Closed Bug 1082550 Opened 10 years ago Closed 10 years ago

Regression: Android MP4/MP3 video/audio playback broken

Categories

(Core :: Audio/Video, defect)

35 Branch
ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla36
Tracking Status
firefox33 --- unaffected
firefox34 --- unaffected
firefox35 --- verified
firefox36 --- verified
fennec 35+ ---

People

(Reporter: CristinaM, Assigned: snorp)

References

()

Details

(Keywords: regression, reproducible)

Attachments

(1 file)

Environment:
Device: Asus Transformer Pad TF300T Android 4.2.1
Build: Firefox for Android 36.0a1 (2014-10-13)

Steps to reproduce:
1. Launch Fennec;
2. Go to http://techslides.com/demos/sample-videos/small.mp4.

Expected result:
Video starts playing.

Actual result:
'Video can't be played because the file is corrupt.' is displayed.

Regression window:
Last good: 2014-10-12
First bad: 2014-10-13

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=44168a7af20d&tochange=f547cf19d104
Is there no Android video playback test?

E/OMX-VDEC-1080P(  269): Extension: OMX.google.android.index.storeMetaDataInBuffers not implemented
E/OMXNodeInstance(  269): OMX_GetExtensionIndex OMX.google.android.index.storeMetaDataInBuffers failed
E/OMX-VDEC-1080P(  269): Extension: OMX.google.android.index.prepareForAdaptivePlayback not implemented
W/OMXNodeInstance(  269): OMX_GetExtensionIndex OMX.google.android.index.prepareForAdaptivePlayback failed
E/OMX-VDEC-1080P(  269): 
E/OMX-VDEC-1080P(  269):  WARNING: failed in ioctl read next msg (-1)
E/OMXMaster( 5044): A component of name 'OMX.qcom.audio.decoder.aac' already exists, ignoring this one.
W/linker  ( 5044): libaricentomxplugin.so has text relocations. This is wasting memory and is a security risk. Please fix.
I/AndroidMediaPluginHost( 4164): Loading OMX Plugin: libomxpluginkk.so
W/GeckoLinker( 4164): /data/app/org.mozilla.fennec-2.apk!/assets/libomxpluginkk.so: unhandled flags #8 not handled
I/AndroidMediaPluginHost( 4164): OMX plugin successfully loaded
Blocks: 1060179
tracking-fennec: --- → ?
Flags: in-testsuite?
Summary: Mp4 videos are broken → Regression: MP4 Mp4 videos are broken
Component: General → Video/Audio
Product: Firefox for Android → Core
Version: Trunk → 35 Branch
Summary: Regression: MP4 Mp4 videos are broken → Regression: MP4 video playback broken
I can repro on my HTC One. But my Linux VM spontaneously combusted, so it's a hassle to work on this. :(

Let's see if backing out the android changes from f14397543760 fix the issue.

https://tbpl.mozilla.org/?tree=Try&rev=d846567fcaf0

(In reply to Aaron Train [:aaronmt] from comment #1)
> Is there no Android video playback test?

Yes. Odd that our mochitests didn't catch this.
Summary: Regression: MP4 video playback broken → Regression: Android MP4 video playback broken
(In reply to Chris Pearce (:cpearce) from comment #3)
> I can repro on my HTC One. But my Linux VM spontaneously combusted, so it's
> a hassle to work on this. :(
> 
> Let's see if backing out the android changes from f14397543760 fix the issue.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=d846567fcaf0


Video doesn't work in this build for me either.
Assignee: nobody → cpearce
tracking-fennec: ? → 35+
Summary: Regression: Android MP4 video playback broken → Regression: Android MP4/MP3 video/audio playback broken
I see an interesting log message when trying to play MP3

E/        (25650): Failed to open file 'zqGQe9COdwlqZEOK'. (No such file or directory)

What.
Ok, that string is supposed to be the full url to the resource server. It looks like change this change busted that:

https://hg.mozilla.org/mozilla-central/rev/f14397543760
I am not sure what the reason was for this change, but it's busted. This is a pretty obvious fix that we need immediately.
Assignee: cpearce → snorp
Status: NEW → ASSIGNED
Attachment #8506971 - Flags: review?(blassey.bugs)
Attachment #8506971 - Flags: review?(blassey.bugs) → review+
Thanks fixing this snorp.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #8)
> Ok, that string is supposed to be the full url to the resource server. It
> looks like change this change busted that:
> 
> https://hg.mozilla.org/mozilla-central/rev/f14397543760

Odd, I tried backing that out via TryServer, but video still didn't work on my HTC One in that build... I will test again once that merges Nightly... But I must have done something wrong...

snorp: It's pretty bad that our mochitests didn't catch this regression, any idea why?
(In reply to Chris Pearce (:cpearce) from comment #11)
> Thanks fixing this snorp.
> 
> (In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #8)
> > Ok, that string is supposed to be the full url to the resource server. It
> > looks like change this change busted that:
> > 
> > https://hg.mozilla.org/mozilla-central/rev/f14397543760
> 
> Odd, I tried backing that out via TryServer, but video still didn't work on
> my HTC One in that build... I will test again once that merges Nightly...
> But I must have done something wrong...

Maybe stuff was busted some other way with the entire patch backed out? Not sure. You can try the inbound build to verify that this patch worked. I just tried the one here: http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android/1413559067/fennec-36.0a1.en-US.android-arm.apk

> 
> snorp: It's pretty bad that our mochitests didn't catch this regression, any
> idea why?

It is pretty bad, yeah. I think most (all?) of the media tests are ok with things just not working, because we don't have incomplete codec support across platforms, especially where mp3/mp4 are concerned. The one place we do actually check (test_can_play_type_mpeg.html) is apparently busted on Android. Still figuring that one out.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #12)
> > snorp: It's pretty bad that our mochitests didn't catch this regression, any
> > idea why?
> 
> It is pretty bad, yeah. I think most (all?) of the media tests are ok with
> things just not working, because we don't have incomplete codec support
> across platforms, especially where mp3/mp4 are concerned. The one place we
> do actually check (test_can_play_type_mpeg.html) is apparently busted on
> Android. Still figuring that one out.

It's not disabled on Android in the manifest.ini file: http://mxr.mozilla.org/mozilla-central/source/content/media/test/mochitest.ini#332
It looks like have_mp4 is false
http://mxr.mozilla.org/mozilla-central/source/content/media/test/test_can_play_type_mpeg.html?force=1#133

The media.gstreamer.enabled pref is not set, it seems, in Fennec:
http://mxr.mozilla.org/mozilla-central/source/content/media/test/manifest.js#893

According to bug 422540, comment 286, the Gstreamer backend should work for all platforms, so I guess on Android too.

Maybe these kinds of lines: if (oldGStreamer !== undefined)
..should be removed in manifest.js?
(I was trying to changed this accidentally, actually with the patch in bug 902686)

In general, I would prefer it if mochitests wouldn't make platform specific decisions, but rather the manifest.ini files. It would make the mochitests easier to read.
(In reply to Martijn Wargers [:mwargers] (QA) from comment #13)
> 
> The media.gstreamer.enabled pref is not set, it seems, in Fennec:
> http://mxr.mozilla.org/mozilla-central/source/content/media/test/manifest.
> js#893
> 
> According to bug 422540, comment 286, the Gstreamer backend should work for
> all platforms, so I guess on Android too.

The Gstreamer backend won't work on Android, unless the particular Android build is using Gstreamer for media rather than libstagefright. I don't know of any Android systems that are doing this in production.
(In reply to cajbir (:cajbir) from comment #14)
> (In reply to Martijn Wargers [:mwargers] (QA) from comment #13)
> The Gstreamer backend won't work on Android, unless the particular Android
> build is using Gstreamer for media rather than libstagefright. I don't know
> of any Android systems that are doing this in production.

Ok, then I guess in a different way have_mp4 has to return true in Android in test_can_play_type_mpeg.html. Perhaps just add some extra UA sniffing for it?
Confirming i'm seeing this on aurora 35.0a2 / galaxy tab, using mozilla-provided binaries.
Bug 1084441 tries to address the testing situation for MP4. I have a patch there which tries to set haveMp4 appropriately.
Other data point - about:buildconfig says my build comes from https://hg.mozilla.org/releases/mozilla-aurora/rev/c278b3312388
https://hg.mozilla.org/mozilla-central/rev/5951468e1e15
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
serious enough to warrant backport to aurora ?
(In reply to Landry Breuil (:gaston) from comment #22)
> serious enough to warrant backport to aurora ?

Yes please.
Flags: needinfo?(snorp)
Comment on attachment 8506971 [details] [diff] [review]
Fix media decoding on Android

Approval Request Comment
[Feature/regressing bug #]: 1060179 
[User impact if declined]: virtually all media decoding is busted (WebM probably works)
[Describe test coverage new/current, TBPL]: none, which is how we got here
[Risks and why]: Low risk
[String/UUID change made/needed]: None
Flags: needinfo?(snorp)
Attachment #8506971 - Flags: approval-mozilla-aurora?
I'm not sure if this is related or not, but using the latest nightly with that bug fixed on the same device, i can play mp4 videos fine now, but i have no controls overlayed on the video when tapping it - no progressbar/timer/fullscreen button.
(In reply to Landry Breuil (:gaston) from comment #25)
> I'm not sure if this is related or not, but using the latest nightly with
> that bug fixed on the same device, i can play mp4 videos fine now, but i
> have no controls overlayed on the video when tapping it - no
> progressbar/timer/fullscreen button.

Yeah we filed that as bug 1086996. We're bad at video.
Attachment #8506971 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed in builds:
Firefox for Android 36.0a1 (2014-11-27)
Firefox for Android 35.0a2 (2014-11-27)
Device: Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.