From 0c4de568a632395ca12c57b0600d5d64b5fc91f1 Mon Sep 17 00:00:00 2001 From: Sergey Ulanov Date: Mon, 21 Mar 2016 11:18:42 -0700 Subject: [PATCH] Fix potential crashes in the screen capturer on Mac ScreenCapturerMac wasn't handling the following two cases properly which could cause crashes: 1. CGDisplayCreateImage() returns image with depth other than 32-bit 2. CGDisplayCreateImage() returns image with dimensions different from expected (e.g. when screen resolution is being changed). I suspect that (2) was causing the linked bug. BUG=crbug.com/504927 R=jiayl@webrtc.org Review URL: https://codereview.webrtc.org/1816723002 . Cr-Commit-Position: refs/heads/master@{#12077} --- .../desktop_capture/screen_capturer_mac.mm | 93 ++++++++++--------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/webrtc/modules/desktop_capture/screen_capturer_mac.mm b/webrtc/modules/desktop_capture/screen_capturer_mac.mm index 7cb468f708..c41dc4d7a3 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()); - 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); + 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); + } + + CFRelease(excluded_image_data); CFRelease(excluded_image); } - - CFRelease(data); - CFRelease(image); } if (window_list) CFRelease(window_list);