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

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.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 438382: Patch

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




--- Comment #7 from Darin Adler <darin at apple.com> ---
Comment on attachment 438382
  --> https://bugs.webkit.org/attachment.cgi?id=438382
Patch

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

> Source/WebCore/platform/graphics/cv/CVUtilities.mm:72
> +    if (format == kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange || format
== kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) {

I think this needs a brief "why" comment.

> Source/WebCore/platform/graphics/cv/CVUtilities.mm:100
> +    ASSERT(surface);

Why assert this? We’re not asserting pixelBufferPool above, for example.

Separately, but related: Maybe we could do a null check, return an unexpected
value in that case, and remove the null check in
ImageTransferSessionVT::createCMSampleBuffer?

>
Source/WebCore/platform/mediastream/mac/RealtimeIncomingVideoSourceCocoa.mm:102
> +	   if (auto pool = createIOSurfaceCVPixelBufferPool({
static_cast<int>(width), static_cast<int>(height) }, poolBufferType)) {

Not new, but what guarantees the width and height fit into int?

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

Not new, but what guarantees the width and height fit into int?