1166169 - [FFOS] [Regression] MSE cannot work on B2G

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

This is a regression bug since it can work well previously. 
I am going to check what is the cause.

Comment 1

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

Repro steps:
1. Go to youtube and change it to be desktop version via setting.
2. Choose one video to play
3. Once it plays, long press the video frame and select "stats for nerds". 
4. Check the "DASH" field.

Actual result:
It shows "no (18)"

Expected result:
It should show "yes"

Comment 2

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

Attachment: Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch

One of root causes is MOZ_GONK_MEDIACODEC is not defined in gecko/dom/media/fmp4 which makes IsGonkMP4DecoderAvailable() return false[1]. 

This patch is to add MOZ_GONK_MEDIACODEC to configure.in and add the define in its required makefile.  


[1]https://dxr.mozilla.org/mozilla-central/source/dom/media/fmp4/MP4Decoder.cpp?from=MP4Decoder.cpp&case=true#199

Comment 3

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

(In reply to Blake Wu [:bwu][:blakewu] from comment #2)
> Created attachment 8607935 [details] [diff] [review]
> Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch
> 
> One of root causes is MOZ_GONK_MEDIACODEC is not defined in
> gecko/dom/media/fmp4 which makes IsGonkMP4DecoderAvailable() return
> false[1]. 
It looks like Bug 1163458 causes this.

Comment 4

[Jean-Yves Avenard [:jya]]

As the PlatformDecoderModule can now be used outside FMP4, defining MOZ_GONK_MEDIACODEC when MOZ_FMP4 wasn't defined caused a compilation on B2G L try run.

So now, MOZ_GONK_MEDIACODEC is defined when:
if CONFIG['MOZ_FMP4'] and CONFIG['ANDROID_VERSION'] >= '18'and CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk':

MOZ_FMP4 isn't defined on B2G L

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8607935 [details] [diff] [review]
Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch

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

I got compilation failure on B2G L when MOZ_GONK_CODEC was defined when MOZ_FMP4 wasn't.
MOZ_FMP4 isn't set on B2G L.

Here is an example:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4931df04bfa
(which is what I changed to only set MOZ_GONK_CODEC when FMP4 is set)

cpearce knows the build system much better than I do.

Comment 6

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

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> Comment on attachment 8607935 [details] [diff] [review]
> Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch
> 
> Review of attachment 8607935 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I got compilation failure on B2G L when MOZ_GONK_CODEC was defined when
> MOZ_FMP4 wasn't.
> MOZ_FMP4 isn't set on B2G L.
> 
> Here is an example:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4931df04bfa
> (which is what I changed to only set MOZ_GONK_CODEC when FMP4 is set)
Thanks for your info! 
I will test my patch and check how to fix this problem. 
Gonk's PDM is based on Android's MediaCodec (L supports, but MOZ_FMP4 is not enabled on it), so if MOZ_GONK_MEDIACODEC is not defined, the codes related to it should not be to build.

Comment 7

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

The build looks good for all versions. 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ecccd62a03c0

Comment 8

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

With this patch, attachment 8607935 [details] [diff] [review], MSE still cannot work well on Youtube; it continually shows the spinner which is similar to bug 1163227.

Comment 9

[Jean-Yves Avenard [:jya]]

(In reply to Blake Wu [:bwu][:blakewu] from comment #8)
> With this patch, attachment 8607935 [details] [diff] [review], MSE still
> cannot work well on Youtube; it continually shows the spinner which is
> similar to bug 1163227.

To test if it's bug 1163227 you can set media.mediasource.format-reader.mp4 to false

If it still doesn't play, it's unlikely to be due to 1163227

Comment 10

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

(In reply to Jean-Yves Avenard [:jya] from comment #9)
> (In reply to Blake Wu [:bwu][:blakewu] from comment #8)
> > With this patch, attachment 8607935 [details] [diff] [review], MSE still
> > cannot work well on Youtube; it continually shows the spinner which is
> > similar to bug 1163227.
> 
> To test if it's bug 1163227 you can set media.mediasource.format-reader.mp4
> to false
> 
> If it still doesn't play, it's unlikely to be due to 1163227
Thanks for this info.
I found no matter I set media.mediasource.format-reader.mp4 false or true, that spinner does not continually show at first time, but if I refresh the browser, sometimes (repro rate: 25%) the spinner continually shows and playback will not start (have wait for over 5 mins). It maybe result from other cause, not bug 1163227. But would like to wait bug 1163227 to be fixed first and then continue to check further.

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8607935 [details] [diff] [review]
Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch

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

::: configure.in
@@ +5303,4 @@
>      MOZ_EME=1
>  fi;
>  
> +if test "$MOZ_WIDGET_TOOLKIT" = "gonk" -a "$MOZ_FMP4" -eq "1" -a "$ANDROID_VERSION" -ge "18"; then

replace "$MOZ_FMP4" -eq "1" with -n "$MOZ_FMP4"

Comment 12

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

Attachment: hg-Bug-1166169-Add-MOZ_GONK_MEDIACODEC-in-configure-and-define-it-in-moz.build.patch

1. Have changes per comment 11.
2. Carry r+ from glandium and cpearce.

Comment 13

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

Try server: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e96559c059a5
https://treeherder.mozilla.org/#/jobs?repo=try&revision=047802b2b09d

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/d02884b7a126

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d02884b7a126

Comment 16

[Jean-Yves Avenard [:jya]]

Shouldn't we enable mochitests on b2g so regressions like this are caught on try run?
Including webref tests.

Comment 17

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

(In reply to Jean-Yves Avenard [:jya] from comment #16)
> Shouldn't we enable mochitests on b2g so regressions like this are caught on
> try run?
> Including webref tests.
Yeah, we should and will. Bug 1107678 is intended to enable tests on b2g emulator KK which MSE and PDM can work on.

Comment 18

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

Bug 1168996 should have nothing to do with this bug.