On 4/10/2020 11:56 AM, Marton Balint wrote: > > > On Fri, 10 Apr 2020, James Almer wrote: > >> On 4/10/2020 5:25 AM, Marton Balint wrote: > > [...] > >>>>>> >>>>>> Finally, (I feel bad having to tell you this given that you have >>>>>> clearly >>>>>> invested a lot of time in this) we currently have no case where we >>>>>> need >>>>>> more than one automatically inserted bitstream filter per stream. >>>>>> We do >>>>>> not know whether the current code does the right thing in case more >>>>>> than >>>>>> one packet needs to be analyzed to determine whether to insert >>>>>> bitstream >>>>>> filters (maybe the packets should instead be cached so that no bsf >>>>>> misses out on the initial packets?); of course what is the right >>>>>> thing >>>>>> might be format and codec-dependent. So I don't know whether we >>>>>> should >>>>>> add this function now (given that public API is hard to remove). >>>>>> >>>>>> This does not mean that we should not use a bsf_list bsf to simplify >>>>>> automatic bitstream filtering in mux.c (it really simplifies this >>>>>> alot >>>>>> and gets rid of code duplication). But I think we should leave the >>>>>> topic >>>>>> of adding a bsf to an already existing bsf open so that it can be >>>>>> determined when the need indeed arises. >>>>> >>>>> I see no sane way of sharing list bsf code other than adding public >>>>> API, >>>>> because mux.c is libavformat, bsf.c is libavcodec. This at least >>>>> allows >>>>> you to replace AVStreamInternal->nb_bsfcs and AVStreamInternal->bsfcs >>>>> with a single bitstream filter to which you join additional ones. >>>>> >>>>> Unless you want to remove the existing (admittedly unused, and >>>>> potentially inadequate) support for multiple bitstream filters in >>>>> AVStreamInternal, I really don't see a way which postpones adding >>>>> this API. >>>> >>>> Can't you achieve the same using existing bsf list API from within >>>> lavf? >>>> av_bsf_list_alloc(), followed by any required amount of calls to >>>> av_bsf_list_append(), av_bsf_list_append2() or av_bsf_list_parse_str() >>>> to add each bsf, then av_bsf_list_finalize() at the end? >>> >>> That API only works if you have all the bitstream filters before >>> finalizing the list, you can't append a BSF to an already finalized >>> list. The implementation in mux.c supports adding a BSF, then maybe >>> passing a few packets through it, then appending another BSF to the list >>> - this can happen if .check_bitstream_filters() adds a bsf but returns >>> 0, and at a later packet it adds another bsf. >>> >>> Regards, >>> Marton >> >> I see what you mean. Do we really need the option to insert a bsf to a >> given stream after a few packets have already been processed? Any >> required bsf should be present since the beginning. They are all written >> to either modify the data or pass it through depending if something must >> be done or not. >> >> What could show up say five packets into the stream that would require a >> bsf to be inserted at that point and couldn't have been present since >> the beginning? Passing packets through within a bsf is almost free, so >> there's not a lot to win by postponing inserting a bsf. You'll simply be >> delegating the task of analyzing the bitstream to the muxer's >> check_bitstream(), when the bsf can already do it without any extra >> steps. > > Ok, so this also points me to the direction of removing the support of > multiple BSFs from AVStreamInternal, because if a demuxer wants multiple > bsfs it can simply create a single list bsf in one go.
This will also allow you to simplify things by moving the oformat->check_bitstream() calls from do_packet_auto_bsf() to init_muxer(), right after the call to oformat->init(), since you're not longer constrained by the need for packets mid muxing. > > Is there anybody who objects removing support for multiple bsfs is > AVStreamInternal? > > Thanks, > Marton > _______________________________________________ > 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".
