From 6ff7713a706e65ff2dfe2b1fe2fbb82558db9cc3 Mon Sep 17 00:00:00 2001 From: Salman Date: Fri, 27 Jan 2023 22:58:35 +0000 Subject: [PATCH] base_capturer_pipewire: Send frames via callback Earlier, we were relying on CRD to periodically do frame captures. This is however not needed since Wayland captures are event based and once the compositor has send the event/frame to us, we can just handover the frame to a registered callback. This ensures that we have a single frame clock that (i.e. one present in the compositor). Without this change, there is a chance that CRD capture clock is run out of sync with the compositor's clock and cause unnecessary frame delays. See the following ideal scenario, for example, where '+' indicates when a frame is sent by the compositor and when CRD tries to capture it. This assumes that the same clock cycle for both CRD and the compositor, each cycle length is enclosed within | .... |). Compositor Frame Ready | +... | | +... | CRD Frame Capture | .+.. | | .+.. | In this case, when both the clocks are in sync, CRD is able to capture frame right after it is generated by the compositor. But if they are completely out of sync, then CRD can always see a stale frame (delayed by one cycle and it can cause users to feel stutter). Compositor Frame Ready | .+.. | | .+.. | CRD Frame Capture | +... | | +... | This stutter can become worse if the compositor is delayed in generating the frames for some reason (e.g. load on the system). Bug: chromium:1291247 Change-Id: I7c10c724fbbd87dc523d474e7ece8e8aa146fd7b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291080 Reviewed-by: Alexander Cooper Commit-Queue: Salman Malik Cr-Commit-Position: refs/heads/main@{#39218} --- .../desktop_and_cursor_composer.cc | 4 + .../desktop_and_cursor_composer.h | 1 + modules/desktop_capture/desktop_capturer.h | 3 + .../linux/wayland/base_capturer_pipewire.cc | 7 +- .../linux/wayland/base_capturer_pipewire.h | 5 + .../linux/wayland/shared_screencast_stream.cc | 109 ++++++++++++------ .../linux/wayland/shared_screencast_stream.h | 4 +- 7 files changed, 96 insertions(+), 37 deletions(-) diff --git a/modules/desktop_capture/desktop_and_cursor_composer.cc b/modules/desktop_capture/desktop_and_cursor_composer.cc index 3bdc1ee008..988d1740d3 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.cc +++ b/modules/desktop_capture/desktop_and_cursor_composer.cc @@ -218,6 +218,10 @@ DesktopCaptureMetadata DesktopAndCursorComposer::GetMetadata() { } #endif // defined(WEBRTC_USE_GIO) +void DesktopAndCursorComposer::OnFrameCaptureStart() { + callback_->OnFrameCaptureStart(); +} + void DesktopAndCursorComposer::OnCaptureResult( DesktopCapturer::Result result, std::unique_ptr frame) { diff --git a/modules/desktop_capture/desktop_and_cursor_composer.h b/modules/desktop_capture/desktop_and_cursor_composer.h index d9208b04a7..28ea25c8c9 100644 --- a/modules/desktop_capture/desktop_and_cursor_composer.h +++ b/modules/desktop_capture/desktop_and_cursor_composer.h @@ -80,6 +80,7 @@ class RTC_EXPORT DesktopAndCursorComposer MouseCursorMonitor* mouse_monitor); // DesktopCapturer::Callback interface. + void OnFrameCaptureStart() override; void OnCaptureResult(DesktopCapturer::Result result, std::unique_ptr frame) override; diff --git a/modules/desktop_capture/desktop_capturer.h b/modules/desktop_capture/desktop_capturer.h index 9a054b6f13..04991f20ea 100644 --- a/modules/desktop_capture/desktop_capturer.h +++ b/modules/desktop_capture/desktop_capturer.h @@ -56,6 +56,9 @@ class RTC_EXPORT DesktopCapturer { // Interface that must be implemented by the DesktopCapturer consumers. class Callback { public: + // Called before a frame capture is started. + virtual void OnFrameCaptureStart() {} + // Called after a frame has been captured. `frame` is not nullptr if and // only if `result` is SUCCESS. virtual void OnCaptureResult(Result result, diff --git a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc index 66d0c30f22..4ef00e68ab 100644 --- a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc +++ b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.cc @@ -75,7 +75,8 @@ void BaseCapturerPipeWire::OnScreenCastRequestResult(RequestResponse result, if (result != RequestResponse::kSuccess || !options_.screencast_stream()->StartScreenCastStream( stream_node_id, fd, options_.get_width(), options_.get_height(), - options_.prefer_cursor_embedded())) { + options_.prefer_cursor_embedded(), + send_frames_immediately_ ? callback_ : nullptr)) { capturer_failed_ = true; RTC_LOG(LS_ERROR) << "ScreenCastPortal failed: " << static_cast(result); @@ -234,4 +235,8 @@ ScreenCastPortal* BaseCapturerPipeWire::GetScreenCastPortal() { : nullptr; } +void BaseCapturerPipeWire::SendFramesImmediately(bool send_frames_immediately) { + send_frames_immediately_ = send_frames_immediately; +} + } // namespace webrtc diff --git a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h index 3b708078ad..083d373c17 100644 --- a/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h +++ b/modules/desktop_capture/linux/wayland/base_capturer_pipewire.h @@ -65,11 +65,16 @@ class RTC_EXPORT BaseCapturerPipeWire xdg_portal::SessionDetails GetSessionDetails(); + // Notifies the callback about the available frames as soon as a frame is + // received. + void SendFramesImmediately(bool send_frames_immediately); + private: ScreenCastPortal* GetScreenCastPortal(); DesktopCaptureOptions options_ = {}; Callback* callback_ = nullptr; + bool send_frames_immediately_ = false; bool capturer_failed_ = false; bool is_screencast_portal_ = false; bool is_portal_open_ = false; diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc index 7c4a78de64..c9b1f8dee5 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc @@ -78,7 +78,8 @@ class SharedScreenCastStreamPrivate { int fd, uint32_t width = 0, uint32_t height = 0, - bool is_cursor_embedded = false); + bool is_cursor_embedded = false, + DesktopCapturer::Callback* callback = nullptr); void UpdateScreenCastStreamResolution(uint32_t width, uint32_t height); void UpdateScreenCastStreamFrameRate(uint32_t frame_rate); void SetUseDamageRegion(bool use_damage_region) { @@ -156,6 +157,9 @@ class SharedScreenCastStreamPrivate { void ProcessBuffer(pw_buffer* buffer); void ConvertRGBxToBGRx(uint8_t* frame, uint32_t size); + void UpdateFrameUpdatedRegions(const spa_buffer* spa_buffer, + DesktopFrame& frame); + void NotifyCallbackOfNewFrame(std::unique_ptr frame); // PipeWire callbacks static void OnCoreError(void* data, @@ -178,6 +182,8 @@ class SharedScreenCastStreamPrivate { // failed to use and try to use a different one or fallback to shared memory // buffers. static void OnRenegotiateFormat(void* data, uint64_t); + + DesktopCapturer::Callback* callback_; }; void SharedScreenCastStreamPrivate::OnCoreError(void* data, @@ -392,9 +398,11 @@ bool SharedScreenCastStreamPrivate::StartScreenCastStream( int fd, uint32_t width, uint32_t height, - bool is_cursor_embedded) { + bool is_cursor_embedded, + DesktopCapturer::Callback* callback) { width_ = width; height_ = height; + callback_ = callback; is_cursor_embedded_ = is_cursor_embedded; if (!InitializePipeWire()) { RTC_LOG(LS_ERROR) << "Unable to open PipeWire library"; @@ -633,8 +641,59 @@ DesktopVector SharedScreenCastStreamPrivate::CaptureCursorPosition() { return mouse_cursor_position_; } +void SharedScreenCastStreamPrivate::UpdateFrameUpdatedRegions( + const spa_buffer* spa_buffer, + DesktopFrame& frame) { + if (!use_damage_region_) { + frame.mutable_updated_region()->SetRect( + DesktopRect::MakeSize(frame.size())); + return; + } + + const struct spa_meta* video_damage = static_cast( + spa_buffer_find_meta(spa_buffer, SPA_META_VideoDamage)); + if (!video_damage) { + damage_region_.SetRect(DesktopRect::MakeSize(frame.size())); + return; + } + + frame.mutable_updated_region()->Clear(); + spa_meta_region* meta_region; + spa_meta_for_each(meta_region, video_damage) { + // Skip empty regions + if (meta_region->region.size.width == 0 || + meta_region->region.size.height == 0) { + continue; + } + + damage_region_.AddRect(DesktopRect::MakeXYWH( + meta_region->region.position.x, meta_region->region.position.y, + meta_region->region.size.width, meta_region->region.size.height)); + } +} + +void SharedScreenCastStreamPrivate::NotifyCallbackOfNewFrame( + std::unique_ptr frame) { + if (!pw_stream_ || !frame->data()) { + callback_->OnCaptureResult(DesktopCapturer::Result::ERROR_TEMPORARY, + nullptr); + return; + } + + if (use_damage_region_) { + frame->mutable_updated_region()->Swap(&damage_region_); + damage_region_.Clear(); + } + callback_->OnCaptureResult(DesktopCapturer::Result::SUCCESS, + std::move(frame)); +} + RTC_NO_SANITIZE("cfi-icall") void SharedScreenCastStreamPrivate::ProcessBuffer(pw_buffer* buffer) { + if (callback_) { + callback_->OnFrameCaptureStart(); + } + spa_buffer* spa_buffer = buffer->buffer; ScopedBuf map; std::unique_ptr src_unique_ptr; @@ -878,34 +937,12 @@ void SharedScreenCastStreamPrivate::ProcessBuffer(pw_buffer* buffer) { observer_->OnDesktopFrameChanged(); } - if (use_damage_region_) { - const struct spa_meta* video_damage = static_cast( - spa_buffer_find_meta(spa_buffer, SPA_META_VideoDamage)); - if (video_damage) { - spa_meta_region* meta_region; - - queue_.current_frame()->mutable_updated_region()->Clear(); - - spa_meta_for_each(meta_region, video_damage) { - // Skip empty regions - if (meta_region->region.size.width == 0 || - meta_region->region.size.height == 0) { - continue; - } - - damage_region_.AddRect(DesktopRect::MakeXYWH( - meta_region->region.position.x, meta_region->region.position.y, - meta_region->region.size.width, meta_region->region.size.height)); - } - } else { - damage_region_.SetRect( - DesktopRect::MakeSize(queue_.current_frame()->size())); - } - } else { - queue_.current_frame()->mutable_updated_region()->SetRect( - DesktopRect::MakeSize(queue_.current_frame()->size())); - } + UpdateFrameUpdatedRegions(spa_buffer, *queue_.current_frame()); queue_.current_frame()->set_may_contain_cursor(is_cursor_embedded_); + + if (callback_) { + NotifyCallbackOfNewFrame(queue_.current_frame()->Share()); + } } void SharedScreenCastStreamPrivate::ConvertRGBxToBGRx(uint8_t* frame, @@ -934,13 +971,15 @@ bool SharedScreenCastStream::StartScreenCastStream(uint32_t stream_node_id) { return private_->StartScreenCastStream(stream_node_id, -1); } -bool SharedScreenCastStream::StartScreenCastStream(uint32_t stream_node_id, - int fd, - uint32_t width, - uint32_t height, - bool is_cursor_embedded) { +bool SharedScreenCastStream::StartScreenCastStream( + uint32_t stream_node_id, + int fd, + uint32_t width, + uint32_t height, + bool is_cursor_embedded, + DesktopCapturer::Callback* callback) { return private_->StartScreenCastStream(stream_node_id, fd, width, height, - is_cursor_embedded); + is_cursor_embedded, callback); } void SharedScreenCastStream::UpdateScreenCastStreamResolution(uint32_t width, diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h index a130e53bbe..f57e22cb69 100644 --- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.h +++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.h @@ -16,6 +16,7 @@ #include "absl/types/optional.h" #include "api/ref_counted_base.h" #include "api/scoped_refptr.h" +#include "modules/desktop_capture/desktop_capturer.h" #include "modules/desktop_capture/mouse_cursor.h" #include "modules/desktop_capture/screen_capture_frame_queue.h" #include "modules/desktop_capture/shared_desktop_frame.h" @@ -49,7 +50,8 @@ class RTC_EXPORT SharedScreenCastStream int fd, uint32_t width = 0, uint32_t height = 0, - bool is_cursor_embedded = false); + bool is_cursor_embedded = false, + DesktopCapturer::Callback* callback = nullptr); void UpdateScreenCastStreamResolution(uint32_t width, uint32_t height); void UpdateScreenCastStreamFrameRate(uint32_t frame_rate); void SetUseDamageRegion(bool use_damage_region);