[FFmpeg-devel] [PATCH 2/2] avfilter: add sidechaingate filter
Paul B Mahol
onemda at gmail.com
Tue Dec 1 09:50:03 CET 2015
On 12/1/15, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> 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.
Mark something, that leaks on error that never happens, as experimental
sounds unfair to me.
Anyway, shouldn't this be called in one place instead of hundreds in each
filter?
This all is really very low hanging fruit to me.
>
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list