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)
Core
Audio/Video: GMP
Tracking
()
RESOLVED
FIXED
mozilla54
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
(Keywords: regression)
Attachments
(4 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
mozbugz
:
review+
|
Details |
8.73 KB,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•7 years ago
|
||
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.
Assignee | ||
Comment 2•7 years ago
|
||
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8841814 [details] Bug 1342822 - Backed out changeset a379d64f8496 (Bug 1338924 patch 3). https://reviewboard.mozilla.org/r/115910/#review117298
Attachment #8841814 -
Flags: review+
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8841815 [details] Bug 1342822 - Backed out changeset 70bc7d4e8512 (bug 1338924). https://reviewboard.mozilla.org/r/115912/#review117300
Attachment #8841815 -
Flags: review+
Comment 8•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
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
Comment 13•7 years ago
|
||
backout bugherder |
https://hg.mozilla.org/mozilla-central/rev/a6cfc821a073 https://hg.mozilla.org/mozilla-central/rev/af9dd74bba62
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3dc5e735df4f
Assignee | ||
Comment 15•7 years ago
|
||
Beta rebase of 3dc5e735df4f.
Assignee | ||
Comment 16•7 years ago
|
||
Beta rebase of 3dc5e735df4f.
Assignee | ||
Updated•7 years ago
|
Attachment #8846898 -
Attachment is obsolete: true
Assignee | ||
Comment 17•7 years ago
|
||
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?
Assignee | ||
Comment 18•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8a521d760cb
Comment 19•7 years ago
|
||
Marking 53 as affected based on bug 1338924. Is 52 also affected?
Comment 20•7 years ago
|
||
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+
Comment 21•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/b6c66c7c52bd
Comment 22•7 years ago
|
||
Setting qe-verify- based on Chris' assessment on manual testing needs (see Comment 17).
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•