On Fri, Apr 8, 2022 at 10:41 PM Wang Cao <wangcao-at-google....@ffmpeg.org> wrote:
> On Fri, Apr 8, 2022 at 11:40 AM Paul B Mahol <one...@gmail.com> wrote: > > > On Thu, Apr 7, 2022 at 11:56 PM Wang Cao < > wangcao-at-google....@ffmpeg.org > > > > > wrote: > > > > > On Thu, Apr 7, 2022 at 12:44 AM Paul B Mahol <one...@gmail.com> wrote: > > > > > > > On Wed, Apr 6, 2022 at 1:49 PM Paul B Mahol <one...@gmail.com> > wrote: > > > > > > > > > > > > > > > > > > > On Tue, Apr 5, 2022 at 8:57 PM Wang Cao < > > > > wangcao-at-google....@ffmpeg.org> > > > > > wrote: > > > > > > > > > >> On Mon, Apr 4, 2022 at 3:28 PM Marton Balint <c...@passwd.hu> > wrote: > > > > >> > > > > >> > > > > > >> > > > > > >> > On Mon, 4 Apr 2022, Paul B Mahol wrote: > > > > >> > > > > > >> > > On Sun, Mar 27, 2022 at 11:41 PM Marton Balint <c...@passwd.hu > > > > > > 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-wang...@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 > option 1 does not add extra delay in processing chain at all, and option 2 adds extra delay. Just look at latest version of af_ladspa.c filter code. > metadata. > -- > > Wang Cao | Software Engineer | wang...@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-requ...@ffmpeg.org with subject "unsubscribe". > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".