On Sun, 1 Jan 2017, Kyle Swanson wrote:

On Wed, Dec 28, 2016 at 9:51 AM, Kyle Swanson <k...@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...@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...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to