James Almer: > 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. > If it is decided that we no longer need to actually look at packet data any more for adding bitstream filters, then we can actually remove the check_bitstream functions altogether and let the muxer set things up during init.
But this is not how I understood Marton. He only wanted to take away the muxer's ability to add bitstream filters during multiple calls to check_bitstream(). Btw: need_auto_bsf() from [1] can also be simplified a lot: One adds a new field to AVStreamInternal bsf_checking_status. Possible values are 1 (checking is finished and one needs bsf), 0 (checking is finished and no bsf are needed) and -1 (checking not finished). If the AVFMT_FLAG_AUTO_BSF is not set or if the AVOutputFormat doesn't have a check_bitstream() function, it is set to 0 during init, otherwise to -1. need_auto_bsf() would then begin with "if (st->internal->bsf_checking_status >= 0) return st->internal->bsf_checking_status;". Otherwise, the bitstream would really be checked (the check for the existence of the check_bitstream function could then be omitted, too, of course), which would update bsf_checking_status accordingly. - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/259204.html _______________________________________________ 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".
