Last Comment Bug 785340 - crash in OmxDecoder::ReadAudio
: crash in OmxDecoder::ReadAudio
Status: RESOLVED FIXED
[native-crash], [hwdecoder]
: crash, regression, reproducible
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 17 Branch
: ARM Android
: -- critical (vote)
: mozilla18
Assigned To: Chris Peterson [:cpeterson]
:
: Maire Reavy [:mreavy]
Mentors:
http://www.objet.com
Depends on:
Blocks: 787227 782508
  Show dependency treegraph
 
Reported: 2012-08-24 02:46 PDT by Scoobidiver (away)
Modified: 2012-09-21 17:25 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed


Attachments
part-1-initialize-mHasAudio-false.patch (2.34 KB, patch)
2012-09-18 17:45 PDT, Chris Peterson [:cpeterson]
cajbir.bugzilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
part-2-fail-if-read-fails.patch (2.07 KB, patch)
2012-09-18 17:47 PDT, Chris Peterson [:cpeterson]
cajbir.bugzilla: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2012-08-24 02:46:12 PDT
It first appeared in 17.0a1/20120822. The regression window is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=360ab7771e27&tochange=abc17059522b
It's likely a regression from bug 782508.

Signature 	OmxDecoder::ReadAudio More Reports Search
UUID	15cd43eb-7591-435a-a273-3b7f22120823
Date Processed	2012-08-23 02:31:09
Uptime	535
Last Crash	16.6 minutes before submission
Install Age	12.2 hours since version was first installed.
Install Time	2012-08-22 14:17:03
Product	FennecAndroid
Version	17.0a1
Build ID	20120822030558
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux 2.6.39.4-gc9932b1 #1 SMP PREEMPT Thu May 10 00:22:08 CST 2012 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0x0
App Notes 	
AdapterDescription: 'NVIDIA Corporation -- NVIDIA Tegra 3 -- OpenGL ES 2.0 14.01002 -- Model: HTC One X, Product: endeavoru, Manufacturer: HTC, Hardware: endeavoru'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ 
HTC HTC One X
htc_europe/endeavoru/endeavoru:4.0.3/IML74K/62864.11:user/release-keys
EMCheckCompatibility	True
Adapter Vendor ID	NVIDIA Corporation
Adapter Device ID	NVIDIA Tegra 3

Frame 	Module 	Signature 	Source
0 	libomxplugin.so 	OmxDecoder::ReadAudio 	OmxPlugin.cpp:566
1 	libomxplugin.so 	ReadAudio 	OmxPlugin.cpp:626
2 	libxul.so 	nsMediaPluginReader::DecodeAudioData 	nsMediaPluginReader.cpp:253
3 	libxul.so 	nsBuiltinDecoderReader::FindStartTime 	nsBuiltinDecoderReader.cpp:324
4 	libxul.so 	nsBuiltinDecoderStateMachine::FindStartTime 	nsBuiltinDecoderStateMachine.cpp:2380
5 	libxul.so 	nsBuiltinDecoderStateMachine::DecodeMetadata 	nsBuiltinDecoderStateMachine.cpp:1776
6 	libxul.so 	nsBuiltinDecoderStateMachine::DecodeThreadRun 	nsBuiltinDecoderStateMachine.cpp:477
7 	libxul.so 	nsRunnableMethodImpl<void , true>::Run 	nsThreadUtils.h:349
8 	libxul.so 	nsThread::ProcessNextEvent 	nsThread.cpp:624
9 	libxul.so 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:220
10 	libxul.so 	nsThread::ThreadFunc 	nsThread.cpp:257
11 	libnspr4.so 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c:156
12 	libc.so 	libc.so@0x1319e 	
13 	libc.so 	libc.so@0x12cd6

More reports at:
https://crash-stats.mozilla.com/report/list?signature=OmxDecoder%3A%3AReadAudio
Comment 1 Chris Peterson [:cpeterson] 2012-08-24 10:38:51 PDT
OmxDecoder::ReadAudio() is crashing on a null mAudioSource here:

      err = mAudioSource->read(&mAudioBuffer);
Comment 2 Joe Drew (not getting mail) 2012-08-30 18:21:29 PDT
100% reproducible for me when loading objet.com on my galaxy tab 10.1 running ICS.
Comment 3 Benjamin Smedberg [:bsmedberg] 2012-09-17 10:37:12 PDT
Per crashkill today, this is not quite a mobile topcrash but looks reproducible and is probably low-hanging fruit. cpeterson or cdouble, can one of you take this?
Comment 4 Chris Peterson [:cpeterson] 2012-09-17 18:03:14 PDT
I can repro this crash using joedrew's STR in comment 2.
Comment 5 Chris Peterson [:cpeterson] 2012-09-17 18:10:37 PDT
This crash is not specifically about audio decoding. The video decoder reports an error about the video and we crash when trying to unload the video as the audio decoder is still initializing.
Comment 6 Chris Peterson [:cpeterson] 2012-09-18 17:45:13 PDT
Created attachment 662371 [details] [diff] [review]
part-1-initialize-mHasAudio-false.patch

Part 1: Fix crash where uninitialized nsMediaPluginReader::mHasAudio looked true.

Also fix a typo and initialize bool mEndOfStream to false, not 0.
Comment 7 Chris Peterson [:cpeterson] 2012-09-18 17:47:42 PDT
Created attachment 662372 [details] [diff] [review]
part-2-fail-if-read-fails.patch

Part 2: OmxDecoder::ReadVideo() should fail if mVideoSource->read() fails.

These patches fix this audio crash, BUT uncover a bug where our decoder never stops retrying the failed video.
Comment 10 Chris Peterson [:cpeterson] 2012-09-20 12:00:03 PDT
Comment on attachment 662371 [details] [diff] [review]
part-1-initialize-mHasAudio-false.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 782508 in 18 and 17. 16 is unaffected.
User impact if declined: Android video crashes
Testing completed (on m-c, etc.): Local testing with 100% STR
Risk to taking this patch (and alternatives if risky): Low risk because we're just initializing some boolean flags to false instead of garbage non-false value.
String or UUID changes made by this patch: N/A
Comment 11 Chris Peterson [:cpeterson] 2012-09-20 12:01:41 PDT
Comment on attachment 662372 [details] [diff] [review]
part-2-fail-if-read-fails.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 782508 in 18 and 17. 16 is unaffected.
User impact if declined: Android video crashes
Testing completed (on m-c, etc.): Local testing with 100% STR
Risk to taking this patch (and alternatives if risky): Low risk. Playing some invalid videos may cause lots of error messages to be logged (but we want to find and fix those).
String or UUID changes made by this patch: N/A
Comment 12 Alex Keybl [:akeybl] 2012-09-21 16:48:00 PDT
Comment on attachment 662371 [details] [diff] [review]
part-1-initialize-mHasAudio-false.patch

[Triage Comment]
Still early enough in the cycle to take a low risk fix for a reproducible crash.

Note You need to log in before you can comment on or make changes to this bug.