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

[Michael Niedermayer]

Hi

On Sun, Mar 16, 2008 at 12:26:15AM +0000, M?ns Rullg?rd wrote:
> M?ns Rullg?rd <mans at mansr.com> writes:
> 
> > 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.
> 
> I've tracked down the real reason for the difference.  It results from
> lost precision by use of floats in mdct.c.  Attached patch makes a few
> temporary values double instead.  I don't know what it might do to
> performance, though.

I dont know about performance either, i guess there are some situations
where it would be slower but systems should have asm MDCTS anyway ...
So iam ok with the change.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080316/8446f14d/attachment.pgp>