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 <zijiehe@chromium.org>
Reviewed-by: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22846}
This commit is contained in:
Julien Isorce 2018-04-11 15:21:19 -07:00 committed by Commit Bot
parent bbf21a3fd6
commit bb005b6b96
3 changed files with 11 additions and 5 deletions

View file

@ -35,6 +35,8 @@ std::unique_ptr<DesktopFrameIOSurface> 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;
}

View file

@ -17,7 +17,7 @@
#include <map>
#include <memory>
#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<DesktopFrame> TakeLatestFrameForDisplay(
CGDirectDisplayID display_id);
@ -48,7 +50,7 @@ class DesktopFrameProvider {
const std::unique_ptr<RWLockWrapper> io_surfaces_lock_;
// Most recent IOSurface that contains a capture of matching display.
std::map<CGDirectDisplayID, std::unique_ptr<DesktopFrame>> io_surfaces_;
std::map<CGDirectDisplayID, std::unique_ptr<SharedDesktopFrame>> io_surfaces_;
RTC_DISALLOW_COPY_AND_ASSIGN(DesktopFrameProvider);
};

View file

@ -37,7 +37,7 @@ std::unique_ptr<DesktopFrame> 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() {