Disable WebM support for MSE

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ajones, Assigned: ajones)

Tracking

(Blocks: 1 bug)

36 Branch
mozilla39
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox36 unaffected, firefox37+ unaffected, firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

We're currently focussing on MSE support for MP4 so for now we'll disable WebM support.
Assignee: nobody → ajones
Status: NEW → ASSIGNED
Comment on attachment 8567786 [details] [diff] [review]
Disable WebM support for MSE

Alfredo - if I land this it will affect the prefs you need to enable MSE.
Attachment #8567786 - Flags: review?(giles)
Attachment #8567786 - Flags: feedback?(ayang)
Comment on attachment 8567786 [details] [diff] [review]
Disable WebM support for MSE

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

Thanks for taking this. r=me with nit addressed.

::: modules/libpref/init/all.js
@@ -461,5 @@
> -#ifdef MOZ_WIDGET_GONK
> -pref("media.mediasource.mp4.enabled", false);
> -pref("media.mediasource.webm.enabled", false);
> -#else
> -#if defined(XP_WIN) || defined(XP_MACOSX)

Can we use #elif here? Much easier to read. Otherwise please indent the second # if ... # else .. # endif.
Attachment #8567786 - Flags: review?(giles) → review+
Attachment #8567786 - Flags: feedback?(ayang) → feedback+
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #2)
> Comment on attachment 8567786 [details] [diff] [review]
> Disable WebM support for MSE
> 
> Alfredo - if I land this it will affect the prefs you need to enable MSE.

Thanks for notification.
However, I'd worry about the emulator-kk try test is not enable yet.

Blake, what do you think?
Flags: needinfo?(bwu)
I think it is fine. This patch only enable MSE for MP4 on B2G. For MP4 local playback, it continues using the original OMX decoder/reader due to [1]. Emulator-kk try test would be more helpful on local playback case.

[1]https://dxr.mozilla.org/mozilla-central/source/dom/media/DecoderTraits.cpp#356
Flags: needinfo?(bwu)
(In reply to Ralph Giles (:rillian) from comment #3)
> Comment on attachment 8567786 [details] [diff] [review]
> Disable WebM support for MSE
> 
> Review of attachment 8567786 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for taking this. r=me with nit addressed.
> 
> ::: modules/libpref/init/all.js
> @@ -461,5 @@
> > -#ifdef MOZ_WIDGET_GONK
> > -pref("media.mediasource.mp4.enabled", false);
> > -pref("media.mediasource.webm.enabled", false);
> > -#else
> > -#if defined(XP_WIN) || defined(XP_MACOSX)
> 
> Can we use #elif here? Much easier to read. Otherwise please indent the
> second # if ... # else .. # endif.

That's the bit I'm taking out...
Given that we're currently planning to ship MSE in 37, I take it that we'll need to disable WebM in 37 and 38 as well.
status-firefox36: --- → unaffected
status-firefox37: --- → affected
status-firefox38: --- → affected
status-firefox39: --- → affected
tracking-firefox37: --- → +
(In reply to Lawrence Mandel [:lmandel] (use needinfo) from comment #9)
> Given that we're currently planning to ship MSE in 37, I take it that we'll
> need to disable WebM in 37 and 38 as well.

WebM was only enabled for Aurora/Nightly by the RELEASE_BUILD flag.
status-firefox37: affected → unaffected
https://hg.mozilla.org/mozilla-central/rev/002bca9bfb55
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox39: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8567786 [details] [diff] [review]
Disable WebM support for MSE

We should have this on 38 as well. As mentioned, it's already off for release builds, so this just matters for aurora.

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Stalls playing MSE video, in particular YouTube.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: This flips MSE pref off for non-Windows, non-Mac plaforms, giving the same behaviour as release builds. Youtube will likely serve low resolution video to systems without flash or mp4 support, but at this point we think that's better than unreliable playback. 
[String/UUID change made/needed]: None.
Attachment #8567786 - Flags: approval-mozilla-aurora?
Comment on attachment 8567786 [details] [diff] [review]
Disable WebM support for MSE

Approving for aurora. Sounds relatively low risk and also sensible!
Attachment #8567786 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Posted patch Disable WebM support for MSE (obsolete) — Splinter Review
Fix for beta/aurora.
Assignee: ajones → jyavenard
Attachment #8572263 - Attachment is obsolete: true
sorry wrong bug #
Assignee: jyavenard → ajones
You need to log in before you can comment on or make changes to this bug.