1077159 - [EME] Make GMP APIs versionable

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

We should fill GMPPlatform::version with a meaningful value so that a GMP can be certain that the plugin container has a compatible ABI.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Note: OpenH264 does not check the GMPPlatform.version, so making this change won't break gmp-openh264, yet at least:

https://github.com/cisco/openh264/blob/0dd0b06287890993f3b799e12b253ab7c0cc6169/module/gmp-openh264.cpp#L756

Comment 2

[Chris Peterson [:cpeterson]]

This bug is necessary for basic playback of EME video on Windows.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

I think the better idea is to version individual interfaces, by changing the string we use to identify and instantiate them in the GMPGetAPI() function. We can have a #define on the header, e.g.:

#define GMP_IID_VIDEO_DECODER "VideoDecoder"

which we can use in the GMP browser code, and the GMP can use the same #define in its code. So a GMP compiled with a different header for that interface will have a different ID string, and so won't instantiate.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Add and use GMP_API_* macros

Create GMP_API_* macros for the "decode-video" etc API names, and use them to compare against API names, and to request for APIs. If we need to rev an API, we can just append/increment an integer suffix to the API name.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8533360 [details] [diff] [review]
Patch: Add and use GMP_API_* macros

Review of attachment 8533360 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp/gmp-api/gmp-async-shutdown.h
@@ +34,5 @@
>  //
>  // Note: Your GMP's GMPShutdown function will still be called after your
>  // call to ShutdownComplete().
>  //
> +// API name macro: GMP_IID_ASYNC_SHUTDOWN

API not IID...

::: dom/media/gmp/gmp-api/gmp-audio-decode.h
@@ +45,5 @@
> +
> +// Audio decoding for a single stream. A GMP may be asked to create multiple
> +// decoders concurrently.
> +//
> +// API name macro: GMP_IID_AUDIO_DECODER

API

::: dom/media/gmp/gmp-api/gmp-video-decode.h
@@ +67,5 @@
> +
> +// Video decoding for a single stream. A GMP may be asked to create multiple
> +// decoders concurrently.
> +//
> +// API name macro: GMP_IID_VIDEO_DECODER

API

::: dom/media/gmp/gmp-api/gmp-video-encode.h
@@ +61,5 @@
> +
> +// Video encoding for a single stream. A GMP may be asked to create multiple
> +// encoders concurrently.
> +//
> +// API name macro: GMP_IID_VIDEO_ENCODER

API

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/817f770bff54

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/817f770bff54

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Mass update firefox-status to track EME uplift.