Closed Bug 1077159 Opened 5 years ago Closed 5 years ago

[EME] Make GMP APIs versionable

Categories

(Core :: Audio/Video, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla37
Tracking Status
firefox37 --- fixed
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We should fill GMPPlatform::version with a meaningful value so that a GMP can be certain that the plugin container has a compatible ABI.
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
This bug is necessary for basic playback of EME video on Windows.
Blocks: eme-m1
No longer blocks: eme-m1
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.
Summary: [EME] GMPPlatform::version should be filled with a version number → [EME] Make GMP APIs versionable
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.
Attachment #8533360 - Flags: review?(rjesup)
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
Attachment #8533360 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/817f770bff54
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Mass update firefox-status to track EME uplift.
You need to log in before you can comment on or make changes to this bug.