[FFmpeg-devel] [PATCH] HE-AAC v1 decoder

Michael Niedermayer michaelni
Wed Jan 27 17:34:32 CET 2010


On Wed, Jan 27, 2010 at 11:15:23AM -0500, Alex Converse wrote:
> On Wed, Jan 27, 2010 at 8:53 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Wed, Jan 27, 2010 at 05:53:24AM -0500, Alexander Strange wrote:
> >>
> >> On Jan 26, 2010, at 10:10 PM, Alex Converse wrote:
> > [...]
> >> > + ? ?for (k = 0; k < num_bands-1; k++) {
> >> > + ? ? ? ?prod *= base;
> >> > + ? ? ? ?present ?= lroundf(start * prod);
> >> > + ? ? ? ?bands[k] = present - previous;
> >> > + ? ? ? ?previous = present;
> >> > + ? ?}
> >>
> >> You use lroundf() a lot, which is a library call for me. Can't you set the rounding direction (or just ignore it) and cast instead?
> >
> > whats wrong with lrintf() ?
> 
> *round*() family functions round their argument to the nearest integer
> value, rounding away from zero, regardless of the current rounding
> direction. *rint*() uses the current rounding direction. There are a
> few cases where things can end up at exactly 0.5 and it makes a big
> difference.

then the code is broken, floats are not exact, especially not floats in
C. floats in ASM might be useable like that but in C assuming that a
calculation like lround(a/b) will have a 0.5 result rounded the way
lround should is russian roullet.
you only need a lround(c/b) close by and many compilers includig gcc
will calculate x=1/b and do lround(a*x) and lround(c*x) also
the compiler can choose to keep values in registers that are more
precisse than doubles or store as floats in intermediate memory loctions
Code that depends on the rounding direction of 0.5 definitly is a
time bomb.

Besides that a quick grep for round indicates that its used for many
calculations with pure integer input and pure integer output
like lroundf((float)a / (float)b);
such calculations should be done purely with integers, that way
you get always the correct result ...

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- 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/20100127/a46112e0/attachment.pgp>



More information about the ffmpeg-devel mailing list