From bb005b6b9695a34ea5ffb1af8f991e93b0b09b31 Mon Sep 17 00:00:00 2001 From: Julien Isorce Date: Wed, 11 Apr 2018 15:21:19 -0700 Subject: [PATCH] Make ScreenCapturerMac more robust when using IOSurface The issue is visible when reconfiguring the screen arrangement while sharing the displays. Can sometimes be seen right after starting the screen sharing. Indeed CaptureFrame can be called at any time so TakeLatestFrameForDisplay should always return a valid frame and the call should not empty the internal container. Also add missing teardown in the provider on failure case. Bug: webrtc:8652 Change-Id: Ice151c1da92b9ad2b86ca9368d30d9d21114e53e Reviewed-on: https://webrtc-review.googlesource.com/69420 Commit-Queue: Zijie He Reviewed-by: Zijie He Cr-Commit-Position: refs/heads/master@{#22846} --- modules/desktop_capture/mac/desktop_frame_iosurface.mm | 2 ++ modules/desktop_capture/mac/desktop_frame_provider.h | 8 +++++--- modules/desktop_capture/mac/desktop_frame_provider.mm | 6 ++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/desktop_capture/mac/desktop_frame_iosurface.mm b/modules/desktop_capture/mac/desktop_frame_iosurface.mm index 7df01b7c85..b59b319db9 100644 --- a/modules/desktop_capture/mac/desktop_frame_iosurface.mm +++ b/modules/desktop_capture/mac/desktop_frame_iosurface.mm @@ -35,6 +35,8 @@ std::unique_ptr DesktopFrameIOSurface::Wrap( if (bytes_per_pixel != DesktopFrame::kBytesPerPixel) { RTC_LOG(LS_ERROR) << "CGDisplayStream handler returned IOSurface with " << (8 * bytes_per_pixel) << " bits per pixel. Only 32-bit depth is supported."; + IOSurfaceUnlock(io_surface.get(), kIOSurfaceLockReadOnly, nullptr); + IOSurfaceDecrementUseCount(io_surface.get()); return nullptr; } diff --git a/modules/desktop_capture/mac/desktop_frame_provider.h b/modules/desktop_capture/mac/desktop_frame_provider.h index d4bbdb1690..b2abca83c7 100644 --- a/modules/desktop_capture/mac/desktop_frame_provider.h +++ b/modules/desktop_capture/mac/desktop_frame_provider.h @@ -17,7 +17,7 @@ #include #include -#include "modules/desktop_capture/desktop_frame.h" +#include "modules/desktop_capture/shared_desktop_frame.h" #include "rtc_base/synchronization/rw_lock_wrapper.h" #include "sdk/objc/Framework/Classes/Common/scoped_cftyperef.h" @@ -29,7 +29,9 @@ class DesktopFrameProvider { ~DesktopFrameProvider(); // The caller takes ownership of the returned desktop frame. Otherwise - // returns null if |display_id| is invalid or not ready. + // returns null if |display_id| is invalid or not ready. Note that this + // function does not remove the frame from the internal container. Caller + // has to call the Release function. std::unique_ptr TakeLatestFrameForDisplay( CGDirectDisplayID display_id); @@ -48,7 +50,7 @@ class DesktopFrameProvider { const std::unique_ptr io_surfaces_lock_; // Most recent IOSurface that contains a capture of matching display. - std::map> io_surfaces_; + std::map> io_surfaces_; RTC_DISALLOW_COPY_AND_ASSIGN(DesktopFrameProvider); }; diff --git a/modules/desktop_capture/mac/desktop_frame_provider.mm b/modules/desktop_capture/mac/desktop_frame_provider.mm index 3e13125682..8417227107 100644 --- a/modules/desktop_capture/mac/desktop_frame_provider.mm +++ b/modules/desktop_capture/mac/desktop_frame_provider.mm @@ -37,7 +37,7 @@ std::unique_ptr DesktopFrameProvider::TakeLatestFrameForDisplay( // handler. Indeed chromium's content uses a dedicates thread. WriteLockScoped scoped_io_surfaces_lock(*io_surfaces_lock_); if (io_surfaces_[display_id]) { - return std::move(io_surfaces_[display_id]); + return io_surfaces_[display_id]->Share(); } return nullptr; @@ -54,7 +54,9 @@ void DesktopFrameProvider::InvalidateIOSurface(CGDirectDisplayID display_id, // Call from the thread which runs the CGDisplayStream handler. WriteLockScoped scoped_io_surfaces_lock(*io_surfaces_lock_); - io_surfaces_[display_id] = std::move(desktop_frame_iosurface); + io_surfaces_[display_id] = desktop_frame_iosurface ? + SharedDesktopFrame::Wrap(std::move(desktop_frame_iosurface)) : + nullptr; } void DesktopFrameProvider::Release() {