[FFmpeg-devel] [PATCH] lavfi/af_ebur128: update filter to use new ebur128 API

Marton Balint cus at passwd.hu
Mon Jan 2 00:06:25 EET 2017


On Sun, 1 Jan 2017, Kyle Swanson wrote:

> On Wed, Dec 28, 2016 at 9:51 AM, Kyle Swanson <k at ylo.ph> wrote:
>
>>
>> Finally had a minute to look at this again. Attached patch addresses
>> Michael's and Marton's comments.
>>
>
> If no one has anything else, I'll push this in the next couple of days.
>

Hi, I still got some comments :)

> From d9d07177455c8f5e31b94c046be8e831c3f6234e Mon Sep 17 00:00:00 2001
> From: Kyle Swanson <k at ylo.ph>
> Date: Wed, 28 Dec 2016 11:19:23 -0600
> Subject: [PATCH] lavfi/ebur128: add ebur128_check_true_peak()
> 
> Signed-off-by: Kyle Swanson <k at ylo.ph>
> ---
>  libavfilter/ebur128.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  libavfilter/ebur128.h |  16 +++++
>  2 files changed, 172 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/ebur128.c b/libavfilter/ebur128.c
> index a46692e..bcce84b 100644
> --- a/libavfilter/ebur128.c
> +++ b/libavfilter/ebur128.c
> @@ -50,6 +50,11 @@
>  #include "libavutil/common.h"
>  #include "libavutil/mem.h"
>  #include "libavutil/thread.h"
> +#include "libavutil/channel_layout.h"
> +#include "libswresample/swresample.h"
> +#if CONFIG_SWRESAMPLE
> +#include "libavutil/opt.h"
> +#endif
>
>  #define CHECK_ERROR(condition, errorcode, goto_point)                          \
>      if ((condition)) {                                                         \
> @@ -91,6 +96,16 @@ struct FFEBUR128StateInternal {
>      size_t short_term_frame_counter;
>      /** Maximum sample peak, one per channel */
>      double *sample_peak;
> +    /** Maximum true peak, one per channel */
> +    double* true_peak;
> +#if CONFIG_SWRESAMPLE
> +    SwrContext *resampler;
> +    size_t oversample_factor;
> +    float* resampler_buffer_input;
> +    size_t resampler_buffer_input_frames;
> +    float* resampler_buffer_output;
> +    size_t resampler_buffer_output_frames;
> +#endif
>      /** The maximum window duration in ms. */
>      unsigned long window;
>      /** Data pointer array for interleaved data */
> @@ -214,6 +229,70 @@ static inline void init_histogram(void)
>      }
>  }
> 
> +#if CONFIG_SWRESAMPLE
> +static int ebur128_init_resampler(FFEBUR128State* st) {
> +    int64_t channel_layout;
> +    int errcode;
> +
> +    if (st->samplerate < 96000) {
> +        st->d->oversample_factor = 4;
> +    } else if (st->samplerate < 192000) {
> +        st->d->oversample_factor = 2;
> +    } else {
> +        st->d->oversample_factor = 1;
> +        st->d->resampler_buffer_input = NULL;
> +        st->d->resampler_buffer_output = NULL;
> +        st->d->resampler = NULL;
> +    }
> +
> +    st->d->resampler_buffer_input_frames = st->d->samples_in_100ms * 4;
> +    st->d->resampler_buffer_input = av_malloc_array(st->d->resampler_buffer_input_frames *
> +                                                    st->channels,
> +                                                    sizeof(float));
> +    CHECK_ERROR(!st->d->resampler_buffer_input, 0, exit)
> +
> +    st->d->resampler_buffer_output_frames =
> +    st->d->resampler_buffer_input_frames *
> +    st->d->oversample_factor;

Missing indentation.

> +    st->d->resampler_buffer_output = av_malloc_array(st->d->resampler_buffer_output_frames *
> +                                                     st->channels,
> +                                                     sizeof(float));
> +    CHECK_ERROR(!st->d->resampler_buffer_output, 0, free_input)
> +
> +    st->d->resampler = swr_alloc();
> +    CHECK_ERROR(!st->d->resampler, 0, free_output)
> +
> +    channel_layout = av_get_default_channel_layout(st->channels);
> +
> +    av_opt_set_int(st->d->resampler, "in_channel_layout", channel_layout, 0);
> +    av_opt_set_int(st->d->resampler, "in_sample_rate", st->samplerate, 0);
> +    av_opt_set_sample_fmt(st->d->resampler, "in_sample_fmt", AV_SAMPLE_FMT_FLT, 0);
> +    av_opt_set_int(st->d->resampler, "out_channel_layout", channel_layout, 0);
> +    av_opt_set_int(st->d->resampler, "out_sample_rate", st->samplerate * st->d->oversample_factor, 0);
> +    av_opt_set_sample_fmt(st->d->resampler, "out_sample_fmt", AV_SAMPLE_FMT_FLT, 0);

Do not guess or use the channel layout, you can pass the channel count directly
to the resampler using the "in_channel_count" and "out_channel_count" options.
Also without the reference to av_get_default_channel_layout the
channel_layout.h #include at the top should become uneeded.

> +
> +    swr_init(st->d->resampler);
> +    return 0;

return swr_init() instead?

> +
> +free_output:
> +    av_freep(st->d->resampler_buffer_output);
> +    st->d->resampler_buffer_output = NULL;

av_freep requires a pointer to the pointer, that is how you eliminate the NULL
assigment. So this should be av_freep(&st->d->resampler_buffer_output); and
the NULL assignment is uneeded.

> +free_input:
> +    av_freep(st->d->resampler_buffer_input);
> +    st->d->resampler_buffer_input = NULL;

Same here.

> +exit:
> +    return AVERROR(ENOMEM);
> +}
> +
> +static void ebur128_destroy_resampler(FFEBUR128State* st) {
> +    av_freep(st->d->resampler_buffer_input);
> +    st->d->resampler_buffer_input = NULL;

Same here.

> +    av_freep(st->d->resampler_buffer_output);
> +    st->d->resampler_buffer_output = NULL;

Same here.

> +    swr_free(&st->d->resampler);
> +}
> +#endif
> +
>  FFEBUR128State *ff_ebur128_init(unsigned int channels,
>                                  unsigned long samplerate,
>                                  unsigned long window, int mode)
> @@ -233,6 +312,9 @@ FFEBUR128State *ff_ebur128_init(unsigned int channels,
>      st->d->sample_peak =
>          (double *) av_mallocz_array(channels, sizeof(double));
>      CHECK_ERROR(!st->d->sample_peak, 0, free_channel_map)
> +    st->d->true_peak =
> +        (double*) av_mallocz_array(channels, sizeof(double));
> +    CHECK_ERROR(!st->d->true_peak, 0, free_sample_peak)
>
>      st->samplerate = samplerate;
>      st->d->samples_in_100ms = (st->samplerate + 5) / 10;
> @@ -242,7 +324,7 @@ FFEBUR128State *ff_ebur128_init(unsigned int channels,
>      } else if ((mode & FF_EBUR128_MODE_M) == FF_EBUR128_MODE_M) {
>          st->d->window = FFMAX(window, 400);
>      } else {
> -        goto free_sample_peak;
> +        goto free_true_peak;
>      }
>      st->d->audio_data_frames = st->samplerate * st->d->window / 1000;
>      if (st->d->audio_data_frames % st->d->samples_in_100ms) {
> @@ -254,7 +336,7 @@ FFEBUR128State *ff_ebur128_init(unsigned int channels,
>      st->d->audio_data =
>          (double *) av_mallocz_array(st->d->audio_data_frames,
>                                      st->channels * sizeof(double));
> -    CHECK_ERROR(!st->d->audio_data, 0, free_sample_peak)
> +    CHECK_ERROR(!st->d->audio_data, 0, free_true_peak)
>
>      ebur128_init_filter(st);
> 
> @@ -267,6 +349,12 @@ FFEBUR128State *ff_ebur128_init(unsigned int channels,
>                  free_block_energy_histogram)
>      st->d->short_term_frame_counter = 0;
> 
> +#if CONFIG_SWRESAMPLE
> +    int result;
> +    result = ebur128_init_resampler(st);

You should simply re-use the already existing errcode variable instead of result,
inline variable declaration is not allowed. Sorry for not spotting this 
the first time.

> +    CHECK_ERROR(result, 0, free_short_term_block_energy_histogram)
> +#endif
> +
>      /* the first block needs 400ms of audio data */
>      st->d->needed_frames = st->d->samples_in_100ms * 4;
>      /* start at the beginning of the buffer */
> @@ -287,6 +375,8 @@ free_block_energy_histogram:
>      av_free(st->d->block_energy_histogram);
>  free_audio_data:
>      av_free(st->d->audio_data);
> +free_true_peak:
> +    av_free(st->d->true_peak);
>  free_sample_peak:
>      av_free(st->d->sample_peak);
>  free_channel_map:
> @@ -306,12 +396,53 @@ void ff_ebur128_destroy(FFEBUR128State ** st)
>      av_free((*st)->d->audio_data);
>      av_free((*st)->d->channel_map);
>      av_free((*st)->d->sample_peak);
> +    av_free((*st)->d->true_peak);
>      av_free((*st)->d->data_ptrs);
> +#if CONFIG_SWRESAMPLE
> +  ebur128_destroy_resampler(*st);

Strange indentation.

> +#endif
>      av_free((*st)->d);
>      av_free(*st);
>      *st = NULL;
>  }
> 
> +static int ebur128_use_swresample(FFEBUR128State* st) {
> +#if CONFIG_SWRESAMPLE
> +    return ((st->mode & FF_EBUR128_MODE_TRUE_PEAK) == FF_EBUR128_MODE_TRUE_PEAK);
> +#else
> +    (void) st;
> +    return 0;
> +#endif
> +}
> +
> +static void ebur128_check_true_peak(FFEBUR128State* st, size_t frames) {
> +#if CONFIG_SWRESAMPLE
> +    size_t c, i;
> +
> +    const int in_len  = frames;
> +    const int out_len = st->d->resampler_buffer_output_frames;
> +    swr_convert(st->d->resampler, (uint8_t **)&st->d->resampler_buffer_output, out_len,
> +                (const uint8_t **)&st->d->resampler_buffer_input, in_len);
> +
> +    for (c = 0; c < st->channels; ++c) {
> +        for (i = 0; i < out_len; ++i) {
> +            if (st->d->resampler_buffer_output[i * st->channels + c] >
> +                                                               st->d->true_peak[c]) {
> +              st->d->true_peak[c] =
> +                  st->d->resampler_buffer_output[i * st->channels + c];
> +            } else if (-st->d->resampler_buffer_output[i * st->channels + c] >
> +                                                               st->d->true_peak[c]) {
> +              st->d->true_peak[c] =
> +                 -st->d->resampler_buffer_output[i * st->channels + c];
> +            }

Maybe just nitpicking, I wouldn't wrap the lines in this case.

> +        }
> +    }
> +#else
> +    (void) st; (void) frames;
> +#endif
> +}
> +
> +
>  #define EBUR128_FILTER(type, scaling_factor)                                       \
>  static void ebur128_filter_##type(FFEBUR128State* st, const type** srcs,           \
>                                    size_t src_index, size_t frames,                 \
> @@ -334,6 +465,15 @@ static void ebur128_filter_##type(FFEBUR128State* st, const type** srcs,
>              if (max > st->d->sample_peak[c]) st->d->sample_peak[c] = max;          \
>          }                                                                          \
>      }                                                                              \
> +    if (ebur128_use_swresample(st)) {                                              \
> +        for (c = 0; c < st->channels; ++c) {                                       \
> +            for (i = 0; i < frames; ++i) {                                         \
> +                st->d->resampler_buffer_input[i * st->channels + c] =              \
> +                    (float) (srcs[c][src_index + i * stride] / scaling_factor);    \
> +            }                                                                      \
> +        }                                                                          \
> +        ebur128_check_true_peak(st, frames);                                       \
> +    }                                                                              \
>      for (c = 0; c < st->channels; ++c) {                                           \
>          int ci = st->d->channel_map[c] - 1;                                        \
>          if (ci < 0) continue;                                                      \
> @@ -781,3 +921,17 @@ int ff_ebur128_sample_peak(FFEBUR128State * st,
>      *out = st->d->sample_peak[channel_number];
>      return 0;
>  }
> +
> +int ff_ebur128_true_peak(FFEBUR128State * st,
> +                         unsigned int channel_number,
> +                         double* out) {
> +  if ((st->mode & FF_EBUR128_MODE_TRUE_PEAK) != FF_EBUR128_MODE_TRUE_PEAK) {
> +      return AVERROR(EINVAL);
> +  } else if (channel_number >= st->channels) {
> +      return AVERROR(EINVAL);
> +  }
> +  *out = st->d->true_peak[channel_number] > st->d->sample_peak[channel_number]
> +       ? st->d->true_peak[channel_number]
> +       : st->d->sample_peak[channel_number];
> +  return 0;
> +}
> diff --git a/libavfilter/ebur128.h b/libavfilter/ebur128.h
> index b94cd24..5d6cd70 100644
> --- a/libavfilter/ebur128.h
> +++ b/libavfilter/ebur128.h
> @@ -91,6 +91,8 @@ enum mode {
>      FF_EBUR128_MODE_LRA = (1 << 3) | FF_EBUR128_MODE_S,
>    /** can call ff_ebur128_sample_peak */
>      FF_EBUR128_MODE_SAMPLE_PEAK = (1 << 4) | FF_EBUR128_MODE_M,
> +  /** can call ff_ebur128_true_peak */
> +    FF_EBUR128_MODE_TRUE_PEAK   = (1 << 5) | FF_EBUR128_MODE_SAMPLE_PEAK
>  };
>
>  /** forward declaration of FFEBUR128StateInternal */
> @@ -283,6 +285,20 @@ int ff_ebur128_loudness_range_multiple(FFEBUR128State ** sts,
>  int ff_ebur128_sample_peak(FFEBUR128State * st,
>                             unsigned int channel_number, double *out);
> 
> +/** \brief Get maximum true peak from all frames that have been processed.
> + *
> + *  @param st library state
> + *  @param channel_number channel to analyse
> + *  @param out maximum true peak in float format (1.0 is 0 dBTP)
> + *  @return
> + *    - 0 on success.
> + *    - AVERROR(EINVAL) if mode "FF_EBUR128_MODE_TRUE_PEAK" has not
> + *      been set.
> + *    - AVERROR(EINVAL) if invalid channel index.
> + */
> +int ff_ebur128_true_peak(FFEBUR128State* st,
> +                      unsigned int channel_number, double* out);
> +
>  /** \brief Get relative threshold in LUFS.
>   *
>   *  @param st library state
> -- 
> 2.10.1

Thanks,
Marton


More information about the ffmpeg-devel mailing list