1

Closed

Some ogg files improperly decode

description

Attached is a file that doesn't decode properly.

file attachments

Closed Sep 13, 2013 at 3:07 PM by ioctlLR
Released build with fix

comments

ioctlLR wrote Jul 31, 2013 at 1:02 PM

Wow, you weren't kidding. My gut feeling is that the windows are wrong, but I won't be able to verify that until next week (had to ship the dev machine off for repair).

If you'd like to debug this using the latest source, feel free. The code should be relatively easy to follow.

thoqbk wrote Jul 31, 2013 at 8:41 PM

I don't know anything about vorbis, so debugging this would be slow going. I did spot check the window values with another decoder and they seem correct (didn't do a block compare so might still be wrong).

However, I'm not entirely sure the correct window is being used all the time; GetWindow may be returning the wrong one (1 and 2 swapped maybe), but fixing it didn't solve the problem. Do you have any other ideas?

If I find the time, I'll try different encoding parameters to narrow down the problem scope.

ioctlLR wrote Aug 1, 2013 at 1:29 AM

Sounds like the windows are fine... What are the block sizes for this file? You can find this by putting a breakpoint at the end of VorbisStreamDecoder.ProcessStreamHeader(...) and reading Block0Size and Block1Size.

The next places to look are (simplest to hardest):
  • RingBuffer (.CopyTo(...), .Write(...), & .RemoveItems(...))
  • VorbisStreamDecoder (.OverlapSamples(), .DecodePacket(), .UnpackPacket())
  • VorbisFloor
  • VorbisResidue
  • Mdct

thoqbk wrote Aug 1, 2013 at 5:15 AM

0x100 and 0x800

It looks like this bug may only affect mono files.

ioctlLR wrote Aug 1, 2013 at 11:49 AM

Interesting... Can you try decoding with version 0.5.5? I'd like to verify that newer versions didn't break it...

thoqbk wrote Aug 1, 2013 at 5:00 PM

So yeah 0.7.2 and later is broken. Commit 310289367342 is the culprit.

ioctlLR wrote Aug 1, 2013 at 7:46 PM

OK, it's almost certainly an issue in VorbisResidue.Residue1.WriteVectors(...). I thought I had it right, but don't have any files to test with...

Please put a breakpoint in that method and make sure it "makes sense" (ignore the comments; see the relevant section of the spec for details). You may consider looking at the libvorbis source to verify, as well.

Here's hoping my dev machine returns quickly...

ioctlLR wrote Aug 2, 2013 at 5:13 PM

I feel sheepish... My dev machine is back, so I was able to determine that residue 1 was missing a "+" on line 273. That's now fixed in my local copy of the code. I'll release it in a bit.