[FFmpeg-devel] Fix for incorrect Vorbis decoding.

[David Conrad]

On Nov 11, 2010, at 1:15 PM, Gregory Maxwell wrote:

> Last night Alex Converse mentioned on IRC that a Ogg/Theora+Vorbis
> file on Wikipedia was producing a garbled decode
> (http://upload.wikimedia.org/wikipedia/commons/7/79/Big_Buck_Bunny_small.ogv).
>  I performed an automated sweep and identified many files for which
> the ffmpeg decode produced bad sounding output.
> 
> e.g. compare
> http://myrandomnode.dyndns.org:8080/~gmaxwell/ffmpeg_broken_decode/f065.wav
> http://myrandomnode.dyndns.org:8080/~gmaxwell/ffmpeg_broken_decode/v065.wav
> 
> I didn't have a chance to look at it more in depth at the time. But
> Monty listened to the files and said they sounded like an error
> decoding the floor slope. Fortunately the logic in the ffmpeg vorbis
> decoder is mostly identical to libvorbis, so it was fairly simple to
> compare it.
> 
> The root cause turned out to be an overflow in the floor 1
> computation. I've attached a working fix, though my feelings won't be
> bruised if you prefer another fix for stylistic reasons.

It it's probably better to use plain int rather than the fast types; IIRC on some platforms they're mapped to 64-bit types which is certainly not the fastest for division.

But the patch looks OK.

> It seems to me that the ffmpeg implementation rather aggressively uses
> the smallest types? and it seems to me that this may be resulting in
> other issues, some of them may be rare or difficult to catch. So I
> make no promises the the decode is now completely correct. However, I
> did run automated testing against a few hundred files and found no
> cases where the patched code gave significantly worse results than
> libvorbis.

The int_fast*_t types are pretty annoying, since their actual sizes are platform-dependant...