| Message ID | 20220415184934.1891758-1-wangcao@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avfilter/alimiter: Add an option "comp_delay" that removes the delay introduced by lookahead buffer | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | fail | Make fate failed |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | fail | Make fate failed |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | fail | Make fate failed |
On Fri, Apr 15, 2022 at 11:50 AM Wang Cao <wangcao@google.com> wrote: > 1. The option also flushes all the valid audio samples in the lookahead > buffer so the audio integrity is preserved. Previously the the output > audio will lose the amount of audio samples equal to the size of > lookahead buffer > 2. Add a FATE test to verify that when the filter is working as > passthrough filter, all audio samples are properly handled from the > input to the output. > > Signed-off-by: Wang Cao <wangcao@google.com> > --- > doc/filters.texi | 5 +++ > libavfilter/af_alimiter.c | 74 +++++++++++++++++++++++++++++++++++++ > tests/fate/filter-audio.mak | 12 ++++++ > 3 files changed, 91 insertions(+) > > diff --git a/doc/filters.texi b/doc/filters.texi > index a161754233..2af0953c89 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -1978,6 +1978,11 @@ in release time while 1 produces higher release > times. > @item level > Auto level output signal. Default is enabled. > This normalizes audio back to 0dB if enabled. > + > +@item comp_delay > +Compensate the delay introduced by using the lookahead buffer set with > attack > +parameter. Also flush the valid audio data in the lookahead buffer when > the > +stream hits EOF. > @end table > > Depending on picked setting it is recommended to upsample input 2x or 4x > times > diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c > index 133f98f165..d10a90859b 100644 > --- a/libavfilter/af_alimiter.c > +++ b/libavfilter/af_alimiter.c > @@ -55,6 +55,12 @@ typedef struct AudioLimiterContext { > int *nextpos; > double *nextdelta; > > + int lookahead_delay_samples; > + int lookahead_flush_samples; > + int64_t output_pts; > + int64_t next_output_pts; > + int comp_delay; > + > double delta; > int nextiter; > int nextlen; > @@ -73,6 +79,7 @@ static const AVOption alimiter_options[] = { > { "asc", "enable asc", OFFSET(auto_release), > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > { "asc_level", "set asc level", OFFSET(asc_coeff), > AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, AF }, > { "level", "auto level", OFFSET(auto_level), > AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, AF }, > + { "comp_delay","compensate delay", OFFSET(comp_delay), > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > { NULL } > }; > > @@ -129,6 +136,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame > *in) > AVFrame *out; > double *buf; > int n, c, i; > + int num_output_samples = in->nb_samples; > + int trim_offset; > > if (av_frame_is_writable(in)) { > out = in; > @@ -271,10 +280,71 @@ static int filter_frame(AVFilterLink *inlink, > AVFrame *in) > > if (in != out) > av_frame_free(&in); > + > + if (!s->comp_delay) { > + return ff_filter_frame(outlink, out); > + } > + > + if (s->output_pts == AV_NOPTS_VALUE) { > + s->output_pts = in->pts; > + } > + > + if (s->lookahead_delay_samples > 0) { > + // The current output frame is completely silence > + if (s->lookahead_delay_samples >= in->nb_samples) { > + s->lookahead_delay_samples -= in->nb_samples; > + return 0; > + } > + > + // Trim the silence part > + trim_offset = av_samples_get_buffer_size( > + NULL, inlink->ch_layout.nb_channels, > s->lookahead_delay_samples, > + (enum AVSampleFormat)out->format, 1); > + out->data[0] += trim_offset; > + out->nb_samples = in->nb_samples - s->lookahead_delay_samples; > + s->lookahead_delay_samples = 0; > + num_output_samples = out->nb_samples; > + } > + > + if (s->lookahead_delay_samples < 0) { > + return AVERROR_BUG; > + } > + > + out->pts = s->output_pts; > + s->next_output_pts = s->output_pts + num_output_samples; > + s->output_pts = s->next_output_pts; > > return ff_filter_frame(outlink, out); > } > > +static int request_frame(AVFilterLink* outlink) > +{ > + AVFilterContext *ctx = outlink->src; > + AudioLimiterContext *s = (AudioLimiterContext*)ctx->priv; > + int ret; > + AVFilterLink *inlink; > + AVFrame *silence_frame; > + > + ret = ff_request_frame(ctx->inputs[0]); > + > + if (ret != AVERROR_EOF || s->lookahead_flush_samples == 0 || > !s->comp_delay) { > + // Not necessarily an error, just not EOF. > + return ret; > + } > + > + // We reach here when input filters have finished producing data > (i.e. EOF), > + // but because of the attack param, s->buffer still has meaningful > + // audio content that needs flushing. > + inlink = ctx->inputs[0]; > + // Pushes silence frame to flush valid audio in the s->buffer > + silence_frame = ff_get_audio_buffer(inlink, > s->lookahead_flush_samples); > + ret = filter_frame(inlink, silence_frame); > + if (ret < 0) { > + return ret; > + } > + return AVERROR_EOF; > +} > + > static int config_input(AVFilterLink *inlink) > { > AVFilterContext *ctx = inlink->dst; > @@ -294,6 +364,9 @@ static int config_input(AVFilterLink *inlink) > memset(s->nextpos, -1, obuffer_size * sizeof(*s->nextpos)); > s->buffer_size = inlink->sample_rate * s->attack * > inlink->ch_layout.nb_channels; > s->buffer_size -= s->buffer_size % inlink->ch_layout.nb_channels; > + // the current logic outputs the next sample from the lookahead > buffer from the beginning so the amount of delay > + // compensation is less than the lookahead buffer size by 1 . > + s->lookahead_delay_samples = s->lookahead_flush_samples = > s->buffer_size / inlink->ch_layout.nb_channels - 1; > > if (s->buffer_size <= 0) { > av_log(ctx, AV_LOG_ERROR, "Attack is too small.\n"); > @@ -325,6 +398,7 @@ static const AVFilterPad alimiter_outputs[] = { > { > .name = "default", > .type = AVMEDIA_TYPE_AUDIO, > + .request_frame = request_frame, > }, > }; > > diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak > index eff32b9f81..3a51ca18a6 100644 > --- a/tests/fate/filter-audio.mak > +++ b/tests/fate/filter-audio.mak > @@ -63,6 +63,18 @@ fate-filter-agate: tests/data/asynth-44100-2.wav > fate-filter-agate: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav > fate-filter-agate: CMD = framecrc -i $(SRC) -af > aresample,agate=level_in=10:range=0:threshold=1:ratio=1:attack=1:knee=1:makeup=4,aresample > > +tests/data/filter-alimiter-passthrough: TAG = GEN > +tests/data/filter-alimiter-passthrough: ffmpeg$(PROGSSUF)$(EXESUF) | > tests/data > + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ > + -i $(TARGET_PATH)/tests/data/asynth-44100-2.wav -af aresample -f > crc $(TARGET_PATH)/$@ -y 2>/dev/null > + > +FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, PCM_S16LE, > WAV) += fate-filter-alimiter-passthrough > +fate-filter-alimiter-passthrough: tests/data/asynth-44100-2.wav > +fate-filter-alimiter-passthrough: tests/data/filter-alimiter-passthrough > +fate-filter-alimiter-passthrough: SRC = > $(TARGET_PATH)/tests/data/asynth-44100-2.wav > +fate-filter-alimiter-passthrough: REF = > $(TARGET_PATH)/tests/data/filter-alimiter-passthrough > +fate-filter-alimiter-passthrough: CMD = crc -i $(SRC) -af > aresample,alimiter=level_in=1:level_out=1:limit=1:level=0:comp_delay=1,aresample > + > FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, PCM_S16LE, > WAV) += fate-filter-alimiter > fate-filter-alimiter: tests/data/asynth-44100-2.wav > fate-filter-alimiter: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav > -- > 2.36.0.rc0.470.gd361397f0d-goog > > Hello folks, we would really appreciate any feedback on my patch. It looks confusing to me that "FATE" failed on the server while the test I added passed locally. I use "make fate-filter-alimiter-passthrough" to run the test FYI. Thank you!
On Fri, Apr 29, 2022 at 10:49 PM Wang Cao <wangcao-at-google.com@ffmpeg.org> wrote: > On Fri, Apr 15, 2022 at 11:50 AM Wang Cao <wangcao@google.com> wrote: > > > 1. The option also flushes all the valid audio samples in the lookahead > > buffer so the audio integrity is preserved. Previously the the output > > audio will lose the amount of audio samples equal to the size of > > lookahead buffer > > 2. Add a FATE test to verify that when the filter is working as > > passthrough filter, all audio samples are properly handled from the > > input to the output. > > > > Signed-off-by: Wang Cao <wangcao@google.com> > > --- > > doc/filters.texi | 5 +++ > > libavfilter/af_alimiter.c | 74 +++++++++++++++++++++++++++++++++++++ > > tests/fate/filter-audio.mak | 12 ++++++ > > 3 files changed, 91 insertions(+) > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > index a161754233..2af0953c89 100644 > > --- a/doc/filters.texi > > +++ b/doc/filters.texi > > @@ -1978,6 +1978,11 @@ in release time while 1 produces higher release > > times. > > @item level > > Auto level output signal. Default is enabled. > > This normalizes audio back to 0dB if enabled. > > + > > +@item comp_delay > > +Compensate the delay introduced by using the lookahead buffer set with > > attack > > +parameter. Also flush the valid audio data in the lookahead buffer when > > the > > +stream hits EOF. > > @end table > > > > Depending on picked setting it is recommended to upsample input 2x or 4x > > times > > diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c > > index 133f98f165..d10a90859b 100644 > > --- a/libavfilter/af_alimiter.c > > +++ b/libavfilter/af_alimiter.c > > @@ -55,6 +55,12 @@ typedef struct AudioLimiterContext { > > int *nextpos; > > double *nextdelta; > > > > + int lookahead_delay_samples; > > + int lookahead_flush_samples; > > + int64_t output_pts; > > + int64_t next_output_pts; > > + int comp_delay; > > + > > double delta; > > int nextiter; > > int nextlen; > > @@ -73,6 +79,7 @@ static const AVOption alimiter_options[] = { > > { "asc", "enable asc", OFFSET(auto_release), > > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > > { "asc_level", "set asc level", OFFSET(asc_coeff), > > AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, AF }, > > { "level", "auto level", OFFSET(auto_level), > > AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, AF }, > > + { "comp_delay","compensate delay", OFFSET(comp_delay), > > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > > { NULL } > > }; > > > > @@ -129,6 +136,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame > > *in) > > AVFrame *out; > > double *buf; > > int n, c, i; > > + int num_output_samples = in->nb_samples; > > + int trim_offset; > > > > if (av_frame_is_writable(in)) { > > out = in; > > @@ -271,10 +280,71 @@ static int filter_frame(AVFilterLink *inlink, > > AVFrame *in) > > > > if (in != out) > > av_frame_free(&in); > > + > > + if (!s->comp_delay) { > > + return ff_filter_frame(outlink, out); > > + } > > + > > + if (s->output_pts == AV_NOPTS_VALUE) { > > + s->output_pts = in->pts; > > + } > > + > > + if (s->lookahead_delay_samples > 0) { > > + // The current output frame is completely silence > > + if (s->lookahead_delay_samples >= in->nb_samples) { > > + s->lookahead_delay_samples -= in->nb_samples; > > + return 0; > > + } > > + > > + // Trim the silence part > > + trim_offset = av_samples_get_buffer_size( > > + NULL, inlink->ch_layout.nb_channels, > > s->lookahead_delay_samples, > > + (enum AVSampleFormat)out->format, 1); > > + out->data[0] += trim_offset; > > + out->nb_samples = in->nb_samples - s->lookahead_delay_samples; > > + s->lookahead_delay_samples = 0; > > + num_output_samples = out->nb_samples; > > + } > > + > > + if (s->lookahead_delay_samples < 0) { > > + return AVERROR_BUG; > > + } > > + > > + out->pts = s->output_pts; > > + s->next_output_pts = s->output_pts + num_output_samples; > > + s->output_pts = s->next_output_pts; > > > > return ff_filter_frame(outlink, out); > > } > > > > +static int request_frame(AVFilterLink* outlink) > > +{ > > + AVFilterContext *ctx = outlink->src; > > + AudioLimiterContext *s = (AudioLimiterContext*)ctx->priv; > > + int ret; > > + AVFilterLink *inlink; > > + AVFrame *silence_frame; > > + > > + ret = ff_request_frame(ctx->inputs[0]); > > + > > + if (ret != AVERROR_EOF || s->lookahead_flush_samples == 0 || > > !s->comp_delay) { > > + // Not necessarily an error, just not EOF. > > + return ret; > > + } > > + > > + // We reach here when input filters have finished producing data > > (i.e. EOF), > > + // but because of the attack param, s->buffer still has meaningful > > + // audio content that needs flushing. > > + inlink = ctx->inputs[0]; > > + // Pushes silence frame to flush valid audio in the s->buffer > > + silence_frame = ff_get_audio_buffer(inlink, > > s->lookahead_flush_samples); > > + ret = filter_frame(inlink, silence_frame); > > + if (ret < 0) { > > + return ret; > > + } > > + return AVERROR_EOF; > > +} > > + > > static int config_input(AVFilterLink *inlink) > > { > > AVFilterContext *ctx = inlink->dst; > > @@ -294,6 +364,9 @@ static int config_input(AVFilterLink *inlink) > > memset(s->nextpos, -1, obuffer_size * sizeof(*s->nextpos)); > > s->buffer_size = inlink->sample_rate * s->attack * > > inlink->ch_layout.nb_channels; > > s->buffer_size -= s->buffer_size % inlink->ch_layout.nb_channels; > > + // the current logic outputs the next sample from the lookahead > > buffer from the beginning so the amount of delay > > + // compensation is less than the lookahead buffer size by 1 . > > + s->lookahead_delay_samples = s->lookahead_flush_samples = > > s->buffer_size / inlink->ch_layout.nb_channels - 1; > > > > if (s->buffer_size <= 0) { > > av_log(ctx, AV_LOG_ERROR, "Attack is too small.\n"); > > @@ -325,6 +398,7 @@ static const AVFilterPad alimiter_outputs[] = { > > { > > .name = "default", > > .type = AVMEDIA_TYPE_AUDIO, > > + .request_frame = request_frame, > > }, > > }; > > > > diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak > > index eff32b9f81..3a51ca18a6 100644 > > --- a/tests/fate/filter-audio.mak > > +++ b/tests/fate/filter-audio.mak > > @@ -63,6 +63,18 @@ fate-filter-agate: tests/data/asynth-44100-2.wav > > fate-filter-agate: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > fate-filter-agate: CMD = framecrc -i $(SRC) -af > > > aresample,agate=level_in=10:range=0:threshold=1:ratio=1:attack=1:knee=1:makeup=4,aresample > > > > +tests/data/filter-alimiter-passthrough: TAG = GEN > > +tests/data/filter-alimiter-passthrough: ffmpeg$(PROGSSUF)$(EXESUF) | > > tests/data > > + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ > > + -i $(TARGET_PATH)/tests/data/asynth-44100-2.wav -af aresample -f > > crc $(TARGET_PATH)/$@ -y 2>/dev/null > > + > > +FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, > PCM_S16LE, > > WAV) += fate-filter-alimiter-passthrough > > +fate-filter-alimiter-passthrough: tests/data/asynth-44100-2.wav > > +fate-filter-alimiter-passthrough: tests/data/filter-alimiter-passthrough > > +fate-filter-alimiter-passthrough: SRC = > > $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > +fate-filter-alimiter-passthrough: REF = > > $(TARGET_PATH)/tests/data/filter-alimiter-passthrough > > +fate-filter-alimiter-passthrough: CMD = crc -i $(SRC) -af > > > aresample,alimiter=level_in=1:level_out=1:limit=1:level=0:comp_delay=1,aresample > > + > > FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, > PCM_S16LE, > > WAV) += fate-filter-alimiter > > fate-filter-alimiter: tests/data/asynth-44100-2.wav > > fate-filter-alimiter: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > -- > > 2.36.0.rc0.470.gd361397f0d-goog > > > > Hello folks, we would really appreciate any feedback on my patch. It > looks > confusing to > me that "FATE" failed on the server while the test I added passed locally. > > I use "make fate-filter-alimiter-passthrough" to run the test FYI. Thank > you! > Still not using logic as in af_ladspa.c Check latest version of FFmpeg source code. Please remove excessive self-explanatory comments in patch. I dunno how FATE test can work if non-trivial filter uses floats. Please follow dev guidelines. And make commit message follow other commits in repo. > -- > > Wang Cao | Software Engineer | wangcao@google.com | 650-203-7807 > <(650)%20203-7807> > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Mon, May 2, 2022 at 1:24 AM Paul B Mahol <onemda@gmail.com> wrote: > On Fri, Apr 29, 2022 at 10:49 PM Wang Cao < > wangcao-at-google.com@ffmpeg.org> > wrote: > > > On Fri, Apr 15, 2022 at 11:50 AM Wang Cao <wangcao@google.com> wrote: > > > > > 1. The option also flushes all the valid audio samples in the lookahead > > > buffer so the audio integrity is preserved. Previously the the > output > > > audio will lose the amount of audio samples equal to the size of > > > lookahead buffer > > > 2. Add a FATE test to verify that when the filter is working as > > > passthrough filter, all audio samples are properly handled from the > > > input to the output. > > > > > > Signed-off-by: Wang Cao <wangcao@google.com> > > > --- > > > doc/filters.texi | 5 +++ > > > libavfilter/af_alimiter.c | 74 +++++++++++++++++++++++++++++++++++++ > > > tests/fate/filter-audio.mak | 12 ++++++ > > > 3 files changed, 91 insertions(+) > > > > > > diff --git a/doc/filters.texi b/doc/filters.texi > > > index a161754233..2af0953c89 100644 > > > --- a/doc/filters.texi > > > +++ b/doc/filters.texi > > > @@ -1978,6 +1978,11 @@ in release time while 1 produces higher release > > > times. > > > @item level > > > Auto level output signal. Default is enabled. > > > This normalizes audio back to 0dB if enabled. > > > + > > > +@item comp_delay > > > +Compensate the delay introduced by using the lookahead buffer set with > > > attack > > > +parameter. Also flush the valid audio data in the lookahead buffer > when > > > the > > > +stream hits EOF. > > > @end table > > > > > > Depending on picked setting it is recommended to upsample input 2x or > 4x > > > times > > > diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c > > > index 133f98f165..d10a90859b 100644 > > > --- a/libavfilter/af_alimiter.c > > > +++ b/libavfilter/af_alimiter.c > > > @@ -55,6 +55,12 @@ typedef struct AudioLimiterContext { > > > int *nextpos; > > > double *nextdelta; > > > > > > + int lookahead_delay_samples; > > > + int lookahead_flush_samples; > > > + int64_t output_pts; > > > + int64_t next_output_pts; > > > + int comp_delay; > > > + > > > double delta; > > > int nextiter; > > > int nextlen; > > > @@ -73,6 +79,7 @@ static const AVOption alimiter_options[] = { > > > { "asc", "enable asc", OFFSET(auto_release), > > > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > > > { "asc_level", "set asc level", OFFSET(asc_coeff), > > > AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, AF }, > > > { "level", "auto level", OFFSET(auto_level), > > > AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, AF }, > > > + { "comp_delay","compensate delay", OFFSET(comp_delay), > > > AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, > > > { NULL } > > > }; > > > > > > @@ -129,6 +136,8 @@ static int filter_frame(AVFilterLink *inlink, > AVFrame > > > *in) > > > AVFrame *out; > > > double *buf; > > > int n, c, i; > > > + int num_output_samples = in->nb_samples; > > > + int trim_offset; > > > > > > if (av_frame_is_writable(in)) { > > > out = in; > > > @@ -271,10 +280,71 @@ static int filter_frame(AVFilterLink *inlink, > > > AVFrame *in) > > > > > > if (in != out) > > > av_frame_free(&in); > > > + > > > + if (!s->comp_delay) { > > > + return ff_filter_frame(outlink, out); > > > + } > > > + > > > + if (s->output_pts == AV_NOPTS_VALUE) { > > > + s->output_pts = in->pts; > > > + } > > > + > > > + if (s->lookahead_delay_samples > 0) { > > > + // The current output frame is completely silence > > > + if (s->lookahead_delay_samples >= in->nb_samples) { > > > + s->lookahead_delay_samples -= in->nb_samples; > > > + return 0; > > > + } > > > + > > > + // Trim the silence part > > > + trim_offset = av_samples_get_buffer_size( > > > + NULL, inlink->ch_layout.nb_channels, > > > s->lookahead_delay_samples, > > > + (enum AVSampleFormat)out->format, 1); > > > + out->data[0] += trim_offset; > > > + out->nb_samples = in->nb_samples - s->lookahead_delay_samples; > > > + s->lookahead_delay_samples = 0; > > > + num_output_samples = out->nb_samples; > > > + } > > > + > > > + if (s->lookahead_delay_samples < 0) { > > > + return AVERROR_BUG; > > > + } > > > + > > > + out->pts = s->output_pts; > > > + s->next_output_pts = s->output_pts + num_output_samples; > > > + s->output_pts = s->next_output_pts; > > > > > > return ff_filter_frame(outlink, out); > > > } > > > > > > +static int request_frame(AVFilterLink* outlink) > > > +{ > > > + AVFilterContext *ctx = outlink->src; > > > + AudioLimiterContext *s = (AudioLimiterContext*)ctx->priv; > > > + int ret; > > > + AVFilterLink *inlink; > > > + AVFrame *silence_frame; > > > + > > > + ret = ff_request_frame(ctx->inputs[0]); > > > + > > > + if (ret != AVERROR_EOF || s->lookahead_flush_samples == 0 || > > > !s->comp_delay) { > > > + // Not necessarily an error, just not EOF. > > > + return ret; > > > + } > > > + > > > + // We reach here when input filters have finished producing data > > > (i.e. EOF), > > > + // but because of the attack param, s->buffer still has meaningful > > > + // audio content that needs flushing. > > > + inlink = ctx->inputs[0]; > > > + // Pushes silence frame to flush valid audio in the s->buffer > > > + silence_frame = ff_get_audio_buffer(inlink, > > > s->lookahead_flush_samples); > > > + ret = filter_frame(inlink, silence_frame); > > > + if (ret < 0) { > > > + return ret; > > > + } > > > + return AVERROR_EOF; > > > +} > > > + > > > static int config_input(AVFilterLink *inlink) > > > { > > > AVFilterContext *ctx = inlink->dst; > > > @@ -294,6 +364,9 @@ static int config_input(AVFilterLink *inlink) > > > memset(s->nextpos, -1, obuffer_size * sizeof(*s->nextpos)); > > > s->buffer_size = inlink->sample_rate * s->attack * > > > inlink->ch_layout.nb_channels; > > > s->buffer_size -= s->buffer_size % inlink->ch_layout.nb_channels; > > > + // the current logic outputs the next sample from the lookahead > > > buffer from the beginning so the amount of delay > > > + // compensation is less than the lookahead buffer size by 1 . > > > + s->lookahead_delay_samples = s->lookahead_flush_samples = > > > s->buffer_size / inlink->ch_layout.nb_channels - 1; > > > > > > if (s->buffer_size <= 0) { > > > av_log(ctx, AV_LOG_ERROR, "Attack is too small.\n"); > > > @@ -325,6 +398,7 @@ static const AVFilterPad alimiter_outputs[] = { > > > { > > > .name = "default", > > > .type = AVMEDIA_TYPE_AUDIO, > > > + .request_frame = request_frame, > > > }, > > > }; > > > > > > diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak > > > index eff32b9f81..3a51ca18a6 100644 > > > --- a/tests/fate/filter-audio.mak > > > +++ b/tests/fate/filter-audio.mak > > > @@ -63,6 +63,18 @@ fate-filter-agate: tests/data/asynth-44100-2.wav > > > fate-filter-agate: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > > fate-filter-agate: CMD = framecrc -i $(SRC) -af > > > > > > aresample,agate=level_in=10:range=0:threshold=1:ratio=1:attack=1:knee=1:makeup=4,aresample > > > > > > +tests/data/filter-alimiter-passthrough: TAG = GEN > > > +tests/data/filter-alimiter-passthrough: ffmpeg$(PROGSSUF)$(EXESUF) | > > > tests/data > > > + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ > > > + -i $(TARGET_PATH)/tests/data/asynth-44100-2.wav -af aresample > -f > > > crc $(TARGET_PATH)/$@ -y 2>/dev/null > > > + > > > +FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, > > PCM_S16LE, > > > WAV) += fate-filter-alimiter-passthrough > > > +fate-filter-alimiter-passthrough: tests/data/asynth-44100-2.wav > > > +fate-filter-alimiter-passthrough: > tests/data/filter-alimiter-passthrough > > > +fate-filter-alimiter-passthrough: SRC = > > > $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > > +fate-filter-alimiter-passthrough: REF = > > > $(TARGET_PATH)/tests/data/filter-alimiter-passthrough > > > +fate-filter-alimiter-passthrough: CMD = crc -i $(SRC) -af > > > > > > aresample,alimiter=level_in=1:level_out=1:limit=1:level=0:comp_delay=1,aresample > > > + > > > FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, > > PCM_S16LE, > > > WAV) += fate-filter-alimiter > > > fate-filter-alimiter: tests/data/asynth-44100-2.wav > > > fate-filter-alimiter: SRC = > $(TARGET_PATH)/tests/data/asynth-44100-2.wav > > > -- > > > 2.36.0.rc0.470.gd361397f0d-goog > > > > > > Hello folks, we would really appreciate any feedback on my patch. It > > looks > > confusing to > > me that "FATE" failed on the server while the test I added passed > locally. > > > > I use "make fate-filter-alimiter-passthrough" to run the test FYI. Thank > > you! > > > > Still not using logic as in af_ladspa.c > > Check latest version of FFmpeg source code. > > Please remove excessive self-explanatory comments in patch. > > I dunno how FATE test can work if non-trivial filter uses floats. > > Please follow dev guidelines. And make commit message follow other commits > in repo. > Hi Paul, I have prepared the patch according to your suggestion. During the development, I have also found the issue in af_ladspa, which caused the latency compensation not to work as intended. It's also fixed in this patch. FATE tests pass on my local development too. Please take a look, thank you!
diff --git a/doc/filters.texi b/doc/filters.texi index a161754233..2af0953c89 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -1978,6 +1978,11 @@ in release time while 1 produces higher release times. @item level Auto level output signal. Default is enabled. This normalizes audio back to 0dB if enabled. + +@item comp_delay +Compensate the delay introduced by using the lookahead buffer set with attack +parameter. Also flush the valid audio data in the lookahead buffer when the +stream hits EOF. @end table Depending on picked setting it is recommended to upsample input 2x or 4x times diff --git a/libavfilter/af_alimiter.c b/libavfilter/af_alimiter.c index 133f98f165..d10a90859b 100644 --- a/libavfilter/af_alimiter.c +++ b/libavfilter/af_alimiter.c @@ -55,6 +55,12 @@ typedef struct AudioLimiterContext { int *nextpos; double *nextdelta; + int lookahead_delay_samples; + int lookahead_flush_samples; + int64_t output_pts; + int64_t next_output_pts; + int comp_delay; + double delta; int nextiter; int nextlen; @@ -73,6 +79,7 @@ static const AVOption alimiter_options[] = { { "asc", "enable asc", OFFSET(auto_release), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, { "asc_level", "set asc level", OFFSET(asc_coeff), AV_OPT_TYPE_DOUBLE, {.dbl=0.5}, 0, 1, AF }, { "level", "auto level", OFFSET(auto_level), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, AF }, + { "comp_delay","compensate delay", OFFSET(comp_delay), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, AF }, { NULL } }; @@ -129,6 +136,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) AVFrame *out; double *buf; int n, c, i; + int num_output_samples = in->nb_samples; + int trim_offset; if (av_frame_is_writable(in)) { out = in; @@ -271,10 +280,71 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in) if (in != out) av_frame_free(&in); + + if (!s->comp_delay) { + return ff_filter_frame(outlink, out); + } + + if (s->output_pts == AV_NOPTS_VALUE) { + s->output_pts = in->pts; + } + + if (s->lookahead_delay_samples > 0) { + // The current output frame is completely silence + if (s->lookahead_delay_samples >= in->nb_samples) { + s->lookahead_delay_samples -= in->nb_samples; + return 0; + } + + // Trim the silence part + trim_offset = av_samples_get_buffer_size( + NULL, inlink->ch_layout.nb_channels, s->lookahead_delay_samples, + (enum AVSampleFormat)out->format, 1); + out->data[0] += trim_offset; + out->nb_samples = in->nb_samples - s->lookahead_delay_samples; + s->lookahead_delay_samples = 0; + num_output_samples = out->nb_samples; + } + + if (s->lookahead_delay_samples < 0) { + return AVERROR_BUG; + } + + out->pts = s->output_pts; + s->next_output_pts = s->output_pts + num_output_samples; + s->output_pts = s->next_output_pts; return ff_filter_frame(outlink, out); } +static int request_frame(AVFilterLink* outlink) +{ + AVFilterContext *ctx = outlink->src; + AudioLimiterContext *s = (AudioLimiterContext*)ctx->priv; + int ret; + AVFilterLink *inlink; + AVFrame *silence_frame; + + ret = ff_request_frame(ctx->inputs[0]); + + if (ret != AVERROR_EOF || s->lookahead_flush_samples == 0 || !s->comp_delay) { + // Not necessarily an error, just not EOF. + return ret; + } + + // We reach here when input filters have finished producing data (i.e. EOF), + // but because of the attack param, s->buffer still has meaningful + // audio content that needs flushing. + inlink = ctx->inputs[0]; + // Pushes silence frame to flush valid audio in the s->buffer + silence_frame = ff_get_audio_buffer(inlink, s->lookahead_flush_samples); + ret = filter_frame(inlink, silence_frame); + if (ret < 0) { + return ret; + } + return AVERROR_EOF; +} + static int config_input(AVFilterLink *inlink) { AVFilterContext *ctx = inlink->dst; @@ -294,6 +364,9 @@ static int config_input(AVFilterLink *inlink) memset(s->nextpos, -1, obuffer_size * sizeof(*s->nextpos)); s->buffer_size = inlink->sample_rate * s->attack * inlink->ch_layout.nb_channels; s->buffer_size -= s->buffer_size % inlink->ch_layout.nb_channels; + // the current logic outputs the next sample from the lookahead buffer from the beginning so the amount of delay + // compensation is less than the lookahead buffer size by 1 . + s->lookahead_delay_samples = s->lookahead_flush_samples = s->buffer_size / inlink->ch_layout.nb_channels - 1; if (s->buffer_size <= 0) { av_log(ctx, AV_LOG_ERROR, "Attack is too small.\n"); @@ -325,6 +398,7 @@ static const AVFilterPad alimiter_outputs[] = { { .name = "default", .type = AVMEDIA_TYPE_AUDIO, + .request_frame = request_frame, }, }; diff --git a/tests/fate/filter-audio.mak b/tests/fate/filter-audio.mak index eff32b9f81..3a51ca18a6 100644 --- a/tests/fate/filter-audio.mak +++ b/tests/fate/filter-audio.mak @@ -63,6 +63,18 @@ fate-filter-agate: tests/data/asynth-44100-2.wav fate-filter-agate: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav fate-filter-agate: CMD = framecrc -i $(SRC) -af aresample,agate=level_in=10:range=0:threshold=1:ratio=1:attack=1:knee=1:makeup=4,aresample +tests/data/filter-alimiter-passthrough: TAG = GEN +tests/data/filter-alimiter-passthrough: ffmpeg$(PROGSSUF)$(EXESUF) | tests/data + $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< -nostdin \ + -i $(TARGET_PATH)/tests/data/asynth-44100-2.wav -af aresample -f crc $(TARGET_PATH)/$@ -y 2>/dev/null + +FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-alimiter-passthrough +fate-filter-alimiter-passthrough: tests/data/asynth-44100-2.wav +fate-filter-alimiter-passthrough: tests/data/filter-alimiter-passthrough +fate-filter-alimiter-passthrough: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav +fate-filter-alimiter-passthrough: REF = $(TARGET_PATH)/tests/data/filter-alimiter-passthrough +fate-filter-alimiter-passthrough: CMD = crc -i $(SRC) -af aresample,alimiter=level_in=1:level_out=1:limit=1:level=0:comp_delay=1,aresample + FATE_AFILTER-$(call FILTERDEMDECENCMUX, AFADE, WAV, PCM_S16LE, PCM_S16LE, WAV) += fate-filter-alimiter fate-filter-alimiter: tests/data/asynth-44100-2.wav fate-filter-alimiter: SRC = $(TARGET_PATH)/tests/data/asynth-44100-2.wav
1. The option also flushes all the valid audio samples in the lookahead buffer so the audio integrity is preserved. Previously the the output audio will lose the amount of audio samples equal to the size of lookahead buffer 2. Add a FATE test to verify that when the filter is working as passthrough filter, all audio samples are properly handled from the input to the output. Signed-off-by: Wang Cao <wangcao@google.com> --- doc/filters.texi | 5 +++ libavfilter/af_alimiter.c | 74 +++++++++++++++++++++++++++++++++++++ tests/fate/filter-audio.mak | 12 ++++++ 3 files changed, 91 insertions(+) -- 2.36.0.rc0.470.gd361397f0d-goog