[FFmpeg-devel] [PATCHv2 2/2] avcodec/sinewin_tablegen: use sin instead of sinf for fixed point to improve precision

[Marton Balint]

On Sun, 13 Mar 2016, Ganesh Ajjanagadde wrote:

> On Sun, Mar 13, 2016 at 6:50 PM, Marton Balint <cus at passwd.hu> wrote:
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavcodec/sinewin_tablegen.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/sinewin_tablegen.h b/libavcodec/sinewin_tablegen.h
>> index 4432135..9c912aa 100644
>> --- a/libavcodec/sinewin_tablegen.h
>> +++ b/libavcodec/sinewin_tablegen.h
>> @@ -50,9 +50,9 @@ SINETABLE(8192);
>>  #endif
>>
>>  #if USE_FIXED
>> -#define SIN_FIX(a) (int)floor((a) * 0x80000000 + 0.5)
>> +#define SIN_FIX(a) (int)floor(sin(a) * 0x80000000 + 0.5)
>
> Note that lrint may be preferred, it is faster and better rounded.
> Hosts/hardcoded tables can use lavu/tablegen.h, which fall back to
> floor based hacks.

Ok.

>
>>  #else
>> -#define SIN_FIX(a) a
>> +#define SIN_FIX(a) sinf(a)
>>  #endif
>
> Also, why is it essential that SIN_FIX is redefined, can't you just
> change sinf to sin on line 69?

I just didn't want to touch existing floating point code, sin may be 
slower then sinf (?), and if the result is rounded to float from double 
anyway, what is the point.

>>
>>  SINETABLE_CONST INTFLOAT * const AAC_RENAME(ff_sine_windows)[] = {
>> @@ -66,7 +66,7 @@ SINETABLE_CONST INTFLOAT * const AAC_RENAME(ff_sine_windows)[] = {
>>  av_cold void AAC_RENAME(ff_sine_window_init)(INTFLOAT *window, int n) {
>>      int i;
>>      for(i = 0; i < n; i++)
>> -        window[i] = SIN_FIX(sinf((i + 0.5) * (M_PI / (2.0 * n))));
>> +        window[i] = SIN_FIX((i + 0.5) * (M_PI / (2.0 * n)));
>>  }
>
> Could you please add a link in the commit message describing more
> verbosely why you need this (e.g which platform)?

Ok, as far as I recall the fixed point table was different on x86_32 and 
x86_64, x86_32 was precise, x86_64 wasn't which is strange.

Regards,
Marton