Support in-band metadata cues when loading media in the GPU Process

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

Support in-band metadata cues when loading media in the GPU Process

[Radar WebKit Bug Importer]

<rdar://problem/59561647>

[Eric Carlson]

Created attachment 391199 [details]
Patch

[Eric Carlson]

Created attachment 391243 [details]
Patch

[Eric Carlson]

Created attachment 391245 [details]
Patch

[Eric Carlson]

Created attachment 391282 [details]
Patch

[Sam Weinig]

Comment on attachment 391282 [details]
Patch

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

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:27
> -    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id, enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration playerConfiguration) Synchronous
> -    DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)
> +    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id, enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration playerConfiguration) Async

😘

[Eric Carlson]

Created attachment 391284 [details]
Patch

[Eric Carlson]

Created attachment 391285 [details]
Patch

[Eric Carlson]

Created attachment 391292 [details]
Patch

[Eric Carlson]

Created attachment 391300 [details]
Patch

[Eric Carlson]

Created attachment 391319 [details]
Patch

[Eric Carlson]

Created attachment 391331 [details]
Patch

[Dean Jackson]

Comment on attachment 391331 [details]
Patch

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

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.h:49
> +    WEBCORE_EXPORT static NSArray* allowedClassesForNativeValues();

Nit, I think the * goes near the function name.

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:66
> +SerializedPlatformDataCueMac::~SerializedPlatformDataCueMac()

Do you need this here now? Could it just be left out or marked = default?

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:116
> +NSArray* SerializedPlatformDataCueMac::allowedClassesForNativeValues()

Nit: * again (I think... I'm never sure of the rules in .mm files)

> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:211
>  static JSValue *jsValueWithAVMetadataItemInContext(AVMetadataItem *item, JSContext *context)

Yeah, I guess it does go on that side.

> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:28
> -    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id, enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration playerConfiguration) Synchronous
> -    DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)
> +    CreateMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id, enum:uint8_t WebCore::MediaPlayerEnums::MediaEngineIdentifier remoteEngineIdentifier, struct WebKit::RemoteMediaPlayerProxyConfiguration proxyConfiguration) -> (struct WebKit::RemoteMediaPlayerConfiguration playerConfiguration) Async
> +     DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)

Sam already gave you love for this, so I don't need to!

[Eric Carlson]

Comment on attachment 391331 [details]
Patch

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

Thanks for the review!

>> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.h:49
>> +    WEBCORE_EXPORT static NSArray* allowedClassesForNativeValues();
> 
> Nit, I think the * goes near the function name.

OMG, this is such a RIDICULOUS "rule"...

>> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:66
>> +SerializedPlatformDataCueMac::~SerializedPlatformDataCueMac()
> 
> Do you need this here now? Could it just be left out or marked = default?

Good point!

>> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:116
>> +NSArray* SerializedPlatformDataCueMac::allowedClassesForNativeValues()
> 
> Nit: * again (I think... I'm never sure of the rules in .mm files)

:-/

>> Source/WebCore/platform/mac/SerializedPlatformDataCueMac.mm:211
>>  static JSValue *jsValueWithAVMetadataItemInContext(AVMetadataItem *item, JSContext *context)
> 
> Yeah, I guess it does go on that side.

Ditto

>> Source/WebKit/GPUProcess/media/RemoteMediaPlayerManagerProxy.messages.in:28
>> +     DeleteMediaPlayer(WebKit::MediaPlayerPrivateRemoteIdentifier id)
> 
> Sam already gave you love for this, so I don't need to!

:-)

[Eric Carlson]

Created attachment 391366 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 391366 [details]:

editing/spelling/spellcheck-paste-continuous-disabled.html bug 208016 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 391366 [details]
Patch

Clearing flags on attachment: 391366

Committed r257125: <https://trac.webkit.org/changeset/257125>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.