On 06/26/2011 06:24 PM, Justin Ruggles wrote: > On 06/26/2011 05:56 PM, Måns Rullgård wrote: > >> Justin Ruggles <[email protected]> writes: >> >>> On 06/26/2011 05:01 PM, Måns Rullgård wrote: >>> >>>> Justin Ruggles <[email protected]> writes: >>>> >>>>> diff --git a/libavcodec/ac3enc_template.c b/libavcodec/ac3enc_template.c >>>>> index f6248a8..2e4b01a 100644 >>>>> --- a/libavcodec/ac3enc_template.c >>>>> +++ b/libavcodec/ac3enc_template.c >>>>> @@ -107,6 +107,20 @@ void AC3_NAME(apply_mdct)(AC3EncodeContext *s) >>>>> } >>>>> >>>>> +void AC3_NAME(clip_coefficients)(AC3EncodeContext *s) >>>>> +{ >>>>> +#if CONFIG_AC3ENC_FLOAT >>>>> +#define vector_clip_coef s->dsp.vector_clipf >>>>> +#else >>>>> +#define vector_clip_coef s->dsp.vector_clip_int32 >>>>> +#endif >>>>> + int chan_size = AC3_MAX_COEFS * AC3_MAX_BLOCKS; >>>>> + vector_clip_coef(s->mdct_coef_buffer + chan_size, >>>>> + s->mdct_coef_buffer + chan_size, >>>>> + COEF_MIN, COEF_MAX, chan_size * s->channels); >>>>> +} >>>> >>>> Would it perhaps be prettier to put this function in the _fixed/float.c >>>> files instead? >>> >>> I could go either way with this... I have several other functions to add >>> that have a similar pattern. The point was to reduce code duplication, >>> especially in separate files. Another option would be to put the >>> conditional parts in ac3enc.h. >> >> It's just a single function call. The duplication is scarcely larger than >> the ifdeffery here. >> >> BTW, I'm not too fond of the double-indirect calls here and elsewhere. > > > Hmmm. Well, most of these are called from ff_ac3_encode_frame(), so if > that is moved to ac3enc_template.c then many of the function pointers > could go away I think. > > Does that sound like a better idea?
Doing this does seem cleaner to me. I had to make quite a few functions in ac3enc.c shared, but at least those only have 1 version each. It cuts the number of function pointers down to 5 (not counting the dsp ones). After this, the indirection to one-liner or few-liner functions in ac3enc_fixed/float.c are static instead of function pointers, so at least the compiler can inline them. I'll send a patch in a separate thread. Thanks, Justin _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
