Closed Bug 1192539 Opened 4 years ago Closed 4 years ago

crash in libstagefright.so@0x723e6 when trying to play WebM videos on a Galaxy S3 Mini/Alcatel OT 8008D

Categories

(Core :: Audio/Video: Playback, defect, P1, critical)

42 Branch
Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox42 --- wontfix
firefox43 --- wontfix
firefox44 --- wontfix
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- verified
fennec 44+ ---

People

(Reporter: JanH, Assigned: esawin)

References

Details

(Keywords: crash, reproducible)

Crash Data

Attachments

(2 files, 6 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-aebe5071-f47d-4420-b773-4fbc72150808.
=============================================================
STR: Try opening a WebM file, e.g. http://video.webmfiles.org/big-buck-bunny_trailer.webm. Affected builds immediately crash.

Regression range is this: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bdb808a2dd66&tochange=195776f0148b, i.e. bug 1185792.
How does this build works for you ?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a80885d6786

Be interested also to know if you notice an improvement performance-wise. You had mentioned that before it was very stuttery.

Note that this build is pure software decoding. Can't get the HW decoder to work yet.
Flags: needinfo?(jh+bugzilla)
(In reply to Catalin Suciu from comment #1)
> Reproducible on Alcatel OT 8008D (4.1.2)
> https://crash-stats.mozilla.com/report/index/660844fe-2d55-422c-91c3-
> c8b242150811

That crash would have been fixed with bug 1188870
Unfortunately no improvement:

bp-138c4809-120b-4817-9610-6ce992150817
Flags: needinfo?(jh+bugzilla)
this is weird, this worked yesterday for me ; and now it doesn't :( (doesn't crash however)

stagefright shouldn't be used at all for webm with this build. So unless there's a bug in this version of android that makes it crash on mimetype it doesn't support...
Assignee: nobody → giles
Component: Audio/Video → Audio/Video: Playback
Priority: -- → P1
(In reply to [PTO Until 02/Sep] Jean-Yves Avenard [:jya] from comment #3)
> (In reply to Catalin Suciu from comment #1)
> > Reproducible on Alcatel OT 8008D (4.1.2)
> > https://crash-stats.mozilla.com/report/index/660844fe-2d55-422c-91c3-
> > c8b242150811
> 
> That crash would have been fixed with bug 1188870

I'm still able to reproduce this crash on Alcatel OT 8008D (4.1.2)/latest Nightly when trying to open a webm file 
https://crash-stats.mozilla.com/report/index/a5697448-26ff-4869-bf62-c14b42150901
Duplicate of this bug: 1209486
Will take a look...
Assignee: giles → gsquelart
I believe this was fixed with bug 1190379
(In reply to Jean-Yves Avenard [:jya] from comment #9)
> I believe this was fixed with bug 1190379

Good stuff, thanks.

That landed on central on 2015-09-29, I'll monitor crash reports to see if it helped with this bug here.
Flags: needinfo?(gsquelart)
As Anthony requested.
Flags: needinfo?(ajones)
Latest Nightly (2015-10-13) still crashes when playing Webm files on Alcatel OT 8008D (4.1.1)
https://crash-stats.mozilla.com/report/index/b542fe37-c293-406d-b997-7265b2151014
Is this only an issue for WebM? We could make prefer the internal decoder.
Flags: needinfo?(ajones)
James - is this likely to be fixed or should we be preferring the internal decoder?
Flags: needinfo?(snorp)
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #14)
> James - is this likely to be fixed or should we be preferring the internal
> decoder?

Yeah, it's crashing by simply creating the decoder, so we probably want to use the software one. Do you already have a mechanism for doing that?
Flags: needinfo?(snorp) → needinfo?(ajones)
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #15)
> (In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #14)
> > James - is this likely to be fixed or should we be preferring the internal
> > decoder?
> 
> Yeah, it's crashing by simply creating the decoder, so we probably want to
> use the software one. Do you already have a mechanism for doing that?

We do.

the Android PDM's SupportsMimeType should return false for the VP8 or VP9 codec (ideally only doing so on machines we know will crash)

The problem is the AndroidDecoderModule::SupportsMimeType test if a codec is supported by creating a decoder and checking that it succeeded. Maybe defined a blacklist, or simply always return false without attempting to create the decoder.
Flags: needinfo?(ajones)
James - do you know what Android phones support VP9 hardware decoding? We could use libvpx for all Android versions prior.
Flags: needinfo?(snorp)
We do test that the device supports vp9; and only do so if it says it does :(
(In reply to Gerald Squelart [:gerald] from comment #10)
> (In reply to Jean-Yves Avenard [:jya] from comment #9)
> > I believe this was fixed with bug 1190379
> 
> Good stuff, thanks.
> 
> That landed on central on 2015-09-29, I'll monitor crash reports to see if
> it helped with this bug here.

I still see reports with the apparent same error (SEGV @0x8) with 42.0 built on 20151029024147. E.g.:
https://crash-stats.mozilla.com/report/index/568f95b9-5151-49a9-a2dc-a8cc52151107
There are about 250 reports involving libstagefright and webm in the past week:
https://crash-stats.mozilla.com/search/?signature=~libstagefright&url=~webm&_facets=signature#facet-signature

Reassigning to nobody, as I'm not actively working on it and don't have the hardware to debug (yet?)
Assignee: gsquelart → nobody
Flags: needinfo?(gsquelart)
Can confirm the bug is still there with 42.0
Galaxy i9100, stock android 4.1.2
100% reproducable
was not there 1 or 2 weeks ago.
tracking-fennec: --- → ?
Has STR: --- → yes
Keywords: reproducible
Summary: crash in libstagefright.so@0x723e6 when trying to play WebM videos on a Galaxy S3 Mini → crash in libstagefright.so@0x723e6 when trying to play WebM videos on a Galaxy S3 Mini/Alcatel OT 8008D
Assignee: nobody → snorp
tracking-fennec: ? → 44+
Brad mentions that we could test using software decoding for WebM as a way to uplift to release
[@ libstagefright.so@0x65a4e ] / bp-130bca50-1169-4b68-a340-ddc832160110 with Nexus S and Android 4.1.2 + Firefox 46.0a1 20160110
Crash Signature: [@ libstagefright.so@0x723e6] → [@ libstagefright.so@0x723e6] [@ libstagefright.so@0x65a4e ]
Assignee: snorp → esawin
Samsung S3 Mini's SoC NovaThor U8420 doesn't support hardware VPx decoding, by blacklisting unsupported devices (and falling back to software decoders), we'll avoid the crashes.

We can extend the blacklist with more popular unsupported devices when needed.
Attachment #8711047 - Flags: review?(snorp)
When debugging, it's helpful if you can log in static functions, too.
Attachment #8711051 - Flags: review?(snorp)
Attachment #8711049 - Attachment is obsolete: true
Attachment #8711049 - Flags: review?(snorp)
Attachment #8711075 - Flags: review?(snorp)
Comment on attachment 8711047 [details] [diff] [review]
0001-Bug-1192539-1.1-Add-VPx-hardware-decoding-support-bl.patch

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

::: widget/android/GfxInfo.cpp
@@ +582,5 @@
> +
> +    if (aFeature == FEATURE_VP8_HW_DECODE) {
> +      NS_LossyConvertUTF16toASCII model(mModel);
> +      bool isBlocklisted =
> +        model.Equals("GT-I8190", nsCaseInsensitiveCStringComparator()) ||

Maybe add a reference to this bug, so we know why it's blocked.
Attachment #8711047 - Flags: review?(snorp) → review+
Attachment #8711051 - Flags: review?(snorp) → review+
Comment on attachment 8711075 [details] [diff] [review]
0002-Bug-1192539-2.2-Don-t-attempt-to-create-VPx-decoder-.patch

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

::: dom/media/platforms/android/AndroidDecoderModule.cpp
@@ +254,4 @@
>      return true;
>    }
>  
> +  auto GetFeatureStatus = [](int32_t aFeature) -> bool

This doesn't really make sense as a lambda. Just use a normal static function.
Attachment #8711075 - Flags: review?(snorp) → review-
Instead of blacklisting, this patch allows us to test for native support using MediaCodecList.

Alternative function name: GetHWDecoderCapabilityForMimeType.
Attachment #8711047 - Attachment is obsolete: true
Attachment #8711147 - Flags: review?(snorp)
Replace the opportunistic decoder creation with an actual support check.
Attachment #8711075 - Attachment is obsolete: true
Attachment #8711148 - Flags: review?(snorp)
Comment on attachment 8711147 [details] [diff] [review]
0001-Bug-1192539-1.2-Add-MediaCodecList-JNI-query-support.patch

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

::: mobile/android/base/java/org/mozilla/gecko/GeckoAppShell.java
@@ +992,5 @@
>        return HardwareCodecCapabilityUtils.getHWDecoderCapability();
>      }
>  
> +    @WrapForJNI(allowMultithread = true, stubName = "FindDecoderCodecInfoForMimeType")
> +    static boolean findDecoderCodecInfoForMimeType(final String aMimeType) {

Please don't add more stuff to GeckoAppShell. Just put the JNI call in HardwareCodecCapabilityUtils.

::: widget/android/AndroidBridge.cpp
@@ +444,4 @@
>  }
>  
>  bool
> +AndroidBridge::FindDecoderCodecInfoForMimeType(const nsACString& aMimeType) const

Don't add stuff to AndroidBridge, just let the caller use the generated wrapper directly.
Attachment #8711147 - Flags: review?(snorp) → review-
Comment on attachment 8711148 [details] [diff] [review]
0002-Bug-1192539-2.3-Use-MediaCodecList-to-check-for-deco.patch

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

This one is ok after you fix it up for the changes in the other patch.
Attachment #8711148 - Flags: review?(snorp) → review+
Addressed comments.
Attachment #8711147 - Attachment is obsolete: true
Attachment #8711867 - Flags: review?(snorp)
Addressed comments, carrying over r+.
Attachment #8711148 - Attachment is obsolete: true
Attachment #8711868 - Flags: review+
Comment on attachment 8711867 [details] [diff] [review]
0001-Bug-1192539-1.3-Add-MediaCodecList-JNI-query-support.patch

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

Thanks
Attachment #8711867 - Flags: review?(snorp) → review+
https://hg.mozilla.org/mozilla-central/rev/0fa9ed048fbe
https://hg.mozilla.org/mozilla-central/rev/8da3aa84b643
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
No longer crashing with build 20160128030208 on a S3 Mini, thanks for fixing this.
Status: RESOLVED → VERIFIED
This is fixed for Alcatel One Touch too on Nightly . 
Will this be uplifted in time for 45 transition to Release? 

PS: I will leave it Resolve till we can verify it on all branches ( as per QA process) and set the flag to verified.
Status: VERIFIED → RESOLVED
Closed: 4 years ago4 years ago
Attachment #8711051 - Attachment is obsolete: true
Comment on attachment 8711867 [details] [diff] [review]
0001-Bug-1192539-1.3-Add-MediaCodecList-JNI-query-support.patch

Approval Request Comment
[Feature/regressing bug #]: media playback
[User impact if declined]: crashes on some devices when trying to play media
[Describe test coverage new/current, TreeHerder]: has been on Nightly for 2 weeks
[Risks and why]: low, simple code change, no reported issues on Nightly
[String/UUID change made/needed]: none
Attachment #8711867 - Flags: approval-mozilla-beta?
Attachment #8711867 - Flags: approval-mozilla-aurora?
Attachment #8711868 - Flags: approval-mozilla-beta?
Attachment #8711868 - Flags: approval-mozilla-aurora?
Comment on attachment 8711867 [details] [diff] [review]
0001-Bug-1192539-1.3-Add-MediaCodecList-JNI-query-support.patch

Fix a crash, taking it.
Attachment #8711867 - Flags: approval-mozilla-beta?
Attachment #8711867 - Flags: approval-mozilla-beta+
Attachment #8711867 - Flags: approval-mozilla-aurora?
Attachment #8711867 - Flags: approval-mozilla-aurora+
Comment on attachment 8711868 [details] [diff] [review]
0002-Bug-1192539-2.4-Use-MediaCodecList-to-check-for-deco.patch

Should be in 45 beta 5.
Attachment #8711868 - Flags: approval-mozilla-beta?
Attachment #8711868 - Flags: approval-mozilla-beta+
Attachment #8711868 - Flags: approval-mozilla-aurora?
Attachment #8711868 - Flags: approval-mozilla-aurora+
Duplicate of this bug: 1244485
Duplicate of this bug: 1235614
You need to log in before you can comment on or make changes to this bug.