812881 - crash in nsMediaPluginHost::DestroyDecoder @ libsomxcore.so@0x1826, mainly on Samsung Galaxy SIII, with qcom/samsunggolden/espresso/espresso10 hw running JB

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Scoobidiver (away)]

The affected devices are:
* samsung SPH-L710
* samsung SGH-T999
* samsung SCH-I535
* samsung SGH-I747

Signature 	libsomxcore.so@0x1826 More Reports Search
UUID	75975562-22f5-4b76-b757-57e082121118
Date Processed	2012-11-18 01:21:24
Uptime	30
Last Crash	42 seconds before submission
Install Age	30.4 minutes since version was first installed.
Install Time	2012-11-18 00:50:53
Product	FennecAndroid
Version	18.0a2
Build ID	20121116042015
Release Channel	aurora
OS	Android
OS Version	0.0.0 Linux 3.0.31-329968 #1 SMP PREEMPT Tue Oct 16 22:37:51 KST 2012 armv7l samsung/d2spr/d2spr:4.1.1/JRO03L/L710VPBLJ7:user/release-keys
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'Qualcomm -- Adreno (TM) 225 -- OpenGL ES 2.0 2184622 -- Model: SPH-L710, Product: d2spr, Manufacturer: samsung, Hardware: qcom'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
samsung SPH-L710
samsung/d2spr/d2spr:4.1.1/JRO03L/L710VPBLJ7:user/release-keys
Processor Notes 	This dump is too long and has triggered the automatic truncation routine
EMCheckCompatibility	True
Adapter Vendor ID	Qualcomm
Adapter Device ID	Adreno (TM) 225
Device	samsung SPH-L710
Android API Version	16 (REL)
Android CPU ABI	armeabi-v7a

Bugzilla - Report this bug in FennecAndroid, Core, Plug-Ins, or Toolkit
Crashing Thread
Frame 	Module 	Signature 	Source
0 	libc.so 	libc.so@0xe3dc 	
1 	libsomxcore.so 	libsomxcore.so@0x1826 	
2 	libsomxcore.so 	libsomxcore.so@0x31e2 	
3 	libsomxcore.so 	libsomxcore.so@0x2102 	
4 	libsomxcore.so 	libsomxcore.so@0x24fe 	
5 	libsomxcore.so 	libsomxcore.so@0x252a 	
6 	libstagefright_omx.so 	libstagefright_omx.so@0xc187 	
7 	libstagefright_omx.so 	libstagefright_omx.so@0xc1bb 	
8 	libstagefright_omx.so 	libstagefright_omx.so@0xc203 	
9 	libstagefright_omx.so 	libstagefright_omx.so@0xaef3 	
10 	libstagefright_omx.so 	libstagefright_omx.so@0xaf7f 	
11 	libutils.so 	libutils.so@0xef11 	
12 	libstagefright.so 	libstagefright.so@0x54e7d 	
13 	libstagefright.so 	libstagefright.so@0x81a61 	
14 	libstagefright.so 	libstagefright.so@0x81aa3 	
15 	libutils.so 	libutils.so@0xef11 	
16 	libstagefright.so 	libstagefright.so@0x8992b 	
17 	libc.so 	libc.so@0x1582f 	
18 	libstagefright_foundation.so 	libstagefright_foundation.so@0x9dab 	
19 	libc.so 	libc.so@0x1582f 	
20 	libstagefright.so 	libstagefright.so@0x899cf 	
21 	libutils.so 	libutils.so@0xef11 	
22 	libomxplugin.so 	android::sp<android::MediaSource>::~sp 	StrongPointer.h:149
23 	libomxplugin.so 	OmxPlugin::OmxDecoder::~OmxDecoder 	OmxPlugin.cpp:223
24 	libomxplugin.so 	OmxPlugin::DestroyDecoder 	OmxPlugin.cpp:843
25 	libxul.so 	nsMediaPluginHost::DestroyDecoder 	nsMediaPluginHost.cpp:183
26 	libxul.so 	nsMediaPluginReader::ResetDecode 	nsMediaPluginReader.cpp:105
27 	libxul.so 	nsMediaPluginReader::~nsMediaPluginReader 	nsMediaPluginReader.cpp:30
28 	libxul.so 	nsMediaPluginReader::~nsMediaPluginReader 	nsMediaPluginReader.cpp:31
29 	libxul.so 	nsBuiltinDecoderStateMachine::~nsBuiltinDecoderStateMachine 	nsAutoPtr.h:38
30 	libxul.so 	nsBuiltinDecoderStateMachine::~nsBuiltinDecoderStateMachine 	nsBuiltinDecoderStateMachine.cpp:454
31 	libxul.so 	nsRunnable::Release 	nsThreadUtils.cpp:30
32 	libxul.so 	nsCOMPtr_base::assign_assuming_AddRef 	nsCOMPtr.h:440
33 	libxul.so 	nsCOMPtr_base::assign_with_AddRef 	nsCOMPtr.cpp:49
34 	libxul.so 	nsDecoderDisposeEvent::Run 	nsCOMPtr.h:622
35 	libxul.so 	nsThread::ProcessNextEvent 	nsThread.cpp:620
36 	libxul.so 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:220
37 	libxul.so 	mozilla::ipc::MessagePump::Run 	MessagePump.cpp:82
38 	libxul.so 	MessageLoop::RunInternal 	message_loop.cc:215
39 	libxul.so 	MessageLoop::Run 	message_loop.cc:208
40 	libxul.so 	nsBaseAppShell::Run 	nsBaseAppShell.cpp:163
41 	libxul.so 	nsAppStartup::Run 	nsAppStartup.cpp:290
42 	libxul.so 	XREMain::XRE_mainRun 	nsAppRunner.cpp:3794
43 	libxul.so 	XREMain::XRE_main 	nsAppRunner.cpp:3860
44 	libxul.so 	XRE_main 	nsAppRunner.cpp:3935 

More reports at:
https://crash-stats.mozilla.com/report/list?signature=libsomxcore.so%400x1826

Comment 1

[Robert Kaiser]

This is rising in 17.0 data, now at #16 in yesterday's data there. Chris, I've seen you worked on the OMX plugin, could you take a look at this?

Comment 2

[Chris Peterson [:cpeterson]]

CC'ing doublec. I'm busy on B2G right now.

This crash looks like a decoder refcount bug or a double free.

Comment 3

[Robert Kaiser]

(In reply to Chris Peterson (:cpeterson) from comment #2)
> CC'ing doublec. I'm busy on B2G right now.

Ping? Can you please look at this? It's now topcrash #13 on 17.0 for Android.

Comment 4

[cajbir (:cajbir)]

The general approach for crashes on specific devices is for the android team to blacklist the phone via the blacklist/whitelist that was added recently while a device is sourced so the bug can be worked on. I suggest we do this. For the bug to be fixed we need to identify a device where it happens and send one to me or Edwin here in NZ.

Comment 5

[Scoobidiver (away)]

The latest correlation per device for the last day is:
Device                  GPU                Hardware      Number of crashes
Samsung SPH-L710 	Adreno (TM) 225    qcom          139
Samsung SGH-T999 	Adreno (TM) 225    qcom          52
Samsung SGH-I747M 	Adreno (TM) 225    qcom          27
Samsung GT-I8190 	Mali-400 MP        samsunggolden 22
Samsung SGH-I747 	Adreno (TM) 225    qcom          9

Maybe more devices are impacted but the FennecAndroid Release 17 Weekly Device Crash Report file doesn't exist.
I don't know if blocklisting Adreno (TM) 225/qcom and Mali-400 MP/samsunggolden will be too conservative (too many devices blocked).

Comment 6

[Alex Keybl [:akeybl]]

(In reply to Chris Double (:doublec) from comment #4)
> The general approach for crashes on specific devices is for the android team
> to blacklist the phone via the blacklist/whitelist that was added recently
> while a device is sourced so the bug can be worked on. I suggest we do this.
> For the bug to be fixed we need to identify a device where it happens and
> send one to me or Edwin here in NZ.

Are you suggesting we block h.264 playback on the Galaxy S3? Or am I misunderstanding something here...

That would seem to be a non-starter.

Comment 7

[Alex Keybl [:akeybl]]

Whoops, not yet assigning to Brad, since we don't have any action to take yet.

Comment 8

[cajbir (:cajbir)]

(In reply to Alex Keybl [:akeybl] from comment #6)
> 
> Are you suggesting we block h.264 playback on the Galaxy S3? Or am I
> misunderstanding something here...
> 
> That would seem to be a non-starter.

Is the S3 one of the crashing devices? What are these devices:

Samsung SPH-L710
Samsung SGH-T999 
Samsung SGH-I747M
Samsung GT-I8190
Samsung SGH-I747

That said, blocking is better than crashing surely. I'm assuming blocking is an easy task to do and reverse. If not, then it may be better to wait until it's fixed.

Comment 9

[Alex Keybl [:akeybl]]

Well let's see...

http://lmgtfy.com/?q=Samsung+SPH-L710
http://lmgtfy.com/?q=Samsung+SGH-T999
http://lmgtfy.com/?q=Samsung+SGH-I747M
http://lmgtfy.com/?q=Samsung+SGH-I747
http://lmgtfy.com/?q=Samsung+GT-I8190

All signs point to the fact that they're all the S3, except the last, which is an unreleased S3.

(In reply to Chris Double (:doublec) from comment #8)
> That said, blocking is better than crashing surely. I'm assuming blocking is
> an easy task to do and reverse. If not, then it may be better to wait until
> it's fixed.

Sure - if it happens 100% of the time on these devices... do you have that info?

Comment 10

[Alex Keybl [:akeybl]]

KaiRo - let's pull URLs.
Kevin - let's try to repro given those URLs asap, in preparation for sending a device with STR to Chris asap. S3s from AT&T/Sprint appear to be affected.

Comment 11

[cajbir (:cajbir)]

(In reply to Alex Keybl [:akeybl] from comment #9)
>
> Sure - if it happens 100% of the time on these devices... do you have that
> info?

I'm just giving my advice. It's the Android's team call whether to blacklist or not. You asked me to look at it and based on what's in this bug, which is the only information I have, that was it.

Comment 12

[Robert Kaiser]

The URLs are widespread, but all seem to be Flash video stuff (I have cut out a number of URLs, all of which seem to be adult video stuff):

1 	http://www.weatherbell.com/login/
1 	http://www.lifeisgood.com/shop/gear/fun-gear/gear,default,sc.html
1 	http://prtienecojones.com/sin-comentarios-o-o-en-la-escuela-buscando-a-mi-hijo/
1 	http://www.upbulk.com/media/video.php?id=1238814
1 	http://triblive.com/sports/steelers/
1 	about:home
1 	http://www.boeing.com/Features/2012/10/bds_champ_10_22_12.html
1 	https://m.facebook.com/profile.php?comment_id=2386428&story_fbid=annmtz11&v=feed...
1 	http://teamtreehouse.com/library/marketing-strategy
1 	http://www.clutchmagonline.com/
1 	http://store.nike.com/us/en_us/product/free-run-id-running-shoe/?piid=28922&pbid
1 	http://www.patrickrothfuss.com/content/index.asp
1 	http://www.steynonline.com/
1 	about:blank
1 	http://www.dailymotion.com/video/x2mzyz_shake-it_news?start=6#from=embediframe
1 	http://www.disclose.tv/action/viewvideo/117466/Have_China_Built_A_Stargate__2012_HD/
1 	http://www.watchpinoytube.com/abs-cbn/be-careful-with-my-heart/51120/be-careful-with-my-heart-december-5-2012.html
1 	http://www.popsci.com/diy/article/2012-11/you-built-what-remote-controlled-robo-arm
1 	http://m.worldstarhiphop.com/?category=&start=14&limit=1
1 	http://www.rollingstones.com/watch/
1 	http://www.nzherald.co.nz/nz/news/article.cfm?c_id=1&objectid=10851829
1 	http://www.gamespot.com/ni-no-kuni-wrath-of-the-white-witch-/videos/

Comment 13

[Aaron Train [:aaronmt]]

QA does not have any of the aforementioned SIII devices. The single device in our inventory we have is a GT-I9300 which is non-affected.

Comment 14

[Alex Keybl [:akeybl]]

(In reply to Aaron Train [:aaronmt] from comment #13)
> QA does not have any of the aforementioned SIII devices. The single device
> in our inventory we have is a GT-I9300 which is non-affected.

This has been ordered.

Comment 15

[Alex Keybl [:akeybl]]

So we got the Samsung SGH-I747, and I've updated it to 4.1.1 through Samsung Kies to match the crash reports.

I tested the following URLs:

1) http://www.boeing.com/Features/2012/10/bds_champ_10_22_12.html
Samsung SGH-I747 (4.0): No crash, audio automatically plays without interaction, video does not ever play, progress bar and controls never come up (the Play button is the only one present, and never toggles)
Samsung SGH-I747 (4.1.1, log attached): No crash, audio automatically plays without interaction, video does not ever play, progress bar and controls never come up (the Play button is the only one present, and never toggles)
Samsung GT-I9300 (4.1.1): video plays properly once I hit play, no crashes

2) http://www.rollingstones.com/watch/
Samsung SGH-I747 (4.0): No crash, audio automatically plays without interaction, video does not ever play
Samsung SGH-I747 (4.1.1, log attached): No crash, audio automatically plays without interaction, video does not ever play
Samsung GT-I9300 (4.1.1): video plays properly once I hit play, no crashes

3) http://www.quirksmode.org/html5/tests/video.html
Samsung SGH-I747 (4.0): No crash, video only plays when touched, no issues
Samsung SGH-I747 (4.1.1, log attached): No crash, video only plays when touched, no issues
Samsung GT-I9300 (4.1.1): No crash, video only plays when touched, no issues


Basically, I haven't been able to reproduce the crash and video for #1/2 above is just not functioning properly (my intuition tells me that this is very related to the crash). The fact that #3 is working as expected means we shouldn't just wholesale blacklist these devices.

Comment 16

[Alex Keybl [:akeybl]]

Attachment: adb logcat for #1

Comment 17

[Alex Keybl [:akeybl]]

Attachment: adb logcat for #3

Comment 18

[Alex Keybl [:akeybl]]

Chris - the phone's on its way to your office.

Comment 19

[cajbir (:cajbir)]

(In reply to Alex Keybl [:akeybl] from comment #15)
> 
> 1) http://www.boeing.com/Features/2012/10/bds_champ_10_22_12.html
> Samsung SGH-I747 (4.0): No crash, audio automatically plays without
> interaction, video does not ever play, progress bar and controls never come
> up (the Play button is the only one present, and never toggles)

I've seen this on other devices and sites. I expect it's either an implementation difference/bug between what the site expects webkit to do with their video implementation and events/custom controls and what we do. Bug 719694 for example.

Comment 20

[Alex Keybl [:akeybl]]

(In reply to Chris Double (:doublec) from comment #19)
> (In reply to Alex Keybl [:akeybl] from comment #15)
> > 
> > 1) http://www.boeing.com/Features/2012/10/bds_champ_10_22_12.html
> > Samsung SGH-I747 (4.0): No crash, audio automatically plays without
> > interaction, video does not ever play, progress bar and controls never come
> > up (the Play button is the only one present, and never toggles)
> 
> I've seen this on other devices and sites. I expect it's either an
> implementation difference/bug between what the site expects webkit to do
> with their video implementation and events/custom controls and what we do.
> Bug 719694 for example.

This may be true - I just realized that my Samsung GT-I9300 had Flash installed, and was therefore not actually playing h.264 content :(.

When I disable Flash, I see the same behavior on the Samsung GT-I9300. Well, the device is on its way regardless. Hopefully it will still help your investigation, even though we haven't yet found STR.

Comment 21

[Robert Kaiser]

This crash is now topcrash #7 over the last 3 days on 17.0 release for Android, with rising tendency.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Edwin, doublec: Do the crashing stack traces actually implicate the hardware decoder? (e.g., is OMX always hardware decoding?) If so, is there a way to disable the hardware decoder and use the software decoder only until we get a better handle on the crash?

It looks like we're crashing when we tear down the decoder.

Comment 23

[cajbir (:cajbir)]

I'm unable to tell from the stack trace if it's the hardware decoder. OMX can be both hardware and software. We can't currently disable hardware decoder for specific devices. I was not able to reproduce the crash on the test phone sent to me using the URL's in comment 12 with the device.

Comment 24

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

The logcat log shows it's the software decoder being used. Did you try to reproduce with the `prefer software codecs' flag set?

Comment 25

[cajbir (:cajbir)]

(In reply to Edwin Flores [:eflores] [:edwin] (Away 31 Dec to 3 Jan NZDT) from comment #24)
> The logcat log shows it's the software decoder being used. Did you try to
> reproduce with the `prefer software codecs' flag set?

Feel free to try. Phone is on my desk.

Comment 26

[cajbir (:cajbir)]

I've been able to reproduce this on the SGS 3 now. STR:

1) Set flag media.stagefright.omxcodec.flags to 8. This forces software decoding.
2) Load http://cd.pn/b
3) Click Play
4) Wait until video starts playing for a second or so
5) Refresh the page
6) Repeat from Step 3

After 4 or more repeats of steps 3-6 the browser crashes.

Comment 27

[Scoobidiver (away)]

It's #3 top crasher in the first hours of 18.0.

Comment 28

[cajbir (:cajbir)]

This is looking like a threading issue in the destruction of the media plugin decoder. Media Decoder Readers are created on the 'Decoder' thread. They are unfortunately destroyed on a different thread. The Decoder thread is temporary and is destroyed on pausing and recreated on resuming.

I have not been able to reproduce this crash using the steps in comment 26 after making some changes to our code to ensure the shutdown/destruction of the plugin decoder reader occurs on the original decoder thread it was created.

Unfortunately because of the terminating/recreation of the decoder thread on pause/resume a crash occurs immediately on resuming then shutting down with these changes.

I'm working on a fix to ensure destruction occurs on the correct threads that handles pause/resume. I also need to investigate if the same issue exists on B2G.

Comment 29

[Alex Keybl [:akeybl]]

(In reply to Chris Double (:doublec) from comment #28)
> I'm working on a fix to ensure destruction occurs on the correct threads
> that handles pause/resume. I also need to investigate if the same issue
> exists on B2G.

Any updates?

Comment 30

[cajbir (:cajbir)]

(In reply to Alex Keybl [:akeybl] from comment #29)
> Any updates?

I'm still working on a fix. So far no luck.

Comment 31

[Alex Keybl [:akeybl]]

Thanks Chris, we're going to drop this off of the tracking radar for now (given how many releases have been affected). Whenever resolved, we will consider a low risk uplift of course.

Comment 32

[Kevin Brosnan [Ex-Mozilla]]

Re noming for tracking fennec as 18 has shipped.

Comment 33

[Robert Kaiser]

(In reply to Chris Double (:doublec) from comment #30)
> I'm still working on a fix. So far no luck.

Did you find anything there?

Comment 34

[cajbir (:cajbir)]

No progress has been made in finding a fix for this crash, sorry.

Comment 35

[cajbir (:cajbir)]

I'll prepare a patch that blocklists this device so it doesn't crash. This will at least stop the crashes until a fix is available.

Comment 36

[Robert Kaiser]

Thanks, we need *some* progress here at least, as this is on the brink of becoming the #1 topcrash behind the "empty dump" signature (it's currently in a bit of a wrestling match with the pretty concerning "database is locked" problem).

Comment 37

[cajbir (:cajbir)]

Attachment: Blocklist devices

Blocklists Samsung devices mentioned here that suffer from the crash. Once the crash is fixed we'll remove them from the blocklist.

Comment 38

[Robert Kaiser]

FYI, here's a full list of devices from just yesterday's crash reports:

libsomxcore.so@0x1826 	1019
Samsung SCH-I535 	181
Samsung SGH-T999 	174
Samsung SPH-L710 	147
Samsung GT-I8190 	135
Samsung SGH-I747 	134
Samsung SGH-I747M 	102
Samsung GT-I8190N 	20
Samsung SCH-R530U 	19
Samsung SCH-I200 	17
Samsung GT-I9070 	15
Samsung SGH-T999V 	13
Samsung GT-I8190L 	10
Samsung GT-P3113 	6
Samsung SGH-I547 	5
Samsung SGH-I727R 	5
Samsung SCH-R530M 	5
Samsung GT-P5100 	5
Samsung GT-P3110 	5
Samsung GT-P3100 	4
Samsung SCH-I915 	4
Samsung SCH-i705 	4
Samsung SHV-E160K 	3
Samsung SPH-P500 	2
Samsung SCH-L710 	1
Samsung GT-P5110 	1
Samsung GT-I8160 	1
Samsung SHV-E120K 	1

This one was taken off https://crash-analysis.mozilla.com/rkaiser/2013-03-20/2013-03-20.fennecandroid.release.19.0.devices.html#sigs

Comment 39

[Aaron Train [:aaronmt]]

> Samsung GT-P3113 	6

Samsung Galaxy Tab 2 7", I can reproduce using the str in comment #26

Comment 40

[Scoobidiver (away)]

I found espresso10 hw.

Comment 41

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/71fe5b69c834

Requested to leave open to track whether more devices need blocklisting.

Comment 42

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/71fe5b69c834

Comment 43

[Scoobidiver (away)]

It's #2 top crasher in 19.0.2, was #10 in 19.0b6 but in 20.0b5 it's #265 crasher and #209 in 20.0b4 so I think it's already fixed in 20.0 and the blocklist should be backed out.

Comment 44

[Lukas Blakk [:lsblakk] use ?needinfo]

Setting tracking on Firefox 22 since we'll want to make sure this gets backed out and tested.

Comment 45

[Scoobidiver (away)]

It's indeed now a low volume crash in 20.0: #140 crasher.

Comment 46

[Scoobidiver (away)]

Can you back it out from Aurora and m-c?

Comment 47

[cajbir (:cajbir)]

(In reply to Scoobidiver from comment #46)
> Can you back it out from Aurora and m-c?

Why does it need to be backed out? Have QA, or someone, confirmed it no longer crashes?

Comment 48

[Scoobidiver (away)]

(In reply to Chris Double (:doublec) from comment #47)
> Why does it need to be backed out? Have QA, or someone, confirmed it no
> longer crashes?
See comment 45. The blocklist is for stopping top crashers, not low volume crashes.

Comment 49

[Aaron Train [:aaronmt]]

(In reply to Scoobidiver from comment #45)
> It's indeed now a low volume crash in 20.0: #140 crasher.

Isn't this a direct result of the block-list working as intended, or am I missing something here?

Comment 50

[Scoobidiver (away)]

(In reply to Aaron Train [:aaronmt] from comment #49)
> Isn't this a direct result of the block-list working as intended, or am I
> missing something here?
How can the blocklist that first landed in 22.0 decrease the crash volume in 20.0?

Comment 51

[Alex Keybl [:akeybl]]

Chris - rather than disabling functionality unnecessarily for the first time in FF22 (Scoobidiver's suspicion) can we back this out for the next FF22 beta and see how it impacts our stability?

Comment 52

[cajbir (:cajbir)]

(In reply to Alex Keybl [:akeybl] from comment #51)
> Chris - rather than disabling functionality unnecessarily for the first time
> in FF22 (Scoobidiver's suspicion) can we back this out for the next FF22
> beta and see how it impacts our stability?

I don't understand what "rather than disabling functionality unnecessarily for the first time in FF22" means. Can you rephrase?

Comment 53

[Alex Keybl [:akeybl]]

(In reply to Chris Double (:doublec) from comment #52)
> (In reply to Alex Keybl [:akeybl] from comment #51)
> > Chris - rather than disabling functionality unnecessarily for the first time
> > in FF22 (Scoobidiver's suspicion) can we back this out for the next FF22
> > beta and see how it impacts our stability?
> 
> I don't understand what "rather than disabling functionality unnecessarily
> for the first time in FF22" means. Can you rephrase?

Following up in IRC.

Comment 54

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Chris Double (:doublec) from comment #52)
> I don't understand what "rather than disabling functionality unnecessarily
> for the first time in FF22" means. Can you rephrase?

Crash data for FF20 seems to show that the bug went away before FF22. If so, it'd be better to have H264 enabled on these devices than to leave the blocklist in place. So I think we should try backing this out.

Chris, do you agree?

Comment 55

[cajbir (:cajbir)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #54)
> 
> Chris, do you agree?

Sure, if the desire is to back the blocklist out I have no problem with that.

Comment 56

[cajbir (:cajbir)]

Attachment: Backout on beta

Backout on beta. Does it need to be backed out anywhere else?

Comment 57

[cajbir (:cajbir)]

Try build: https://tbpl.mozilla.org/?tree=Try&rev=84b8d893159d

Comment 58

[Alex Keybl [:akeybl]]

Comment on attachment 761267 [details] [diff] [review]
Backout on beta

I think this needs to be applied to 22/23/24 (all pre-release branches).

Comment 59

[Alex Keybl [:akeybl]]

I'll say that it would have been much better to get this into an earlier beta, especially since we knew the backout necessary. Fingers crossed we don't get new related stability issues.

I don't want to cause a known and unnecessary regression in FF22 though.

Comment 60

[cajbir (:cajbir)]

https://hg.mozilla.org/releases/mozilla-beta/rev/fa4f21801382

Comment 61

[cajbir (:cajbir)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b77b99444e56

Comment 62

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8adb8856da4

Comment 63

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a8adb8856da4

Comment 64

[Scoobidiver (away)]

I've updated https://wiki.mozilla.org/Blocklisting/Blocked_Graphics_Drivers#On_Android_2 accordingly.

Comment 65

[Kevin Brosnan [Ex-Mozilla]]

It looks like these phones received an Android 4.1 and 4.3. Is there any progress on a real fix or are we looking at another block?

Comment 66

[cajbir (:cajbir)]

(In reply to Kevin Brosnan [:kbrosnan] from comment #65)
> It looks like these phones received an Android 4.1 and 4.3. Is there any
> progress on a real fix or are we looking at another block?

Edwin would know.

Comment 67

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: Ensure OMX plugins instantiate only one OMXClient instance

Samsung's OMX IL doesn't like being instantiated more than once, often leading
to crashes. This patch changes the OMX plugin so that we statically instantiate
one OMXClient to be shared between decoder instances.

Comment 68

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8400390 [details] [diff] [review]
Ensure OMX plugins instantiate only one OMXClient instance

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

::: media/omx-plugin/OmxPlugin.cpp
@@ +162,5 @@
> +  ~OmxClientInstance()
> +  {
> +    if (mStatus == OK) {
> +      mClient->disconnect();
> +      delete mClient;

Oops, |delete| shouldn't be guarded here.

Comment 69

[cajbir (:cajbir)]

(In reply to Edwin Flores [:eflores] [:edwin] from comment #67)
> Samsung's OMX IL doesn't like being instantiated more than once, often
> leading
> to crashes. This patch changes the OMX plugin so that we statically
> instantiate
> one OMXClient to be shared between decoder instances.

I thought the point of OMXClient was to be able to share the single OMX instance. We used to statically use a single OMX instance but then moved to sharing via OMXClient. Disappointing that that crashes.

Comment 70

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8400390 [details] [diff] [review]
Ensure OMX plugins instantiate only one OMXClient instance

Looks good.

Comment 71

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa89a5d38700

Comment 73

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fa89a5d38700

Comment 74

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

STR:
1. Set about:config -> media.stagefright.omxcodec.flags = 8
2. Go to http://cd.pn/b
3. Start video playing
4. Open new tab and do steps 2 and 3
5. Open new tab to about:memory
6. Close cd.pn/b tabs
7. Hit "Minimize memory usage"

Before patch:
Crash.

Expected (after patch):
No crash.

Comment 75

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

I see no crashes with this signature from the most recent nightlies on crash-stats.mo; resolving fixed.

Still waiting on verification, then will request uplift to aurora and beta.

Comment 76

[Aaron Train [:aaronmt]]

We'll treat the crash volume rate as verification for this one as we don't have any of the affected devices based on inventory check

Comment 77

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8400390 [details] [diff] [review]
Ensure OMX plugins instantiate only one OMXClient instance

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 782508
User impact if declined: Continued crashing on popular Android devices after playing MP3/MP4 media.
Testing completed (on m-c, etc.): Crash volume with this signature on Nightly has dropped to 0.
Risk to taking this patch (and alternatives if risky): None.
String or IDL/UUID changes made by this patch: None.

Comment 78

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8ea30a5b565c
https://hg.mozilla.org/releases/mozilla-beta/rev/14b8222e1a24