From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 83AE240AE3 for ; Thu, 5 May 2022 21:17:47 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4DF1068B2D5; Fri, 6 May 2022 00:17:45 +0300 (EEST) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id C302768ADD4 for ; Fri, 6 May 2022 00:17:38 +0300 (EEST) Received: by mail-yb1-f174.google.com with SMTP id e12so9749984ybc.11 for ; Thu, 05 May 2022 14:17:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=O7+/XseEWOxTOBYKEiTi2b0d+QbtHHzf60JT0giZBrs=; b=GTkP6J91rEgKDDjz1FNgtGr+xyYKv7wtbTBJzm8txtuRyznFG4Fz5gyARdVwPGhXCz iBev3zs1pjRVd6WHgX9GZaxRcxHDL/fzdFmIDnULjsmJb0oCGU9/iM3+ZBS07JoPAytK wNymMwyw0FceOspEoPkZzQtrWkZF6QIlbsurKzVtzI0VeD22nyaN2fPDEK+Br3bT/TFu K4K3jHolIXAGLsB8M+XyUiUm+ftwVM/dOXlg8/q67HgLZscA/7PNR3Vo5fMwle3gCbTh /rNn5XYQVii+BEplkazXuWMgd0TYuQ36QIF51Xygic7WuxwGYYj/1yiEF69GVfE+5R5A ryMA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=O7+/XseEWOxTOBYKEiTi2b0d+QbtHHzf60JT0giZBrs=; b=Mc0W/EvmFXDWu5+h3GMtmmo+fTSO/Yt9zwCs1CRAFjSWBSSXYBS4jOZxXUJE2RJtDt 8UpYU6WKfv2mAn9M88rDfbhgUTzrhfRDghdYjb3uniOjAMBmU6jz5FjZA6kdydfuhoEb AfrKoFH5VfbhwFwW/lZ11CZjLc6aPFMYlr+t79bpviQCO3PFRb2LLlrx+weGBK8jzDZF 7hghMqU9i15wCwe9MBLQwLyQlejcYlgIUtLdfQp5G2jcJgQIsFFj+vtxGRrDsyiTtUqE ZeTO35mEYcrEqDIYp/hWOq+tgEHBshpIPf+QvK0gUxjk2v68J7+3wfGIoaB6NkXJTJ5t 51GQ== X-Gm-Message-State: AOAM531F8ePg97kVVUvW128r0e+69LxA7tYH8Hh2f3Vbzz0NT8rCIX52 mPjXApnIZN+In4wl4UiMQQfZsrB2cm4GyPvhIgEZuxVD99g= X-Google-Smtp-Source: ABdhPJz3egLZSnOD7cQTIG9IqDDJjYpXKFp2GiQKVIOe15sq16JBWx6lOgWR43SN37MXiMoyN1volKjYGnC9CIKPzI0= X-Received: by 2002:a25:cb4b:0:b0:645:d702:eb15 with SMTP id b72-20020a25cb4b000000b00645d702eb15mr107638ybg.500.1651785457046; Thu, 05 May 2022 14:17:37 -0700 (PDT) MIME-Version: 1.0 References: <20220415184934.1891758-1-wangcao@google.com> In-Reply-To: From: Wang Cao Date: Thu, 5 May 2022 14:17:26 -0700 Message-ID: To: FFmpeg development discussions and patches X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [FFmpeg-devel] [PATCH] avfilter/alimiter: Add an option "comp_delay" that removes the delay introduced by lookahead buffer X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Mon, May 2, 2022 at 1:24 AM Paul B Mahol 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 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 > > > --- > > > 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! -- Wang Cao | Software Engineer | wangcao@google.com | 650-203-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".