[FFmpeg-devel] [PATCH 3/4] avcodec/ppc/fdctdsp: use more accurate constants

[Ganesh Ajjanagadde]

On Mon, Nov 16, 2015 at 9:48 AM, Daniel Serpell <dserpell at gmail.com> wrote:
> Hi!,
>
> El Fri, Nov 13, 2015 at 11:42:30AM -0500, Ganesh Ajjanagadde escribio:
>> Whoever wrote this stuff had a pretty bad libm - digits differ pretty
>> quickly.
>
> They where correctly rounded to 24bit precision. I don't know if that
> was intentional, so I can't comment on the correctness of the patch.

Unless I am off here, doing things at 24 bits makes very little sense:
float itself offers 32 bits, and only place where Ci's get used is in
computing a float array statically so there can be no speed loss.
Thus, currently, useless loss of precision happens. This is very
similar to a recent commit I did with Michael on avcodec/faandct:
064ced5dc147c9b5a33807a90f07037feec57cbe.

In fact, in light of that, I may even want to change to long double
floating literals.
One may think that is excessive, but it has an additional advantage:
suppose someone wishes to move to double precision. Then the #defines
don't need to change, only the type signatures of the static float
array and other such things in the file. Using L is a best effort
solution that helps with future work (if any). Of course, one could
let the array have the literals directly instead of computed from the
cosines, but that results in significant loss of readability and is
likely unnecessary work.

>
>     Daniel.
>
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavcodec/ppc/fdctdsp.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/ppc/fdctdsp.c b/libavcodec/ppc/fdctdsp.c
>> index 924d12c..92866d3 100644
>> --- a/libavcodec/ppc/fdctdsp.c
>> +++ b/libavcodec/ppc/fdctdsp.c
>> @@ -37,13 +37,13 @@
>>  #define vu16(v) ((vector unsigned short) (v))
>>  #define vu32(v)   ((vector unsigned int) (v))
>>
>> -#define C1     0.98078525066375732421875000 /* cos(1 * PI / 16) */
>> -#define C2     0.92387950420379638671875000 /* cos(2 * PI / 16) */
>> -#define C3     0.83146959543228149414062500 /* cos(3 * PI / 16) */
>> -#define C4     0.70710676908493041992187500 /* cos(4 * PI / 16) */
>> -#define C5     0.55557024478912353515625000 /* cos(5 * PI / 16) */
>> -#define C6     0.38268342614173889160156250 /* cos(6 * PI / 16) */
>> -#define C7     0.19509032368659973144531250 /* cos(7 * PI / 16) */
>> +#define C1     0.98078528040323044912618224 /* cos(1 * PI / 16) */
>> +#define C2     0.92387953251128675612818319 /* cos(2 * PI / 16) */
>> +#define C3     0.83146961230254523707878838 /* cos(3 * PI / 16) */
>> +#define C4     M_SQRT1_2                    /* cos(4 * PI / 16) */
>> +#define C5     0.55557023301960222474283081 /* cos(5 * PI / 16) */
>> +#define C6     0.38268343236508977172845998 /* cos(6 * PI / 16) */
>> +#define C7     0.19509032201612826784828487 /* cos(7 * PI / 16) */
>>
>>  #define W0 -(2 * C2)
>>  #define W1  (2 * C6)
>> --
>> 2.6.2
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel