Closed Bug 1350279 Opened 3 years ago Closed 3 years ago

Videos (mp4, WebM) broken on several top websites

Categories

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

53 Branch
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 + disabled
firefox54 + fixed
firefox55 + fixed

People

(Reporter: marco, Assigned: jhlin)

References

Details

(Keywords: regression, site-compat)

Attachments

(2 files, 1 obsolete file)

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.
The videos works for me on a Nexus 5, but they don't work on a Sony Xperia Z3 Compact.
Does this work with "media.android-remote-codec.enabled" set to false? In which case bug 1350209 would "fix" it for 53.
(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.
Keywords: site-compat
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?
[Tracking Requested - why for this release]:
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).
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.".
(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?
See Also: → 1350649
(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).
Depends on: 1350209
Hi Blake,
Can you help find someone to investigate this?
Flags: needinfo?(bwu)
John, 
Please check this bug. 
Thanks.
Flags: needinfo?(bwu)
Flags: needinfo?(jolin)
FWIW, I can play the videos on comment 0 with the latest nightly on my Pixel with Android 7.1.1
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.
Flags: needinfo?(jolin) → needinfo?(mcastelluccio)
Also Marco can you confirm this affects 55? The status is currently set to ?. Thanks.
What's the status here for 54? Do we need to disable OOP decoding in 54?
Flags: needinfo?(jolin)
I currently don't have access to the device, so I can't test.
Flags: needinfo?(mcastelluccio)
Keeping needinfo on me so that I don't forget to test as soon as I can.
Flags: needinfo?(mcastelluccio)
(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.
Flags: needinfo?(jolin)
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.
Attachment #8863633 - Flags: review?(esawin) → review+
Priority: -- → P1
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.
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf0d3b39a0b1
try other codecs when first one failed to create. r=esawin
https://hg.mozilla.org/mozilla-central/rev/cf0d3b39a0b1
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Hi Marco,
Do you know if the patch fix the issue?
I think I will be able to test this later today.
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.
Flags: needinfo?(mcastelluccio)
Assignee: nobody → jolin
Per comment #27, mark 54 as fix-optional.
Did you want to consider this for uplift to Beta still?
Flags: needinfo?(jolin)
I'd like to uplift this with bug 1364341. Will request approval after that one is landed.
Flags: needinfo?(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
Attachment #8863633 - Flags: approval-mozilla-beta?
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.
Attachment #8863633 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs a rebased patch Beta.
Flags: needinfo?(jolin)
Attached patch patch for beta(54) (obsolete) — Splinter Review
Rebased patch for beta(54).
Flags: needinfo?(jolin)
So sorry for the inconvinience. Please use attachment 8871591 [details] [diff] [review] for uplifting. Thanks a lot.
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
Flags: needinfo?(jolin)
This is the correct patch. Apologies for the troubles caused.
Attachment #8871591 - Attachment is obsolete: true
Flags: needinfo?(jolin)
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.
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.
@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.
Duplicate of this bug: 1340963
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?
I think so, but I would file a new bug blocking this bug with the details of his phone.
(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.
You need to log in before you can comment on or make changes to this bug.