On Tue, May 17, 2011 at 06:37:58PM +0100, Måns Rullgård wrote:
> 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.

True.  Maybe I have seen too many "mp" prefixes for my own good in
MPlayer.  I'm undecided here, but I don't intend to block your patch.
The filename could use the long name though and fit into our family
of mpegaudio filenames.

> >> --- /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.

No.  However, the API breakage season is drawing to a close, so there
is not much time left to discuss such questions.

Notice the "unrelated comment" moniker; I clearly labeled my remark as
outside the scope of your patch.

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

Just to do me a favor - thank you.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to