Re: [FFmpeg-devel] [PATCH] avfilter/alimiter: Remove the delay introduced by lookahead buffer

2022-04-04 Thread Paul B Mahol
On Wed, Mar 30, 2022 at 11:05 PM Wang Cao 
wrote:

> The change essentially removes the delay introduces by lookahead buffer.
> The valid audio data in the internal buffer is also flushed to ensure
> the integrity of output.
>
>
This adds extra delay,  and complicates current code a lot.



> Signed-off-by: Wang Cao 
> ---
>  doc/filters.texi   |   2 -
>  libavfilter/af_alimiter.c  |  97 +-
>  tests/ref/fate/filter-alimiter | 518 -
>  3 files changed, 344 insertions(+), 273 deletions(-)
>
> Thanks a lot for your time to review my patch. I have made "zero_delay"
> option to default. Now the output samples will only be written after the
> lookahead buffer is full. We also uses "request_frame" to flush the
> internal buffers so that all valid input audio samples are written.
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index d70ac3e237..5d1adf88e1 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -1943,8 +1943,6 @@ aiir=z=1.3057 0 0 0:p=1.3057 2.3892 2.1860 1:f=sf:r=d
>
>  The limiter prevents an input signal from rising over a desired threshold.
>  This limiter uses lookahead technology to prevent your signal from
> distorting.
> -It means that there is a small delay after the signal is processed. Keep
> in mind
> -that the delay it produces is the attack time you set.
>
>  The filter accepts the following options:
>
> diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c
> index 133f98f165..e1fcf98574 100644
> --- a/libavfilter/af_alimiter.c
> +++ b/libavfilter/af_alimiter.c
> @@ -30,6 +30,7 @@
>
>  #include "audio.h"
>  #include "avfilter.h"
> +#include "bufferqueue.h"
>  #include "formats.h"
>  #include "internal.h"
>
> @@ -55,6 +56,13 @@ typedef struct AudioLimiterContext {
>  int *nextpos;
>  double *nextdelta;
>
> +int total_samples_to_flush;
> +int lookahead_buffer_full;
> +
> +struct FFBufQueue output_frame_queue;
> +int output_sample_pos;
> +int output_frame_pos;
> +
>  double delta;
>  int nextiter;
>  int nextlen;
> @@ -129,6 +137,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>  AVFrame *out;
>  double *buf;
>  int n, c, i;
> +int can_free_input_frame = 0;
> +double peak = 0;
>
>  if (av_frame_is_writable(in)) {
>  out = in;
> @@ -138,12 +148,23 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
>  av_frame_free();
>  return AVERROR(ENOMEM);
>  }
> +can_free_input_frame = 1;
>  av_frame_copy_props(out, in);
>  }
> -dst = (double *)out->data[0];
> +
> +if (ff_bufqueue_is_full(>output_frame_queue)) {
> +// In the runtime, the total number of frames in the queue is
> bounded by
> +// attack_time, sample rate and frame size.
> +return AVERROR_BUG;
> +}
> +
> +ff_bufqueue_add(ctx, >output_frame_queue, out);
>
>  for (n = 0; n < in->nb_samples; n++) {
> -double peak = 0;
> +out = ff_bufqueue_peek(>output_frame_queue,
> s->output_frame_pos);
> +dst = (double *)out->data[0];
> +dst += s->output_sample_pos * channels;
> +peak = 0;
>
>  for (c = 0; c < channels; c++) {
>  double sample = src[c] * level_in;
> @@ -213,8 +234,21 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>
>  s->att += s->delta;
>
> -for (c = 0; c < channels; c++)
> -dst[c] = buf[c] * s->att;
> +// Checks lookahead buffer (s->buffer) has been filled
> +if (!s->lookahead_buffer_full && (s->pos + channels) %
> buffer_size == 0) {
> +s->lookahead_buffer_full = 1;
> +}
> +if (s->lookahead_buffer_full) {
> +for (c = 0; c < channels; c++) {
> +dst[c] = buf[c] * s->att;
> +dst[c] = av_clipd(dst[c], -limit, limit) * level *
> level_out;
> +}
> +s->output_sample_pos++;
> +if (s->output_sample_pos == out->nb_samples) {
> +s->output_frame_pos++;
> +s->output_sample_pos = 0;
> +}
> +}
>
>  if ((s->pos + channels) % buffer_size == nextpos[s->nextiter]) {
>  if (s->auto_release) {
> @@ -261,18 +295,55 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame *in)
>  if (s->delta != 0. && fabs(s->delta) < 0.01)
>  s->delta = 0.;
>
> -for (c = 0; c < channels; c++)
> -dst[c] = av_clipd(dst[c], -limit, limit) * level * level_out;
> -
>  s->pos = (s->pos + channels) % buffer_size;
>  src += channels;
> -dst += channels;
>  }
> -
> -if (in != out)
> +if (can_free_input_frame) {
>  av_frame_free();
> +}
>
> -return ff_filter_frame(outlink, out);
> +if (s->output_frame_pos > 0) {
> +s->output_frame_pos--;
> +out = ff_bufqueue_get(>output_frame_queue);
> + 

[FFmpeg-devel] [PATCH] avfilter/alimiter: Remove the delay introduced by lookahead buffer

2022-03-30 Thread Wang Cao
The change essentially removes the delay introduces by lookahead buffer.
The valid audio data in the internal buffer is also flushed to ensure
the integrity of output.

Signed-off-by: Wang Cao 
---
 doc/filters.texi   |   2 -
 libavfilter/af_alimiter.c  |  97 +-
 tests/ref/fate/filter-alimiter | 518 -
 3 files changed, 344 insertions(+), 273 deletions(-)

Thanks a lot for your time to review my patch. I have made "zero_delay"
option to default. Now the output samples will only be written after the
lookahead buffer is full. We also uses "request_frame" to flush the
internal buffers so that all valid input audio samples are written.

diff --git a/doc/filters.texi b/doc/filters.texi
index d70ac3e237..5d1adf88e1 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -1943,8 +1943,6 @@ aiir=z=1.3057 0 0 0:p=1.3057 2.3892 2.1860 1:f=sf:r=d
 
 The limiter prevents an input signal from rising over a desired threshold.
 This limiter uses lookahead technology to prevent your signal from distorting.
-It means that there is a small delay after the signal is processed. Keep in 
mind
-that the delay it produces is the attack time you set.
 
 The filter accepts the following options:
 
diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c
index 133f98f165..e1fcf98574 100644
--- a/libavfilter/af_alimiter.c
+++ b/libavfilter/af_alimiter.c
@@ -30,6 +30,7 @@
 
 #include "audio.h"
 #include "avfilter.h"
+#include "bufferqueue.h"
 #include "formats.h"
 #include "internal.h"
 
@@ -55,6 +56,13 @@ typedef struct AudioLimiterContext {
 int *nextpos;
 double *nextdelta;
 
+int total_samples_to_flush;
+int lookahead_buffer_full;
+
+struct FFBufQueue output_frame_queue;
+int output_sample_pos;
+int output_frame_pos;
+
 double delta;
 int nextiter;
 int nextlen;
@@ -129,6 +137,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 AVFrame *out;
 double *buf;
 int n, c, i;
+int can_free_input_frame = 0;
+double peak = 0;
 
 if (av_frame_is_writable(in)) {
 out = in;
@@ -138,12 +148,23 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 av_frame_free();
 return AVERROR(ENOMEM);
 }
+can_free_input_frame = 1;
 av_frame_copy_props(out, in);
 }
-dst = (double *)out->data[0];
+
+if (ff_bufqueue_is_full(>output_frame_queue)) {
+// In the runtime, the total number of frames in the queue is bounded 
by
+// attack_time, sample rate and frame size.
+return AVERROR_BUG;
+}
+
+ff_bufqueue_add(ctx, >output_frame_queue, out);
 
 for (n = 0; n < in->nb_samples; n++) {
-double peak = 0;
+out = ff_bufqueue_peek(>output_frame_queue, s->output_frame_pos);
+dst = (double *)out->data[0];
+dst += s->output_sample_pos * channels;
+peak = 0;
 
 for (c = 0; c < channels; c++) {
 double sample = src[c] * level_in;
@@ -213,8 +234,21 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 
 s->att += s->delta;
 
-for (c = 0; c < channels; c++)
-dst[c] = buf[c] * s->att;
+// Checks lookahead buffer (s->buffer) has been filled
+if (!s->lookahead_buffer_full && (s->pos + channels) % buffer_size == 
0) {
+s->lookahead_buffer_full = 1;
+}
+if (s->lookahead_buffer_full) {
+for (c = 0; c < channels; c++) {
+dst[c] = buf[c] * s->att;
+dst[c] = av_clipd(dst[c], -limit, limit) * level * level_out;
+}
+s->output_sample_pos++;
+if (s->output_sample_pos == out->nb_samples) {
+s->output_frame_pos++;
+s->output_sample_pos = 0;
+}
+}
 
 if ((s->pos + channels) % buffer_size == nextpos[s->nextiter]) {
 if (s->auto_release) {
@@ -261,18 +295,55 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 if (s->delta != 0. && fabs(s->delta) < 0.01)
 s->delta = 0.;
 
-for (c = 0; c < channels; c++)
-dst[c] = av_clipd(dst[c], -limit, limit) * level * level_out;
-
 s->pos = (s->pos + channels) % buffer_size;
 src += channels;
-dst += channels;
 }
-
-if (in != out)
+if (can_free_input_frame) {
 av_frame_free();
+}
 
-return ff_filter_frame(outlink, out);
+if (s->output_frame_pos > 0) {
+s->output_frame_pos--;
+out = ff_bufqueue_get(>output_frame_queue);
+return ff_filter_frame(outlink, out);
+}
+return 0;
+}
+
+static int request_frame(AVFilterLink* outlink) 
+{
+AVFilterContext *ctx = outlink->src;
+AudioLimiterContext *s = (AudioLimiterContext*)ctx->priv;
+int ret;
+AVFilterLink *inlink;
+AVFrame *out;
+AVFrame *silence_frame;
+
+ret = ff_request_frame(ctx->inputs[0]);
+
+if