[FFmpeg-devel] [PATCH] Port biquads filters from SoX

[Stefano Sabatini]

On date Wednesday 2013-01-30 13:30:43 +0000, Paul B Mahol encoded:
> On 1/29/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> > On date Monday 2013-01-28 20:04:19 +0000, Paul B Mahol encoded:
> >> Adds allpass, bandpass, bandreject, bass, biquad,
> >> equalizer, highpass, lowpass and treble filter.
> >>
> >> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> >> ---
> >>  libavfilter/Makefile     |   9 +
> >>  libavfilter/af_biquads.c | 498
> >> +++++++++++++++++++++++++++++++++++++++++++++++
> >>  libavfilter/allfilters.c |   9 +
> >>  3 files changed, 516 insertions(+)
> >>  create mode 100644 libavfilter/af_biquads.c
> >
> > Reminder: minor bump and changelog entry
> 
> I'm aware of that already. I do not need such reminders.

Then feel free to forget the reminders.

[...]
> >> +    if ((ret = av_set_options_string(p, args, "=", ":")) < 0)
> >> +        return ret;
> >> +
> >> +    return 0;
> >
> > return av_set_options_string(...);
> 
> Not really, I still need to add check for some values which do not have sense,
> but are possible and there is no better way to handle them due design flaws
> is current API.

Please be specific.

OTOH I see that avfilter_init_filter() should return 0 in case of
success, so the additional code may be required.

[...]
> >> +    switch (inlink->format) {
> >> +    case AV_SAMPLE_FMT_S16P: p->filter = biquad_s16; break;
> >> +    case AV_SAMPLE_FMT_S32P: p->filter = biquad_s32; break;
> >> +    case AV_SAMPLE_FMT_FLTP: p->filter = biquad_flt; break;
> >> +    case AV_SAMPLE_FMT_DBLP: p->filter = biquad_dbl; break;
> >
> > assert otherwise
> 
> Why? If somebody who want to modify code and did not notice this switch
> it should better not code at all for him.
> 
> Is there policy to add such bloated stuff in code in such trivial places?
> I'm not going to add it. If you insist, feel free to add it any time later.

I don't insist.

[...]
-- 
FFmpeg = Fascinating and Fascinating Mega Patchable Erotic Gladiator