Chromium Code Reviews

Unified Diff: webrtc/modules/desktop_capture/screen_capturer_mac.mm

Issue 2391743004: Use non-deprecated screen update callbacks. (Closed)
Patch Set: vector -> map Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/desktop_capture/screen_capturer_mac.mm
diff --git a/webrtc/modules/desktop_capture/screen_capturer_mac.mm b/webrtc/modules/desktop_capture/screen_capturer_mac.mm
index 451cd9ed9d8168463d6370ef52b0a012ebfaefaa..a9a16573a600fbb313d3700e1d67f8b301e8583d 100644
--- a/webrtc/modules/desktop_capture/screen_capturer_mac.mm
+++ b/webrtc/modules/desktop_capture/screen_capturer_mac.mm
@@ -18,6 +18,7 @@
#include <ApplicationServices/ApplicationServices.h>
#include <Cocoa/Cocoa.h>
+#include <CoreGraphics/CoreGraphics.h>
#include <dlfcn.h>
#include <OpenGL/CGLMacro.h>
#include <OpenGL/OpenGL.h>
@@ -25,6 +26,7 @@
#include "webrtc/base/checks.h"
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/macutils.h"
+#include "webrtc/base/criticalsection.h"
#include "webrtc/base/timeutils.h"
#include "webrtc/modules/desktop_capture/desktop_capture_options.h"
#include "webrtc/modules/desktop_capture/desktop_frame.h"
@@ -43,6 +45,20 @@ namespace webrtc {
namespace {
+struct DisplayStreamWrapper {
Eugene But (OOO till 7-30) 2016/10/04 23:13:52 Do you want to add comments for this?
erikchen 2016/10/04 23:52:39 Done. Note that I added an |owner| field.
+ CGDisplayStreamRef stream = nullptr;
+ int unique_id = 0;
+ bool active = true;
+};
+
+// CGDisplayStreamRef needs to be destroyed asynchronously, after a callback
+// with status kCGDisplayStreamFrameStatusStopped. There's no guarantee that
+// ScreenCapturerMac will still be around, so we need a global to ensure that
+// the map from callback to CGDisplayStreamRef is still valid.
+static std::map<int, DisplayStreamWrapper> g_display_stream_wrappers;
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 I don't think that static global objects are allow
erikchen 2016/10/04 23:52:39 We do use them: https://cs.chromium.org/search/?q=
Eugene But (OOO till 7-30) 2016/10/05 00:19:53 Maybe a function which returns a map reference?:
Sergey Ulanov 2016/10/06 23:57:24 this pattern is used in several places in WebRTC,
+static int g_unique_id_generator = 0;
+static rtc::CriticalSection g_display_stream_critical_section;
+
// Definitions used to dynamic-link to deprecated OS 10.6 functions.
const char* kApplicationServicesLibraryName =
"/System/Library/Frameworks/ApplicationServices.framework/"
@@ -218,16 +234,11 @@ class ScreenCapturerMac : public ScreenCapturer {
void UnregisterRefreshAndMoveHandlers();
void ScreenRefresh(CGRectCount count, const CGRect *rect_array);
- void ScreenUpdateMove(CGScreenUpdateMoveDelta delta,
+ void ScreenUpdateMove(CGFloat delta_x,
+ CGFloat delta_y,
size_t count,
- const CGRect *rect_array);
- static void ScreenRefreshCallback(CGRectCount count,
- const CGRect *rect_array,
- void *user_parameter);
- static void ScreenUpdateMoveCallback(CGScreenUpdateMoveDelta delta,
- size_t count,
- const CGRect *rect_array,
- void *user_parameter);
+ const CGRect* rect_array);
+ void ScreenRefreshCallback(CGRectCount count, const CGRect* rect_array);
void ReleaseBuffers();
std::unique_ptr<DesktopFrame> CreateFrame();
@@ -311,12 +322,12 @@ ScreenCapturerMac::~ScreenCapturerMac() {
}
bool ScreenCapturerMac::Init() {
- if (!RegisterRefreshAndMoveHandlers()) {
- return false;
- }
desktop_config_monitor_->Lock();
desktop_config_ = desktop_config_monitor_->desktop_configuration();
desktop_config_monitor_->Unlock();
+ if (!RegisterRefreshAndMoveHandlers()) {
+ return false;
+ }
ScreenConfigurationChanged();
return true;
}
@@ -848,28 +859,81 @@ void ScreenCapturerMac::ScreenConfigurationChanged() {
}
bool ScreenCapturerMac::RegisterRefreshAndMoveHandlers() {
- CGError err = CGRegisterScreenRefreshCallback(
- ScreenCapturerMac::ScreenRefreshCallback, this);
- if (err != kCGErrorSuccess) {
- LOG(LS_ERROR) << "CGRegisterScreenRefreshCallback " << err;
- return false;
- }
-
- err = CGScreenRegisterMoveCallback(
- ScreenCapturerMac::ScreenUpdateMoveCallback, this);
- if (err != kCGErrorSuccess) {
- LOG(LS_ERROR) << "CGScreenRegisterMoveCallback " << err;
- return false;
+ desktop_config_ = desktop_config_monitor_->desktop_configuration();
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 Is this thing can be called on different threads?
erikchen 2016/10/04 23:52:39 Based on the usage of desktop_config_ in the rest
Eugene But (OOO till 7-30) 2016/10/05 00:19:53 So if it is single threaded, then what is the reas
+ for (MacDisplayConfigurations::iterator it = desktop_config_.displays.begin();
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 NIT: Maybe use auto and for in?
erikchen 2016/10/04 23:52:39 Done.
+ it != desktop_config_.displays.end(); ++it) {
+ size_t pixel_width = it->pixel_bounds.width();
+ size_t pixel_height = it->pixel_bounds.height();
+ if (pixel_width == 0 || pixel_height == 0)
+ continue;
+ int unique_id;
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 Please initialize
erikchen 2016/10/04 23:52:39 Done.
+ {
+ rtc::CritScope scoper(&g_display_stream_critical_section);
+ unique_id = ++g_unique_id_generator;
+ }
+ CGDirectDisplayID display_id = it->id;
+ CGDisplayStreamFrameAvailableHandler handler =
+ ^(CGDisplayStreamFrameStatus status, uint64_t displayTime,
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 s/displayTime/display_time
erikchen 2016/10/04 23:52:39 Done.
+ IOSurfaceRef frameSurface, CGDisplayStreamUpdateRef updateRef) {
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 s/frameSurface/frame_surface s/updateRef/update_re
Eugene But (OOO till 7-30) 2016/10/04 23:13:53 Is this hander called on different thread (other t
erikchen 2016/10/04 23:52:39 No. The reason for the lock I added was to deal wi
erikchen 2016/10/04 23:52:39 Done.
Eugene But (OOO till 7-30) 2016/10/05 00:19:53 So if |g_display_stream_wrappers| is always access
+ if (status == kCGDisplayStreamFrameStatusStopped) {
+ rtc::CritScope scoper(&g_display_stream_critical_section);
+ auto it = g_display_stream_wrappers.find(unique_id);
+ assert(it != g_display_stream_wrappers.end());
Eugene But (OOO till 7-30) 2016/10/04 23:13:52 Should this be DCHECK?
erikchen 2016/10/04 23:52:39 I think the answer is no. Assuming we don't immedi
+ assert(!it->second.active);
+ CFRelease(it->second.stream);
+ g_display_stream_wrappers.erase(it);
+ return;
+ }
+
+ // Only pay attention to frame updates.
+ if (status != kCGDisplayStreamFrameStatusFrameComplete)
+ return;
+
+ size_t count = 0;
+ const CGRect* rects = CGDisplayStreamUpdateGetRects(
+ updateRef, kCGDisplayStreamUpdateMovedRects, &count);
+ if (count != 0) {
+ CGFloat dx = 0;
+ CGFloat dy = 0;
+ CGDisplayStreamUpdateGetMovedRectsDelta(updateRef, &dx, &dy);
+ ScreenUpdateMove(dx, dy, count, rects);
+ }
+
+ count = 0;
+ rects = CGDisplayStreamUpdateGetRects(
+ updateRef, kCGDisplayStreamUpdateRefreshedRects, &count);
+ if (count != 0) {
+ ScreenRefreshCallback(count, rects);
+ }
+ };
+ CGDisplayStreamRef display_stream = CGDisplayStreamCreate(
+ display_id, pixel_width, pixel_height, 'BGRA', nullptr, handler);
+ if (display_stream) {
+ DisplayStreamWrapper wrapper;
+ wrapper.stream = display_stream;
+ wrapper.unique_id = unique_id;
+ {
+ rtc::CritScope scoper(&g_display_stream_critical_section);
+ assert(g_display_stream_wrappers.find(unique_id) ==
+ g_display_stream_wrappers.end());
+ g_display_stream_wrappers[unique_id] = wrapper;
+ }
+ CGDisplayStreamStart(display_stream);
+ }
}
return true;
}
void ScreenCapturerMac::UnregisterRefreshAndMoveHandlers() {
- CGUnregisterScreenRefreshCallback(
- ScreenCapturerMac::ScreenRefreshCallback, this);
- CGScreenUnregisterMoveCallback(
- ScreenCapturerMac::ScreenUpdateMoveCallback, this);
+ rtc::CritScope scoper(&g_display_stream_critical_section);
+ for (auto& pair : g_display_stream_wrappers) {
+ DisplayStreamWrapper& wrapper = pair.second;
+ if (wrapper.active) {
+ wrapper.active = false;
+ CGDisplayStreamStop(wrapper.stream);
+ }
+ }
}
void ScreenCapturerMac::ScreenRefresh(CGRectCount count,
@@ -891,13 +955,14 @@ void ScreenCapturerMac::ScreenRefresh(CGRectCount count,
helper_.InvalidateRegion(region);
}
-void ScreenCapturerMac::ScreenUpdateMove(CGScreenUpdateMoveDelta delta,
+void ScreenCapturerMac::ScreenUpdateMove(CGFloat delta_x,
+ CGFloat delta_y,
size_t count,
const CGRect* rect_array) {
// Translate |rect_array| to identify the move's destination.
CGRect refresh_rects[count];
for (CGRectCount i = 0; i < count; ++i) {
- refresh_rects[i] = CGRectOffset(rect_array[i], delta.dX, delta.dY);
+ refresh_rects[i] = CGRectOffset(rect_array[i], delta_x, delta_y);
}
// Currently we just treat move events the same as refreshes.
@@ -905,23 +970,10 @@ void ScreenCapturerMac::ScreenUpdateMove(CGScreenUpdateMoveDelta delta,
}
void ScreenCapturerMac::ScreenRefreshCallback(CGRectCount count,
- const CGRect* rect_array,
- void* user_parameter) {
- ScreenCapturerMac* capturer =
- reinterpret_cast<ScreenCapturerMac*>(user_parameter);
- if (capturer->screen_pixel_bounds_.is_empty())
- capturer->ScreenConfigurationChanged();
- capturer->ScreenRefresh(count, rect_array);
-}
-
-void ScreenCapturerMac::ScreenUpdateMoveCallback(
- CGScreenUpdateMoveDelta delta,
- size_t count,
- const CGRect* rect_array,
- void* user_parameter) {
- ScreenCapturerMac* capturer =
- reinterpret_cast<ScreenCapturerMac*>(user_parameter);
- capturer->ScreenUpdateMove(delta, count, rect_array);
+ const CGRect* rect_array) {
+ if (screen_pixel_bounds_.is_empty())
+ ScreenConfigurationChanged();
+ ScreenRefresh(count, rect_array);
}
std::unique_ptr<DesktopFrame> ScreenCapturerMac::CreateFrame() {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine