Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(562)

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

Issue 1816723002: Fix potential crashes in the screen capturer on Mac (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« 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 7cb468f7087195fd8d7462996edb03dcad8aeba4..c41dc4d7a3b4ba59e4ebe6880f1e090dae5386b5 100644
--- a/webrtc/modules/desktop_capture/screen_capturer_mac.mm
+++ b/webrtc/modules/desktop_capture/screen_capturer_mac.mm
@@ -168,8 +168,7 @@ DesktopRect GetExcludedWindowPixelBounds(CGWindowID window,
// pixels. The caller should release the returned CGImageRef and CFDataRef.
CGImageRef CreateExcludedWindowRegionImage(const DesktopRect& pixel_bounds,
float dip_to_pixel_scale,
- CFArrayRef window_list,
- CFDataRef* data_ref) {
+ CFArrayRef window_list) {
CGRect window_bounds;
// The origin is in DIP while the size is in physical pixels. That's what
// CGWindowListCreateImageFromArray expects.
@@ -178,13 +177,8 @@ CGImageRef CreateExcludedWindowRegionImage(const DesktopRect& pixel_bounds,
window_bounds.size.width = pixel_bounds.width();
window_bounds.size.height = pixel_bounds.height();
- CGImageRef excluded_image = CGWindowListCreateImageFromArray(
+ return CGWindowListCreateImageFromArray(
window_bounds, window_list, kCGWindowImageDefault);
-
- CGDataProviderRef provider = CGImageGetDataProvider(excluded_image);
- *data_ref = CGDataProviderCopyData(provider);
- assert(*data_ref);
- return excluded_image;
}
// A class to perform video frame capturing for mac.
@@ -694,7 +688,6 @@ bool ScreenCapturerMac::CgBlitPostLion(const DesktopFrame& frame,
DesktopRect excluded_window_bounds;
CGImageRef excluded_image = NULL;
- CFDataRef excluded_window_region_data = NULL;
if (excluded_window_ && window_list) {
// Get the region of the excluded window relative the primary display.
excluded_window_bounds = GetExcludedWindowPixelBounds(
@@ -705,17 +698,30 @@ bool ScreenCapturerMac::CgBlitPostLion(const DesktopFrame& frame,
// than captuing the whole display.
if (!excluded_window_bounds.is_empty()) {
excluded_image = CreateExcludedWindowRegionImage(
- excluded_window_bounds,
- display_config.dip_to_pixel_scale,
- window_list,
- &excluded_window_region_data);
+ excluded_window_bounds, display_config.dip_to_pixel_scale,
+ window_list);
}
}
// Create an image containing a snapshot of the display.
CGImageRef image = CGDisplayCreateImage(display_config.id);
- if (image == NULL)
+ if (image == NULL) {
+ if (excluded_image)
+ CFRelease(excluded_image);
continue;
+ }
+
+ // Verify that the image has 32-bit depth.
+ int bits_per_pixel = CGImageGetBitsPerPixel(image);
+ if (bits_per_pixel / 8 != DesktopFrame::kBytesPerPixel) {
+ LOG(LS_ERROR) << "CGDisplayCreateImage() returned imaged with "
+ << bits_per_pixel
+ << " bits per pixel. Only 32-bit depth is supported.";
+ CFRelease(image);
+ if (excluded_image)
+ CFRelease(excluded_image);
+ return false;
+ }
// Request access to the raw pixel data via the image's DataProvider.
CGDataProviderRef provider = CGImageGetDataProvider(image);
@@ -724,50 +730,51 @@ bool ScreenCapturerMac::CgBlitPostLion(const DesktopFrame& frame,
const uint8_t* display_base_address = CFDataGetBytePtr(data);
int src_bytes_per_row = CGImageGetBytesPerRow(image);
- int src_bytes_per_pixel = CGImageGetBitsPerPixel(image) / 8;
- // Calculate where in the output buffer the display's origin is.
- uint8_t* out_ptr = frame.data() +
- (display_bounds.left() * src_bytes_per_pixel) +
- (display_bounds.top() * frame.stride());
+ // |image| size may be different from display_bounds in case the screen was
+ // resized recently.
+ copy_region.IntersectWith(
+ DesktopRect::MakeWH(CGImageGetWidth(image), CGImageGetHeight(image)));
// Copy the dirty region from the display buffer into our desktop buffer.
+ uint8_t* out_ptr = frame.GetFrameDataAtPos(display_bounds.top_left());
for (DesktopRegion::Iterator i(copy_region); !i.IsAtEnd(); i.Advance()) {
- CopyRect(display_base_address,
- src_bytes_per_row,
- out_ptr,
- frame.stride(),
- src_bytes_per_pixel,
- i.rect());
+ CopyRect(display_base_address, src_bytes_per_row, out_ptr, frame.stride(),
+ DesktopFrame::kBytesPerPixel, i.rect());
}
- // Copy the region of the excluded window to the frame.
+ CFRelease(data);
+ CFRelease(image);
+
if (excluded_image) {
- assert(excluded_window_region_data);
- display_base_address = CFDataGetBytePtr(excluded_window_region_data);
+ CGDataProviderRef provider = CGImageGetDataProvider(excluded_image);
+ CFDataRef excluded_image_data = CGDataProviderCopyData(provider);
+ assert(excluded_image_data);
+ display_base_address = CFDataGetBytePtr(excluded_image_data);
src_bytes_per_row = CGImageGetBytesPerRow(excluded_image);
// Translate the bounds relative to the desktop, because |frame| data
// starts from the desktop top-left corner.
DesktopRect window_bounds_relative_to_desktop(excluded_window_bounds);
- window_bounds_relative_to_desktop.Translate(
- -screen_pixel_bounds_.left(), -screen_pixel_bounds_.top());
- out_ptr = frame.data() +
- (window_bounds_relative_to_desktop.left() * src_bytes_per_pixel) +
- (window_bounds_relative_to_desktop.top() * frame.stride());
+ window_bounds_relative_to_desktop.Translate(-screen_pixel_bounds_.left(),
+ -screen_pixel_bounds_.top());
+
+ DesktopRect rect_to_copy =
+ DesktopRect::MakeSize(excluded_window_bounds.size());
+ rect_to_copy.IntersectWith(DesktopRect::MakeWH(
+ CGImageGetWidth(excluded_image), CGImageGetHeight(excluded_image)));
+
+ if (CGImageGetBitsPerPixel(excluded_image) / 8 ==
+ DesktopFrame::kBytesPerPixel) {
+ CopyRect(display_base_address, src_bytes_per_row,
+ frame.GetFrameDataAtPos(
+ window_bounds_relative_to_desktop.top_left()),
+ frame.stride(), DesktopFrame::kBytesPerPixel, rect_to_copy);
+ }
- CopyRect(display_base_address,
- src_bytes_per_row,
- out_ptr,
- frame.stride(),
- src_bytes_per_pixel,
- DesktopRect::MakeSize(excluded_window_bounds.size()));
- CFRelease(excluded_window_region_data);
+ CFRelease(excluded_image_data);
CFRelease(excluded_image);
}
-
- CFRelease(data);
- CFRelease(image);
}
if (window_list)
CFRelease(window_list);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698