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

[Marton Balint]

On Sat, 28 Mar 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.
>> 
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavformat/internal.h |   1 +
>>  libavformat/mux.c      | 128 ++++++++++++++++++++++++++++++++++---------------
>>  2 files changed, 91 insertions(+), 38 deletions(-)
>> 
>> diff --git a/libavformat/internal.h b/libavformat/internal.h
>> index 332477a532..45aeef717a 100644
>> --- a/libavformat/internal.h
>> +++ b/libavformat/internal.h
>> @@ -158,6 +158,7 @@ struct AVStreamInternal {
>>       */
>>      AVBSFContext **bsfcs;
>>      int nb_bsfcs;
>> +    int bsfcs_idx;
>>
>>      /**
>>       * Whether or not check_bitstream should still be run on each packet
>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>> index 8c2d6a8060..3054ab8644 100644
>> --- a/libavformat/mux.c
>> +++ b/libavformat/mux.c
>> @@ -840,14 +840,48 @@ static int prepare_input_packet(AVFormatContext *s, AVPacket *pkt)
>>      return 0;
>>  }
>> 
>> -static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
>> -    AVStream *st = s->streams[pkt->stream_index];
>> -    int i, ret;
>> +static int auto_bsf_receive_packet(AVFormatContext *s, AVStream *st, AVPacket *pkt)
>> +{
>> +    AVStreamInternal *sti = st->internal;
>> +    int ret = AVERROR(EAGAIN);
>> +    int eof = 0;
>> +
>> +    while (sti->bsfcs_idx) {
>> +        /* get a packet from the previous filter up the chain */
>> +        ret = av_bsf_receive_packet(sti->bsfcs[sti->bsfcs_idx - 1], pkt);
>> +        if (ret == AVERROR(EAGAIN)) {
>> +            sti->bsfcs_idx--;
>> +            continue;
>> +        } else if (ret == AVERROR_EOF) {
>> +            eof = 1;
>> +        } else if (ret < 0)
>> +            break;
>> +
>> +        /* send it to the next filter down the chain */
>> +        if (sti->bsfcs_idx < sti->nb_bsfcs) {
>> +            ret = av_bsf_send_packet(sti->bsfcs[sti->bsfcs_idx], eof ? NULL : pkt);
>> +            av_assert2(ret != AVERROR(EAGAIN));
>> +            if (ret < 0)
>> +                break;
>> +            sti->bsfcs_idx++;
>> +            eof = 0;
>> +        } else if (eof) {
>> +            break;
>> +        } else {
>> +            return 0;
>> +        }
>> +    }
>
> Would it actually be possible to simplify this by using the
> av_bsf_list-API (right now code like this exists here and in ffmpeg.c
> and probably at even more places)? The problem I see with this is that
> one cannot send a packet for filtering to an unfinished bsf list whereas
> the current code has this capability (although AFAIK no one uses it; if
> I am not mistaken, all bsf chains that are automatically inserted
> consist of exactly one bsf).

I am not aware of practical uses of more than one bsf, but it *was* 
supported for a long time.

Yes, the main issue with the list_bsf-API is that it does not currently 
supports adding filters to already initialized chains. I wrapped up some 
new API for it which can be used to do exactly that:

+/**
+ * Join a new bitstream filter to an already operating one.
+ *
+ * @param[in,out] out_bsf *out_bsf contains the existing, already initialized bitstream
+ *                        filter, the new filter will be joined to the output of this.
+ *                        It can be NULL, in which case an empty filter is assumed.
+ *                        Upon successful return *out_bsf will contain the new, combined
+ *                        bitstream filter which will be initialized.
+ * @param[in]     bsf     the bitstream filter to be joined to the output of *out_bsf.
+ *                        This filter must be uninitialized but it must be ready to be
+ *                        initalized, so input codec parameters and time base must be
+ *                        set if *out_bsf is NULL, otherwise the function will set these
+ *                        automatically based on the output parameters of *out_bsf.
+ *                        Upon successful return the bsf will be initialized.
+ *
+ * @return 0 on success, negative on error.
+ */
+int av_bsf_join(AVBSFContext **out_bsf, AVBSFContext *bsf);
+

So this API basically creates a list BSF transparenly if somebody wants to 
concatenate two BSF-s. It is a bit fraglie because it is using list bfs 
internals directly, instead of calling the init() method of the list bsf, 
but it works.

If people prefer this option, I can continue on this path and simply 
provide patches on top of the existing series, it is quite 
straightforward to do the conversion.

>
> (This actually brings up a question: If check_bitstream returns that
> more packets from this stream need to be checked, the current packet is
> currently sent to the bsf-list as-is; it would not be sent to any bsf
> that get added to the list later. Makes me wonder whether this is
> actually a problem.)

I always thought that is how it is supposed to work by design. But since 
there are no use cases for that at the moment, we don't know if that is 
satisfactory...

Regards,
Marton