[FFmpeg-devel] Fix for incorrect Vorbis decoding.

Alex Converse alex.converse
Fri Nov 12 05:09:10 CET 2010

On Thu, Nov 11, 2010 at 1:25 PM, David Conrad <lessen42 at gmail.com> wrote:
> 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.

Those fast int types always seem to be creating all kinds of crazy problems.

All the recent fixes to vorbisdec seem to deal with those crazy types.

> 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...

uint_fast8_t x = 3;
int y = -2;

if (y > x)
    printf("sizeof uint_fast8_t>= sizeof int");
    printf("sizeof uint_fast8_t < sizeof int");


see r24716

More information about the ffmpeg-devel mailing list