On 5/10/16, Kyle Swanson <k...@ylo.ph> wrote:
> Hi,
>
> Updated patch attached. Thanks!
>
[...]
>
> +@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
> +@code{--enable-libebur128}.
> +
> +The filter accepts the following options:
> +
> +@table @option
> +@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
