[FFmpeg-devel] [PATCH] Generic sine window init function

[Robert Swain]

2008/6/22 Michael Niedermayer <michaelni at gmx.at>:
> On Sat, Jun 21, 2008 at 09:02:35PM +0100, Robert Swain wrote:
>> Hello,
>>
>> A few of the MDCT codecs (AAC, IMC, COOK, Nellymoser) use sine windows
>> so it would make sense to share the sine window initialisation code
>> between them as the . I have attached a patch for this. If there are
>> other codecs that can use this that I've missed, let me know.
>>
>> It is probably noteworthy that the tables for functions that normalise
>> the window coefficients may differ from the direct calculation due to
>> loss of precision during intermediate stages. For example, in imc.c:
>>
>> +    ff_sine_window_init(q->mdct_sine_window, 2*COEFFS);
>>      for(i = 0; i < COEFFS; i++)
>> -        q->mdct_sine_window[i] = sin((i + 0.5) / 512.0 * M_PI) * sqrt(2.0);
>> +        q->mdct_sine_window[i] *= sqrt(2.0);
>>
>> mdct_sine_window is a float array so the sin() part of the calculation
>> is limited to float precision in ff_sine_window_init and is then
>> normalised with the sqrt(2.0) after. Is this a problem or is the
>> difference insignificant? For the above case the maximum deviation was
>> 0.00000011920928955078125.
>>
>
>> Also, off topic, should that sqrt(2.0) be switched for M_SQRT2?
>
> yes

OK. I'll deal with that afterwards, as well as any indentation or
anything (I generally consider it a given that I will correct
indentation after making functional changes...).

> [...]
>> Index: libavcodec/imc.c
>> ===================================================================
>> --- libavcodec/imc.c  (revision 13854)
>> +++ libavcodec/imc.c  (working copy)
>> @@ -102,8 +102,9 @@
>>          q->old_floor[i] = 1.0;
>>
>>      /* Build mdct window, a simple sine window normalized with sqrt(2) */
>> +    ff_sine_window_init(q->mdct_sine_window, COEFFS);
>>      for(i = 0; i < COEFFS; i++)
>> -        q->mdct_sine_window[i] = sin((i + 0.5) / 512.0 * M_PI) * sqrt(2.0);
>> +        q->mdct_sine_window[i] *= sqrt(2.0);
>>      for(i = 0; i < COEFFS/2; i++){
>>          q->post_cos[i] = cos(i / 256.0 * M_PI);
>>          q->post_sin[i] = sin(i / 256.0 * M_PI);
>> Index: libavcodec/cook.c
>> ===================================================================
>> --- libavcodec/cook.c (revision 13854)
>> +++ libavcodec/cook.c (working copy)
>> @@ -239,9 +239,9 @@
>>        return -1;
>>
>>      /* Initialize the MLT window: simple sine window. */
>> -    alpha = M_PI / (2.0 * (float)mlt_size);
>> +    ff_sine_window_init(q->mlt_window, mlt_size);
>>      for(j=0 ; j<mlt_size ; j++)
>> -        q->mlt_window[j] = sin((j + 0.5) * alpha) * sqrt(2.0 / q->samples_per_channel);
>> +        q->mlt_window[j] *= sqrt(2.0 / q->samples_per_channel);
>
> The window should not be scaled by sqrt(2) there are plenty of other places
> where the sqrt(2) could be applied.
> and it should not be in the context but static and shared
> (of course that is not really your job to do so the patch is ok if you dont
> want to work on that)

It's OK. I'll do it. I don't mind a bit of monkey work now and again. :)

I actually have some improvements. As these normalisation factors are
always that, how about adding another argument and multiplying by the
normalisation factor within the init function? This could also be done
for ff_kbd_window_init() as well which would solve that non-thread
safe situation in aac.c. See attached.

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080622-1247-ff_sine_window_init.diff
Type: text/x-diff
Size: 2868 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080622/95f6922e/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 20080622-1250-ff_kbd_window_init-norm_fac.diff
Type: text/x-diff
Size: 1610 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080622/95f6922e/attachment-0001.diff>