[webkit-reviews] review granted: [Bug 65400] Adopt AVCF media back end on Windows : [Attachment 102865] First cut at implementing AVCF support on Windows.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jeff Miller <jeffm at apple.com>'s
request for review:
Bug 65400: Adopt AVCF media back end on Windows
https://bugs.webkit.org/show_bug.cgi?id=65400

Attachment 102865: First cut at implementing AVCF support on Windows.
https://bugs.webkit.org/attachment.cgi?id=102865&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=102865&action=review


Looks good.

>
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.cp
p:782
> +	    contentsNeedsDisplay();

Indented here by an extra space.

>
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationCF.
cpp:176
> +    static CFArrayRef keys;
> +    if (!keys) {
> +	   static const CFStringRef keyNames[] = {
> +	       AVCFAssetPropertyDuration,
> +	       AVCFAssetPropertyNaturalSize,
> +	       AVCFAssetPropertyPreferredTransform,
> +	       AVCFAssetPropertyPreferredRate,
> +	       AVCFAssetPropertyPlayable,
> +	       AVCFAssetPropertyTracks 
> +	   };
> +	   keys = CFArrayCreate(0, (const void**)keyNames, sizeof(keyNames) /
sizeof(keyNames[0]), &kCFTypeArrayCallBacks);
> +    }
> +    return keys;

I suggest putting the creation of the array in a separate create function, then
you can just do it like this:

    static CFArrayRef keys = createMetadataKeysArray();
    return keys;

>
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationCF.
cpp:613
> +    return static_cast<unsigned>(totalMediaSize);

What makes it safe to cast this to unsigned?

>>
Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundationCF.
cpp:1057
>> +	AVCFAssetLoadValuesAsynchronouslyForProperties(avAsset(),
metadataKeyNames(), loadMetadataCompletionCallback, this);    
> 
> Tab found; better to use spaces  [whitespace/tab] [1]

Tab is at the end of this line.