Crash [@ theora_decode_packetin] with ogg video

RESOLVED FIXED in mozilla1.9.1a2

Status

()

--
critical
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: martijn.martijn, Assigned: cajbir)

Tracking

(Blocks: 1 bug, {crash, testcase, verified1.9.1})

Trunk
mozilla1.9.1a2
x86
Windows XP
crash, testcase, verified1.9.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Created attachment 331834 [details]
testcase

See testcase, which crashes current trunk build on load.
I guess the ogg video file is invalid, but Mozilla should not crash on it.

http://crash-stats.mozilla.com/report/index/f27859f5-5e69-11dd-9801-001cc4e2bf68?p=1
0  	xul.dll  	theora_decode_packetin  	 modules/libtheora/lib/dec/decapiwrapper.c:170
1 	xul.dll 	oggplay_callback_theora 	modules/liboggplay/src/liboggplay/oggplay_callback.c:133
2 	xul.dll 	oggplay_callback_predetected 	modules/liboggplay/src/liboggplay/oggplay_callback.c:552
3 	xul.dll 	oggz_read_deliver_packet 	modules/liboggz/src/liboggz/oggz_read.c:326
4 	xul.dll 	oggz_dlist_deliter 	modules/liboggz/src/liboggz/oggz_dlist.c:149
5 	xul.dll 	oggz_read_sync 	modules/liboggz/src/liboggz/oggz_read.c:474
6 	xul.dll 	oggz_read 	modules/liboggz/src/liboggz/oggz_read.c:629
7 	xul.dll 	oggplay_initialise 	modules/liboggplay/src/liboggplay/oggplay.c:103
8 	xul.dll 	oggplay_open_with_reader 	modules/liboggplay/src/liboggplay/oggplay.c:137
9 	xul.dll 	nsOggDecoder::LoadOggHeaders 	content/media/video/src/nsOggDecoder.cpp:611
10 	xul.dll 	nsRunnableMethod<nsScriptLoader>::Run 	obj-firefox/dist/include/xpcom/nsThreadUtils.h:261
11 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
12 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:254
13 		@0x9afffab
(Reporter)

Comment 1

10 years ago
Created attachment 331835 [details]
testcase
Attachment #331834 - Attachment is obsolete: true

Updated

10 years ago
Component: DOM: HTML → DOM: Core & HTML

Comment 2

10 years ago
Yes, the 3.1alpha2 build crashes indeed. And I had to kill -9 the process <defunct> to restart the browser.
(Assignee)

Comment 3

10 years ago
This appears to be a problem with liboggplay since it's own player crashes with this file, whereas libtheora handles it fine (reporting an error). I'm working with annodex people to see what the problem is.
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 4

10 years ago
Ticket raised with Annodex: http://trac.annodex.net/ticket/409
My hypothesis is that liboggplay (or liboggz) is ignoring the corruption when libogg reports it. The decoder doesn't get initialized, and then segfaults when it tries to decode without being set up by the header packet.

Therefore there are two bugs here: liboggplay needs to detect the corruption, and libtheora needs to do more checking in th_decode_packetin().
(Assignee)

Comment 7

10 years ago
Created attachment 331921 [details] [diff] [review]
Apply libtheora changeset r15144 to fix corruption issues

This applies the changeset r15144 from libtheora. Fix provided by Ralph Giles as per comment 6. 

A change is also required for liboggplay. Patch for that following shortly.
Assignee: nobody → chris.double
Attachment #331921 - Flags: superreview?(roc)
Attachment #331921 - Flags: review?(roc)
(Assignee)

Comment 8

10 years ago
Created attachment 331922 [details] [diff] [review]
Apply liboggplay changset r3673 to fix corruption issues

patch for liboggplay. I applied the fix to liboggplay svn and imported the changes into the mozilla tree.

http://trac.annodex.net/changeset/3673

This patch combined with the theora patch in attachment 331921 [details] [diff] [review] fixes the issue.
Attachment #331922 - Flags: superreview?(roc)
Attachment #331922 - Flags: review?(roc)
I don't want to review Ogg patches. You should feel free to apply Ogg patches from upstream without review. If you write your own Ogg patches, ideally you'd get them reviewed by upstream maintainers.

I'll leave the decision about whether to cherrypick patches or do a full update to you.
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Pushed 249c616e4355 and 4f50285c3a41.

We need a testcase here. It would be nice if Martijn's testcase could be reduced in size. It would also be nice if we could test that the right errors are raised in the element and that fallback is triggered correctly.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Created attachment 332161 [details]
truncated version of the testcase file which still shows the problem

I've trimmed the file so it's a little easier to ship around.

Updated

10 years ago
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1a2

Updated

10 years ago
Component: DOM: Core & HTML → Video/Audio
QA Contact: general → video.audio

Comment 12

10 years ago
verified1.9.1 using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090428 Shiretoko/3.5b5pre on x86 Windows XP SP3
Adding the verified1.9.1 keyword since this is fine based on Comment12 and my testing on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090430 Shiretoko/3.5b5pre.
Keywords: verified1.9.1

Comment 14

10 years ago
I can't get this video to play anywhere - not in current trunk or shiretoko.  It doesn't crash but it also doesn't play.

Furthermore, I can no longer reproduce the crash when testing with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/2008072903 Minefield/3.1a2pre

So we aren't crashing which is good, but we also aren't playing, which is bad?  Should I file a new bug or reopen this one?
(Assignee)

Comment 15

10 years ago
The file is corrupt - it's not a valid file - that's why we don't play it. mplayer also doesn't play it for example.

Comment 16

10 years ago
(In reply to comment #15)
> The file is corrupt - it's not a valid file - that's why we don't play it.
> mplayer also doesn't play it for example.

Ok, thanks, I didn't realize we gated on corrupt files. So a simple crash test should be sufficient to clear the in-testsuite flag, just a simple "does-the-page-load-without-crashing crashtest.  We'll have it coming right up.

Comment 17

10 years ago
(In reply to comment #16)
> (In reply to comment #15)
> > The file is corrupt - it's not a valid file - that's why we don't play it.
> > mplayer also doesn't play it for example.
> 
> Ok, thanks, I didn't realize we gated on corrupt files. So a simple crash test
> should be sufficient to clear the in-testsuite flag, just a simple
> "does-the-page-load-without-crashing crashtest.  We'll have it coming right up.

Chris, Sorry, but I'm confused.  We were going to make a quick reftest for this, but on very recent builds, this renders as a "loading" video.  It has the gray video box and the throbber as though the video is buffering. Is that expected?  If we're not going to play it, then we probably shouldn't show anything the way we did until recently (it just rendered to blank but the controls were still active on mouseover (ideally those should also be disabled)).

What's the expected behavior here?
(Assignee)

Comment 18

9 years ago
Created attachment 392384 [details] [diff] [review]
Testcase added to media testing framework
(Assignee)

Comment 19

9 years ago
Test in attachment 392384 [details] [diff] [review] pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/0099558dc16f
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ theora_decode_packetin]
You need to log in before you can comment on or make changes to this bug.