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

[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. 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).
>> 
> When automatic bitstream filtering was introduced in
> 1f9139b07b8a896b62c1f28f3d04acac33978c0d, writing the header of every
> muxer that potentially might include a bsf automatically (as indicated
> by the existence of the check_bitstream()-function) was delayed until
> the first packet would be written. This meant that there was a high
> probability of having a packet for the relevant stream when using the
> interleaved codepath.
> In particular, this meant that the Matroska muxer now received proper
> extradata for AAC when writing the header even before it received any
> packet with side data containing potential extradata at all. The reason
> was that the aac_adtstoasc bsf has simply overwritten the output
> extradata when dealing with the first packet (that was the old bsf API
> without init functions; when the new bsf API was merged, the merge
> commit (not the original commit) propagated the API violating behaviour).
> 1f6d7eb47070afc4394348721cd149f940ad2386 added the autobsf flag because
> of this delay: After all, the delay existed as soon as the
> AVOutputFormat had a check_bitstream function, even when no stream had a
> codec for which the check_bitstream function would add a bsf at all.
>
> As time passed, the API-violating behaviour of aac_adtstoasc was fixed
> and d6d605eb05c3ca32f591016c345eb2ad9e81c554 stopped delaying writing
> the header. The very same patchset containing said commit also contained
> a patch to deprecate AVFMT_FLAG_AUTO_BSF [1]. It was not applied due to
> concerns what happens when the bsf is not available.
>
> (For the record, ff_stream_add_bitstream_filter() will error out if a
> bsf is not available and the packet discarded. If the caller sends
> another packet for this stream and a bsf that should be added isn't
> available the packet will be rejected as well and so on.)
>
> The fact that one is forced to include certain automatically inserted
> bitstream filters even when one knows that one will only use them in
> scenarios where they merely passthrough the packets is certainly a
> downside of eliminating this flag. But now that I have reread the
> history I am nevertheless in favour of deprecation.

Wow, thanks for checking this. But if we are moving in the direction of 
deprecation, then I'd rather not add the extra check to 
ff_stream_add_bitstream_filter because it will be removed later anyway.

Also when I tried to add that, there was a fate failure caused by the 
segment muxer setting the -autobsf flag but calling the muxer's 
check_bitstream function directly. Or should I add create a patch which 
simply removes the -autobsf additional options from segment.c?

Regards,
Marton