495129 - ogg crash [@ vorbis_synthesis] with attached file

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Mike Shaver (:shaver emeritus)]

Attachment: crash on load in Firefox

Comment 1

[Martijn Wargers (dead)]

http://crash-stats.mozilla.com/report/index/f8ec0704-84c0-49ed-92e3-65fd82090527?p=1
0  	xul.dll  	vorbis_synthesis  	 media/libvorbis/lib/vorbis_synthesis.c:49
1 	xul.dll 	fs_vorbis_decode 	media/libfishsound/src/libfishsound/fishsound_vorbis.c:158
2 	xul.dll 	fish_sound_decode 	media/libfishsound/src/libfishsound/fishsound_decode.c:117
3 	xul.dll 	oggplay_callback_audio 	media/liboggplay/src/liboggplay/oggplay_callback.c:391
4 	xul.dll 	oggz_read_sync 	media/liboggz/src/liboggz/oggz_read.c:495
5 	xul.dll 	oggz_read 	media/liboggz/src/liboggz/oggz_read.c:592
6 	xul.dll 	oggplay_step_decoding 	media/liboggplay/src/liboggplay/oggplay.c:691
7 	xul.dll 	nsOggDecodeStateMachine::Run 	content/media/video/src/nsOggDecoder.cpp:1326
8 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
9 	xul.dll 	NS_ProcessNextEvent_P 	obj-firefox/xpcom/build/nsThreadUtils.cpp:230
10 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:254
11 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
12 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
13 	mozcrt19.dll 	_callthreadstartex 	obj-firefox/memory/jemalloc/crtsrc/threadex.c:348
14 	mozcrt19.dll 	_threadstartex 	obj-firefox/memory/jemalloc/crtsrc/threadex.c:326
15 	kernel32.dll 	kernel32.dll@0xb728 

The ogg file only seems to crash locally (not with the bugzilla url, at least).

Comment 2

[Martijn Wargers (dead)]

Related to bug 487519?

Comment 3

[cajbir (:cajbir)]

I have a fix for this. Preparing it now.

Comment 4

[cajbir (:cajbir)]

Attachment: liboggplay fixes

Cherry picked fixes from liboggplay

Comment 5

[cajbir (:cajbir)]

Attachment: handle liboggplay error for bad input

Comment 6

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

Comment on attachment 380031 [details] [diff] [review]
handle liboggplay error for bad input

+void nsOggDecodeStateMachine::HandleDecodeErrors(OggPlayErrorCode r)

aErrorCode?

Comment 7

[cajbir (:cajbir)]

Comment on attachment 380031 [details] [diff] [review]
handle liboggplay error for bad input

Patch has issue with non-broken content. Fixing.

Comment 8

[cajbir (:cajbir)]

Attachment: Fixes

Address review comment, fix playback of valid files.

Comment 9

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

http://hg.mozilla.org/mozilla-central/rev/95c878a87ccd

Comment 10

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

We need a crashtest here.

Comment 11

[Mike Beltzner [:beltzner, not reading bugmail]]

Follow-up bug for the crashtest filed as bug 495639; I think we should just get
this landed and not block outright on that test.

Comment 12

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

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/da492c1f68fc

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on trunk and 1.9.1 with builds on all platforms like

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre ID:20090604031228

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre ID:20090604031153

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch - add test

Add testcase. Rebased on testcase patch for bug 501279.

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

Pushed patch for testcase to m-c:
http://hg.mozilla.org/mozilla-central/rev/3ce467cf8e0d

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Chris, will those tests also be pushed to 1.9.1? And can we open this bug to
the public now?

Comment 17

[Martijn Wargers (dead)]

This was already fixed in 1.9.1, so it seems this bug can be made public. Also, I think the tests can be pushed to 1.9.1.

Comment 18

[cajbir (:cajbir)]

the test can't be directly pushed as 1.9.1 has a different test harness structure. It will need to be redone.

Comment 19

[Josh Aas]

Opening this up per comment 17. This issue has been resolved for long enough.