Last Comment Bug 666132 - ASSERTION: Decoded samples for Vorbis packet don't match expected
: ASSERTION: Decoded samples for Vorbis packet don't match expected
Status: RESOLVED FIXED
[inbound]
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: mozilla8
Assigned To: cajbir (:cajbir)
:
Mentors:
http://www.pirateslovedaisies.com/aud...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-21 22:32 PDT by PTO until Sep 5 NZ time; Chris Pearce (:cpearce)
Modified: 2011-08-03 02:28 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test file (18.55 KB, video/ogg)
2011-08-01 19:32 PDT, cajbir (:cajbir)
no flags Details
Fix (1.09 KB, patch)
2011-08-01 22:37 PDT, cajbir (:cajbir)
no flags Details | Diff | Splinter Review
Fix (1.09 KB, patch)
2011-08-02 05:14 PDT, cajbir (:cajbir)
cpearce: review+
Details | Diff | Splinter Review

Description PTO until Sep 5 NZ time; Chris Pearce (:cpearce) 2011-06-21 22:32:58 PDT
The code to predict the number of samples in an Ogg Vorbis packet is firing an assertion while decoding many of the Ogg files in the Pirates Love Daisies HTML5 demo. e.g.:

http://www.pirateslovedaisies.com/audio/units/U-Thug8.ogg

7672[bae9d60]: ###!!! ASSERTION: Decoded samples for Vorbis packet don't match expected!: 'mVorbisPacketSamples[aPacket] == aSamples', file c:/Users/cpearce/src/blue/objdir/conten
/media/ogg/../../../../content/media/ogg/nsOggCodecState.cpp, line 126
###!!! ASSERTION: Decoded samples for Vorbis packet don't match expected!: 'mVorbisPacketSamples[aPacket] == aSamples', file c:/Users/cpearce/src/blue/objdir/content/media/ogg/../
./../../content/media/ogg/nsOggCodecState.cpp, line 126
Comment 1 cajbir (:cajbir) 2011-08-01 19:32:25 PDT
Created attachment 550000 [details]
Test file

I've attached a test file that also causes the assertion. The file is from the test in bug 668449 comment 6.
Comment 2 cajbir (:cajbir) 2011-08-01 22:37:48 PDT
Created attachment 550010 [details] [diff] [review]
Fix

Account for trailing data in the last frame of vorbis file when doing debug sanity checks.
Comment 3 Timothy B. Terriberry (:derf) 2011-08-02 04:27:02 PDT
Comment on attachment 550010 [details] [diff] [review]
Fix

>+    // Account for a partial last frame
>+    if (packet->e_o_s && packet->granulepos > mGranulepos) {
>+       samples = packet->granulepos - mGranulepos;
>+    }

Is there a particular reason you're using > instead of >=? I don't see an explicit limit in the spec on how many samples can be trimmed by a short granpos on the last page, but I know that at the start of the stream, removing _all_ the samples from the packet (to give a granpos of 0) is explicitly allowed.
Comment 4 cajbir (:cajbir) 2011-08-02 05:14:15 PDT
Created attachment 550048 [details] [diff] [review]
Fix

Thanks Tim, I wasn't aware of that. Changed to >= as per your suggestion in comment 3.
Comment 6 Marco Bonardo [::mak] 2011-08-03 02:28:20 PDT
http://hg.mozilla.org/mozilla-central/rev/e8fe55848089

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