Closed Bug 1342822 Opened 7 years ago Closed 7 years ago

Widevine throughput limiting causes MacOS playback to fail

Categories

(Core :: Audio/Video: GMP, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox52 --- ?
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

The landing of bug 1338924 caused clearkey playback to fail on MacOS, at least on my 13" MBP.

I believe this is because on MacOS clearkey is decrypting both the audio and video through the CDM. So the rate limit of 2 seconds duration decoded per 1 second will be hit, and we'll fall behind.

Also a problem with Widevine playback is that the GMP's clock has only second precision, and the throttling code is setting timers with millisecond precision. So the throttling code ends up in a hot loop doing rounded-down-to-zero-millisecond timeouts.
If we move the throttling into the EMEDecryptor, we'll solve this problem. Since there is one EMEDecryptor per stream, we'll be throttling audio and (decrypted) video with separate counters, instead of them sharing a single throughput limiter.

The GMP clock precision thing won't then be a problem, as we'll not be relying on it for setting high precision timers. I think we should still improve it's precision, but it's not a problem in itself.
While fixing this, I'll backout the changes from Bug 1342822 (except the NewGMPTask(std::function<void()>&& aFunction) added there, as that's useful), and reimplement the rate limiting in EMEDecryptor.
Comment on attachment 8841816 [details]
Bug 1342822 - Move Widevine decryption throughput limit into Gecko.

https://reviewboard.mozilla.org/r/115914/#review117302

::: dom/media/platforms/agnostic/eme/DecryptThroughputLimit.h:1
(Diff revision 1)
> +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */

In commit description: 'Widvine' -> 'Widevine'
Attachment #8841816 - Flags: review?(gsquelart) → review+
Pushed by cpearce@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6cfc821a073
Backed out changeset a379d64f8496 (Bug 1338924 patch 3). r=gerald
https://hg.mozilla.org/integration/autoland/rev/af9dd74bba62
Backed out changeset 70bc7d4e8512 (bug 1338924). r=gerald
https://hg.mozilla.org/integration/autoland/rev/3dc5e735df4f
Move Widevine decryption throughput limit into Gecko. r=gerald
https://hg.mozilla.org/mozilla-central/rev/a6cfc821a073
https://hg.mozilla.org/mozilla-central/rev/af9dd74bba62
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Attachment #8846898 - Attachment is obsolete: true
Comment on attachment 8846899 [details] [diff] [review]
[BETA REBASE] Move Widevine decryption throughput limit into Gecko

Approval Request Comment
[Feature/Bug causing the regression]: "Showtime" Amazon Live content fails on machines without HDCP compliant monitors; VGA monitors for Windows, and external displays on MacOS and Linux. This fixes bug 1338924 (the fix in bug 1338924 was flawed, this patch addresses the flaw and fixes the bug). 
[User impact if declined]: Users who try to watch "Showtime" Amazon Live content on displays which are not HDCP compliant will have playback fail.
[Is this code covered by automated tests?]: We have lots of mochitests covering EME, but none covering HDCP compliance directly.
[Has the fix been verified in Nightly?]: Yes. Amazon's QA have verified this fixes the issue in Nightly.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: This change throttles DRM protected audio decrypting path. Amazon is the only service I'm aware of that encrypts their audio streams. Every other DRM video streamer I'm aware of only encrypts their video. So it's not likely to affect the most streaming sites other than Amazon, who have tested this fix themselves.
[String changes made/needed]: None.
Attachment #8846899 - Flags: approval-mozilla-beta?
Marking 53 as affected based on bug 1338924. Is 52 also affected?
Comment on attachment 8846899 [details] [diff] [review]
[BETA REBASE] Move Widevine decryption throughput limit into Gecko

Fix for regression in 53, let's take this for beta 3.
Attachment #8846899 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Setting qe-verify- based on Chris' assessment on manual testing needs (see Comment 17).
Flags: qe-verify-
Depends on: 1368564
You need to log in before you can comment on or make changes to this bug.