Title: [274237] trunk/Source/ThirdParty/libwebrtc
Revision
274237
Author
jer.no...@apple.com
Date
2021-03-10 13:56:42 -0800 (Wed, 10 Mar 2021)

Log Message

CRASH in MergeUVRow_AVX2
https://bugs.webkit.org/show_bug.cgi?id=222996
<rdar://75183835>

Reviewed by Geoff Garen.

Crash logging shows occasional crashes in MergeUVRow_AVX2. These crashes  all occur when
calling -[AVAssetImageGenerator copyCGImageAtTime:actualTime:error:]. This path is only used
when there was no prior image generated, and a new image is not available from
AVPlayerItemVideoOutput, which is a scenario which only occurs when doing a software-paint
immedately after a <video> element begins loading. While we should probably stop using
AVAssetImageGenerator, that would be a much riskier change, and wouldn't address the
underlying cause of the crash. Instead, bailing out early when in this state would cause
decoding to fail, but since this scenario only appears to occur for the
AVAssetImageGenerator path, painting would quickly recover as soon as
AVPlayerItemVideoOutput begins emitting frames.

The explanation for these crashes seems to be a mismatch between the size of the libvpx
output frame and the size of the CVPixelBuffer where the converted frame data is being
stored. Add a pre-flight check that will bail out early in this scenario.

* Source/webrtc/sdk/WebKit/WebKitUtilities.mm:
(webrtc::CopyVideoFrameToPixelBuffer):

Modified Paths

Diff

Modified: trunk/Source/ThirdParty/libwebrtc/ChangeLog (274236 => 274237)


--- trunk/Source/ThirdParty/libwebrtc/ChangeLog	2021-03-10 21:45:34 UTC (rev 274236)
+++ trunk/Source/ThirdParty/libwebrtc/ChangeLog	2021-03-10 21:56:42 UTC (rev 274237)
@@ -1,3 +1,29 @@
+2021-03-10  Jer Noble  <jer.no...@apple.com>
+
+        CRASH in MergeUVRow_AVX2
+        https://bugs.webkit.org/show_bug.cgi?id=222996
+        <rdar://75183835>
+
+        Reviewed by Geoff Garen.
+
+        Crash logging shows occasional crashes in MergeUVRow_AVX2. These crashes  all occur when
+        calling -[AVAssetImageGenerator copyCGImageAtTime:actualTime:error:]. This path is only used
+        when there was no prior image generated, and a new image is not available from
+        AVPlayerItemVideoOutput, which is a scenario which only occurs when doing a software-paint
+        immedately after a <video> element begins loading. While we should probably stop using
+        AVAssetImageGenerator, that would be a much riskier change, and wouldn't address the
+        underlying cause of the crash. Instead, bailing out early when in this state would cause
+        decoding to fail, but since this scenario only appears to occur for the
+        AVAssetImageGenerator path, painting would quickly recover as soon as
+        AVPlayerItemVideoOutput begins emitting frames.
+
+        The explanation for these crashes seems to be a mismatch between the size of the libvpx
+        output frame and the size of the CVPixelBuffer where the converted frame data is being
+        stored. Add a pre-flight check that will bail out early in this scenario.
+
+        * Source/webrtc/sdk/WebKit/WebKitUtilities.mm:
+        (webrtc::CopyVideoFrameToPixelBuffer):
+
 2021-03-09  Youenn Fablet  <you...@apple.com>
 
         Duplicate headers in WebRTC

Modified: trunk/Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitUtilities.mm (274236 => 274237)


--- trunk/Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitUtilities.mm	2021-03-10 21:45:34 UTC (rev 274236)
+++ trunk/Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitUtilities.mm	2021-03-10 21:56:42 UTC (rev 274237)
@@ -60,12 +60,27 @@
     if (CVPixelBufferLockBaseAddress(pixel_buffer, 0) != kCVReturnSuccess)
         return false;
 
+    auto src_width_y = frame->width();
+    auto src_height_y = frame->height();
+    auto src_width_uv = frame->ChromaWidth();
+    auto src_height_uv = frame->ChromaHeight();
+
     uint8_t* dst_y = reinterpret_cast<uint8_t*>(CVPixelBufferGetBaseAddressOfPlane(pixel_buffer, 0));
     int dst_stride_y = CVPixelBufferGetBytesPerRowOfPlane(pixel_buffer, 0);
+    auto dst_width_y = CVPixelBufferGetWidthOfPlane(pixel_buffer, 0);
+    auto dst_height_y = CVPixelBufferGetHeightOfPlane(pixel_buffer, 0);
 
     uint8_t* dst_uv = reinterpret_cast<uint8_t*>(CVPixelBufferGetBaseAddressOfPlane(pixel_buffer, 1));
     int dst_stride_uv = CVPixelBufferGetBytesPerRowOfPlane(pixel_buffer, 1);
+    auto dst_width_uv = CVPixelBufferGetWidthOfPlane(pixel_buffer, 1);
+    auto dst_height_uv = CVPixelBufferGetHeightOfPlane(pixel_buffer, 1);
 
+    if (src_width_y != dst_width_y
+        || src_height_y != dst_height_y
+        || src_width_uv != dst_width_uv
+        || src_height_uv != dst_height_uv)
+        return false;
+
     int result = libyuv::I420ToNV12(
         frame->DataY(), frame->StrideY(),
         frame->DataU(), frame->StrideU(),
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to