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

Paul B Mahol onemda at gmail.com
Wed Jan 30 14:30:43 CET 2013


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.

>
> Also missing docs (you could copy-paste and edit the excellent SoX
> documentation).
>
>>
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 5835a7e..938b183 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -53,6 +53,7 @@ OBJS-$(CONFIG_SWSCALE)                       +=
>> lswsutils.o
>>  OBJS-$(CONFIG_ACONVERT_FILTER)               += af_aconvert.o
>>  OBJS-$(CONFIG_AFADE_FILTER)                  += af_afade.o
>>  OBJS-$(CONFIG_AFORMAT_FILTER)                += af_aformat.o
>> +OBJS-$(CONFIG_ALLPASS_FILTER)                += af_biquads.o
>>  OBJS-$(CONFIG_AMERGE_FILTER)                 += af_amerge.o
>>  OBJS-$(CONFIG_AMIX_FILTER)                   += af_amix.o
>>  OBJS-$(CONFIG_ANULL_FILTER)                  += af_anull.o
>> @@ -68,14 +69,22 @@ OBJS-$(CONFIG_ASPLIT_FILTER)                 +=
>> split.o
>>  OBJS-$(CONFIG_ASTREAMSYNC_FILTER)            += af_astreamsync.o
>>  OBJS-$(CONFIG_ASYNCTS_FILTER)                += af_asyncts.o
>>  OBJS-$(CONFIG_ATEMPO_FILTER)                 += af_atempo.o
>> +OBJS-$(CONFIG_BANDPASS_FILTER)               += af_biquads.o
>> +OBJS-$(CONFIG_BANDREJECT_FILTER)             += af_biquads.o
>> +OBJS-$(CONFIG_BASS_FILTER)                   += af_biquads.o
>> +OBJS-$(CONFIG_BIQUAD_FILTER)                 += af_biquads.o
>>  OBJS-$(CONFIG_CHANNELMAP_FILTER)             += af_channelmap.o
>>  OBJS-$(CONFIG_CHANNELSPLIT_FILTER)           += af_channelsplit.o
>>  OBJS-$(CONFIG_EARWAX_FILTER)                 += af_earwax.o
>>  OBJS-$(CONFIG_EBUR128_FILTER)                += f_ebur128.o
>> +OBJS-$(CONFIG_EQUALIZER_FILTER)              += af_biquads.o
>> +OBJS-$(CONFIG_HIGHPASS_FILTER)               += af_biquads.o
>>  OBJS-$(CONFIG_JOIN_FILTER)                   += af_join.o
>> +OBJS-$(CONFIG_LOWPASS_FILTER)                += af_biquads.o
>>  OBJS-$(CONFIG_PAN_FILTER)                    += af_pan.o
>>  OBJS-$(CONFIG_RESAMPLE_FILTER)               += af_resample.o
>>  OBJS-$(CONFIG_SILENCEDETECT_FILTER)          += af_silencedetect.o
>> +OBJS-$(CONFIG_TREBLE_FILTER)                 += af_biquads.o
>>  OBJS-$(CONFIG_VOLUME_FILTER)                 += af_volume.o
>>  OBJS-$(CONFIG_VOLUMEDETECT_FILTER)           += af_volumedetect.o
>>
>> diff --git a/libavfilter/af_biquads.c b/libavfilter/af_biquads.c
>> new file mode 100644
>> index 0000000..26fbc16
>> --- /dev/null
>> +++ b/libavfilter/af_biquads.c
>> @@ -0,0 +1,498 @@
>> +/*
>> + * Copyright (c) 2013 Paul B Mahol
>> + * Copyright (c) 2006-2008 Rob Sykes <robs at users.sourceforge.net>
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>> + */
>> +
>> +/*
>> + * 2-pole filters designed by Robert Bristow-Johnson
>> <rbj at audioimagination.com>
>> + *   see http://www.musicdsp.org/files/Audio-EQ-Cookbook.txt
>> + *
>> + * 1-pole filters based on code (c) 2000 Chris Bagwell
>> <cbagwell at sprynet.com>
>> + *   Algorithms: Recursive single pole low/high pass filter
>> + *   Reference: The Scientist and Engineer's Guide to Digital Signal
>> Processing
>> + *
>> + *   low-pass: output[N] = input[N] * A + output[N-1] * B
>> + *     X = exp(-2.0 * pi * Fc)
>> + *     A = 1 - X
>> + *     B = X
>> + *     Fc = cutoff freq / sample rate
>> + *
>> + *     Mimics an RC low-pass filter:
>> + *
>> + *     ---/\/\/\/\----------->
>> + *                   |
>> + *                  --- C
>> + *                  ---
>> + *                   |
>> + *                   |
>> + *                   V
>> + *
>> + *   high-pass: output[N] = A0 * input[N] + A1 * input[N-1] + B1 *
>> output[N-1]
>> + *     X  = exp(-2.0 * pi * Fc)
>> + *     A0 = (1 + X) / 2
>> + *     A1 = -(1 + X) / 2
>> + *     B1 = X
>> + *     Fc = cutoff freq / sample rate
>> + *
>> + *     Mimics an RC high-pass filter:
>> + *
>> + *         || C
>> + *     ----||--------->
>> + *         ||    |
>> + *               <
>> + *               > R
>> + *               <
>> + *               |
>> + *               V
>> + */
>> +
>> +#include "libavutil/opt.h"
>> +#include "audio.h"
>> +#include "avfilter.h"
>> +#include "internal.h"
>> +
>> +enum filter_type {
>> +    biquad,
>> +    equalizer,
>> +    bass,
>> +    treble,
>> +    band,
>> +    bandpass,
>> +    bandreject,
>> +    allpass,
>> +    highpass,
>> +    lowpass,
>> +};
>> +
>> +typedef struct chan_cache {
>> +    double i1, i2;
>> +    double o1, o2;
>> +} chan_cache;
>> +
>> +typedef struct {
>> +    const AVClass *class;
>> +
>> +    int filter_type;
>> +    int poles;
>> +    int csg;
>> +
>> +    double gain;
>> +    double frequency;
>> +    double width;
>> +
>> +    double a0, a1, a2;
>> +    double b0, b1, b2;
>> +
>> +    chan_cache *cache;
>> +
>> +    void (*filter)(const void *ibuf, void *obuf, int len,
>> +                   double *i1, double *i2, double *o1, double *o2,
>> +                   double b0, double b1, double b2, double a1, double
>> a2);
>> +} BiquadsContext;
>> +
>> +static av_cold int init(AVFilterContext *ctx, const char *args)
>> +{
>> +    BiquadsContext *p = ctx->priv;
>> +    int ret;
>> +
>> +    av_opt_set_defaults(p);
>> +
>
>> +    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.

>
>> +}
>> +
>> +static int query_formats(AVFilterContext *ctx)
>> +{
>> +    AVFilterFormats *formats;
>> +    AVFilterChannelLayouts *layouts;
>> +    static const enum AVSampleFormat sample_fmts[] = {
>> +        AV_SAMPLE_FMT_S16P,
>> +        AV_SAMPLE_FMT_S32P,
>> +        AV_SAMPLE_FMT_FLTP,
>> +        AV_SAMPLE_FMT_DBLP,
>> +        AV_SAMPLE_FMT_NONE
>> +    };
>> +
>> +    layouts = ff_all_channel_layouts();
>> +    if (!layouts)
>> +        return AVERROR(ENOMEM);
>> +    ff_set_common_channel_layouts(ctx, layouts);
>> +
>> +    formats = ff_make_format_list(sample_fmts);
>> +    if (!formats)
>> +        return AVERROR(ENOMEM);
>> +    ff_set_common_formats(ctx, formats);
>> +
>> +    formats = ff_all_samplerates();
>> +    if (!formats)
>> +        return AVERROR(ENOMEM);
>> +    ff_set_common_samplerates(ctx, formats);
>> +
>> +    return 0;
>> +}
>
> Note: we need an helper to reduce the boilerplate.

I'm not going to do it. I did not designed such nonsense at first place.
If I cared for every similar code in tree I would never do anything
relevant.

>
>> +
>> +#define BIQUAD_FILTER(name, type)
>>     \
>> +static void biquad_## name (const void *input, void *output, int len,
>>     \
>> +                            double *i1, double *i2, double *o1, double
>> *o2,   \
>> +                            double b0, double b1, double b2,
>>     \
>> +                            double a1, double a2)
>>     \
>> +{
>>     \
>> +    const type *ibuf = input;
>>     \
>> +    type *obuf = output;
>>     \
>> +    int i;
>>     \
>> +
>>     \
>> +    for (i = 0; i < len; i++) {
>>     \
>> +        double o0 = ibuf[i] * b0 + *i1 * b1 + *i2 * b2 - *o1 * a1 - *o2 *
>> a2; \
>> +        *i2 = *i1;
>>     \
>> +        *i1 = ibuf[i];
>>     \
>> +        *o2 = *o1;
>>     \
>> +        *o1 = o0;
>>     \
>> +        obuf[i] = o0;
>>     \
>> +    }
>>     \
>> +}
>> +
>> +BIQUAD_FILTER(s16, int16_t)
>> +BIQUAD_FILTER(s32, int32_t)
>> +BIQUAD_FILTER(flt, float)
>> +BIQUAD_FILTER(dbl, double)
>> +
>> +static int config_output(AVFilterLink *outlink)
>> +{
>> +    AVFilterContext *ctx    = outlink->src;
>> +    BiquadsContext *p       = ctx->priv;
>> +    AVFilterLink *inlink    = ctx->inputs[0];
>> +    double A = exp(p->gain / 40 * log(10.));
>> +    double w0 = 2 * M_PI * p->frequency / inlink->sample_rate;
>> +    double alpha = sin(w0) / (2 * p->frequency / p->width);
>> +
>> +    if (w0 > M_PI) {
>> +        av_log(ctx, AV_LOG_ERROR,
>> +               "frequency %f must be less than half the sample-rate
>> %d\n",
>> +               p->frequency, inlink->sample_rate);
>> +        return AVERROR(EINVAL);
>> +    }
>> +
>> +    switch (p->filter_type) {
>> +    case equalizer:
>> +        p->a0 =   1 + alpha / A;
>> +        p->a1 =  -2 * cos(w0);
>> +        p->a2 =   1 - alpha / A;
>> +        p->b0 =   1 + alpha * A;
>> +        p->b1 =  -2 * cos(w0);
>> +        p->b2 =   1 - alpha * A;
>> +        break;
>> +    case bass:
>> +        p->a0 =          (A + 1) + (A - 1) * cos(w0) + 2 * sqrt(A) *
>> alpha;
>> +        p->a1 =    -2 * ((A - 1) + (A + 1) * cos(w0));
>> +        p->a2 =          (A + 1) + (A - 1) * cos(w0) - 2 * sqrt(A) *
>> alpha;
>> +        p->b0 =     A * ((A + 1) - (A - 1) * cos(w0) + 2 * sqrt(A) *
>> alpha);
>> +        p->b1 = 2 * A * ((A - 1) - (A + 1) * cos(w0));
>> +        p->b2 =     A * ((A + 1) - (A - 1) * cos(w0) - 2 * sqrt(A) *
>> alpha);
>> +        break;
>> +    case treble:
>> +        p->a0 =          (A + 1) - (A - 1) * cos(w0) + 2 * sqrt(A) *
>> alpha;
>> +        p->a1 =     2 * ((A - 1) - (A + 1) * cos(w0));
>> +        p->a2 =          (A + 1) - (A - 1) * cos(w0) - 2 * sqrt(A) *
>> alpha;
>> +        p->b0 =     A * ((A + 1) + (A - 1) * cos(w0) + 2 * sqrt(A) *
>> alpha);
>> +        p->b1 =-2 * A * ((A - 1) + (A + 1) * cos(w0));
>> +        p->b2 =     A * ((A + 1) + (A - 1) * cos(w0) - 2 * sqrt(A) *
>> alpha);
>> +        break;
>> +    case bandpass:
>> +        if (p->csg) {
>> +            p->a0 =  1 + alpha;
>> +            p->a1 = -2 * cos(w0);
>> +            p->a2 =  1 - alpha;
>> +            p->b0 =  sin(w0) / 2;
>> +            p->b1 =  0;
>> +            p->b2 = -sin(w0) / 2;
>> +        } else {
>> +            p->a0 =  1 + alpha;
>> +            p->a1 = -2 * cos(w0);
>> +            p->a2 =  1 - alpha;
>> +            p->b0 =  alpha;
>> +            p->b1 =  0;
>> +            p->b2 = -alpha;
>> +        }
>> +        break;
>> +    case bandreject:
>> +        p->a0 =  1 + alpha;
>> +        p->a1 = -2 * cos(w0);
>> +        p->a2 =  1 - alpha;
>> +        p->b0 =  1;
>> +        p->b1 = -2 * cos(w0);
>> +        p->b2 =  1;
>> +        break;
>> +    case lowpass:
>> +        if (p->poles == 1) {
>> +            p->a0 = 1;
>> +            p->a1 = -exp(-w0);
>> +            p->a2 = 0;
>> +            p->b0 = 1 + p->a1;
>> +            p->b1 = 0;
>> +            p->b2 = 0;
>> +        } else {
>> +            p->a0 =  1 + alpha;
>> +            p->a1 = -2 * cos(w0);
>> +            p->a2 =  1 - alpha;
>> +            p->b0 = (1 - cos(w0)) / 2;
>> +            p->b1 =  1 - cos(w0);
>> +            p->b2 = (1 - cos(w0)) / 2;
>> +        }
>> +        break;
>> +    case highpass:
>> +        if (p->poles == 1) {
>> +            p->a0 = 1;
>> +            p->a1 = -exp(-w0);
>> +            p->a2 = 0;
>> +            p->b0 = (1 - p->a1) / 2;
>> +            p->b1 = -p->b0;
>> +            p->b2 = 0;
>> +        } else {
>> +            p->a0 =   1 + alpha;
>> +            p->a1 =  -2 * cos(w0);
>> +            p->a2 =   1 - alpha;
>> +            p->b0 =  (1 + cos(w0)) / 2;
>> +            p->b1 = -(1 + cos(w0));
>> +            p->b2 =  (1 + cos(w0)) / 2;
>> +        }
>> +        break;
>> +    case allpass:
>> +        p->a0 =  1 + alpha;
>> +        p->a1 = -2 * cos(w0);
>> +        p->a2 =  1 - alpha;
>> +        p->b0 =  1 - alpha;
>> +        p->b1 = -2 * cos(w0);
>> +        p->b2 =  1 + alpha;
>> +        break;
>> +    }
>> +
>> +    p->a1 /= p->a0;
>> +    p->a2 /= p->a0;
>> +    p->b0 /= p->a0;
>> +    p->b1 /= p->a0;
>> +    p->b2 /= p->a0;
>> +
>> +    p->cache = av_realloc(p->cache, sizeof(chan_cache) *
>> inlink->channels);
>> +    if (!p->cache)
>> +        return AVERROR(ENOMEM);
>> +
>> +    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.

>
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *buf)
>> +{
>> +    BiquadsContext *p       = inlink->dst->priv;
>> +    AVFilterLink *outlink   = inlink->dst->outputs[0];
>> +    AVFilterBufferRef *out_buf;
>> +    int nb_samples = buf->audio->nb_samples;
>> +    int ch;
>> +
>> +    if (buf->perms & AV_PERM_WRITE) {
>> +        out_buf = buf;
>> +    } else {
>> +        out_buf = ff_get_audio_buffer(inlink, AV_PERM_WRITE,
>> nb_samples);
>> +        if (!out_buf)
>> +            return AVERROR(ENOMEM);
>> +        out_buf->pts = buf->pts;
>> +    }
>> +
>> +    for (ch = 0; ch < buf->audio->channels; ch++)
>> +        p->filter((const float *)buf->extended_data[ch],
>> +                   (float *)out_buf->extended_data[ch], nb_samples,
>> +                   &p->cache[ch].i1, &p->cache[ch].i2,
>> +                   &p->cache[ch].o1, &p->cache[ch].o2,
>> +                   p->b0, p->b1, p->b2, p->a1, p->a2);
>> +
>> +    if (buf != out_buf)
>> +        avfilter_unref_buffer(buf);
>> +
>> +    return ff_filter_frame(outlink, out_buf);
>> +}
>> +
>> +static av_cold void uninit(AVFilterContext *ctx)
>> +{
>> +    BiquadsContext *p = ctx->priv;
>> +
>> +    av_freep(&p->cache);
>
> av_opt_free(p);
>
>
>> +}
>> +
>> +static const AVFilterPad inputs[] = {
>> +    {
>> +        .name         = "default",
>> +        .type         = AVMEDIA_TYPE_AUDIO,
>> +        .filter_frame = filter_frame,
>> +    },
>> +    { NULL }
>> +};
>> +
>> +static const AVFilterPad outputs[] = {
>> +    {
>> +        .name         = "default",
>> +        .type         = AVMEDIA_TYPE_AUDIO,
>> +        .config_props = config_output,
>> +    },
>> +    { NULL }
>> +};
>> +
>> +#define OFFSET(x) offsetof(BiquadsContext, x)
>> +#define FLAGS AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>> +
>> +#define DEFINE_BIQUAD_FILTER(name_, description_)
>> \
>> +AVFILTER_DEFINE_CLASS(name_);
>> \
>> +static av_cold int name_##_init(AVFilterContext *ctx, const char *args)
>> \
>> +{
>> \
>> +    BiquadsContext *p = ctx->priv;
>> \
>> +    p->class = &name_##_class;
>> \
>> +    p->filter_type = name_;
>> \
>> +    return init(ctx, args);
>> \
>> +}
>> \
>> +                                                         \
>> +AVFilter avfilter_af_##name_ = {                         \
>> +    .name          = #name_,                             \
>> +    .description   = NULL_IF_CONFIG_SMALL(description_), \
>> +    .priv_size     = sizeof(BiquadsContext),             \
>> +    .init          = name_##_init,                       \
>> +    .uninit        = uninit,                             \
>> +    .query_formats = query_formats,                      \
>> +    .inputs        = inputs,                             \
>> +    .outputs       = outputs,                            \
>> +    .priv_class    = &name_##_class,                     \
>> +}
>> +
>> +#if CONFIG_EQUALIZER_FILTER
>> +static const AVOption equalizer_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=0}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=0}, 0, 999999, FLAGS},
>> +    {"width", "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=1}, 0, 999, FLAGS},
>> +    {"w",     "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=1}, 0, 999, FLAGS},
>> +    {"gain", "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {"g",    "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(equalizer, "Apply two-pole peaking equalization (EQ)
>> filter.");
>> +#endif  /* CONFIG_EQUALIZER_FILTER */
>
> Nit: add an empty line to mark the end of a filter template, same below.

I prefer to keep current state.

>
>> +#if CONFIG_BASS_FILTER
>> +static const AVOption bass_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=100}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=100}, 0, 999999, FLAGS},
>> +    {"width", "set shelf transition steep", OFFSET(width),
>> AV_OPT_TYPE_DOUBLE, {.dbl=202.99}, 0, 99999, FLAGS},
>> +    {"w",     "set shelf transition steep", OFFSET(width),
>> AV_OPT_TYPE_DOUBLE, {.dbl=202.99}, 0, 99999, FLAGS},
>> +    {"gain", "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {"g",    "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {NULL},
>> +};
>> +
>
>> +DEFINE_BIQUAD_FILTER(bass, "Bost or cut lower frequencies.");
>
> Boost?
>
>> +#endif  /* CONFIG_BASS_FILTER */
>> +#if CONFIG_TREBLE_FILTER
>> +static const AVOption treble_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"width", "set shelf transition steep", OFFSET(width),
>> AV_OPT_TYPE_DOUBLE, {.dbl=202.99}, 0, 99999, FLAGS},
>> +    {"w",     "set shelf transition steep", OFFSET(width),
>> AV_OPT_TYPE_DOUBLE, {.dbl=202.99}, 0, 99999, FLAGS},
>> +    {"gain", "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {"g",    "set gain", OFFSET(gain), AV_OPT_TYPE_DOUBLE, {.dbl=0},
>> -900, 900, FLAGS},
>> +    {NULL},
>> +};
>
> I'm a bit unhappy with the options duplication (set lut* to see how to
> avoid that), OTOH I see the point of having them separated.

It is separated because of different default values. It's not really obvious
how to get unclipped output, unless there is some way to plot values.
Instead you are forced to start guessing numbers and for
every guess you check if clipping happened somewhere in stream.
I try to work on every aspect of this issue as I can.

>
>> +
>> +DEFINE_BIQUAD_FILTER(treble, "Bost or cut upper frequencies.");
>
> Ditto.
>
>> +#endif  /* CONFIG_TREBLE_FILTER */
>> +#if CONFIG_BANDPASS_FILTER
>> +static const AVOption bandpass_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"width", "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=0.5}, 0, 999, FLAGS},
>> +    {"w",     "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=0.5}, 0, 999, FLAGS},
>> +    {"csg",   "use constant skirt gain", OFFSET(csg), AV_OPT_TYPE_INT,
>> {.i64=0}, 0, 1, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(bandpass, "Apply a two-pole Butterworth band-pass
>> filter.");
>> +#endif  /* CONFIG_BANDPASS_FILTER */
>> +#if CONFIG_BANDREJECT_FILTER
>> +static const AVOption bandreject_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"width", "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=0.5}, 0, 999, FLAGS},
>> +    {"w",     "set band-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=0.5}, 0, 999, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(bandreject, "Apply a two-pole Butterworth
>> band-reject filter.");
>> +#endif  /* CONFIG_BANDREJECT_FILTER */
>> +#if CONFIG_LOWPASS_FILTER
>> +static const AVOption lowpass_options[] = {
>> +    {"frequency", "set frequency", OFFSET(frequency), AV_OPT_TYPE_DOUBLE,
>> {.dbl=500}, 0, 999999, FLAGS},
>> +    {"f",         "set frequency", OFFSET(frequency), AV_OPT_TYPE_DOUBLE,
>> {.dbl=500}, 0, 999999, FLAGS},
>> +    {"width", "set width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {"w",     "set width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {"poles", "set number of poles", OFFSET(poles), AV_OPT_TYPE_INT,
>> {.i64=2}, 1, 2, FLAGS},
>> +    {"p",     "set number of poles", OFFSET(poles), AV_OPT_TYPE_INT,
>> {.i64=2}, 1, 2, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(lowpass, "Apply a low-pass filter with 3dB point
>> frequency.");
>> +#endif  /* CONFIG_LOWPASS_FILTER */
>> +#if CONFIG_HIGHPASS_FILTER
>> +static const AVOption highpass_options[] = {
>> +    {"frequency", "set frequency", OFFSET(frequency), AV_OPT_TYPE_DOUBLE,
>> {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"f",         "set frequency", OFFSET(frequency), AV_OPT_TYPE_DOUBLE,
>> {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"width", "set width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {"w",     "set width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {"poles", "set number of poles", OFFSET(poles), AV_OPT_TYPE_INT,
>> {.i64=2}, 1, 2, FLAGS},
>> +    {"p",     "set number of poles", OFFSET(poles), AV_OPT_TYPE_INT,
>> {.i64=2}, 1, 2, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(highpass, "Apply a high-pass filter with 3dB point
>> frequency.");
>> +#endif  /* CONFIG_HIGHPASS_FILTER */
>> +#if CONFIG_ALLPASS_FILTER
>> +static const AVOption allpass_options[] = {
>> +    {"frequency", "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"f",         "set central frequency", OFFSET(frequency),
>> AV_OPT_TYPE_DOUBLE, {.dbl=3000}, 0, 999999, FLAGS},
>> +    {"width", "set filter-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {"w",     "set filter-width", OFFSET(width), AV_OPT_TYPE_DOUBLE,
>> {.dbl=707.1}, 0, 99999, FLAGS},
>> +    {NULL},
>> +};
>> +
>> +DEFINE_BIQUAD_FILTER(allpass, "Apply a two-pole all-pass filter.");
>> +#endif  /* CONFIG_ALLPASS_FILTER */
>> +#if CONFIG_BIQUAD_FILTER
>> +static const AVOption biquad_options[] = {
>
>> +    {"a0", NULL, OFFSET(a0), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>> +    {"a1", NULL, OFFSET(a1), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>> +    {"a2", NULL, OFFSET(a2), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>> +    {"b0", NULL, OFFSET(b0), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>> +    {"b1", NULL, OFFSET(b1), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>> +    {"b2", NULL, OFFSET(b2), AV_OPT_TYPE_DOUBLE, {.dbl=1}, INT16_MAX,
>> INT16_MAX, FLAGS},
>
> uhm no help?

What help, those are just parameters for function?
If you did not notice all other filters in file are just user friendly
variant of this one.

>
> [...]
>
> I'd prefer to leave review of the internal code to someone with more
> signal processing expertise (Rob?).

Code funcionality is indentical and gives indentical (except rounding) output.

> --
> FFmpeg = Fostering & Fast Mortal Perennial Elastic Geek
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list