[FFmpeg-devel] FATE & Regressions (and PPC is broken)

[Måns Rullgård]

Diego Biurrun <diego at biurrun.de> writes:

> On Sat, Mar 15, 2008 at 02:24:41AM +0100, Michael Niedermayer wrote:
>> On Sat, Mar 15, 2008 at 12:36:36AM +0000, M?ns Rullg?rd wrote:
>> > Mike Melanson <mike at multimedia.cx> writes:
>> > 
>> > > The following regression test is broken on PowerPC (gcc 4.1.2):
>> > >
>> > > diff -u -w
>> > > "/home/melanson/ffmpeg/ffmpeg-main"/tests/ffmpeg.regression.ref
>> > > tests/data/vsynth.regression
>> > > --- /home/melanson/ffmpeg/ffmpeg-main/tests/ffmpeg.regression.ref
>> > > 2008-03-08 19:10:07.000000000 -0800
>> > > +++ tests/data/vsynth.regression        2008-03-12 14:27:53.000000000 -0700
>> > > @@ -197,7 +197,7 @@
>> > >  353368 ./tests/data/a-flac.flac
>> > >  c4228df189aad9567a037727d0e763e4 *./tests/data/flac.vsynth.out.wav
>> > >  stddev: 33.31 PSNR:65.87 bytes:1040384
>> > > -a1e56bb034c2773438ba1c830c4cea07 *./tests/data/a-wmav1.asf
>> > > +59575b4756d674cd8c8d53d86c08e8cb *./tests/data/a-wmav1.asf
>> > >  106004 ./tests/data/a-wmav1.asf
>> > >  stddev:12251.50 PSNR:14.56 bytes:1056768
>> > >  stddev:2106.00 PSNR:29.85 bytes:1048576
>> > > make: *** [codectest] Error 1
>> > 
>> > OK, I tracked this one down.  It turns out lrint() rounds slightly
>> > differently for whatever reason (bug or otherwise).  Attached patch
>> > changes it to lround() (which has well-defined rounding behaviour).
>> > With this change, the results are the same on x86_64 and ppc64.
>> > 
>> > This raises the broader question of lrint() usage in FFmpeg.  The
>> > documentation for the lrint() family says:
>> > 
>> >   These functions shall round their argument to the nearest integer
>> >   value, rounding according to the current rounding direction.
>> > 
>> > Clearly, in a library we do not know the current rounding direction,
>> > and it would be rude of us to change it.  The lround() family, on the
>> > other hand, always rounds the same way:
>> > 
>> >   These functions shall round their argument to the nearest integer
>> >   value, rounding halfway cases away from zero, regardless of the
>> >   current rounding direction.
>> 
>> We have to use lrint* because its faster see:
>
> Hmm, does speed really matter that much in that part of the WMA encoder?
> Or is that just a comment for the general case and Mans' patch is OK?

Yes, speed matters.  Besides, further investigation shows the
inaccuracy arises elsewhere.  This change worked by accident.

-- 
M?ns Rullg?rd
mans at mansr.com