785340 - crash in OmxDecoder::ReadAudio

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Scoobidiver (away)]

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

OmxDecoder::ReadAudio() is crashing on a null mAudioSource here:

      err = mAudioSource->read(&mAudioBuffer);

Comment 2

[Joe Drew (not getting mail)]

100% reproducible for me when loading objet.com on my galaxy tab 10.1 running ICS.

Comment 3

[Benjamin Smedberg]

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

I can repro this crash using joedrew's STR in comment 2.

Comment 5

[Chris Peterson [:cpeterson]]

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

Attachment: 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]]

Attachment: 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 8

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/989003f06039
https://hg.mozilla.org/integration/mozilla-inbound/rev/345abc9535a0

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/989003f06039
https://hg.mozilla.org/mozilla-central/rev/345abc9535a0

Comment 10

[Chris Peterson [:cpeterson]]

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

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

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.

Comment 13

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/00f3ce989b27
https://hg.mozilla.org/releases/mozilla-aurora/rev/96204c1cf65c