Diego Biurrun <[email protected]> writes:

> On Tue, May 17, 2011 at 05:24:24PM +0100, Mans Rullgard wrote:
>> This separation allows these functions to be used in a cleaner
>> fashion from other codecs (e.g. qdm2) and simplifies creating
>> optimised versions of them.
>> ---
>>  17 files changed, 388 insertions(+), 255 deletions(-)
>>  create mode 100644 libavcodec/mpadsp.c
>>  create mode 100644 libavcodec/mpadsp.h
>>  create mode 100644 libavcodec/mpadsp_fixed.c
>>  create mode 100644 libavcodec/mpadsp_float.c
>>  create mode 100644 libavcodec/mpadsp_template.c
>
> I find the name mpadsp non-intuitive, I would suggest mpegaudiodsp
> instead.

That's very long, especially as a base for function names etc.

> The general idea of the patch LGTM otherwise.
>
>> --- /dev/null
>> +++ b/libavcodec/mpadsp.h
>> @@ -0,0 +1,63 @@
>> +
>> +#ifndef AVCODEC_MPADSP_H
>> +#define AVCODEC_MPADSP_H
>> +
>> +#include <stdint.h>
>> +
>> +typedef struct MPADSPContext {
>> +    void (*apply_window_float)(float *synth_buf, float *window,
>> +                               int *dither_state, float *samples, int incr);
>> +    void (*apply_window_fixed)(int32_t *synth_buf, int32_t *window,
>> +                               int *dither_state, int16_t *samples, int 
>> incr);
>> +    void (*dct32_float)(float *dst, const float *src);
>> +    void (*dct32_fixed)(int *dst, const int *src);
>> +} MPADSPContext;
>
> unrelated comment: Will we keep typedeffing structs forever?

Somehow I suspect you would have asked me to add the typedef (for
consistency) had I left it out.

>> +#endif
>
> Please add an #endif comment.

Why?

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

Reply via email to