This bug was filed from the Socorro interface and is report bp-bba2f311-18b8-4c1d-8861-cc3562150728. ============================================================= Using a Samsung Galaxy S3 Mini, affected builds of Firefox crash when visiting http://podiobooks.com/title/a-coffee-crusade/ Regression range is this: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=192cf71f768f&tochange=06340c449def which points to the changes made in bug 1148102. There's also an old bug (bug 812881) for this crash signature, but I don't know in how far it's relevant for the current problem.

Code's from bug 1148102 isn't active and is turned off by default The crash is deep within Android's stagefright ; we have no control over this. Jan, some ideas?

Unfortunately not. This came up while confirming a bug report on mozillazine which turned into bug 1188873.

That page has one audio element with an mp3[1], dont see how it could be related to webm. The only place would be the hookup part. Possibly: CreateDecoder calls SupportsMimeType now, AndroidDecoderModule::SupportsMimeType would do something like this: return static_cast<bool>(mozilla::CreateDecoder(aMimeType)); -> mozilla::CreateDecoder: NS_ENSURE_SUCCESS(MediaCodec::CreateDecoderByType(PromiseFlatCString(aMimeType).get(), &codec), nullptr); -- 1 - <audio id="jp_audio_0" preload="metadata" src="http://media.podiobooks.com/coffeecrusade/PB-CoffeeCrusade-01.mp3"></audio>

If indeed the problem is that MediaCodec::CreateDecoderByType crashes, it might be an option to handle audio/mpeg like video/mp4 and return true without trying to create a decoder in AndroidDecoderModule::SupportsMimeType.

Comment on attachment 8640789 [details] [diff] [review] Bug-1188870-AndroidDecoderModule-supports-MP3.patch Review of attachment 8640789 [details] [diff] [review]: ----------------------------------------------------------------- sure, but how is that any different to what was before the webm patch ?

oh, I see... SupportsMimeType is called in CreateDecoder() However, Right before CreateDecoder() , in the MediaDecoderReader you have NS_ENSURE_TRUE(IsSupportedAudioMimeType(mInfo.mAudio.mMimeType), false); which also calls SupportsMimeType() . So it would be the twice in a row call that would cause the crash ? Likely another android bug being revealed (like bug 1186965).

Comment on attachment 8640789 [details] [diff] [review] Bug-1188870-AndroidDecoderModule-supports-MP3.patch Review of attachment 8640789 [details] [diff] [review]: ----------------------------------------------------------------- Passing the review to snorp. Does all android support mp3, like they all support AAC and h264 ? seems reasonable to think so

Comment on attachment 8640789 [details] [diff] [review] Bug-1188870-AndroidDecoderModule-supports-MP3.patch Review of attachment 8640789 [details] [diff] [review]: ----------------------------------------------------------------- This looks right, but we are supposedly already using AndroidDecoderModule for MP3. I'm confused how it could've ever worked without this patch. Passing review to Eugen.

Comment on attachment 8640789 [details] [diff] [review] Bug-1188870-AndroidDecoderModule-supports-MP3.patch Review of attachment 8640789 [details] [diff] [review]: ----------------------------------------------------------------- I don't have the device to reproduce the crash, so I can't say whether that's the right fix, but it looks good to me. Audio playback remains broken, however that seems to a long-standing issue and is handled in bug 1188873. @snorp: AndroidDecoderModule::SupportsMimeType returns true for "audio/mpeg" because mozilla::CreateDecoder should succeed in this case.

STR from Bug 812881 comment #74 crashes my Firefox Beta for Android 40b10. Crash report is at https://crash-stats.mozilla.com/report/index/ffb47ed7-5657-404d-b278-8a5702150806

Any particular reason this has stalled? If it needs testing, I'm happy to test whether this patch has actually fixed the crash as long as someone can provide me with a local/try build.

Here is a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a25dfe6913e8 not sure how that could fix anything ... but time will tell.

Mystery as it may be, but I'm happy to report that - latest Incoming build is still broken - that try build looks fixed, i.e. it doesn't crash neither on that audiobook site, nor upon MP3 playback in general

All this indicates to me is that the current code is wrong, and things only work with AndroidDecoderModule::CreateDecoder() isn't called to check if the mimetype is supported. Looking at the android source code, you can see that MediaCodec::CreateByType will return an initialized MediaCodec instance ; in the MediaCodec destructor; there's CHECK_EQ(mState, UNINITIALIZED); which will assert if the MediaCodec isn't in uninitialized stated. The decoder is initialized asynchronously; as such the code is racy. It's possible that the destructor will be called whil mState isn't UNINITIALIZED and therefore will crash. The code used to be only used for H264 and AAC for which the call to CreateDecoder is bypassed. I'll try to wrap a quick work around.

Solution was simple actually: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf14e5ce8a58 Could you give this a try once compilation is done? Also, could you check that webm player works for you? This is to test for using HW acceleration works on Android thanks to bug 1190379. You can find some webm samples there: http://www.webmfiles.org/demo-files/ I don't know if your S3 mini supports hardware VP8/VP9 ; but at least this will also test that the patch for this bug properly handles it. So regardless, it will be useful information

Okay, regarding MP3 playback, the second Try build is working fine as well. Regarding WebM, I've tried playing the Big Buck Bunny trailer, however both the Incoming and your second Try build are crashing after opening the file/pressing the playback button on the embedded player: bp-2b6a53db-fd1c-40b4-87a6-d33f32150808 for an example from the Try build and bp-8ffdb78e-1df3-4c3c-b1e5-5aeae2150808 for the Incoming build I've been using On FF39, playback is still working normally, albeit with a few stutters. Should I file a new bug and get a regression range for those WebM crashes, or do you already have something on file?

What do you call the incoming build? webm with HW decoding isn't enabled yet. so just that may be the problem. That the problem is fixed for MP3 indicates that the problem was indeed that the decoder wasn't released and triggered the assert in stagefright

On second thought, if it's crashing with a standard nightly build ; then yes please lodge a bug. Likely a regression due to bug 1185792. Would be good to confirm.

The comment in android's source code is rather explicit: // Client MUST call release before releasing final reference to this // object.

Comment on attachment 8640789 [details] [diff] [review] Bug-1188870-AndroidDecoderModule-supports-MP3.patch This is just a work around the real problem.

I meant the build from mozilla-incoming I had used to make sure that without your patch applied, Firefox was still crashing for MP3 files. Regarding the crash, will do.

Comment on attachment 8645356 [details] [diff] [review] Properly release decoder before destruction. Review of attachment 8645356 [details] [diff] [review]: ----------------------------------------------------------------- Good catch

Verified as working on yesterday's Nightly (build 20150810030205). Thanks very much for fixing this. @xabolcs: If this hasn't fixed your crash, and if your crash doesn't have the same regression window, it's probably best to open a new bug for it.