PipeWire capturer: fix some possible threading issues

- avoid holding a lock across OnCaptureResult() callback to avoid a risk
  of a possible deadlock
- annotate damage region as guarded by the same lock as latest frame as
  both belong together
- document the acqusition order between locks

Bug: chromium:333945842
Change-Id: I9c65beed720ba54e40b85fb243a07d40524695f4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/353600
Commit-Queue: Jan Grulich <grulja@gmail.com>
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Andreas Pehrson <apehrson@mozilla.com>
Cr-Commit-Position: refs/heads/main@{#42432}
This commit is contained in:
Jan Grulich 2024-06-04 19:38:21 +02:00 committed by WebRTC LUCI CQ
parent 500e1e1c98
commit 06e88bbb5a

View File

@ -73,7 +73,7 @@ class SharedScreenCastStreamPrivate {
// Track damage region updates that were reported since the last time
// frame was captured
DesktopRegion damage_region_;
DesktopRegion damage_region_ RTC_GUARDED_BY(&latest_frame_lock_);
uint32_t pw_stream_node_id_ = 0;
@ -83,7 +83,7 @@ class SharedScreenCastStreamPrivate {
webrtc::Mutex queue_lock_;
ScreenCaptureFrameQueue<SharedDesktopFrame> queue_
RTC_GUARDED_BY(&queue_lock_);
webrtc::Mutex latest_frame_lock_;
webrtc::Mutex latest_frame_lock_ RTC_ACQUIRED_AFTER(queue_lock_);
SharedDesktopFrame* latest_available_frame_
RTC_GUARDED_BY(&latest_frame_lock_) = nullptr;
std::unique_ptr<MouseCursor> mouse_cursor_;
@ -140,7 +140,6 @@ class SharedScreenCastStreamPrivate {
void ConvertRGBxToBGRx(uint8_t* frame, uint32_t size);
void UpdateFrameUpdatedRegions(const spa_buffer* spa_buffer,
DesktopFrame& frame);
void NotifyCallbackOfNewFrame(std::unique_ptr<SharedDesktopFrame> frame);
// PipeWire callbacks
static void OnCoreError(void* data,
@ -636,6 +635,8 @@ DesktopVector SharedScreenCastStreamPrivate::CaptureCursorPosition() {
void SharedScreenCastStreamPrivate::UpdateFrameUpdatedRegions(
const spa_buffer* spa_buffer,
DesktopFrame& frame) {
latest_frame_lock_.AssertHeld();
if (!use_damage_region_) {
frame.mutable_updated_region()->SetRect(
DesktopRect::MakeSize(frame.size()));
@ -664,22 +665,6 @@ void SharedScreenCastStreamPrivate::UpdateFrameUpdatedRegions(
}
}
void SharedScreenCastStreamPrivate::NotifyCallbackOfNewFrame(
std::unique_ptr<SharedDesktopFrame> 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) {
int64_t capture_start_time_nanos = rtc::TimeNanos();
@ -865,9 +850,8 @@ void SharedScreenCastStreamPrivate::ProcessBuffer(pw_buffer* buffer) {
observer_->OnDesktopFrameChanged();
}
// We have to hold the lock over the latest_frame here already, since
// we are going to update damage region, which corresponds to the latest
// frame and is accessed in CaptureFrame()
std::unique_ptr<SharedDesktopFrame> frame;
{
webrtc::MutexLock latest_frame_lock(&latest_frame_lock_);
UpdateFrameUpdatedRegions(spa_buffer, *queue_.current_frame());
@ -875,12 +859,23 @@ void SharedScreenCastStreamPrivate::ProcessBuffer(pw_buffer* buffer) {
latest_available_frame_ = queue_.current_frame();
if (callback_) {
std::unique_ptr<SharedDesktopFrame> frame = queue_.current_frame()->Share();
if (!callback_) {
return;
}
frame = latest_available_frame_->Share();
frame->set_capturer_id(DesktopCapturerId::kWaylandCapturerLinux);
frame->set_capture_time_ms((rtc::TimeNanos() - capture_start_time_nanos) /
rtc::kNumNanosecsPerMillisec);
NotifyCallbackOfNewFrame(std::move(frame));
if (use_damage_region_) {
frame->mutable_updated_region()->Swap(&damage_region_);
damage_region_.Clear();
}
}
if (callback_) {
callback_->OnCaptureResult(DesktopCapturer::Result::SUCCESS,
std::move(frame));
}
}