From 7add207ada479c4b9087fec2178ece9086523dc6 Mon Sep 17 00:00:00 2001 From: Lambros Lambrou Date: Wed, 1 Sep 2021 23:58:50 -0700 Subject: [PATCH] Remove DCHECK when overwriting shared DesktopFrame. The desktop capturer uses a circular queue (currently of length 2) to implement a double-buffer scheme. This allows a C++ API consumer to keep a reference to the latest captured image without the pixels being overwritten by a pending capture request. The DCHECK was intended to warn that the application is still holding a reference to a recycled frame that is being captured into. This made sense when the capturer implementations were originally part of the Chromoting host process. Now that the capturers are part of the WebRTC C++ library, a DCHECK seems too harsh. A DCHECK should be reserved for impossible conditions, but this one triggers simply because an API consumer holds onto a reference for too long. This CL changes these DCHECKs into log warnings. The DCHECK is sometimes triggered by the Chromoting host process (because of the recent change to use the standard encoding pipeline). This is tracked by http://crbug.com/1239746. Bug: None Change-Id: Iad9ef38b4800315bd17c93b27d287e115d4fe54c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230881 Commit-Queue: Joe Downing Reviewed-by: Joe Downing Cr-Commit-Position: refs/heads/main@{#34910} --- modules/desktop_capture/linux/screen_capturer_x11.cc | 4 +++- modules/desktop_capture/mac/screen_capturer_mac.mm | 4 +++- modules/desktop_capture/win/screen_capturer_win_gdi.cc | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/modules/desktop_capture/linux/screen_capturer_x11.cc b/modules/desktop_capture/linux/screen_capturer_x11.cc index 7cdbb2a8a8..33e332613f 100644 --- a/modules/desktop_capture/linux/screen_capturer_x11.cc +++ b/modules/desktop_capture/linux/screen_capturer_x11.cc @@ -228,7 +228,9 @@ void ScreenCapturerX11::CaptureFrame() { int64_t capture_start_time_nanos = rtc::TimeNanos(); queue_.MoveToNextFrame(); - RTC_DCHECK(!queue_.current_frame() || !queue_.current_frame()->IsShared()); + if (queue_.current_frame() && queue_.current_frame()->IsShared()) { + RTC_DLOG(LS_WARNING) << "Overwriting frame that is still shared."; + } // Process XEvents for XDamage and cursor shape tracking. options_.x_display()->ProcessPendingXEvents(); diff --git a/modules/desktop_capture/mac/screen_capturer_mac.mm b/modules/desktop_capture/mac/screen_capturer_mac.mm index 29d2017356..701d44476e 100644 --- a/modules/desktop_capture/mac/screen_capturer_mac.mm +++ b/modules/desktop_capture/mac/screen_capturer_mac.mm @@ -198,7 +198,9 @@ void ScreenCapturerMac::CaptureFrame() { int64_t capture_start_time_nanos = rtc::TimeNanos(); queue_.MoveToNextFrame(); - RTC_DCHECK(!queue_.current_frame() || !queue_.current_frame()->IsShared()); + if (queue_.current_frame() && queue_.current_frame()->IsShared()) { + RTC_DLOG(LS_WARNING) << "Overwriting frame that is still shared."; + } MacDesktopConfiguration new_config = desktop_config_monitor_->desktop_configuration(); if (!desktop_config_.Equals(new_config)) { diff --git a/modules/desktop_capture/win/screen_capturer_win_gdi.cc b/modules/desktop_capture/win/screen_capturer_win_gdi.cc index dc27344f82..231754c952 100644 --- a/modules/desktop_capture/win/screen_capturer_win_gdi.cc +++ b/modules/desktop_capture/win/screen_capturer_win_gdi.cc @@ -78,7 +78,9 @@ void ScreenCapturerWinGdi::CaptureFrame() { int64_t capture_start_time_nanos = rtc::TimeNanos(); queue_.MoveToNextFrame(); - RTC_DCHECK(!queue_.current_frame() || !queue_.current_frame()->IsShared()); + if (queue_.current_frame() && queue_.current_frame()->IsShared()) { + RTC_DLOG(LS_WARNING) << "Overwriting frame that is still shared."; + } // Make sure the GDI capture resources are up-to-date. PrepareCaptureResources();