[EME] Handle CDM updating during playback

VERIFIED FIXED in Firefox 38

Status

()

defect
P1
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: cpearce, Assigned: eflores)

Tracking

(Blocks 1 bug)

39 Branch
mozilla40
x86_64
Windows 8.1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox38 verified, firefox39 verified, firefox40 verified)

Details

Attachments

(3 attachments, 2 obsolete attachments)

If the CDM is updated while the CDM is in use, we need to ensure that playback does not fail.

I suspect that it could be the case that the GMPDecryptor from the old version would be in use, but a new GMPVideoDecoder will be created from the new plugin.

The nodeId will get us part of the way, but we may need to do more work.
Priority: -- → P1
Instructions to test a CDM update during playback:

1. Download a copy of the update.xml [1].
2. Obtain two Primetime CDM zip files of different versions, both able to play back encrypted content. These will be referred to as "v1" and "v2" respectively in the steps below.
3. Obtain or generate the sha512 of both Primetime zip files.
4. Host the CDM zip files on a web server.
5. Change the URL for "gmp-eme-adobe" in the update.xml file (obtained in point 1) to point to the v1 Primetime CDM hosted in point 4.
6. Change the hashValue for "gmp-eme-adobe" in the update.xml file to the sha512 of the v1 Primetime CDM.
7. Host the update.xml file on a web server.
8. Start Firefox with a new profile.
9. Type "about:config" in the URL bar.
10. Create a new string key "media.gmp-manager.url.override". Set the value to the URL of your update.xml file hosted in point 7.
11. Reset the "media.gmp-manager.lastCheck" pref if one is present.
12. Quickly shut down Firefox (but do NOT kill process via Activity Manager or similar).
13. Remove the gmp-eme-adobe directory (if any) from your user profile.
14. Start Firefox and play back encrypted content. Wait for content to play back before proceeding to the next step. This will ensure that the v1 Primetime CDM has been downloaded properly.
15. At this point, you may want to create a bookmark to the encrypted content player to have quick access to this page for step 21.
16. Change the URL for "gmp-eme-adobe" in the update.xml file on the web server (step 7) to point to the v2 Primetime CDM hosted in point 4.
17. Change the hashValue for "gmp-eme-adobe" in the update.xml file to the sha512 of the v2 Primetime CDM.
18. Open a new tab and type "about:config" in the URL bar.
19. Reset the "media.gmp-manager.lastCheck" pref.
20. Shut down Firefox.
21. Start Firefox and quickly play back encrypted content (the bookmark from step 15 may be handy). Content playback needs to start within 60 seconds of Firefox startup to ensure that the v1 Primetime CDM is used.
22. Wait several minutes to ensure that the v2 Primetime CDM is downloaded and registered. You can monitor this by opening a new tab, entering "about:addons" in the URL bar and clicking on Plugins. Once the v2 Primetime CDM has been downloaded, the new version will be reflected there.

[1] https://aus4.mozilla.org/update/3/GMP/39.0a1/20150320030211/WINNT_x86-msvc/en-US/nightly/Windows_NT%206.1.1.0%20%28x64%29/default/default/update.xml
A few things going on in this patch:
 * Add CDM version to node ID hash so that we get the same CDM every time for a given CDMProxy.
 * Add a `CanDefer' flag to removePluginDirectory and RemoveOnGMPThread to tell the GMPService to wait for the CDM to die before removing it properly.
 * Make sure the child process using the CDM DLL is dead before trying to delete it.
Comment on attachment 8588896 [details] [diff] [review]
1148071-handle-stream-switch-pre-e10s.patch

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

Aurora-based patch. Patch for trunk coming soon to a bug near you (this one).
Attachment #8588896 - Flags: review?(cpearce)
Comment on attachment 8588896 [details] [diff] [review]
1148071-handle-stream-switch-pre-e10s.patch

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

r+ with nits.

Really need an automated test for this somehow.

::: dom/media/gmp/GMPService.cpp
@@ +793,5 @@
>      nsCString api(aAPI);
> +    size_t index = 0;
> +
> +    while (GMPParent* gmp = FindPluginForAPIFrom(index, api, *aTags, &index)) {
> +      if (aOutVersion.IsEmpty() || gmp->GetVersion() > aOutVersion) {

You're doing a string comparison here. This won't actually work, as "9" < "10" returns false for example. You need to actually convert the version string to a number. This is complicated by the fact that OpenH264 uses an X.x format, and EME plugins using a X format. So you probably need to parse the version to a float, and do the less than comparision on that.

::: dom/media/gmp/mozIGeckoMediaPluginService.idl
@@ +103,3 @@
>     * @note Main-thread API.
>     */
> +  void removeAndDeletePluginDirectory(in AString directory,

Need to rev the uuid.
Attachment #8588896 - Flags: review?(cpearce) → review+
Depends on: 1146192
Flags: needinfo?(cpearce)
Comment on attachment 8590041 [details] [diff] [review]
Handle stream switch. Review comments addressed; based on Aurora.

Approval Request Comment
[Feature/regressing bug #]: Encrypted Media Extensions/Gecko Media Plugins
[User impact if declined]: Playback stops when an EME CDM is updated, and the old CDM is not deleted from disk. Further, the CDM is not deleted from disk when EME is disabled.
[Describe test coverage new/current, TreeHerder]: None. This patch cannot be applied to trunk since bug 1057908 landed and differs significantly from the trunk version.
[Risks and why]: Small possibility of breaking encrypted media playback.
[String/UUID change made/needed]: Touches mozIGeckoMediaPluginService UUID.
Attachment #8590041 - Flags: approval-mozilla-aurora?
Now with more e10s action
Attachment #8590102 - Flags: review?(cpearce)
Now with more e10s _and_ |hg qref| action!
Attachment #8590102 - Attachment is obsolete: true
Attachment #8590102 - Flags: review?(cpearce)
Attachment #8590103 - Flags: review?(cpearce)
Comment on attachment 8590103 [details] [diff] [review]
1148071-handle-stream-switch-post-e10s.patch

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

r+, but if it's possible to remove the extra round trip through the main thread in CDMProxy::Init and friends, that would be good.

::: dom/media/eme/CDMProxy.cpp
@@ +197,5 @@
> +  // Add plugin version string to node ID so that we get the same
> +  // CDM every time during a stream's playback.
> +  // We have to do this on the main thread in |mt_GetPluginVersionRunnable|
> +  // which then calls back to gmp_InitGetNodeId().
> +  NS_DispatchToMainThread(new mt_GetPluginVersionRunnable(this, Move(aData)));

Why can't you read the plugin version from the main thread in CDMProxy::Init() and just add it as a field to struct InitData? Why does Init() have to dispatch to the GMP thread, only to have it dispatch back tot he main thread and then dispatch back to the GMP thread to continue initializing?
Attachment #8590103 - Flags: review?(cpearce) → review+
Edwin: does this all need to uplift together once it lands?  Or is the patch in comment 7 the part that's different for aurora, and the rest of the work you're doing will stay on 40/nightly?
Flags: needinfo?(edwin)
I think I see now. These are the same patch but rebased for beta, aurora, and nightly. 

Let's uplift this now and verify it on aurora using the steps in comment 1.  Florin if your team can work on verification on Monday that would be awesome.
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
Comment on attachment 8590041 [details] [diff] [review]
Handle stream switch. Review comments addressed; based on Aurora.

Approving for uplift to aurora; while there is some risk we do have steps to verify the fix.
Attachment #8590041 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(edwin)
Comment on attachment 8590042 [details] [diff] [review]
Handle stream switch. Rebased for Beta.

Approval Request Comment
[Feature/regressing bug #]: Encrypted Media Extensions/Gecko Media Plugins
[User impact if declined]: Playback stops when an EME CDM is updated, and the old CDM is not deleted from disk. Further, the CDM is not deleted from disk when EME is disabled. Adobe will be spinning a new CDM version soon, so it would be good to get this in before the CDM updates.
[Describe test coverage new/current, TreeHerder]: The aurora and central version of this patch have been landed for a few days now.
[Risks and why]: Small possibility of breaking encrypted media playback.
[String/UUID change made/needed]: Touches mozIGeckoMediaPluginService UUID.
Attachment #8590042 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/260b09772972
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Jorge, is that acceptable? Thanks
> [String/UUID change made/needed]: Touches mozIGeckoMediaPluginService UUID.
Flags: needinfo?(jorge)
(In reply to Liz Henry (:lizzard) from comment #13)
> I think I see now. These are the same patch but rebased for beta, aurora,
> and nightly. 
> 
> Let's uplift this now and verify it on aurora using the steps in comment 1. 
> Florin if your team can work on verification on Monday that would be awesome.

Sorry, Monday was a holiday for us, so no verification possible. Assigning to Alexandra since she monitors EME related fixes.
Flags: needinfo?(florin.mezei)
QA Contact: alexandra.lucinet
(In reply to Sylvestre Ledru [:sylvestre] from comment #19)
> Jorge, is that acceptable? Thanks
> > [String/UUID change made/needed]: Touches mozIGeckoMediaPluginService UUID.

Looks okay to me.
Flags: needinfo?(jorge)
Comment on attachment 8590042 [details] [diff] [review]
Handle stream switch. Rebased for Beta.

Jorge approved, I am happy!
Should be in 38 beta 5.
Attachment #8590042 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed with latest Developer Edition 39.0a2 (2015-04-14) and Nightly 40.0a1 (2015-04-15) with str provided in comment 1: while CDM is in use via http://people.mozilla.org/~cpearce/mse-clearkey/, updates are properly applied and playback does not fail. Verified with updates from:
- version 6 (bug 1149876) to version 7 (bug 1150781) on 39.0a2
- version 3 (bug 1140944) to version 7 and version 7 to version 8 (bug 1154590) on 40.0a1
Status: RESOLVED → VERIFIED
(In reply to Alexandra Lucinet, QA Mentor [:adalucinet] from comment #23)
> Verified as fixed with latest Developer Edition 39.0a2 (2015-04-14) and
> Nightly 40.0a1 (2015-04-15) with str provided in comment 1: while CDM is in
> use via http://people.mozilla.org/~cpearce/mse-clearkey/, updates are
> properly applied and playback does not fail. Verified with updates from:
> - version 6 (bug 1149876) to version 7 (bug 1150781) on 39.0a2
> - version 3 (bug 1140944) to version 7 and version 7 to version 8 (bug
> 1154590) on 40.0a1

http://people.mozilla.org/~cpearce/mse-clearkey/ does not use the Adobe CDM, it uses the "ClearKey CDM", which is not updated by the update mechanism in question here. 

So you tested that playback using the ClearKey CDM is not affected while the Adobe CDM is updated.

Whereas we need to test that playback using the *Adobe* CDM is not affected while the Adobe CDM is updated.

So please can you retest, but using this URL:

http://drmtest2.adobe.com/HTML5AAXSPlayer/2/mse-access/

This uses the Adobe CDM.

Note that if you're testing in beta, you will also need to set the pref media.mediasource.whitelist to false.

Thanks!
Status: VERIFIED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(cpearce) → needinfo?(alexandra.lucinet)
Chris, thanks for the heads up!
Encountered the same results with http://drmtest2.adobe.com/HTML5AAXSPlayer/2/mse-access/ test page: updates are properly applied and playback does not fail on latest Developer Edition 39.0a2 (2015-04-15) and Nightly 40.0a1 (2015-04-15).
Flags: needinfo?(alexandra.lucinet)
Verified as fixed on Firefox 38 beta 5 (Build ID: 20150416143048) as well, under Windows 8.1 64 bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+ → qe-verify-
You need to log in before you can comment on or make changes to this bug.