[FFmpeg-devel] [PATCH 2/2] avcodec/bsf: add av_bsf_join() to chain bitstream filters

[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.

> 
> Is there anybody who objects removing support for multiple bsfs is
> AVStreamInternal?
> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".