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 AB74640C77 for ; Fri, 8 Apr 2022 20:41:52 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5EB3368B235; Fri, 8 Apr 2022 23:41:48 +0300 (EEST) Received: from mail-yb1-f180.google.com (mail-yb1-f180.google.com [209.85.219.180]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B70A968B137 for ; Fri, 8 Apr 2022 23:41:41 +0300 (EEST) Received: by mail-yb1-f180.google.com with SMTP id z33so17102487ybh.5 for ; Fri, 08 Apr 2022 13:41:41 -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=QjZZU2R0upfWoWIrWi7OHEnIjr5LAn+ATwHf4O6Z0w0=; b=p6RHWPua9HMFBo4+Mf7tRRPbzBiY39SRzLsMQqVMVzpOBbdWcGJQ8nNzc20Oe6FV+e xhIHuGpnxwW+E3QJ9ulqXGz90VLNmUDIsOh0lEM5iRZX9HDv22oRM31MmKOyS60wWtpw BOSSzBNkbC10YKken622CqSD8d6+reTwbirg2Sj8J7f9E92P4MklP9463q+Kn/QJZpXI VY16KTLd9fRjHTSBNb9A9UUSufcM+pbmVbggR9w1L0oJf8+X1QUqLosQO+BNsJ6m7lDe 8x1CQ627jmlWdIFhDpR0WrvD36tJxBwlz7vcF91NnPUUw+Z2XghJ4zKUnulNItA7RPd2 SN+A== 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=QjZZU2R0upfWoWIrWi7OHEnIjr5LAn+ATwHf4O6Z0w0=; b=usFmxNEQARJ7f/DAu1DAlkOZPQnDmBtEME+IgGsMCk69Y4JSsfMTea2IqulbnPqLt9 iOwHB2Erpd9UVuwd0i8+PF/Nh1biyfxpF3roHYVCFH8vfKTG7VsHScANS2LpgeIOFKqz kHGfKy4gru2NlnGbwelwxTKwY85QWpB3Dn9gKDpcwceos6I7RtjBalfgKI830oVqlpAY T0/ajMGazuJDXhSdreLmCWuD4LBHVXs/9HWt39jL5Ud7WnLmEcgZDJ+M0AoyVdXfivdw 0ed6kRP2DdxdrObF2InUDFETkrU3foxeDiY2zGXRRNRisJTCKsCiwy34J8HmpldjoRCZ /jXw== X-Gm-Message-State: AOAM531Av7SOLjZh6XG0vh37UAikdL/F7fokG955L6dkNHbrp+We+bQa sfaU1oNMRK3O5CSpk7AUws9Mi25oCaW9i1PE2XxV9Ymf4AE= X-Google-Smtp-Source: ABdhPJxpKjTah1Xia6gvfAIMqFeGaVTiprRGk3JXbRvPkwe1X+JVjxzzHrkRVVAlq2kR2fnKriG0DUJ1Ye4/ZdX/ySo= X-Received: by 2002:a25:b984:0:b0:629:6b2a:8328 with SMTP id r4-20020a25b984000000b006296b2a8328mr16414298ybg.112.1649450499383; Fri, 08 Apr 2022 13:41:39 -0700 (PDT) MIME-Version: 1.0 References: <20220327060800.3732289-1-wangcao@google.com> <2ec3971d-f698-1369-1ace-eac613afc980@passwd.hu> In-Reply-To: From: Wang Cao Date: Fri, 8 Apr 2022 13:41:28 -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 "flush_buffer" option to flush the remaining valid data to the output 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 Fri, Apr 8, 2022 at 11:40 AM Paul B Mahol wrote: > On Thu, Apr 7, 2022 at 11:56 PM Wang Cao > > wrote: > > > On Thu, Apr 7, 2022 at 12:44 AM Paul B Mahol wrote: > > > > > On Wed, Apr 6, 2022 at 1:49 PM Paul B Mahol wrote: > > > > > > > > > > > > > > > On Tue, Apr 5, 2022 at 8:57 PM Wang Cao < > > > wangcao-at-google.com@ffmpeg.org> > > > > wrote: > > > > > > > >> On Mon, Apr 4, 2022 at 3:28 PM Marton Balint wrote: > > > >> > > > >> > > > > >> > > > > >> > On Mon, 4 Apr 2022, Paul B Mahol wrote: > > > >> > > > > >> > > On Sun, Mar 27, 2022 at 11:41 PM Marton Balint > > > wrote: > > > >> > > > > > >> > >> > > > >> > >> > > > >> > >> On Sat, 26 Mar 2022, Wang Cao wrote: > > > >> > >> > > > >> > >>> The change in the commit will add some samples to the end of > the > > > >> audio > > > >> > >>> stream. The intention is to add a "zero_delay" option > eventually > > > to > > > >> not > > > >> > >>> have the delay in the begining the output from alimiter due to > > > >> > >>> lookahead. > > > >> > >> > > > >> > >> I was very much suprised to see that the alimiter filter > actually > > > >> delays > > > >> > >> the audio - as in extra samples are inserted in the beginning > and > > > >> some > > > >> > >> samples are cut in the end. This trashes A-V sync, so it is a > bug > > > >> IMHO. > > > >> > >> > > > >> > >> So unless somebody has some valid usecase for the legacy way of > > > >> > operation > > > >> > >> I'd just simply change it to be "zero delay" without any > > additional > > > >> user > > > >> > >> option, in a single patch. > > > >> > >> > > > >> > > > > > >> > > > > > >> > > This is done by this patch in very complicated way and also it > > > really > > > >> > > should be optional. > > > >> > > > > >> > But why does it make sense to keep the current (IMHO buggy) > > > operational > > > >> > mode which adds silence in the beginning and trims the end? I > > > understand > > > >> > that the original implementation worked like this, but libavfilter > > has > > > >> > packet timestamps and N:M filtering so there is absolutely no > reason > > > to > > > >> > use an 1:1 implementation and live with its limitations. > > > >> > > > > >> Hello Paul and Marton, thank you so much for taking time to review > my > > > >> patch. > > > >> I totally understand that my patch may seem a little bit complicated > > > but I > > > >> can > > > >> show with a FATE test that if we set the alimiter to behave as a > > > >> passthrough filter, > > > >> the output frames will be the same from "framecrc" with my patch. > The > > > >> existing > > > >> behavior will not work for all gapless audio processing. > > > >> > > > >> The complete patch to fix this issue is at > > > >> > > > >> > > > > > > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20220330210314.2055201-1-wangcao@google.com/ > > > >> > > > >> Regarding Paul's concern, I personally don't have any preference > > whether > > > >> to > > > >> put > > > >> the patch as an extra option or not. With respect to the > > implementation, > > > >> the patch > > > >> is the best I can think of by preserving as much information as > > possible > > > >> from input > > > >> frames. I also understand it may break concept that "filter_frame" > > > outputs > > > >> one frame > > > >> at a time. For alimiter with my patch, depending on the size of the > > > >> lookahead buffer, > > > >> it may take a few frames before one output frame can be generated. > > This > > > is > > > >> inevitable > > > >> to compensate for the delay of the lookahead buffer. > > > >> > > > >> Thanks again for reviewing my patch and I'm looking forward to > hearing > > > >> from > > > >> you :) > > > >> > > > > > > > > Better than (because its no more 1 frame X nb_samples in, 1 frame X > > > > nb_samples out) to replace .filter_frame/.request_frame with > .activate > > > > logic. > > > > > > > > And make this output delay compensation filtering optional. > > > > > > > > In this process make sure that output PTS frame timestamps are > > unchanged > > > > from input one, by keeping reference of needed frames in filter > queue. > > > > > > > > Look how speechnorm/dynaudnorm does it. > > > > > > > > > > > > > Alternatively, use current logic in ladspa filter, (also add option > with > > > same name). > > > > > > This would need less code, and probably better approach, as there is no > > > extra latency introduced. > > > > > > Than mapping 1:1 between same number of samples per frame is not hold > any > > > more, but I think that is not much important any more. > > > > > Thank you for replying to me with your valuable feedback! I have checked > > af_ladspa > > and the "request_frame" function in af_ladspa looks similar to what I'm > > doing. The > > difference comes from the fact that I need an internal frame buffer to > keep > > track of > > output frames. Essentially I add a frame to the internal buffer when an > > input frame > > comes in. The frames in this buffer will be the future output frames. We > > start writing > > these output frame data buffers only when the internal lookahead buffer > has > > been filled. > > When there is no more input frames, we flushing all the remaining data in > > the internal > > frame buffers and lookahead buffers. Can you advise on my approach here? > > Thank you! > > > > I can put my current implementation of "filter_frame" and "request_frame" > > into the "activate" approach as you suggested with speechnorm/dynaudnorm. > > > > No need to wait for all buffers to fill up, only lookahead buffer. > > Just trim initial samples that is size of lookahead, and than start > outputing samples > just once you get whatever modulo of current frame number of samples. > > So there should not be need for extra buffers to keep audio samples. > Just buffers to keep input pts and number of samples of previous input > frames, like in ladspa filter. > Thank you for the suggestion! From my understanding, we have two ways to achieve "zero_delay" functionality here. Option 1: as you mentioned, we can trim the initial samples of lookahead size. The size of the lookahead buffer can be multiple frames. For example when the attack is 0.08 second, sample rate is 44100 and frame size is 1024, the lookahead buffer size is about 3 frames so the filter needs to see at least 3 input audio frames before it can output one audio frame. We also need to make assumptions about the size of the audio frame (meaning the number of audio samples per frame) when flushing. The frame is probably 1024 conventionally but I think it's better to make less assumption as possible to allow the filter to be used as flexible as possible. Option 2: this is what I proposed before. We basically map the same number of input frames to the output and we also make sure everything about the frame the same as the input except for the audio signal data itself, which will be changed by whatever processing alimiter has to do with that. I think it is safer to make the filter only work on the signal itself. It can help other people who use this filter without worrying about any unexpected change on the frame. The downside is that the filter internally needs to store some empty frames, which will be written as the lookahead buffer is filled. I don't see any performance difference between these two options but option 2 looks better to me because it works solely on the signals without any changes on the frame metadata. -- 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".