[FFmpeg-devel] Fix for incorrect Vorbis decoding.
David Conrad
lessen42
Thu Nov 11 22:25:19 CET 2010
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...
More information about the ffmpeg-devel
mailing list