Bug 147642 – Adding ability to generate image from sample buffer for MediaPlayerPrivateMediaStream

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Matthew Daiter]

Need the ability to generate CGImageRefs from CMSampleBuffers in MediaPlayerPrivateMediaStreamAVFObjC, in order to generate proper buffers for methods that need to paint to a context. If we don't have this method, we can't make an image, and we can't make a context out of a MediaStream.

[Matthew Daiter]

Created attachment 258194 [details]
Patch

[Eric Carlson]

Comment on attachment 258194 [details]
Patch

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

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:350
> +    // Get a CMSampleBuffer's Core Video image buffer for the media data

Nit: WebKit style is to not add a comment for something that is obvious from reading the code. This doesn't reach that bar.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:353
> +    // Lock the base address of the pixel buffer

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:356
> +    // Get the number of bytes per row for the pixel buffer

This comment is just wrong.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:361
> +    // Get the number of bytes per row for the pixel buffer
> +    size_t bytesPerRow = CVPixelBufferGetBytesPerRow(imageBuffer);
> +    // Get the pixel buffer width and height

These are obvious.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:365
> +    // Create a device-dependent RGB color space

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:372
> +    // Unlock the pixel buffer

Ditto.

[Matthew Daiter]

Created attachment 258203 [details]
Patch

[Matthew Daiter]

Comment on attachment 258194 [details]
Patch

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

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:350
>> +    // Get a CMSampleBuffer's Core Video image buffer for the media data
> 
> Nit: WebKit style is to not add a comment for something that is obvious from reading the code. This doesn't reach that bar.

Fixed.

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:353
>> +    // Lock the base address of the pixel buffer
> 
> Ditto.

Fixed.

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:356
>> +    // Get the number of bytes per row for the pixel buffer
> 
> This comment is just wrong.

Fixed.

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:361
>> +    // Get the pixel buffer width and height
> 
> These are obvious.

Fixed.

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:365
>> +    // Create a device-dependent RGB color space
> 
> Ditto.

Fixed.

>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:372
>> +    // Unlock the pixel buffer
> 
> Ditto.

Fixed.

[WebKit Commit Bot]

Comment on attachment 258203 [details]
Patch

Clearing flags on attachment: 258203

Committed r187883: <http://trac.webkit.org/changeset/187883>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.