1135558 - Disable WebM support for MSE

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

We're currently focussing on MSE support for MP4 so for now we'll disable WebM support.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Disable WebM support for MSE

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

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.

Comment 3

[Ralph Giles (:rillian)]

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.

Comment 4

[Alfredo Yang (:alfredo)]

(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.

Comment 5

[Alfredo Yang (:alfredo)]

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

Blake, what do you think?

Comment 6

[Blake Wu [:bwu][:blakewu]]

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

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(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...

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/002bca9bfb55

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

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.

Comment 10

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(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.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/002bca9bfb55

Comment 12

[Ralph Giles (:rillian)]

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.

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8567786 [details] [diff] [review]
Disable WebM support for MSE

Approving for aurora. Sounds relatively low risk and also sensible!

Comment 14

[Ralph Giles (:rillian)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f1507c74dd1f

Comment 15

[Jean-Yves Avenard [:jya]]

Attachment: Disable WebM support for MSE

Fix for beta/aurora.

Comment 16

[Jean-Yves Avenard [:jya]]

sorry wrong bug #