[FFmpeg-devel] [PATCH 2/2] avfilter: add sidechaingate filter

Ganesh Ajjanagadde gajjanag at mit.edu
Tue Dec 1 02:25:07 CET 2015


On Sun, Nov 29, 2015 at 6:03 PM, Paul B Mahol <onemda at gmail.com> wrote:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  doc/filters.texi         |  61 +++++++++++++++++
>  libavfilter/Makefile     |   1 +
>  libavfilter/af_agate.c   | 170 ++++++++++++++++++++++++++++++++++++++++++++++-
>  libavfilter/allfilters.c |   1 +
>  4 files changed, 230 insertions(+), 3 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 0395c7a..ed4a376 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2531,6 +2531,67 @@ ffmpeg -i main.flac -i sidechain.flac -filter_complex "[1:a]asplit=2[sc][mix];[0
>  @end example
>  @end itemize
>
> + at section sidechaingate
> +
> +A sidechain gate acts like a normal (wideband) gate but has the ability to
> +filter the detected signal before sending it to the gain reduction stage.
> +Normally a gate uses the full range signal to detect a level above the
> +threshold.
> +For example: If you cut all lower frequencies from your sidechain signal
> +the gate will decrease the volume of your track only if not enough highs
> +appear. With this technique you are able to reduce the resonation of a
> +natural drum or remove "rumbling" of muted strokes from a heavily distorted
> +guitar.
> +It needs two input streams and returns one output stream.
> +First input stream will be processed depending on second stream signal.
> +
> +The filter accepts the following options:
> +
> + at table @option
> + at item level_in
> +Set input level before filtering.
> +Default is 1. Allowed range is from 0.015625 to 64.
> +
> + at item range
> +Set the level of gain reduction when the signal is below the threshold.
> +Default is 0.06125. Allowed range is from 0 to 1.
> +
> + at item threshold
> +If a signal rises above this level the gain reduction is released.
> +Default is 0.125. Allowed range is from 0 to 1.
> +
> + at item ratio
> +Set a ratio about which the signal is reduced.
> +Default is 2. Allowed range is from 1 to 9000.
> +
> + at item attack
> +Amount of milliseconds the signal has to rise above the threshold before gain
> +reduction stops.
> +Default is 20 milliseconds. Allowed range is from 0.01 to 9000.
> +
> + at item release
> +Amount of milliseconds the signal has to fall below the threshold before the
> +reduction is increased again. Default is 250 milliseconds.
> +Allowed range is from 0.01 to 9000.
> +
> + at item makeup
> +Set amount of amplification of signal after processing.
> +Default is 1. Allowed range is from 1 to 64.
> +
> + at item knee
> +Curve the sharp knee around the threshold to enter gain reduction more softly.
> +Default is 2.828427125. Allowed range is from 1 to 8.
> +
> + at item detection
> +Choose if exact signal should be taken for detection or an RMS like one.
> +Default is peak. Can be peak or rms.
> +
> + at item link
> +Choose if the average level between all channels or the louder channel affects
> +the reduction.
> +Default is average. Can be average or maximum.
> + at end table
> +
>  @section silencedetect
>
>  Detect silence in an audio stream.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index e31bdaa..582fd0e 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -82,6 +82,7 @@ OBJS-$(CONFIG_REPLAYGAIN_FILTER)             += af_replaygain.o
>  OBJS-$(CONFIG_RESAMPLE_FILTER)               += af_resample.o
>  OBJS-$(CONFIG_RUBBERBAND_FILTER)             += af_rubberband.o
>  OBJS-$(CONFIG_SIDECHAINCOMPRESS_FILTER)      += af_sidechaincompress.o
> +OBJS-$(CONFIG_SIDECHAINGATE_FILTER)          += af_agate.o
>  OBJS-$(CONFIG_SILENCEDETECT_FILTER)          += af_silencedetect.o
>  OBJS-$(CONFIG_SILENCEREMOVE_FILTER)          += af_silenceremove.o
>  OBJS-$(CONFIG_STEREOTOOLS_FILTER)            += af_stereotools.o
> diff --git a/libavfilter/af_agate.c b/libavfilter/af_agate.c
> index d3f74fb..23e45c6 100644
> --- a/libavfilter/af_agate.c
> +++ b/libavfilter/af_agate.c
> @@ -18,6 +18,12 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>
> +/**
> + * @file
> + * Audio (Sidechain) Gate filter
> + */
> +
> +#include "libavutil/avassert.h"
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/opt.h"
>  #include "avfilter.h"
> @@ -46,12 +52,14 @@ typedef struct AudioGateContext {
>      double lin_slope;
>      double attack_coeff;
>      double release_coeff;
> +
> +    AVFrame *input_frame[2];
>  } AudioGateContext;
>
>  #define OFFSET(x) offsetof(AudioGateContext, x)
>  #define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>
> -static const AVOption agate_options[] = {
> +static const AVOption options[] = {
>      { "level_in",  "set input level",        OFFSET(level_in),  AV_OPT_TYPE_DOUBLE, {.dbl=1},           0.015625,   64, A },
>      { "range",     "set max gain reduction", OFFSET(range),     AV_OPT_TYPE_DOUBLE, {.dbl=0.06125},     0, 1, A },
>      { "threshold", "set threshold",          OFFSET(threshold), AV_OPT_TYPE_DOUBLE, {.dbl=0.125},       0, 1, A },
> @@ -69,6 +77,7 @@ static const AVOption agate_options[] = {
>      { NULL }
>  };
>
> +#define agate_options options
>  AVFILTER_DEFINE_CLASS(agate);
>
>  static int query_formats(AVFilterContext *ctx)
> @@ -97,7 +106,7 @@ static int query_formats(AVFilterContext *ctx)
>      return ff_set_common_samplerates(ctx, formats);
>  }
>
> -static int config_input(AVFilterLink *inlink)
> +static int agate_config_input(AVFilterLink *inlink)
>  {
>      AVFilterContext *ctx = inlink->dst;
>      AudioGateContext *s = ctx->priv;
> @@ -221,7 +230,7 @@ static const AVFilterPad inputs[] = {
>          .name         = "default",
>          .type         = AVMEDIA_TYPE_AUDIO,
>          .filter_frame = filter_frame,
> -        .config_props = config_input,
> +        .config_props = agate_config_input,
>      },
>      { NULL }
>  };
> @@ -243,3 +252,158 @@ AVFilter ff_af_agate = {
>      .inputs         = inputs,
>      .outputs        = outputs,
>  };
> +
> +#if CONFIG_SIDECHAINGATE_FILTER
> +
> +#define sidechaingate_options options
> +AVFILTER_DEFINE_CLASS(sidechaingate);
> +
> +static int scfilter_frame(AVFilterLink *link, AVFrame *in)
> +{
> +    AVFilterContext *ctx = link->dst;
> +    AudioGateContext *s = ctx->priv;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    const double *scsrc;
> +    double *sample;
> +    int nb_samples;
> +    int ret, i;
> +
> +    for (i = 0; i < 2; i++)
> +        if (link == ctx->inputs[i])
> +            break;
> +    av_assert0(i < 2 && !s->input_frame[i]);
> +    s->input_frame[i] = in;
> +
> +    if (!s->input_frame[0] || !s->input_frame[1])
> +        return 0;
> +
> +    nb_samples = FFMIN(s->input_frame[0]->nb_samples,
> +                       s->input_frame[1]->nb_samples);
> +
> +    sample = (double *)s->input_frame[0]->data[0];
> +    scsrc = (const double *)s->input_frame[1]->data[0];
> +
> +    gate(s, sample, sample, scsrc, nb_samples,
> +         ctx->inputs[0], ctx->inputs[1]);
> +    ret = ff_filter_frame(outlink, s->input_frame[0]);
> +
> +    s->input_frame[0] = NULL;
> +    av_frame_free(&s->input_frame[1]);
> +
> +    return ret;
> +}
> +
> +static int screquest_frame(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    AudioGateContext *s = ctx->priv;
> +    int i, ret;
> +
> +    /* get a frame on each input */
> +    for (i = 0; i < 2; i++) {
> +        AVFilterLink *inlink = ctx->inputs[i];
> +        if (!s->input_frame[i] &&
> +            (ret = ff_request_frame(inlink)) < 0)
> +            return ret;
> +
> +        /* request the same number of samples on all inputs */
> +        if (i == 0)
> +            ctx->inputs[1]->request_samples = s->input_frame[0]->nb_samples;
> +    }
> +
> +    return 0;
> +}
> +
> +static int scquery_formats(AVFilterContext *ctx)
> +{
> +    AVFilterFormats *formats;
> +    AVFilterChannelLayouts *layouts = NULL;
> +    static const enum AVSampleFormat sample_fmts[] = {
> +        AV_SAMPLE_FMT_DBL,
> +        AV_SAMPLE_FMT_NONE
> +    };
> +    int ret, i;
> +
> +    if (!ctx->inputs[0]->in_channel_layouts ||
> +        !ctx->inputs[0]->in_channel_layouts->nb_channel_layouts) {
> +        av_log(ctx, AV_LOG_WARNING,
> +               "No channel layout for input 1\n");
> +            return AVERROR(EAGAIN);
> +    }
> +
> +    if ((ret = ff_add_channel_layout(&layouts, ctx->inputs[0]->in_channel_layouts->channel_layouts[0])) < 0 ||
> +        (ret = ff_channel_layouts_ref(layouts, &ctx->outputs[0]->in_channel_layouts)) < 0)
> +        return ret;
> +
> +    for (i = 0; i < 2; i++) {
> +        layouts = ff_all_channel_counts();
> +        if ((ret = ff_channel_layouts_ref(layouts, &ctx->inputs[i]->out_channel_layouts)) < 0)
> +            return ret;
> +    }
> +
> +    formats = ff_make_format_list(sample_fmts);
> +    if ((ret = ff_set_common_formats(ctx, formats)) < 0)
> +        return ret;
> +
> +    formats = ff_all_samplerates();
> +    return ff_set_common_samplerates(ctx, formats);

Please fix the memleak issue - similar such things have been pointed
out for previous reviews by me. Please repost - you have ignored this
point in the past: see commit ID
3f895dcb0dcbcbf10a621bf4bfae6d8879899015 which you pushed recently, in
spite of the issue being pointed out in review:
https://ffmpeg.org/pipermail/ffmpeg-devel/2015-November/183861.html.

This does not seem cool to me. What is even worse is that this is
something that flags Coverity (legitimately AFAIK), see a patch I have
posted: https://ffmpeg.org/pipermail/ffmpeg-devel/2015-November/183335.html.

I do not mind fixing broken code that went past review, but I do mind
when it is recognized, ignored during patch review, and could have
easily been avoided with a 5 minute effort addressing the comment.

If this continues to be ignored, I am going to find a way to mark
filters as "experimental" and send out a patch, similar to
encoders/decoders. Users should know about problematic filters like
this that have easily recognized problems.

BTW (for all FFmpeg users out there though this may be well known),
but for completeness I remark that avfilter tends to be far worse in
Coverity than the other components. It is not a surprise at all given
incidents like this. I entirely sympathize with wm4 regarding the
quality of filters entering the repo.

> +}
> +
> +static int scconfig_output(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +
> +    if (ctx->inputs[0]->sample_rate != ctx->inputs[1]->sample_rate) {
> +        av_log(ctx, AV_LOG_ERROR,
> +               "Inputs must have the same sample rate "
> +               "%d for in0 vs %d for in1\n",
> +               ctx->inputs[0]->sample_rate, ctx->inputs[1]->sample_rate);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    outlink->sample_rate = ctx->inputs[0]->sample_rate;
> +    outlink->time_base   = ctx->inputs[0]->time_base;
> +    outlink->channel_layout = ctx->inputs[0]->channel_layout;
> +    outlink->channels = ctx->inputs[0]->channels;
> +
> +    agate_config_input(ctx->inputs[0]);
> +
> +    return 0;
> +}
> +
> +static const AVFilterPad sidechaingate_inputs[] = {
> +    {
> +        .name           = "main",
> +        .type           = AVMEDIA_TYPE_AUDIO,
> +        .filter_frame   = scfilter_frame,
> +        .needs_writable = 1,
> +        .needs_fifo     = 1,
> +    },{
> +        .name           = "sidechain",
> +        .type           = AVMEDIA_TYPE_AUDIO,
> +        .filter_frame   = scfilter_frame,
> +        .needs_fifo     = 1,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad sidechaingate_outputs[] = {
> +    {
> +        .name          = "default",
> +        .type          = AVMEDIA_TYPE_AUDIO,
> +        .config_props  = scconfig_output,
> +        .request_frame = screquest_frame,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter ff_af_sidechaingate = {
> +    .name           = "sidechaingate",
> +    .description    = NULL_IF_CONFIG_SMALL("Audio sidechain gate."),
> +    .priv_size      = sizeof(AudioGateContext),
> +    .priv_class     = &sidechaingate_class,
> +    .query_formats  = scquery_formats,
> +    .inputs         = sidechaingate_inputs,
> +    .outputs        = sidechaingate_outputs,
> +};
> +#endif  /* CONFIG_SIDECHAINGATE_FILTER */
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index ccd3f35..9c3778d 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -104,6 +104,7 @@ void avfilter_register_all(void)
>      REGISTER_FILTER(RESAMPLE,       resample,       af);
>      REGISTER_FILTER(RUBBERBAND,     rubberband,     af);
>      REGISTER_FILTER(SIDECHAINCOMPRESS, sidechaincompress, af);
> +    REGISTER_FILTER(SIDECHAINGATE,  sidechaingate,  af);
>      REGISTER_FILTER(SILENCEDETECT,  silencedetect,  af);
>      REGISTER_FILTER(SILENCEREMOVE,  silenceremove,  af);
>      REGISTER_FILTER(STEREOTOOLS,    stereotools,    af);
> --
> 1.9.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list