[webkit-reviews] review granted: [Bug 93591] Use the new AVPlayerItemVideoOutput API in MediaPlayerPrivateAVFoundation. : [Attachment 157752] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 93591: Use the new AVPlayerItemVideoOutput API in
MediaPlayerPrivateAVFoundation.
https://bugs.webkit.org/show_bug.cgi?id=93591

Attachment 157752: Patch
https://bugs.webkit.org/attachment.cgi?id=157752&action=review

------- Additional Comments from Eric Carlson <eric.carlson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=157752&action=review


> Source/WebCore/ChangeLog:23
> +	   Pull out existing the AVAssetImageGenerator into its own functions:

Nit: typo "Pull out existing the ..." -> "Pull out the existing ..."

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundati
onObjC.mm:833
> +    NSDictionary* attributes = [NSDictionary
dictionaryWithObjectsAndKeys:[NSNumber
numberWithUnsignedInt:k32BGRAPixelFormat], kCVPixelBufferPixelFormatTypeKey,

I assume you are using k32BGRAPixelFormat because it is the most efficient
format to draw? A comment about why this is so (is that the format <canvas>
uses?) would be helpful.

Nit: I would prefer (slightly) if we didn't use autoreleased NSObjects.