Closed
Bug 1144875
Opened 10 years ago
Closed 10 years ago
Disable EME in ESR 38
Categories
(Core :: Audio/Video, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
firefox37 | --- | unaffected |
firefox38 | + | fixed |
firefox39 | --- | unaffected |
firefox40 | --- | unaffected |
firefox-esr38 | --- | verified |
firefox-esr45 | --- | unaffected |
People
(Reporter: cpeterson, Assigned: rillian)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Dolske
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]:
We would like to disable EME in ESR (38) so we and Adobe do not need to maintain EME and CDM compatibility for the next twelve months. We don't think typical organizations using ESR to care about EME support. They can use Flash or Silverlight to watch movies, if they care.
* Anthony: this was the conclusion from a meeting with Javaun and Chad this morning. Does this sound reasonable to you?
Flags: needinfo?(ajones)
Reporter | ||
Updated•10 years ago
|
Component: General → Video/Audio
Comment 1•10 years ago
|
||
Will youtube end up using flash in ESR38, or try to use EME and just not show anything?
Reporter | ||
Comment 2•10 years ago
|
||
Firefox EME is not relevant to YouTube in Firefox 38. We are working with YouTube to support MSE (for HTML5 video) but not EME yet. YouTube supports Google's Widevine and Microsoft's PlayReady DRM, not the Adobe Primetime DRM we are currently integrating.
YouTube decides on the server whether to play Flash or HTML5 video. They don't do client-side feature detection because they want to minimize video playback latency.
Comment 3•10 years ago
|
||
When you say disable, do you mean off-by-default or broken?
Flags: needinfo?(ajones)
Reporter | ||
Comment 4•10 years ago
|
||
I think we just want to default the "media.eme.enabled" pref to false as spohl describes in bug 1144903 comment 5 for the DRM-free build.
No longer blocks: 1144878
Comment 5•10 years ago
|
||
Tracking as it is important we follow that.
And this will need to be properly documented and managed as we build the ESR release.
Reporter | ||
Updated•10 years ago
|
Priority: -- → P1
Comment 6•10 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #4)
> I think we just want to default the "media.eme.enabled" pref to false as
> spohl describes in bug 1144903 comment 5 for the DRM-free build.
Yep (and the UI pref too).
The #defines for this should be MOZ_UPDATE_CHANNEL, something like
#if defined(MOZ_UPDATE_CHANNEL) && MOZ_UPDATE_CHANNEL == esr
Reporter | ||
Comment 7•10 years ago
|
||
Ralph, Anthony nominated you to write this patch to disable EME in ESR and uplift it to Beta 38. :)
Assignee: nobody → giles
status-firefox40:
--- → unaffected
Comment 8•10 years ago
|
||
Ralph: Is there something we can do here before the ESR branch, or do we have to wait until the branch before we can close this bug?
Flags: needinfo?(giles)
Comment 9•10 years ago
|
||
We have to wait but preparing the patch now would help (except if you think we will have rebase issues).
Flags: needinfo?(giles)
Assignee | ||
Comment 10•10 years ago
|
||
Here's a patch which just flips the prefs unconditionally. This would need to land on the 38 branch after merge day to disable eme for the esr release. This approach was recommended by Gavin, and is the planned approach from disabling Hello in 1141058.
This should disable the UI as well, but I want to verify that the button to re-enable in browser-eme.js isn't exposed.
Assignee | ||
Comment 11•10 years ago
|
||
This version is conditional on MOZ_UPDATE_CHANNEL as :cpeterson suggested. Also needs to be verified, but it looks like we do set this properly for esr31.
We can land this version on 38 while it's still in beta and it won't become active until we do esr builds in May.
Assignee | ||
Comment 12•10 years ago
|
||
Sylvestre, do you have a preference as to which approach we take? We can land a conditional pref during 38 beta to become active in 38 esr, or we can land an unconditional pref after release before the first esr build.
I prefer te conditional pref as there's less to forget, but we'll need to verify either way.
Flags: needinfo?(sledru)
Comment 13•10 years ago
|
||
Just like you, I prefer the conditional pref, something less to worry about :)
Flags: needinfo?(sledru)
Comment 14•10 years ago
|
||
Comment on attachment 8593738 [details] [diff] [review]
Disable EME for Firefox 38 ESR (conditional patch)
This may not quite work as expected. For example, we query the value of "media.eme.enabled" in several places and default to true if the pref isn't set (see [1] for example). We probably need to set these prefs to false explicitly.
[1] http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/GMPProvider.jsm#158
Assignee | ||
Comment 15•10 years ago
|
||
Thanks for the review, spohl. I thought there was another define for media.eme.enabled in the mozilla-beta firefox.js the previous patch would fall back to, like there is for browser.eme.ui.enabled, but I was mistaken.
Here's an updated patch with an explicit fallback and everything in one place. Going with the conditional approach per sledru.
Attachment #8593737 -
Attachment is obsolete: true
Attachment #8593738 -
Attachment is obsolete: true
Attachment #8594083 -
Flags: review?(cpearce)
Comment 16•10 years ago
|
||
Comment on attachment 8594083 [details] [diff] [review]
Disable EME for Firefox 38 ESR (conditional patch v2)
Review of attachment 8594083 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM, but should get a Firefox peer to r+ this.
Attachment #8594083 -
Flags: review?(cpearce) → review?(dolske)
Updated•10 years ago
|
Attachment #8594083 -
Flags: review?(dolske) → review+
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8594083 [details] [diff] [review]
Disable EME for Firefox 38 ESR (conditional patch v2)
Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: 38 ESR users will be stuck with a minimal initial implementation, increasing our support burden.
[Describe test coverage new/current, TreeHerder]: local testing only.
[Risks and why]: This conditionally disables the EME feature for ESR build. If it accidentally disables eme otherwise, that will be obvious in beta builds. The main risk is that it doesn't completely disable eme in the eventual ESR builds, so we will need to verify that when the time comes.
[String/UUID change made/needed]: None, interface is already pref-controlled.
Attachment #8594083 -
Flags: approval-mozilla-beta?
Comment 18•10 years ago
|
||
Comment on attachment 8594083 [details] [diff] [review]
Disable EME for Firefox 38 ESR (conditional patch v2)
Thanks. Should be in 38 beta 6 or 7.
Attachment #8594083 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox-esr38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Updated•10 years ago
|
Comment 20•10 years ago
|
||
Verified as fixed on ESR 38.0 (Build ID: 20150505103531) under Windows 7 x64 and Windows 8.1 x32 - EME is properly disabled.
Updated•10 years ago
|
Flags: qe-verify+
Comment 21•9 years ago
|
||
Chris, now that we are working on 45 (the next ESR), are we going to do the same for this release? thanks
Flags: needinfo?(cpeterson)
Reporter | ||
Comment 22•9 years ago
|
||
Good question. Now that Netflix has launched with EME on Windows, the code is stable enough that we don't need to disable it in ESR 45.
status-firefox-esr45:
--- → unaffected
Flags: needinfo?(cpeterson)
You need to log in
before you can comment on or make changes to this bug.
Description
•