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

Status: RESOLVED FIXED

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

Description

[Jan Henning [:JanH]]

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.

Comment 1

[Catalin Suciu [:csuciu]]

Reproducible on Alcatel OT 8008D (4.1.2)
https://crash-stats.mozilla.com/report/index/660844fe-2d55-422c-91c3-c8b242150811

Comment 2

[Jean-Yves Avenard [:jya]]

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.

Comment 3

[Jean-Yves Avenard [:jya]]

(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

Comment 4

[Jan Henning [:JanH]]

Unfortunately no improvement:

bp-138c4809-120b-4817-9610-6ce992150817

Comment 5

[Jean-Yves Avenard [:jya]]

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...

Comment 6

[Catalin Suciu [:csuciu]]

(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

Comment 8

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Will take a look...

Comment 9

[Jean-Yves Avenard [:jya]]

I believe this was fixed with bug 1190379

Comment 10

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(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.

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

As Anthony requested.

Comment 12

[Catalin Suciu [:csuciu]]

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

Comment 13

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Is this only an issue for WebM? We could make prefer the internal decoder.

Comment 14

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

James - is this likely to be fixed or should we be preferring the internal decoder?

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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?

Comment 16

[Jean-Yves Avenard [:jya]]

(In reply to James Willcox (:snorp) ([e-mail]) 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.

Comment 17

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

James - do you know what Android phones support VP9 hardware decoding? We could use libvpx for all Android versions prior.

Comment 18

[Jean-Yves Avenard [:jya]]

We do test that the device supports vp9; and only do so if it says it does :(

Comment 19

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

(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?)

Comment 20

[degalaxys2]

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.

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

Brad mentions that we could test using software decoding for WebM as a way to uplift to release

Comment 22

[Ioana Chiorean]

This is always reproducible on Alcatel OT 8008D (4.1.2) using Fennec 44 Beta 1:
https://crash-stats.mozilla.com/report/index/52b833a8-45ab-4f01-b5b9-647a52151217
https://crash-stats.mozilla.com/report/index/6758ed98-cd13-425f-8d23-00c0c2151217

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

[@ libstagefright.so@0x65a4e ] / bp-130bca50-1169-4b68-a340-ddc832160110 with Nexus S and Android 4.1.2 + Firefox 46.0a1 20160110

Comment 24

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1192539-1.1-Add-VPx-hardware-decoding-support-bl.patch

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.

Comment 25

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1192539-2.1-Don-t-attempt-to-create-VPx-decoder-.patch

Comment 26

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1192539-3.1-Allow-logging-using-LOG-within-stati.patch

When debugging, it's helpful if you can log in static functions, too.

Comment 27

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1192539-2.2-Don-t-attempt-to-create-VPx-decoder-.patch

Comment 28

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 29

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 30

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1192539-1.2-Add-MediaCodecList-JNI-query-support.patch

Instead of blacklisting, this patch allows us to test for native support using MediaCodecList.

Alternative function name: GetHWDecoderCapabilityForMimeType.

Comment 31

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1192539-2.3-Use-MediaCodecList-to-check-for-deco.patch

Replace the opportunistic decoder creation with an actual support check.

Comment 32

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 33

[James Willcox (:snorp) ([e-mail]) (he/him)]

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.

Comment 34

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1192539-1.3-Add-MediaCodecList-JNI-query-support.patch

Addressed comments.

Comment 35

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1192539-2.4-Use-MediaCodecList-to-check-for-deco.patch

Addressed comments, carrying over r+.

Comment 36

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

Comment 37

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0fa9ed048fbe
https://hg.mozilla.org/integration/mozilla-inbound/rev/8da3aa84b643

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0fa9ed048fbe
https://hg.mozilla.org/mozilla-central/rev/8da3aa84b643

Comment 39

[Jan Henning [:JanH]]

No longer crashing with build 20160128030208 on a S3 Mini, thanks for fixing this.

Comment 40

[Ioana Chiorean]

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.

Comment 41

[Eugen Sawin [:esawin]]

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

Comment 42

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8711867 [details] [diff] [review]
0001-Bug-1192539-1.3-Add-MediaCodecList-JNI-query-support.patch

Fix a crash, taking it.

Comment 43

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/92255a6c239b
https://hg.mozilla.org/releases/mozilla-aurora/rev/8e11eaf0cafb

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dfdbdf2f2a8c
https://hg.mozilla.org/releases/mozilla-beta/rev/a369f4cef498

Comment 46

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dfdbdf2f2a8c
https://hg.mozilla.org/releases/mozilla-beta/rev/a369f4cef498