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

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-November/220993.html