[FFmpeg-devel] [PATCH v4 4/8] avformat/mux: add proper support for full N:M bitstream filtering

[Andreas Rheinhardt]

Marton Balint:
> 
> 
> On Tue, 5 May 2020, Andreas Rheinhardt wrote:
> 
>> Marton Balint:
>>>
>>>
>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote:
>>>
>>>> Marton Balint:
>>>>>
>>>>>
>>>>> On Tue, 5 May 2020, Andreas Rheinhardt wrote:
>>>>>
>>>>>> Marton Balint:
>>>>>>> Previously only 1:1 bitstream filters were supported, the end of the
>>>>>>> stream was
>>>>>>> not signalled to the bitstream filters and time base changes were
>>>>>>> ignored.
>>>>>>>
>>>>>>> This change also allows muxers to set up bitstream filters
>>>>>>> regardless
>>>>>>> of the
>>>>>>> autobsf flag during write_header instead of during check_bitstream
>>>>>>> and those
>>>>>>> bitstream filters will always be executed.
>>>>>>
>>>>>> Ignoring the autobsf flag is not ok IMO. The muxer should not add
>>>>>> a bsf
>>>>>> when the user explicitly didn't want this.
>>>>>>
>>>>>
>>>>> The user should not be allowed to create broken files, and the only
>>>>> way
>>>>> to achieve that is to force the BSF.
>>>>
>>>> No, one could also check in the muxer that the packets are ok (these
>>>> checks could be disabled if the bsf is enabled, of course) and error
>>>> out
>>>> if it is not.
>>>
>>> And that would duplicate some functionaltiy for negigable performance
>>> gain of not always doing a BSF.
>>>
>>>>
>>>>> I don't see a use case for
>>>>> disabling BSF either for MXFenc of GXFenc. (in fact, from the top
>>>>> of my
>>>>> head I can't see a use case for disabling automatically inserted BSF-s
>>>>> in other muxers).
>>>>>
>>>> A user who already knows that his packets are ok can do so to avoid the
>>>> (admittedly small) overhead of these bsfs. (And even if you think that
>>>> the option should be removed/should be made non-public (for internal
>>>> testing only), then this does not change that this option exists and
>>>> removing it would require the usual deprecation period (and consensus
>>>> from other devs, of course; I personally don't have formed an opinion
>>>> about this yet).)
>>>
>>> With my patch the autobsf option is still honored for all existing
>>> bitstream filters which are inserted mid-stream (in check_bitstream).
>>> Only the ones which are inserted at write_header(), so in mxfenc and
>>> gxfenc are forced.
>>>
>>>> One use-case is if a user runs the pcm_rechunk bsf filter manually and
>>>> then sends the output to both a framecrc as well as the mxf muxers in
>>>> order to be able to test more thoroughly whether any write errors
>>>> happened. Or where you want to write the same audio to multiple mxf
>>>> muxers, all with the same audio settings.
>>>
>>> And for them the additional BSF which is auto inserted will only pass on
>>> their packets, it will only *check* for correct packets, exactly what
>>> you suggested. I cannot accept the performance gain as argument.
>>>
>>>>
>>>>> So if you feel strongly about this and we still want to avoid creating
>>>>> corrupt files then the BSFs should be processed in mxf_interleave and
>>>>> gxf_interleave separately from the auto inserted bitstream filters. Do
>>>>> you think that is better? I don't think so.
>>>>>
>>>> That would just be a worse form of automatically inserted bsf (and it
>>>> would not work for av_write_frame()).
>>>
>>> Please suggest something reasonable to follow up on this.
>>>
>> How about: You error out in write_header if there is an audio track and
>> the user does not want automatically inserted bitstream filters?
> 
> Then maybe better to do it ff_stream_add_bitstream_filter?
> 
Yeah, that sounds like the right place for this.

- Andreas