[FFmpeg-devel] [PATCH 3/3] Allow either float or int16_t input and output in ff_iir_filter().

[Ronald S. Bultje]

Hi,

2011/1/19 M?ns Rullg?rd <mans at mansr.com>:
> Justin Ruggles <justin.ruggles at gmail.com> writes:
>
>> ---
>> ?libavcodec/iirfilter.c | ?103 +++++++++++++++++++++++++++++++----------------
>> ?libavcodec/iirfilter.h | ? ?3 +-
>> ?libavcodec/psymodel.c ?| ? ?3 +-
>> ?3 files changed, 72 insertions(+), 37 deletions(-)
>>
>>
>> ?void ff_iir_filter(const struct FFIIRFilterCoeffs *c, struct FFIIRFilterState *s,
>> - ? ? ? ? ? ? ? ? ? int size, const int16_t *src, int sstep, int16_t *dst, int dstep)
>> + ? ? ? ? ? ? ? ? ? int size, const void *src, int sstep, void *dst, int dstep,
>> + ? ? ? ? ? ? ? ? ? enum AVSampleFormat sample_fmt)
>> ?{
>> - ? ?int i;
>> -
>> - ? ?if(c->order == 4){
>> - ? ? ? ?for(i = 0; i < size; i += 4){
>> - ? ? ? ? ? ?float in, res;
>> -
>> - ? ? ? ? ? ?FILTER(0, 1, 2, 3);
>> - ? ? ? ? ? ?FILTER(1, 2, 3, 0);
>> - ? ? ? ? ? ?FILTER(2, 3, 0, 1);
>> - ? ? ? ? ? ?FILTER(3, 0, 1, 2);
>> - ? ? ? ?}
>> - ? ?}else{
>> - ? ? ? ?for(i = 0; i < size; i++){
>> - ? ? ? ? ? ?int j;
>> - ? ? ? ? ? ?float in, res;
>> - ? ? ? ? ? ?in = *src * c->gain;
>> - ? ? ? ? ? ?for(j = 0; j < c->order; j++)
>> - ? ? ? ? ? ? ? ?in += c->cy[j] * s->x[j];
>> - ? ? ? ? ? ?res = s->x[0] + in + s->x[c->order >> 1] * c->cx[c->order >> 1];
>> - ? ? ? ? ? ?for(j = 1; j < c->order >> 1; j++)
>> - ? ? ? ? ? ? ? ?res += (s->x[j] + s->x[c->order - j]) * c->cx[j];
>> - ? ? ? ? ? ?for(j = 0; j < c->order - 1; j++)
>> - ? ? ? ? ? ? ? ?s->x[j] = s->x[j + 1];
>> - ? ? ? ? ? ?*dst = av_clip_int16(lrintf(res));
>> - ? ? ? ? ? ?s->x[c->order - 1] = in;
>> - ? ? ? ? ? ?src += sstep;
>> - ? ? ? ? ? ?dst += dstep;
>> - ? ? ? ?}
>> + ? ?if (!(sample_fmt == AV_SAMPLE_FMT_S16 || sample_fmt == AV_SAMPLE_FMT_FLT))
>> + ? ? ? ?return;
>> +
>> + ? ?if (sample_fmt == AV_SAMPLE_FMT_S16) {
>> + ? ? ? ?void (*conv_filter_output)(float, void *) = conv_filter_output_s16;
>> + ? ? ? ?if (c->order == 4)
>> + ? ? ? ? ? ?FILTER4(int16_t)
>> + ? ? ? ?else
>> + ? ? ? ? ? ?FILTER_DIRECT_FORM_II(int16_t)
>> + ? ?} else {
>> + ? ? ? ?void (*conv_filter_output)(float, void *) = conv_filter_output_flt;
>> + ? ? ? ?if (c->order == 4)
>> + ? ? ? ? ? ?FILTER4(float)
>> + ? ? ? ?else
>> + ? ? ? ? ? ?FILTER_DIRECT_FORM_II(float)
>
> I think two different functions would be preferable. ?Fewer branches
> (and parameters) is always better, and I dread to think what some
> compilers might do with code like the above. ?Also consider that we
> might want to write asm versions of those some day. ?That is much
> easier if they are separated.

I tend to agree. For voice codecs, we wrote a bunch of these filters
already anyway, and there's always 2 of each, one int (function()) and
one float (functionf()), works fine there, is probably OK here also.

Also, how similar are voice-codec iir filters
(ff_celp_lp_zero_synthesis_filterf()) from this one?

Ronald