1350279 - Videos (mp4, WebM) broken on several top websites

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P1)

Description

[Marco Castelluccio [:marco]]

Not only HLS videos are broken (bug 1247433), but mp4 videos too (from YouTube, Facebook). Infact, it's possible all mp4 and also WebM videos are broken.

For example, http://techslides.com/demos/sample-videos/small.ogv is playable, http://techslides.com/demos/sample-videos/small.webm and http://techslides.com/demos/sample-videos/small.mp4 are not.

Comment 1

[Marco Castelluccio [:marco]]

The videos works for me on a Nexus 5, but they don't work on a Sony Xperia Z3 Compact.

Comment 2

[Jan Henning [:JanH]]

Does this work with "media.android-remote-codec.enabled" set to false? In which case bug 1350209 would "fix" it for 53.

Comment 3

[Marco Castelluccio [:marco]]

(In reply to Jan Henning [:JanH] from comment #2)
> Does this work with "media.android-remote-codec.enabled" set to false? In
> which case bug 1350209 would "fix" it for 53.

Yes, it does.

Comment 4

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

OK, I can track this for now but sounds like if we uplift the patch from 1350209 it should be fixed
How did we not catch this in testing/with automated tests?

Comment 5

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

[Tracking Requested - why for this release]:

Comment 6

[Marco Castelluccio [:marco]]

Actually, I've tested all options now, setting that pref to false only fixes playing a video when you have a direct URL (e.g. http://techslides.com/demos/sample-videos/small.mp4 and videos from Facebook, which for now open in a separate tab), but videos on web pages are still broken (e.g. on YouTube).

Playing videos in web pages is already broken in 52, so that one is not a regression in 52 (but it is still definitely a concerning bug).

Comment 7

[Marco Castelluccio [:marco]]

On YouTube and other web sites, the video element is replaced with an error message saying "Video can't be played because the file is corrupt.".

Comment 8

[Jan Henning [:JanH]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #4)
> OK, I can track this for now but sounds like if we uplift the patch from
> 1350209 it should be fixed

Yes and no - it'll disable OOP decoding in 53, but 54+ still needs a real fix.

(In reply to Marco Castelluccio [:marco] (PTO until April 3) from comment #6)
> Playing videos in web pages is already broken in 52, so that one is not a
> regression in 52 (but it is still definitely a concerning bug).

Can you open a separate bug for anything that shows up even with OOP decoding disabled?

Comment 9

[Marco Castelluccio [:marco]]

(In reply to Jan Henning [:JanH] from comment #8)
> (In reply to Marco Castelluccio [:marco] (PTO until April 3) from comment #6)
> > Playing videos in web pages is already broken in 52, so that one is not a
> > regression in 52 (but it is still definitely a concerning bug).
> 
> Can you open a separate bug for anything that shows up even with OOP
> decoding disabled?

Filed bug 1350649, which is probably more important than this one (as this one only affects videos directly linked, which I suspect are a small subset; most videos are probably played via a <video> element).

Comment 10

[Gerry Chang [:gchang]]

Hi Blake,
Can you help find someone to investigate this?

Comment 11

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

John, 
Please check this bug. 
Thanks.

Comment 12

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

FWIW, I can play the videos on comment 0 with the latest nightly on my Pixel with Android 7.1.1

Comment 13

[John Lin [:jhlin][:jolin]]

I tried play those video files on my Z3C with my local aurora build and all works.

Marco, did you by any chance captured logcat output when error occurred? This could be similar to bug 1340963 but I need the log to confirm it. Thanks a lot.

Comment 14

[Marcia Knous [:marcia]]

Also Marco can you confirm this affects 55? The status is currently set to ?. Thanks.

Comment 15

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

What's the status here for 54? Do we need to disable OOP decoding in 54?

Comment 16

[Marco Castelluccio [:marco]]

I currently don't have access to the device, so I can't test.

Comment 17

[Marco Castelluccio [:marco]]

Keeping needinfo on me so that I don't forget to test as soon as I can.

Comment 18

[John Lin [:jhlin][:jolin]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> What's the status here for 54? Do we need to disable OOP decoding in 54?

 Disabling OOP decoding is no longer an option in 54 because the code has been changed so much that backing out could possibly only make thing worse.

Comment 19

[John Lin [:jhlin][:jolin]]

Attachment: Bug 1350279 - try other codecs when first one failed to create.

Video fails to play on Sony Z3C when the media server is in a state that no hardware codec can be created unless reboot. Fallback to software codec when that happens to workaround the issue.

Review commit: https://reviewboard.mozilla.org/r/135430/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/135430/

Comment 20

[Eugen Sawin [:esawin]]

Comment on attachment 8863633 [details]
Bug 1350279 - try other codecs when first one failed to create.

https://reviewboard.mozilla.org/r/135430/#review138524

Do we know what breaks the media server? Bug 1357287 is a similar issue which we have to work around.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/media/Codec.java:445
(Diff revision 1)
> +                    AsyncCodec codec = AsyncCodecFactory.create(info.getName());
> +                    mName = info.getName();
> +                    return codec;
> +                } catch (IllegalArgumentException e) {
> +                    Log.w(LOGTAG, "unable to create " + info.getName() + ". Try next codec.");
> +                    continue;

Redundant continue.

Comment 21

[John Lin [:jhlin][:jolin]]

Comment on attachment 8863633 [details]
Bug 1350279 - try other codecs when first one failed to create.

https://reviewboard.mozilla.org/r/135430/#review138524

It seems to be some sort of leakage. QCOM driver reported ENOMEM when opening vdec, and I saw a lot of tasks/threads under /proc/(mediaserver pid)/task/ when this happens.

Comment 22

[John Lin [:jhlin][:jolin]]

Comment on attachment 8863633 [details]
Bug 1350279 - try other codecs when first one failed to create.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/135430/diff/1-2/

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf0d3b39a0b1
try other codecs when first one failed to create. r=esawin

Comment 24

[Carsten Book [:Tomcat]]

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

Comment 25

[Gerry Chang [:gchang]]

Hi Marco,
Do you know if the patch fix the issue?

Comment 26

[Marco Castelluccio [:marco]]

I think I will be able to test this later today.

Comment 27

[Marco Castelluccio [:marco]]

It's working now, but I can't reproduce the problem in any Firefox version.
I suppose it's intermittent, so it might be hard to verify.

Comment 28

[Gerry Chang [:gchang]]

Per comment #27, mark 54 as fix-optional.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Did you want to consider this for uplift to Beta still?

Comment 30

[John Lin [:jhlin][:jolin]]

I'd like to uplift this with bug 1364341. Will request approval after that one is landed.

Comment 31

[John Lin [:jhlin][:jolin]]

Comment on attachment 8863633 [details]
Bug 1350279 - try other codecs when first one failed to create.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1257777
[User impact if declined]: on some devices (Z3C), sometimes video won't play unless reboot
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[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?]: it falls back to SW decoder when HW is not usable
[String changes made/needed]: none

Comment 32

[Gerry Chang [:gchang]]

Comment on attachment 8863633 [details]
Bug 1350279 - try other codecs when first one failed to create.

This one should be together with bug 1364341. Beta54+. Should be in 54 beta 12 for mobile.

Comment 33

[Ryan VanderMeulen [:RyanVM]]

Needs a rebased patch Beta.

Comment 34

[John Lin [:jhlin][:jolin]]

Attachment: patch for beta(54)

Rebased patch for beta(54).

Comment 35

[John Lin [:jhlin][:jolin]]

So sorry for the inconvinience. Please use attachment 8871591 [details] [diff] [review] for uplifting. Thanks a lot.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

applying bz://1350279
Fetching... done
Parsing... done
patching file mobile/android/base/java/org/mozilla/gecko/media/Codec.java
Hunk #2 FAILED at 362
Hunk #4 FAILED at 424
2 out of 4 hunks FAILED -- saving rejects to file mobile/android/base/java/org/mozilla/gecko/media/Codec.java.rej
abort: patch failed to apply

Comment 37

[John Lin [:jhlin][:jolin]]

Attachment: patch for beta(54)

This is the correct patch. Apologies for the troubles caused.

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c4c3049f649e

Comment 39

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - Nexus 6 (Android 7.0);
 - Nexus 5 (Android 6.0.1);
 - LG G4 (Android 5.1).

Hello,

 I've tried the scenarios described by Marco in Comment 0 taking into consideration scenarios from Comment 6 and pushing different media files and running them from the device's internal memory. I was not able to reproduce the issue on any of the devices that I tested on. Marking this as verified.

Comment 40

[Marco Castelluccio [:marco]]

I believe this issue is really specific to certain devices with certain drivers (and it's intermittent), so testing on other devices is not enough to verify.

Comment 41

[Bogdan Surd, QA [:bsurd, NI]]

@Marco

I can verify this on multiple other devices if it would help but sadly we do not have any Sony devices here on hand. Changing the status back to fixed until further updates or we manage to get some Sony devices.

Comment 43

[Caspy7]

This reddit user reports that videos are not working for them on 54.

https://www.reddit.com/r/firefox/comments/6k3wqi/unable_to_play_videos_on_firefox_54_for_android/

They include screenshots and info about their phone/OS version.

Is it from this bug?

Comment 44

[Marco Castelluccio [:marco]]

I think so, but I would file a new bug blocking this bug with the details of his phone.

Comment 45

[John Lin [:jhlin][:jolin]]

(In reply to Marco Castelluccio [:marco] from comment #44)
> I think so, but I would file a new bug blocking this bug with the details of
> his phone.

 The report says it happened on a Zenfone 2 ZE551ML and sounds like bug 1374556.