[webkit-reviews] review granted: [Bug 230338] Add utility to create CVPixelBuffers from IOSurfaces : [Attachment 438506] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 22 06:41:46 PDT 2021


youenn fablet <youennf at gmail.com> has granted Kimmo Kinnunen
<kkinnunen at apple.com>'s request for review:
Bug 230338: Add utility to create CVPixelBuffers from IOSurfaces
https://bugs.webkit.org/show_bug.cgi?id=230338

Attachment 438506: Patch

https://bugs.webkit.org/attachment.cgi?id=438506&action=review




--- Comment #14 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 438506
  --> https://bugs.webkit.org/attachment.cgi?id=438506
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=438506&action=review

> Source/WebCore/platform/graphics/cv/CVUtilities.mm:55
> +	   return makeUnexpected(status);

We do not seem to log errors at call sites of createIOSurfaceCVPixelBufferPool.
Maybe we should, or should do here.

> Source/WebCore/platform/graphics/cv/CVUtilities.mm:64
> +	   return makeUnexpected(status);

It is a bit odd to have a case where kCVReturnSuccess and pixelBuffer is null.
But it seems good to make it the error path.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:79
> +    auto bufferPool = createIOSurfaceCVPixelBufferPool(size.width(),
size.height(), m_pixelFormat, 6).value_or(nullptr);

Preexisting issue but this value 6 is a bit mysterious.

> Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:214
> +    auto pixelBuffer =
WebCore::createCVPixelBuffer(sample.surface()).value_or(nullptr);

If pixelBuffer is null, we probably want to exit early, maybe log an error.

> Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:500
> +	   m_pixelBufferPool = WebCore::createIOSurfaceCVPixelBufferPool(width,
height, type).value_or(nullptr);

We could not set m_pixelBufferPoolWidth/m_pixelBufferPoolHeight if
m_pixelBufferPool creation failed.


More information about the webkit-reviews mailing list