[FFmpeg-devel] [PATCH] lavfi/af_ebur128: update filter to use new ebur128 API
Kyle Swanson
k at ylo.ph
Wed Dec 28 19:51:59 EET 2016
On Mon, Nov 28, 2016 at 10:59 PM, Marton Balint <cus at passwd.hu> wrote:
>
> On Mon, 28 Nov 2016, Kyle Swanson wrote:
>
> On Thu, Nov 17, 2016 at 11:04 AM, Kyle Swanson <k at ylo.ph> wrote:
>>
>>> Hi,
>>>
>>> Here's a couple of patches which update the ebur128 filter to use the
>>> recently added ebur128 API. This updated filter allows fine-tuned
>>> control over which EBU R128 parameters are measured, and provides
>>> modest speed increases over the previous ebur128 filter. Also
>>> noteworthy: this removes the video output option of the ebur128
>>> filter. This is extraneous for an ebur128 measurement filter IMHO, but
>>> if we wanted to keep similar functionality in FFmpeg, we'd be better
>>> served by a new video source filter where custom meters could be
>>> created via exported frame metadata.
>>>
>>> The first patch adds true peak functionality to the ebur128 API using
>>> swresample (this was already discussed a little bit:
>>> http://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/202583.html)
>>> The second patch is an update to the ebur128 filter.
>>>
>>> Kyle
>>>
>>
>> Does anyone have any problems with the first patch?
>>
>
> From 6912ed3a03cd19f46e96f1f4b9eb3aa69b7ce4df Mon Sep 17 00:00:00 2001
>> From: Kyle Swanson <k at ylo.ph>
>> Date: Thu, 17 Nov 2016 10:32:45 -0600
>> Subject: [PATCH 1/2] lavfi/ebur128: add ebur128_check_true_peak()
>>
>> Signed-off-by: Kyle Swanson <k at ylo.ph>
>> ---
>> libavfilter/ebur128.c | 162 ++++++++++++++++++++++++++++++
>> +++++++++++++++++++-
>> libavfilter/ebur128.h | 17 ++++++
>> 2 files changed, 177 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavfilter/ebur128.c b/libavfilter/ebur128.c
>> index a46692e..dc16647 100644
>> --- a/libavfilter/ebur128.c
>> +++ b/libavfilter/ebur128.c
>> @@ -50,6 +50,9 @@
>> #include "libavutil/common.h"
>> #include "libavutil/mem.h"
>> #include "libavutil/thread.h"
>> +#include "libavutil/channel_layout.h"
>> +#include "libswresample/swresample.h"
>>
>
> Isn't this include needs an ifdef as well?
>
> +#include "libavutil/opt.h"
>>
>> #define CHECK_ERROR(condition, errorcode, goto_point)
>> \
>> if ((condition)) {
>> \
>> @@ -91,6 +94,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,12 +227,78 @@ 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(st->d->resampler_buffer_input_frames
>> *
>> + st->channels *
>> + sizeof(float));
>>
>
> av_malloc_array
>
> + 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;
>> + st->d->resampler_buffer_output = av_malloc(st->d->resampler_buffer_output_frames
>> *
>> + st->channels *
>> + sizeof(float));
>>
>
> av_malloc_array
>
> + 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);
>> +
>> + swr_init(st->d->resampler);
>> + return 0;
>> +
>> +free_output:
>> + av_free(st->d->resampler_buffer_output);
>> + st->d->resampler_buffer_output = NULL;
>>
>
> av_freep
>
> +free_input:
>> + av_free(st->d->resampler_buffer_input);
>> + st->d->resampler_buffer_input = NULL;
>>
>
> av_freep
>
> +exit:
>> + return AVERROR(ENOMEM);
>> +}
>> +
>> +static void ebur128_destroy_resampler(FFEBUR128State* st) {
>> + av_free(st->d->resampler_buffer_input);
>> + st->d->resampler_buffer_input = NULL;
>>
>
> av_freep
>
> + av_free(st->d->resampler_buffer_output);
>> + st->d->resampler_buffer_output = NULL;
>>
>
> av_freep
>
> + swr_free(&st->d->resampler);
>> + st->d->resampler = NULL;
>>
>
> swr_free already sets resampler to NULL.
>
> +}
>> +#endif
>> +
>> FFEBUR128State *ff_ebur128_init(unsigned int channels,
>> unsigned long samplerate,
>> unsigned long window, int mode)
>> {
>> int errcode;
>> FFEBUR128State *st;
>> + unsigned int i;
>>
>> st = (FFEBUR128State *) av_malloc(sizeof(FFEBUR128State));
>> CHECK_ERROR(!st, 0, exit)
>> @@ -233,6 +312,14 @@ 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*) malloc(channels * sizeof(double));
>>
>
> av_mallocz_array
>
> + CHECK_ERROR(!st->d->true_peak, 0, free_sample_peak)
>> +
>> + for (i = 0; i < channels; ++i) {
>> + st->d->sample_peak[i] = 0.0;
>> + st->d->true_peak[i] = 0.0;
>> + }
>>
>
> Technically not portable (AFAIK), but we assume in a lot of places that a
> mallocz-ed double is 0.0, so this initialization is unneeded.
>
>
>> st->samplerate = samplerate;
>> st->d->samples_in_100ms = (st->samplerate + 5) / 10;
>> @@ -242,7 +329,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 +341,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 +354,11 @@ FFEBUR128State *ff_ebur128_init(unsigned int
>> channels,
>> free_block_energy_histogram)
>> st->d->short_term_frame_counter = 0;
>>
>> +#if CONFIG_SWRESAMPLE
>> + unsigned int result = ebur128_init_resampler(st);
>>
>
> Why unsigned?
>
> + 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 +379,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 +400,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);
>> +#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];
>> + }
>> + }
>> + }
>> +#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 +469,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 +925,17 @@ int ff_ebur128_sample_peak(FFEBUR128State * st,
>> *out = st->d->sample_peak[channel_number];
>> return 0;
>>
>
> Hmm, okay, I got a bit of a problem with this performance-wise. The way I
> see it first we convert everything to float, then we resample, then we find
> the maximum in an interleaved output. I'd say performance-wise it would be
> alot better if we could resample directly the input data to a planar float,
> and then measure the maximum there, so no intermediate conversions.
>
> This can be a second step, if you are not interested in this now.
>
> }
>
>> +
>> +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..ca6dd62 100644
>> --- a/libavfilter/ebur128.h
>> +++ b/libavfilter/ebur128.h
>> @@ -91,6 +91,9 @@ 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_M
>> + | FF_EBUR128_MODE_SAMPLE_PEAK
>>
>
> I'd rather not set implicitly MODE_M, because I don't want to give
> loudness measurement to the user, who only wants true peak.
>
> };
>>
>> /** forward declaration of FFEBUR128StateInternal */
>> @@ -283,6 +286,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
>>
>>
> Regards,
> Marton
Finally had a minute to look at this again. Attached patch addresses
Michael's and Marton's comments.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavfi-ebur128-add-ebur128_check_true_peak.patch
Type: application/octet-stream
Size: 11655 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161228/6c59d983/attachment.obj>
More information about the ffmpeg-devel
mailing list