diff mbox

[FFmpeg-devel] avfilter: add acomb filter

Message ID 20191002151129.25175-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol Oct. 2, 2019, 3:11 p.m. UTC
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 doc/filters.texi         |  28 ++++++
 libavfilter/Makefile     |   1 +
 libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
 libavfilter/allfilters.c |   1 +
 4 files changed, 218 insertions(+)
 create mode 100644 libavfilter/af_acomb.c

Comments

James Almer Oct. 2, 2019, 3:29 p.m. UTC | #1
On 10/2/2019 12:11 PM, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  doc/filters.texi         |  28 ++++++
>  libavfilter/Makefile     |   1 +
>  libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
>  libavfilter/allfilters.c |   1 +
>  4 files changed, 218 insertions(+)
>  create mode 100644 libavfilter/af_acomb.c
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index e46839bfec..9c50b2e4b2 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -355,6 +355,34 @@ build.
>  
>  Below is a description of the currently available audio filters.
>  
> +@section acomb
> +Apply comb audio filtering.
> +
> +Amplifies or attenuates certain frequencies by the superposition of a
> +delayed version of the original audio signal onto itself.
> +
> +@table @option
> +@item t
> +Set comb filtering type.
> +
> +It accepts the following values:
> +@table @option
> +@item f
> +set feedforward type
> +@item b
> +set feedback type
> +@end table
> +
> +@item b0
> +Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
> +
> +@item xM
> +Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
> +
> +@item M
> +Set delay in number of samples. Default is 10. Allowed range is from 1 to 327680.
> +@end table
> +
>  @section acompressor
>  
>  A compressor is mainly used to reduce the dynamic range of a signal.
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 182fe9df4b..d8a16d6e15 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -31,6 +31,7 @@ include $(SRC_PATH)/libavfilter/dnn/Makefile
>  
>  # audio filters
>  OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
> +OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
>  OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
>  OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
>  OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
> diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
> new file mode 100644
> index 0000000000..3b0730c363
> --- /dev/null
> +++ b/libavfilter/af_acomb.c
> @@ -0,0 +1,188 @@
> +/*
> + * 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
> + */
> +
> +#include "libavutil/opt.h"
> +#include "audio.h"
> +#include "avfilter.h"
> +#include "internal.h"
> +
> +typedef struct AudioCombContext {
> +    const AVClass *class;
> +
> +    double b0, xM;
> +    int t, M;
> +
> +    int head;
> +    int tail;
> +
> +    AVFrame *delayframe;
> +
> +    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame *out);
> +} AudioCombContext;
> +
> +#define OFFSET(x) offsetof(AudioCombContext, x)
> +#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
> +
> +static const AVOption acomb_options[] = {
> +    { "t",  "set comb filter type",           OFFSET(t),  AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
> +    { "f",  "feedforward",                    0,          AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
> +    { "b",  "feedback",                       0,          AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
> +    { "b0", "set direct signal gain",         OFFSET(b0), AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
> +    { "xM", "set delayed line gain",          OFFSET(xM), AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
> +    { "M",  "set delay in number of samples", OFFSET(M),  AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(acomb);
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    AVFilterFormats *formats = NULL;
> +    AVFilterChannelLayouts *layouts = NULL;
> +    static const enum AVSampleFormat sample_fmts[] = {
> +        AV_SAMPLE_FMT_FLTP,
> +        AV_SAMPLE_FMT_DBLP,
> +        AV_SAMPLE_FMT_NONE
> +    };
> +    int ret;
> +
> +    formats = ff_make_format_list(sample_fmts);
> +    if (!formats)
> +        return AVERROR(ENOMEM);
> +    ret = ff_set_common_formats(ctx, formats);
> +    if (ret < 0)
> +        return ret;
> +
> +    layouts = ff_all_channel_counts();
> +    if (!layouts)
> +        return AVERROR(ENOMEM);
> +
> +    ret = ff_set_common_channel_layouts(ctx, layouts);
> +    if (ret < 0)
> +        return ret;
> +
> +    formats = ff_all_samplerates();
> +    return ff_set_common_samplerates(ctx, formats);
> +}
> +
> +#define COMB(name, type, dir, t)                                \
> +static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
> +                                     AVFrame *in, AVFrame *out) \
> +{                                                               \
> +    const type b0 = s->b0;                                      \
> +    const type xM = s->xM;                                      \
> +    const int M = s->M;                                         \
> +    int head;                                                   \
> +                                                                \
> +    for (int c = 0; c < in->channels; c++) {                    \
> +        const type *src = (const type *)in->extended_data[c];   \
> +        type *delay = (type *)s->delayframe->extended_data[c];  \
> +        type *dst = (type *)out->extended_data[c];              \
> +                                                                \
> +        head = s->head;                                         \
> +        for (int n = 0; n < in->nb_samples; n++) {              \
> +            dst[n] = b0 * src[n] + t * xM * delay[head];        \
> +            if (t == 1)                                         \
> +                delay[head] = src[n];                           \
> +            else                                                \
> +                delay[head] = dst[n];                           \
> +            head++;                                             \
> +            if (head >= M)                                      \
> +                head = 0;                                       \
> +        }                                                       \
> +    }                                                           \
> +                                                                \
> +    s->head = head;                                             \
> +}
> +
> +COMB(fltp, float,  f,  1)
> +COMB(dblp, double, f,  1)
> +COMB(fltp, float,  b, -1)
> +COMB(dblp, double, b, -1)
> +
> +static int config_input(AVFilterLink *inlink)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    AudioCombContext *s = ctx->priv;
> +
> +    s->delayframe = ff_get_audio_buffer(inlink, s->M);

You're leaking s->delayframe every time config_input() is called after
the first time.

> +    if (!s->delayframe)
> +        return AVERROR(ENOMEM);
> +
> +    switch (inlink->format) {
> +    case AV_SAMPLE_FMT_FLTP: s->filter = s->t ? acomb_fltp_b : acomb_fltp_f; break;
> +    case AV_SAMPLE_FMT_DBLP: s->filter = s->t ? acomb_dblp_b : acomb_dblp_f; break;
> +    }
> +
> +    return 0;
> +}
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    AudioCombContext *s = ctx->priv;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    AVFrame *out = ff_get_audio_buffer(outlink, in->nb_samples);
> +
> +    if (!out) {
> +        av_frame_free(&in);
> +        return AVERROR(ENOMEM);
> +    }
> +    av_frame_copy_props(out, in);
> +
> +    s->filter(s, in, out);
> +
> +    av_frame_free(&in);
> +    return ff_filter_frame(outlink, out);
> +}
> +
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    AudioCombContext *s = ctx->priv;
> +
> +    av_frame_free(&s->delayframe);
> +}
> +
> +static const AVFilterPad acomb_inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_AUDIO,
> +        .filter_frame = filter_frame,
> +        .config_props = config_input,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad acomb_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_AUDIO,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter ff_af_acomb = {
> +    .name          = "acomb",
> +    .description   = NULL_IF_CONFIG_SMALL("Apply comb audio filter."),
> +    .query_formats = query_formats,
> +    .priv_size     = sizeof(AudioCombContext),
> +    .priv_class    = &acomb_class,
> +    .uninit        = uninit,
> +    .inputs        = acomb_inputs,
> +    .outputs       = acomb_outputs,
> +};
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 1a26129069..7417f9656d 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -24,6 +24,7 @@
>  #include "config.h"
>  
>  extern AVFilter ff_af_abench;
> +extern AVFilter ff_af_acomb;
>  extern AVFilter ff_af_acompressor;
>  extern AVFilter ff_af_acontrast;
>  extern AVFilter ff_af_acopy;
>
Paul B Mahol Oct. 2, 2019, 3:37 p.m. UTC | #2
On 10/2/19, James Almer <jamrial@gmail.com> wrote:
> On 10/2/2019 12:11 PM, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  doc/filters.texi         |  28 ++++++
>>  libavfilter/Makefile     |   1 +
>>  libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
>>  libavfilter/allfilters.c |   1 +
>>  4 files changed, 218 insertions(+)
>>  create mode 100644 libavfilter/af_acomb.c
>>
>> diff --git a/doc/filters.texi b/doc/filters.texi
>> index e46839bfec..9c50b2e4b2 100644
>> --- a/doc/filters.texi
>> +++ b/doc/filters.texi
>> @@ -355,6 +355,34 @@ build.
>>
>>  Below is a description of the currently available audio filters.
>>
>> +@section acomb
>> +Apply comb audio filtering.
>> +
>> +Amplifies or attenuates certain frequencies by the superposition of a
>> +delayed version of the original audio signal onto itself.
>> +
>> +@table @option
>> +@item t
>> +Set comb filtering type.
>> +
>> +It accepts the following values:
>> +@table @option
>> +@item f
>> +set feedforward type
>> +@item b
>> +set feedback type
>> +@end table
>> +
>> +@item b0
>> +Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
>> +
>> +@item xM
>> +Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
>> +
>> +@item M
>> +Set delay in number of samples. Default is 10. Allowed range is from 1 to
>> 327680.
>> +@end table
>> +
>>  @section acompressor
>>
>>  A compressor is mainly used to reduce the dynamic range of a signal.
>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>> index 182fe9df4b..d8a16d6e15 100644
>> --- a/libavfilter/Makefile
>> +++ b/libavfilter/Makefile
>> @@ -31,6 +31,7 @@ include $(SRC_PATH)/libavfilter/dnn/Makefile
>>
>>  # audio filters
>>  OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
>> +OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
>>  OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
>>  OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
>>  OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
>> diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
>> new file mode 100644
>> index 0000000000..3b0730c363
>> --- /dev/null
>> +++ b/libavfilter/af_acomb.c
>> @@ -0,0 +1,188 @@
>> +/*
>> + * 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
>> + */
>> +
>> +#include "libavutil/opt.h"
>> +#include "audio.h"
>> +#include "avfilter.h"
>> +#include "internal.h"
>> +
>> +typedef struct AudioCombContext {
>> +    const AVClass *class;
>> +
>> +    double b0, xM;
>> +    int t, M;
>> +
>> +    int head;
>> +    int tail;
>> +
>> +    AVFrame *delayframe;
>> +
>> +    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame
>> *out);
>> +} AudioCombContext;
>> +
>> +#define OFFSET(x) offsetof(AudioCombContext, x)
>> +#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>> +
>> +static const AVOption acomb_options[] = {
>> +    { "t",  "set comb filter type",           OFFSET(t),
>> AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
>> +    { "f",  "feedforward",                    0,
>> AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
>> +    { "b",  "feedback",                       0,
>> AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
>> +    { "b0", "set direct signal gain",         OFFSET(b0),
>> AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
>> +    { "xM", "set delayed line gain",          OFFSET(xM),
>> AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
>> +    { "M",  "set delay in number of samples", OFFSET(M),
>> AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
>> +    { NULL }
>> +};
>> +
>> +AVFILTER_DEFINE_CLASS(acomb);
>> +
>> +static int query_formats(AVFilterContext *ctx)
>> +{
>> +    AVFilterFormats *formats = NULL;
>> +    AVFilterChannelLayouts *layouts = NULL;
>> +    static const enum AVSampleFormat sample_fmts[] = {
>> +        AV_SAMPLE_FMT_FLTP,
>> +        AV_SAMPLE_FMT_DBLP,
>> +        AV_SAMPLE_FMT_NONE
>> +    };
>> +    int ret;
>> +
>> +    formats = ff_make_format_list(sample_fmts);
>> +    if (!formats)
>> +        return AVERROR(ENOMEM);
>> +    ret = ff_set_common_formats(ctx, formats);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    layouts = ff_all_channel_counts();
>> +    if (!layouts)
>> +        return AVERROR(ENOMEM);
>> +
>> +    ret = ff_set_common_channel_layouts(ctx, layouts);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    formats = ff_all_samplerates();
>> +    return ff_set_common_samplerates(ctx, formats);
>> +}
>> +
>> +#define COMB(name, type, dir, t)                                \
>> +static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
>> +                                     AVFrame *in, AVFrame *out) \
>> +{                                                               \
>> +    const type b0 = s->b0;                                      \
>> +    const type xM = s->xM;                                      \
>> +    const int M = s->M;                                         \
>> +    int head;                                                   \
>> +                                                                \
>> +    for (int c = 0; c < in->channels; c++) {                    \
>> +        const type *src = (const type *)in->extended_data[c];   \
>> +        type *delay = (type *)s->delayframe->extended_data[c];  \
>> +        type *dst = (type *)out->extended_data[c];              \
>> +                                                                \
>> +        head = s->head;                                         \
>> +        for (int n = 0; n < in->nb_samples; n++) {              \
>> +            dst[n] = b0 * src[n] + t * xM * delay[head];        \
>> +            if (t == 1)                                         \
>> +                delay[head] = src[n];                           \
>> +            else                                                \
>> +                delay[head] = dst[n];                           \
>> +            head++;                                             \
>> +            if (head >= M)                                      \
>> +                head = 0;                                       \
>> +        }                                                       \
>> +    }                                                           \
>> +                                                                \
>> +    s->head = head;                                             \
>> +}
>> +
>> +COMB(fltp, float,  f,  1)
>> +COMB(dblp, double, f,  1)
>> +COMB(fltp, float,  b, -1)
>> +COMB(dblp, double, b, -1)
>> +
>> +static int config_input(AVFilterLink *inlink)
>> +{
>> +    AVFilterContext *ctx = inlink->dst;
>> +    AudioCombContext *s = ctx->priv;
>> +
>> +    s->delayframe = ff_get_audio_buffer(inlink, s->M);
>
> You're leaking s->delayframe every time config_input() is called after
> the first time.

Sorry, but since when its ok to call config_input() multiple times?
It was never ok, only filter is allowed to call it by itself.

>
>> +    if (!s->delayframe)
>> +        return AVERROR(ENOMEM);
>> +
>> +    switch (inlink->format) {
>> +    case AV_SAMPLE_FMT_FLTP: s->filter = s->t ? acomb_fltp_b :
>> acomb_fltp_f; break;
>> +    case AV_SAMPLE_FMT_DBLP: s->filter = s->t ? acomb_dblp_b :
>> acomb_dblp_f; break;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>> +{
>> +    AVFilterContext *ctx = inlink->dst;
>> +    AudioCombContext *s = ctx->priv;
>> +    AVFilterLink *outlink = ctx->outputs[0];
>> +    AVFrame *out = ff_get_audio_buffer(outlink, in->nb_samples);
>> +
>> +    if (!out) {
>> +        av_frame_free(&in);
>> +        return AVERROR(ENOMEM);
>> +    }
>> +    av_frame_copy_props(out, in);
>> +
>> +    s->filter(s, in, out);
>> +
>> +    av_frame_free(&in);
>> +    return ff_filter_frame(outlink, out);
>> +}
>> +
>> +static av_cold void uninit(AVFilterContext *ctx)
>> +{
>> +    AudioCombContext *s = ctx->priv;
>> +
>> +    av_frame_free(&s->delayframe);
>> +}
>> +
>> +static const AVFilterPad acomb_inputs[] = {
>> +    {
>> +        .name         = "default",
>> +        .type         = AVMEDIA_TYPE_AUDIO,
>> +        .filter_frame = filter_frame,
>> +        .config_props = config_input,
>> +    },
>> +    { NULL }
>> +};
>> +
>> +static const AVFilterPad acomb_outputs[] = {
>> +    {
>> +        .name = "default",
>> +        .type = AVMEDIA_TYPE_AUDIO,
>> +    },
>> +    { NULL }
>> +};
>> +
>> +AVFilter ff_af_acomb = {
>> +    .name          = "acomb",
>> +    .description   = NULL_IF_CONFIG_SMALL("Apply comb audio filter."),
>> +    .query_formats = query_formats,
>> +    .priv_size     = sizeof(AudioCombContext),
>> +    .priv_class    = &acomb_class,
>> +    .uninit        = uninit,
>> +    .inputs        = acomb_inputs,
>> +    .outputs       = acomb_outputs,
>> +};
>> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
>> index 1a26129069..7417f9656d 100644
>> --- a/libavfilter/allfilters.c
>> +++ b/libavfilter/allfilters.c
>> @@ -24,6 +24,7 @@
>>  #include "config.h"
>>
>>  extern AVFilter ff_af_abench;
>> +extern AVFilter ff_af_acomb;
>>  extern AVFilter ff_af_acompressor;
>>  extern AVFilter ff_af_acontrast;
>>  extern AVFilter ff_af_acopy;
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer Oct. 2, 2019, 3:57 p.m. UTC | #3
On 10/2/2019 12:37 PM, Paul B Mahol wrote:
> On 10/2/19, James Almer <jamrial@gmail.com> wrote:
>> On 10/2/2019 12:11 PM, Paul B Mahol wrote:
>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>> ---
>>>  doc/filters.texi         |  28 ++++++
>>>  libavfilter/Makefile     |   1 +
>>>  libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
>>>  libavfilter/allfilters.c |   1 +
>>>  4 files changed, 218 insertions(+)
>>>  create mode 100644 libavfilter/af_acomb.c
>>>
>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>> index e46839bfec..9c50b2e4b2 100644
>>> --- a/doc/filters.texi
>>> +++ b/doc/filters.texi
>>> @@ -355,6 +355,34 @@ build.
>>>
>>>  Below is a description of the currently available audio filters.
>>>
>>> +@section acomb
>>> +Apply comb audio filtering.
>>> +
>>> +Amplifies or attenuates certain frequencies by the superposition of a
>>> +delayed version of the original audio signal onto itself.
>>> +
>>> +@table @option
>>> +@item t
>>> +Set comb filtering type.
>>> +
>>> +It accepts the following values:
>>> +@table @option
>>> +@item f
>>> +set feedforward type
>>> +@item b
>>> +set feedback type
>>> +@end table
>>> +
>>> +@item b0
>>> +Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
>>> +
>>> +@item xM
>>> +Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
>>> +
>>> +@item M
>>> +Set delay in number of samples. Default is 10. Allowed range is from 1 to
>>> 327680.
>>> +@end table
>>> +
>>>  @section acompressor
>>>
>>>  A compressor is mainly used to reduce the dynamic range of a signal.
>>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>>> index 182fe9df4b..d8a16d6e15 100644
>>> --- a/libavfilter/Makefile
>>> +++ b/libavfilter/Makefile
>>> @@ -31,6 +31,7 @@ include $(SRC_PATH)/libavfilter/dnn/Makefile
>>>
>>>  # audio filters
>>>  OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
>>> +OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
>>>  OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
>>>  OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
>>>  OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
>>> diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
>>> new file mode 100644
>>> index 0000000000..3b0730c363
>>> --- /dev/null
>>> +++ b/libavfilter/af_acomb.c
>>> @@ -0,0 +1,188 @@
>>> +/*
>>> + * 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
>>> + */
>>> +
>>> +#include "libavutil/opt.h"
>>> +#include "audio.h"
>>> +#include "avfilter.h"
>>> +#include "internal.h"
>>> +
>>> +typedef struct AudioCombContext {
>>> +    const AVClass *class;
>>> +
>>> +    double b0, xM;
>>> +    int t, M;
>>> +
>>> +    int head;
>>> +    int tail;
>>> +
>>> +    AVFrame *delayframe;
>>> +
>>> +    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame
>>> *out);
>>> +} AudioCombContext;
>>> +
>>> +#define OFFSET(x) offsetof(AudioCombContext, x)
>>> +#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>>> +
>>> +static const AVOption acomb_options[] = {
>>> +    { "t",  "set comb filter type",           OFFSET(t),
>>> AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
>>> +    { "f",  "feedforward",                    0,
>>> AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
>>> +    { "b",  "feedback",                       0,
>>> AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
>>> +    { "b0", "set direct signal gain",         OFFSET(b0),
>>> AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
>>> +    { "xM", "set delayed line gain",          OFFSET(xM),
>>> AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
>>> +    { "M",  "set delay in number of samples", OFFSET(M),
>>> AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
>>> +    { NULL }
>>> +};
>>> +
>>> +AVFILTER_DEFINE_CLASS(acomb);
>>> +
>>> +static int query_formats(AVFilterContext *ctx)
>>> +{
>>> +    AVFilterFormats *formats = NULL;
>>> +    AVFilterChannelLayouts *layouts = NULL;
>>> +    static const enum AVSampleFormat sample_fmts[] = {
>>> +        AV_SAMPLE_FMT_FLTP,
>>> +        AV_SAMPLE_FMT_DBLP,
>>> +        AV_SAMPLE_FMT_NONE
>>> +    };
>>> +    int ret;
>>> +
>>> +    formats = ff_make_format_list(sample_fmts);
>>> +    if (!formats)
>>> +        return AVERROR(ENOMEM);
>>> +    ret = ff_set_common_formats(ctx, formats);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    layouts = ff_all_channel_counts();
>>> +    if (!layouts)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    ret = ff_set_common_channel_layouts(ctx, layouts);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    formats = ff_all_samplerates();
>>> +    return ff_set_common_samplerates(ctx, formats);
>>> +}
>>> +
>>> +#define COMB(name, type, dir, t)                                \
>>> +static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
>>> +                                     AVFrame *in, AVFrame *out) \
>>> +{                                                               \
>>> +    const type b0 = s->b0;                                      \
>>> +    const type xM = s->xM;                                      \
>>> +    const int M = s->M;                                         \
>>> +    int head;                                                   \
>>> +                                                                \
>>> +    for (int c = 0; c < in->channels; c++) {                    \
>>> +        const type *src = (const type *)in->extended_data[c];   \
>>> +        type *delay = (type *)s->delayframe->extended_data[c];  \
>>> +        type *dst = (type *)out->extended_data[c];              \
>>> +                                                                \
>>> +        head = s->head;                                         \
>>> +        for (int n = 0; n < in->nb_samples; n++) {              \
>>> +            dst[n] = b0 * src[n] + t * xM * delay[head];        \
>>> +            if (t == 1)                                         \
>>> +                delay[head] = src[n];                           \
>>> +            else                                                \
>>> +                delay[head] = dst[n];                           \
>>> +            head++;                                             \
>>> +            if (head >= M)                                      \
>>> +                head = 0;                                       \
>>> +        }                                                       \
>>> +    }                                                           \
>>> +                                                                \
>>> +    s->head = head;                                             \
>>> +}
>>> +
>>> +COMB(fltp, float,  f,  1)
>>> +COMB(dblp, double, f,  1)
>>> +COMB(fltp, float,  b, -1)
>>> +COMB(dblp, double, b, -1)
>>> +
>>> +static int config_input(AVFilterLink *inlink)
>>> +{
>>> +    AVFilterContext *ctx = inlink->dst;
>>> +    AudioCombContext *s = ctx->priv;
>>> +
>>> +    s->delayframe = ff_get_audio_buffer(inlink, s->M);
>>
>> You're leaking s->delayframe every time config_input() is called after
>> the first time.
> 
> Sorry, but since when its ok to call config_input() multiple times?
> It was never ok, only filter is allowed to call it by itself.

I see, so it's an init function and not something called per frame.
Disregard what i said, then. I'm not familiar with libavfilter internal
workings, which is why i assumed it could happen.

> 
>>
>>> +    if (!s->delayframe)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    switch (inlink->format) {
>>> +    case AV_SAMPLE_FMT_FLTP: s->filter = s->t ? acomb_fltp_b :
>>> acomb_fltp_f; break;
>>> +    case AV_SAMPLE_FMT_DBLP: s->filter = s->t ? acomb_dblp_b :
>>> acomb_dblp_f; break;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>>> +{
>>> +    AVFilterContext *ctx = inlink->dst;
>>> +    AudioCombContext *s = ctx->priv;
>>> +    AVFilterLink *outlink = ctx->outputs[0];
>>> +    AVFrame *out = ff_get_audio_buffer(outlink, in->nb_samples);
>>> +
>>> +    if (!out) {
>>> +        av_frame_free(&in);
>>> +        return AVERROR(ENOMEM);
>>> +    }
>>> +    av_frame_copy_props(out, in);
>>> +
>>> +    s->filter(s, in, out);
>>> +
>>> +    av_frame_free(&in);
>>> +    return ff_filter_frame(outlink, out);
>>> +}
>>> +
>>> +static av_cold void uninit(AVFilterContext *ctx)
>>> +{
>>> +    AudioCombContext *s = ctx->priv;
>>> +
>>> +    av_frame_free(&s->delayframe);
>>> +}
>>> +
>>> +static const AVFilterPad acomb_inputs[] = {
>>> +    {
>>> +        .name         = "default",
>>> +        .type         = AVMEDIA_TYPE_AUDIO,
>>> +        .filter_frame = filter_frame,
>>> +        .config_props = config_input,
>>> +    },
>>> +    { NULL }
>>> +};
>>> +
>>> +static const AVFilterPad acomb_outputs[] = {
>>> +    {
>>> +        .name = "default",
>>> +        .type = AVMEDIA_TYPE_AUDIO,
>>> +    },
>>> +    { NULL }
>>> +};
>>> +
>>> +AVFilter ff_af_acomb = {
>>> +    .name          = "acomb",
>>> +    .description   = NULL_IF_CONFIG_SMALL("Apply comb audio filter."),
>>> +    .query_formats = query_formats,
>>> +    .priv_size     = sizeof(AudioCombContext),
>>> +    .priv_class    = &acomb_class,
>>> +    .uninit        = uninit,
>>> +    .inputs        = acomb_inputs,
>>> +    .outputs       = acomb_outputs,
>>> +};
>>> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
>>> index 1a26129069..7417f9656d 100644
>>> --- a/libavfilter/allfilters.c
>>> +++ b/libavfilter/allfilters.c
>>> @@ -24,6 +24,7 @@
>>>  #include "config.h"
>>>
>>>  extern AVFilter ff_af_abench;
>>> +extern AVFilter ff_af_acomb;
>>>  extern AVFilter ff_af_acompressor;
>>>  extern AVFilter ff_af_acontrast;
>>>  extern AVFilter ff_af_acopy;
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Paul B Mahol Oct. 2, 2019, 3:57 p.m. UTC | #4
On 10/2/19, Paul B Mahol <onemda@gmail.com> wrote:
> On 10/2/19, James Almer <jamrial@gmail.com> wrote:
>> On 10/2/2019 12:11 PM, Paul B Mahol wrote:
>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>> ---
>>>  doc/filters.texi         |  28 ++++++
>>>  libavfilter/Makefile     |   1 +
>>>  libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
>>>  libavfilter/allfilters.c |   1 +
>>>  4 files changed, 218 insertions(+)
>>>  create mode 100644 libavfilter/af_acomb.c
>>>
>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>> index e46839bfec..9c50b2e4b2 100644
>>> --- a/doc/filters.texi
>>> +++ b/doc/filters.texi
>>> @@ -355,6 +355,34 @@ build.
>>>
>>>  Below is a description of the currently available audio filters.
>>>
>>> +@section acomb
>>> +Apply comb audio filtering.
>>> +
>>> +Amplifies or attenuates certain frequencies by the superposition of a
>>> +delayed version of the original audio signal onto itself.
>>> +
>>> +@table @option
>>> +@item t
>>> +Set comb filtering type.
>>> +
>>> +It accepts the following values:
>>> +@table @option
>>> +@item f
>>> +set feedforward type
>>> +@item b
>>> +set feedback type
>>> +@end table
>>> +
>>> +@item b0
>>> +Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
>>> +
>>> +@item xM
>>> +Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
>>> +
>>> +@item M
>>> +Set delay in number of samples. Default is 10. Allowed range is from 1
>>> to
>>> 327680.
>>> +@end table
>>> +
>>>  @section acompressor
>>>
>>>  A compressor is mainly used to reduce the dynamic range of a signal.
>>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>>> index 182fe9df4b..d8a16d6e15 100644
>>> --- a/libavfilter/Makefile
>>> +++ b/libavfilter/Makefile
>>> @@ -31,6 +31,7 @@ include $(SRC_PATH)/libavfilter/dnn/Makefile
>>>
>>>  # audio filters
>>>  OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
>>> +OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
>>>  OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
>>>  OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
>>>  OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
>>> diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
>>> new file mode 100644
>>> index 0000000000..3b0730c363
>>> --- /dev/null
>>> +++ b/libavfilter/af_acomb.c
>>> @@ -0,0 +1,188 @@
>>> +/*
>>> + * 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
>>> + */
>>> +
>>> +#include "libavutil/opt.h"
>>> +#include "audio.h"
>>> +#include "avfilter.h"
>>> +#include "internal.h"
>>> +
>>> +typedef struct AudioCombContext {
>>> +    const AVClass *class;
>>> +
>>> +    double b0, xM;
>>> +    int t, M;
>>> +
>>> +    int head;
>>> +    int tail;
>>> +
>>> +    AVFrame *delayframe;
>>> +
>>> +    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame
>>> *out);
>>> +} AudioCombContext;
>>> +
>>> +#define OFFSET(x) offsetof(AudioCombContext, x)
>>> +#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>>> +
>>> +static const AVOption acomb_options[] = {
>>> +    { "t",  "set comb filter type",           OFFSET(t),
>>> AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
>>> +    { "f",  "feedforward",                    0,
>>> AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
>>> +    { "b",  "feedback",                       0,
>>> AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
>>> +    { "b0", "set direct signal gain",         OFFSET(b0),
>>> AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
>>> +    { "xM", "set delayed line gain",          OFFSET(xM),
>>> AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
>>> +    { "M",  "set delay in number of samples", OFFSET(M),
>>> AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
>>> +    { NULL }
>>> +};
>>> +
>>> +AVFILTER_DEFINE_CLASS(acomb);
>>> +
>>> +static int query_formats(AVFilterContext *ctx)
>>> +{
>>> +    AVFilterFormats *formats = NULL;
>>> +    AVFilterChannelLayouts *layouts = NULL;
>>> +    static const enum AVSampleFormat sample_fmts[] = {
>>> +        AV_SAMPLE_FMT_FLTP,
>>> +        AV_SAMPLE_FMT_DBLP,
>>> +        AV_SAMPLE_FMT_NONE
>>> +    };
>>> +    int ret;
>>> +
>>> +    formats = ff_make_format_list(sample_fmts);
>>> +    if (!formats)
>>> +        return AVERROR(ENOMEM);
>>> +    ret = ff_set_common_formats(ctx, formats);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    layouts = ff_all_channel_counts();
>>> +    if (!layouts)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    ret = ff_set_common_channel_layouts(ctx, layouts);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +
>>> +    formats = ff_all_samplerates();
>>> +    return ff_set_common_samplerates(ctx, formats);
>>> +}
>>> +
>>> +#define COMB(name, type, dir, t)                                \
>>> +static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
>>> +                                     AVFrame *in, AVFrame *out) \
>>> +{                                                               \
>>> +    const type b0 = s->b0;                                      \
>>> +    const type xM = s->xM;                                      \
>>> +    const int M = s->M;                                         \
>>> +    int head;                                                   \
>>> +                                                                \
>>> +    for (int c = 0; c < in->channels; c++) {                    \
>>> +        const type *src = (const type *)in->extended_data[c];   \
>>> +        type *delay = (type *)s->delayframe->extended_data[c];  \
>>> +        type *dst = (type *)out->extended_data[c];              \
>>> +                                                                \
>>> +        head = s->head;                                         \
>>> +        for (int n = 0; n < in->nb_samples; n++) {              \
>>> +            dst[n] = b0 * src[n] + t * xM * delay[head];        \
>>> +            if (t == 1)                                         \
>>> +                delay[head] = src[n];                           \
>>> +            else                                                \
>>> +                delay[head] = dst[n];                           \
>>> +            head++;                                             \
>>> +            if (head >= M)                                      \
>>> +                head = 0;                                       \
>>> +        }                                                       \
>>> +    }                                                           \
>>> +                                                                \
>>> +    s->head = head;                                             \
>>> +}
>>> +
>>> +COMB(fltp, float,  f,  1)
>>> +COMB(dblp, double, f,  1)
>>> +COMB(fltp, float,  b, -1)
>>> +COMB(dblp, double, b, -1)
>>> +
>>> +static int config_input(AVFilterLink *inlink)
>>> +{
>>> +    AVFilterContext *ctx = inlink->dst;
>>> +    AudioCombContext *s = ctx->priv;
>>> +
>>> +    s->delayframe = ff_get_audio_buffer(inlink, s->M);
>>
>> You're leaking s->delayframe every time config_input() is called after
>> the first time.
>
> Sorry, but since when its ok to call config_input() multiple times?
> It was never ok, only filter is allowed to call it by itself.

Fixed locally, but note that bunch of other filters may need to be changed too.

>
>>
>>> +    if (!s->delayframe)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    switch (inlink->format) {
>>> +    case AV_SAMPLE_FMT_FLTP: s->filter = s->t ? acomb_fltp_b :
>>> acomb_fltp_f; break;
>>> +    case AV_SAMPLE_FMT_DBLP: s->filter = s->t ? acomb_dblp_b :
>>> acomb_dblp_f; break;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>>> +{
>>> +    AVFilterContext *ctx = inlink->dst;
>>> +    AudioCombContext *s = ctx->priv;
>>> +    AVFilterLink *outlink = ctx->outputs[0];
>>> +    AVFrame *out = ff_get_audio_buffer(outlink, in->nb_samples);
>>> +
>>> +    if (!out) {
>>> +        av_frame_free(&in);
>>> +        return AVERROR(ENOMEM);
>>> +    }
>>> +    av_frame_copy_props(out, in);
>>> +
>>> +    s->filter(s, in, out);
>>> +
>>> +    av_frame_free(&in);
>>> +    return ff_filter_frame(outlink, out);
>>> +}
>>> +
>>> +static av_cold void uninit(AVFilterContext *ctx)
>>> +{
>>> +    AudioCombContext *s = ctx->priv;
>>> +
>>> +    av_frame_free(&s->delayframe);
>>> +}
>>> +
>>> +static const AVFilterPad acomb_inputs[] = {
>>> +    {
>>> +        .name         = "default",
>>> +        .type         = AVMEDIA_TYPE_AUDIO,
>>> +        .filter_frame = filter_frame,
>>> +        .config_props = config_input,
>>> +    },
>>> +    { NULL }
>>> +};
>>> +
>>> +static const AVFilterPad acomb_outputs[] = {
>>> +    {
>>> +        .name = "default",
>>> +        .type = AVMEDIA_TYPE_AUDIO,
>>> +    },
>>> +    { NULL }
>>> +};
>>> +
>>> +AVFilter ff_af_acomb = {
>>> +    .name          = "acomb",
>>> +    .description   = NULL_IF_CONFIG_SMALL("Apply comb audio filter."),
>>> +    .query_formats = query_formats,
>>> +    .priv_size     = sizeof(AudioCombContext),
>>> +    .priv_class    = &acomb_class,
>>> +    .uninit        = uninit,
>>> +    .inputs        = acomb_inputs,
>>> +    .outputs       = acomb_outputs,
>>> +};
>>> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
>>> index 1a26129069..7417f9656d 100644
>>> --- a/libavfilter/allfilters.c
>>> +++ b/libavfilter/allfilters.c
>>> @@ -24,6 +24,7 @@
>>>  #include "config.h"
>>>
>>>  extern AVFilter ff_af_abench;
>>> +extern AVFilter ff_af_acomb;
>>>  extern AVFilter ff_af_acompressor;
>>>  extern AVFilter ff_af_acontrast;
>>>  extern AVFilter ff_af_acopy;
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Paul B Mahol Oct. 2, 2019, 3:59 p.m. UTC | #5
On 10/2/19, James Almer <jamrial@gmail.com> wrote:
> On 10/2/2019 12:37 PM, Paul B Mahol wrote:
>> On 10/2/19, James Almer <jamrial@gmail.com> wrote:
>>> On 10/2/2019 12:11 PM, Paul B Mahol wrote:
>>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>>> ---
>>>>  doc/filters.texi         |  28 ++++++
>>>>  libavfilter/Makefile     |   1 +
>>>>  libavfilter/af_acomb.c   | 188 +++++++++++++++++++++++++++++++++++++++
>>>>  libavfilter/allfilters.c |   1 +
>>>>  4 files changed, 218 insertions(+)
>>>>  create mode 100644 libavfilter/af_acomb.c
>>>>
>>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>>> index e46839bfec..9c50b2e4b2 100644
>>>> --- a/doc/filters.texi
>>>> +++ b/doc/filters.texi
>>>> @@ -355,6 +355,34 @@ build.
>>>>
>>>>  Below is a description of the currently available audio filters.
>>>>
>>>> +@section acomb
>>>> +Apply comb audio filtering.
>>>> +
>>>> +Amplifies or attenuates certain frequencies by the superposition of a
>>>> +delayed version of the original audio signal onto itself.
>>>> +
>>>> +@table @option
>>>> +@item t
>>>> +Set comb filtering type.
>>>> +
>>>> +It accepts the following values:
>>>> +@table @option
>>>> +@item f
>>>> +set feedforward type
>>>> +@item b
>>>> +set feedback type
>>>> +@end table
>>>> +
>>>> +@item b0
>>>> +Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
>>>> +
>>>> +@item xM
>>>> +Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
>>>> +
>>>> +@item M
>>>> +Set delay in number of samples. Default is 10. Allowed range is from 1
>>>> to
>>>> 327680.
>>>> +@end table
>>>> +
>>>>  @section acompressor
>>>>
>>>>  A compressor is mainly used to reduce the dynamic range of a signal.
>>>> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
>>>> index 182fe9df4b..d8a16d6e15 100644
>>>> --- a/libavfilter/Makefile
>>>> +++ b/libavfilter/Makefile
>>>> @@ -31,6 +31,7 @@ include $(SRC_PATH)/libavfilter/dnn/Makefile
>>>>
>>>>  # audio filters
>>>>  OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
>>>> +OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
>>>>  OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
>>>>  OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
>>>>  OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
>>>> diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
>>>> new file mode 100644
>>>> index 0000000000..3b0730c363
>>>> --- /dev/null
>>>> +++ b/libavfilter/af_acomb.c
>>>> @@ -0,0 +1,188 @@
>>>> +/*
>>>> + * 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
>>>> + */
>>>> +
>>>> +#include "libavutil/opt.h"
>>>> +#include "audio.h"
>>>> +#include "avfilter.h"
>>>> +#include "internal.h"
>>>> +
>>>> +typedef struct AudioCombContext {
>>>> +    const AVClass *class;
>>>> +
>>>> +    double b0, xM;
>>>> +    int t, M;
>>>> +
>>>> +    int head;
>>>> +    int tail;
>>>> +
>>>> +    AVFrame *delayframe;
>>>> +
>>>> +    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame
>>>> *out);
>>>> +} AudioCombContext;
>>>> +
>>>> +#define OFFSET(x) offsetof(AudioCombContext, x)
>>>> +#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
>>>> +
>>>> +static const AVOption acomb_options[] = {
>>>> +    { "t",  "set comb filter type",           OFFSET(t),
>>>> AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
>>>> +    { "f",  "feedforward",                    0,
>>>> AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
>>>> +    { "b",  "feedback",                       0,
>>>> AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
>>>> +    { "b0", "set direct signal gain",         OFFSET(b0),
>>>> AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
>>>> +    { "xM", "set delayed line gain",          OFFSET(xM),
>>>> AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
>>>> +    { "M",  "set delay in number of samples", OFFSET(M),
>>>> AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
>>>> +    { NULL }
>>>> +};
>>>> +
>>>> +AVFILTER_DEFINE_CLASS(acomb);
>>>> +
>>>> +static int query_formats(AVFilterContext *ctx)
>>>> +{
>>>> +    AVFilterFormats *formats = NULL;
>>>> +    AVFilterChannelLayouts *layouts = NULL;
>>>> +    static const enum AVSampleFormat sample_fmts[] = {
>>>> +        AV_SAMPLE_FMT_FLTP,
>>>> +        AV_SAMPLE_FMT_DBLP,
>>>> +        AV_SAMPLE_FMT_NONE
>>>> +    };
>>>> +    int ret;
>>>> +
>>>> +    formats = ff_make_format_list(sample_fmts);
>>>> +    if (!formats)
>>>> +        return AVERROR(ENOMEM);
>>>> +    ret = ff_set_common_formats(ctx, formats);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    layouts = ff_all_channel_counts();
>>>> +    if (!layouts)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>> +    ret = ff_set_common_channel_layouts(ctx, layouts);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    formats = ff_all_samplerates();
>>>> +    return ff_set_common_samplerates(ctx, formats);
>>>> +}
>>>> +
>>>> +#define COMB(name, type, dir, t)                                \
>>>> +static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
>>>> +                                     AVFrame *in, AVFrame *out) \
>>>> +{                                                               \
>>>> +    const type b0 = s->b0;                                      \
>>>> +    const type xM = s->xM;                                      \
>>>> +    const int M = s->M;                                         \
>>>> +    int head;                                                   \
>>>> +                                                                \
>>>> +    for (int c = 0; c < in->channels; c++) {                    \
>>>> +        const type *src = (const type *)in->extended_data[c];   \
>>>> +        type *delay = (type *)s->delayframe->extended_data[c];  \
>>>> +        type *dst = (type *)out->extended_data[c];              \
>>>> +                                                                \
>>>> +        head = s->head;                                         \
>>>> +        for (int n = 0; n < in->nb_samples; n++) {              \
>>>> +            dst[n] = b0 * src[n] + t * xM * delay[head];        \
>>>> +            if (t == 1)                                         \
>>>> +                delay[head] = src[n];                           \
>>>> +            else                                                \
>>>> +                delay[head] = dst[n];                           \
>>>> +            head++;                                             \
>>>> +            if (head >= M)                                      \
>>>> +                head = 0;                                       \
>>>> +        }                                                       \
>>>> +    }                                                           \
>>>> +                                                                \
>>>> +    s->head = head;                                             \
>>>> +}
>>>> +
>>>> +COMB(fltp, float,  f,  1)
>>>> +COMB(dblp, double, f,  1)
>>>> +COMB(fltp, float,  b, -1)
>>>> +COMB(dblp, double, b, -1)
>>>> +
>>>> +static int config_input(AVFilterLink *inlink)
>>>> +{
>>>> +    AVFilterContext *ctx = inlink->dst;
>>>> +    AudioCombContext *s = ctx->priv;
>>>> +
>>>> +    s->delayframe = ff_get_audio_buffer(inlink, s->M);
>>>
>>> You're leaking s->delayframe every time config_input() is called after
>>> the first time.
>>
>> Sorry, but since when its ok to call config_input() multiple times?
>> It was never ok, only filter is allowed to call it by itself.
>
> I see, so it's an init function and not something called per frame.
> Disregard what i said, then. I'm not familiar with libavfilter internal
> workings, which is why i assumed it could happen.

It actually happens with astreamselect filter. But that filter is not used much.
diff mbox

Patch

diff --git a/doc/filters.texi b/doc/filters.texi
index e46839bfec..9c50b2e4b2 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -355,6 +355,34 @@  build.
 
 Below is a description of the currently available audio filters.
 
+@section acomb
+Apply comb audio filtering.
+
+Amplifies or attenuates certain frequencies by the superposition of a
+delayed version of the original audio signal onto itself.
+
+@table @option
+@item t
+Set comb filtering type.
+
+It accepts the following values:
+@table @option
+@item f
+set feedforward type
+@item b
+set feedback type
+@end table
+
+@item b0
+Set direct signal gain. Default is 1. Allowed range is from 0 to 1.
+
+@item xM
+Set delayed line gain. Default is 0.5. Allowed range is from 0 to 1.
+
+@item M
+Set delay in number of samples. Default is 10. Allowed range is from 1 to 327680.
+@end table
+
 @section acompressor
 
 A compressor is mainly used to reduce the dynamic range of a signal.
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 182fe9df4b..d8a16d6e15 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -31,6 +31,7 @@  include $(SRC_PATH)/libavfilter/dnn/Makefile
 
 # audio filters
 OBJS-$(CONFIG_ABENCH_FILTER)                 += f_bench.o
+OBJS-$(CONFIG_ACOMB_FILTER)                  += af_acomb.o
 OBJS-$(CONFIG_ACOMPRESSOR_FILTER)            += af_sidechaincompress.o
 OBJS-$(CONFIG_ACONTRAST_FILTER)              += af_acontrast.o
 OBJS-$(CONFIG_ACOPY_FILTER)                  += af_acopy.o
diff --git a/libavfilter/af_acomb.c b/libavfilter/af_acomb.c
new file mode 100644
index 0000000000..3b0730c363
--- /dev/null
+++ b/libavfilter/af_acomb.c
@@ -0,0 +1,188 @@ 
+/*
+ * 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
+ */
+
+#include "libavutil/opt.h"
+#include "audio.h"
+#include "avfilter.h"
+#include "internal.h"
+
+typedef struct AudioCombContext {
+    const AVClass *class;
+
+    double b0, xM;
+    int t, M;
+
+    int head;
+    int tail;
+
+    AVFrame *delayframe;
+
+    void (*filter)(struct AudioCombContext *s, AVFrame *in, AVFrame *out);
+} AudioCombContext;
+
+#define OFFSET(x) offsetof(AudioCombContext, x)
+#define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
+
+static const AVOption acomb_options[] = {
+    { "t",  "set comb filter type",           OFFSET(t),  AV_OPT_TYPE_INT,    {.i64=0}, 0, 1, A, "t" },
+    { "f",  "feedforward",                    0,          AV_OPT_TYPE_CONST,  {.i64=0}, 0, 0, A, "t" },
+    { "b",  "feedback",                       0,          AV_OPT_TYPE_CONST,  {.i64=1}, 0, 0, A, "t" },
+    { "b0", "set direct signal gain",         OFFSET(b0), AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, A },
+    { "xM", "set delayed line gain",          OFFSET(xM), AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, A },
+    { "M",  "set delay in number of samples", OFFSET(M),  AV_OPT_TYPE_INT,    {.i64=10}, 1, 327680, A },
+    { NULL }
+};
+
+AVFILTER_DEFINE_CLASS(acomb);
+
+static int query_formats(AVFilterContext *ctx)
+{
+    AVFilterFormats *formats = NULL;
+    AVFilterChannelLayouts *layouts = NULL;
+    static const enum AVSampleFormat sample_fmts[] = {
+        AV_SAMPLE_FMT_FLTP,
+        AV_SAMPLE_FMT_DBLP,
+        AV_SAMPLE_FMT_NONE
+    };
+    int ret;
+
+    formats = ff_make_format_list(sample_fmts);
+    if (!formats)
+        return AVERROR(ENOMEM);
+    ret = ff_set_common_formats(ctx, formats);
+    if (ret < 0)
+        return ret;
+
+    layouts = ff_all_channel_counts();
+    if (!layouts)
+        return AVERROR(ENOMEM);
+
+    ret = ff_set_common_channel_layouts(ctx, layouts);
+    if (ret < 0)
+        return ret;
+
+    formats = ff_all_samplerates();
+    return ff_set_common_samplerates(ctx, formats);
+}
+
+#define COMB(name, type, dir, t)                                \
+static void acomb_## name ## _ ##dir(AudioCombContext *s,       \
+                                     AVFrame *in, AVFrame *out) \
+{                                                               \
+    const type b0 = s->b0;                                      \
+    const type xM = s->xM;                                      \
+    const int M = s->M;                                         \
+    int head;                                                   \
+                                                                \
+    for (int c = 0; c < in->channels; c++) {                    \
+        const type *src = (const type *)in->extended_data[c];   \
+        type *delay = (type *)s->delayframe->extended_data[c];  \
+        type *dst = (type *)out->extended_data[c];              \
+                                                                \
+        head = s->head;                                         \
+        for (int n = 0; n < in->nb_samples; n++) {              \
+            dst[n] = b0 * src[n] + t * xM * delay[head];        \
+            if (t == 1)                                         \
+                delay[head] = src[n];                           \
+            else                                                \
+                delay[head] = dst[n];                           \
+            head++;                                             \
+            if (head >= M)                                      \
+                head = 0;                                       \
+        }                                                       \
+    }                                                           \
+                                                                \
+    s->head = head;                                             \
+}
+
+COMB(fltp, float,  f,  1)
+COMB(dblp, double, f,  1)
+COMB(fltp, float,  b, -1)
+COMB(dblp, double, b, -1)
+
+static int config_input(AVFilterLink *inlink)
+{
+    AVFilterContext *ctx = inlink->dst;
+    AudioCombContext *s = ctx->priv;
+
+    s->delayframe = ff_get_audio_buffer(inlink, s->M);
+    if (!s->delayframe)
+        return AVERROR(ENOMEM);
+
+    switch (inlink->format) {
+    case AV_SAMPLE_FMT_FLTP: s->filter = s->t ? acomb_fltp_b : acomb_fltp_f; break;
+    case AV_SAMPLE_FMT_DBLP: s->filter = s->t ? acomb_dblp_b : acomb_dblp_f; break;
+    }
+
+    return 0;
+}
+
+static int filter_frame(AVFilterLink *inlink, AVFrame *in)
+{
+    AVFilterContext *ctx = inlink->dst;
+    AudioCombContext *s = ctx->priv;
+    AVFilterLink *outlink = ctx->outputs[0];
+    AVFrame *out = ff_get_audio_buffer(outlink, in->nb_samples);
+
+    if (!out) {
+        av_frame_free(&in);
+        return AVERROR(ENOMEM);
+    }
+    av_frame_copy_props(out, in);
+
+    s->filter(s, in, out);
+
+    av_frame_free(&in);
+    return ff_filter_frame(outlink, out);
+}
+
+static av_cold void uninit(AVFilterContext *ctx)
+{
+    AudioCombContext *s = ctx->priv;
+
+    av_frame_free(&s->delayframe);
+}
+
+static const AVFilterPad acomb_inputs[] = {
+    {
+        .name         = "default",
+        .type         = AVMEDIA_TYPE_AUDIO,
+        .filter_frame = filter_frame,
+        .config_props = config_input,
+    },
+    { NULL }
+};
+
+static const AVFilterPad acomb_outputs[] = {
+    {
+        .name = "default",
+        .type = AVMEDIA_TYPE_AUDIO,
+    },
+    { NULL }
+};
+
+AVFilter ff_af_acomb = {
+    .name          = "acomb",
+    .description   = NULL_IF_CONFIG_SMALL("Apply comb audio filter."),
+    .query_formats = query_formats,
+    .priv_size     = sizeof(AudioCombContext),
+    .priv_class    = &acomb_class,
+    .uninit        = uninit,
+    .inputs        = acomb_inputs,
+    .outputs       = acomb_outputs,
+};
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index 1a26129069..7417f9656d 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -24,6 +24,7 @@ 
 #include "config.h"
 
 extern AVFilter ff_af_abench;
+extern AVFilter ff_af_acomb;
 extern AVFilter ff_af_acompressor;
 extern AVFilter ff_af_acontrast;
 extern AVFilter ff_af_acopy;