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

[Alex Converse]

On Wed, Jan 27, 2010 at 11:34 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 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.
>

Numbers like 0.f, 2.f, 4.f, 0.5f, 1.5f are all exactly representable
with IEEE 754.

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

According to the original thread
<http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/2009-November/008425.html>
there are two lroundf()s that are important:

+            sbr->n_master =         lroundf((sbr->k[2] - sbr->k[0]) *
0.25f)  << 1;
That one looks trivially fixable.

+        int temp = lroundf(abs_bord_trail / (float)ch_data->bs_num_env[1]);
when bs_num_env[1] is 2 or 4.

In general it seems to be a good idea to follow the recommendation of
the spec when possible.

Is (int)(x+0.5) any better than lroundf()? Most of these variables are
strictly positive.