Bug 147642 - Adding ability to generate image from sample buffer for MediaPlayerPrivateMediaStream
Summary: Adding ability to generate image from sample buffer for MediaPlayerPrivateMed...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Matthew Daiter
URL:
Keywords: PlatformOnly
Depends on:
Blocks:
 
Reported: 2015-08-04 11:50 PDT by Matthew Daiter
Modified: 2015-08-04 13:30 PDT (History)
9 users (show)

See Also:


Attachments
Patch (3.69 KB, patch)
2015-08-04 11:56 PDT, Matthew Daiter
no flags Details | Formatted Diff | Diff
Patch (3.31 KB, patch)
2015-08-04 13:06 PDT, Matthew Daiter
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Daiter 2015-08-04 11:50:56 PDT
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.
Comment 1 Matthew Daiter 2015-08-04 11:56:08 PDT
Created attachment 258194 [details]
Patch
Comment 2 Eric Carlson 2015-08-04 13:01:40 PDT
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.
Comment 3 Matthew Daiter 2015-08-04 13:06:01 PDT
Created attachment 258203 [details]
Patch
Comment 4 Matthew Daiter 2015-08-04 13:06:44 PDT
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.
Comment 5 WebKit Commit Bot 2015-08-04 13:30:04 PDT
Comment on attachment 258203 [details]
Patch

Clearing flags on attachment: 258203

Committed r187883: <http://trac.webkit.org/changeset/187883>
Comment 6 WebKit Commit Bot 2015-08-04 13:30:06 PDT
All reviewed patches have been landed.  Closing bug.