Diego Biurrun <[email protected]> writes:

> On Tue, May 17, 2011 at 11:48:38AM +0100, Mans Rullgard wrote:
>> This builds the float and fixed-point versions of dct32 separately
>> instead of #including the file in dct.c and mpegaudiodec.c.
>
> neat idea
>
>> --- /dev/null
>> +++ b/libavcodec/dct32.h
>> @@ -0,0 +1,25 @@
>> +
>> +#ifndef AVCODEC_DCT32_H
>> +#define AVCODEC_DCT32_H
>> +
>> +void ff_dct32_float(float *dst, const float *src);
>> +void ff_dct32_fixed(int *dst, const int *src);
>> +
>> +#endif
>
> nit: Please add the standard #endif comment.
>
>> --- a/libavcodec/dct32.c
>> +++ b/libavcodec/dct32.c
>> @@ -19,10 +19,19 @@
>>  
>> -#ifdef DCT32_FLOAT
>> +#include "dct32.h"
>> +#include "mathops.h"
>> +
>> +#if DCT32_FLOAT
>> +#   define dct32 ff_dct32_float
>>  #   define FIXHR(x)       ((float)(x))
>>  #   define MULH3(x, y, s) ((s)*(y)*(x))
>>  #   define INTFLOAT float
>> +#else
>> +#   define dct32 ff_dct32_fixed
>> +#   define FIXHR(a)       ((int)((a) * (1LL<<32) + 0.5))
>> +#   define MULH3(x, y, s) MULH((s)*(x), y)
>> +#   define INTFLOAT int
>>  #endif
>>  
>> @@ -103,7 +112,7 @@
>>  
>>  /* DCT32 without 1/sqrt(2) coef zero scaling. */
>> -static void dct32(INTFLOAT *out, const INTFLOAT *tab)
>> +void dct32(INTFLOAT *out, const INTFLOAT *tab)
>>  {
>> --- /dev/null
>> +++ b/libavcodec/dct32_fixed.c
>> @@ -0,0 +1,2 @@
>> +#define DCT32_FLOAT 0
>> +#include "dct32.c"
>> --- /dev/null
>> +++ b/libavcodec/dct32_float.c
>> @@ -0,0 +1,2 @@
>> +#define DCT32_FLOAT 1
>> +#include "dct32.c"
>
> I think it would be cleaner to move the defines to dct32_fixed.c /
> dct32_float.c, that saves the #ifdeffery.

I disagree.  I think it's better to have the definitions in the same
file that uses them.

> Also, the files are missing standard license headers.

Last time I did this you told me to skip them.  Which way do you want it?

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to