[FFmpeg-devel] [PATCH] avfilter: add loudnorm

[Paul B Mahol]

On 5/10/16, Kyle Swanson <k at ylo.ph> wrote:
> Hi,
>
> Updated patch attached. Thanks!
>


[...]

>
> + at section loudnorm
> +
> +EBU R128 loudness normalization. Includes both dynamic and linear normalization modes.
> +Support for both single pass (livestreams, files) and double pass (files) modes.
> +This algorithm can target IL, LRA, and maximum true peak.
> +
> +To enable compilation of this filter you need to configure FFmpeg with
> + at code{--enable-libebur128}.
> +
> +The filter accepts the following options:
> +
> + at table @option
> + at item I, i
> +Set integrated loudness target

Could you document range and default values here and below?

[...]

> +    ebur128_state *r128_in;
> +    ebur128_state *r128_out;
> +} LoudNormContext;
> +
> +enum {
> +    FIRST_FRAME,
> +    INNER_FRAME,
> +    FINAL_FRAME,
> +    LINEAR_MODE
> +};

Can't you name those enums? And add NB as last one?

[...]

> +static int config_input(AVFilterLink *inlink)
> +{
> +    AVFilterContext *ctx = inlink->dst;
> +    LoudNormContext *s = ctx->priv;
> +
> +    s->r128_in = av_malloc((size_t) sizeof(ebur128_state*));
> +    if (!s->r128_in)
> +        return AVERROR(ENOMEM);
> +    s->r128_in = ebur128_init(inlink->channels, inlink->sample_rate, EBUR128_MODE_I | EBUR128_MODE_S | EBUR128_MODE_LRA | EBUR128_MODE_SAMPLE_PEAK);
> +

Can this fail? One should check return value.
Doesn't this leaks memory?

> +    s->r128_out = av_malloc((size_t) sizeof(ebur128_state*));
> +    if (!s->r128_out)
> +        return AVERROR(ENOMEM);
> +    s->r128_out = ebur128_init(inlink->channels, inlink->sample_rate, EBUR128_MODE_I | EBUR128_MODE_S | EBUR128_MODE_LRA | EBUR128_MODE_SAMPLE_PEAK);

ditto.

[...]

> +static const AVFilterPad avfilter_af_loudnorm_outputs[] = {
> +    {
> +        .name = "default",
> +        .request_frame = request_frame,
> +        .type = AVMEDIA_TYPE_AUDIO,

Vertical alignment please.

[...]

rest LGTM