[FFmpeg-devel] [PATCH] avformat/flvdec: Ignore the first two data/subtitle streams.

[Martin Storsjö]

On Thu, 13 May 2021, Josh Allmann wrote:

> Previously, one or the other would have been ignored, but not both.
> Since the probe terminates at three streams, it could exit
> prematurely if both data and subtitles are present along with
> slightly trailing media, usually video trailing audio.
>
> Trailing media is common in RTMP, and encoders write strange metadata.
> ---
> libavformat/flvdec.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index 4b9f46902b..1be8d98618 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -134,18 +134,32 @@ static void add_keyframes_index(AVFormatContext *s)
>     }
> }
>
> +static int is_ignorable(enum AVMediaType codec_type)
> +{
> +    switch(codec_type) {
> +    case AVMEDIA_TYPE_SUBTITLE:
> +    case AVMEDIA_TYPE_DATA:
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> static AVStream *create_stream(AVFormatContext *s, int codec_type)
> {
> +    int streams_to_ignore = 0, nb_streams = 0;
>     FLVContext *flv   = s->priv_data;
>     AVStream *st = avformat_new_stream(s, NULL);
>     if (!st)
>         return NULL;
>     st->codecpar->codec_type = codec_type;
> -    if (s->nb_streams>=3 ||(   s->nb_streams==2
> -                           && s->streams[0]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE
> -                           && s->streams[1]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE
> -                           && s->streams[0]->codecpar->codec_type != AVMEDIA_TYPE_DATA
> -                           && s->streams[1]->codecpar->codec_type != AVMEDIA_TYPE_DATA))
> +
> +    if (s->nb_streams >= 1)
> +        streams_to_ignore += is_ignorable(s->streams[0]->codecpar->codec_type);
> +    if (s->nb_streams >= 2)
> +        streams_to_ignore += is_ignorable(s->streams[1]->codecpar->codec_type);
> +
> +    nb_streams = s->nb_streams - streams_to_ignore;
> +    if (nb_streams >= 2)
>         s->ctx_flags &= ~AVFMTCTX_NOHEADER;

Overall, the idea of the patch seems sensible, but it could be done 
slightly differently...

Instead of explicitly inspecting streams [0] and [1], wouldn't it be more 
straightforward to just loop over all existing streams and count the ones 
where is_ignorable returns 0? Otherwise, if you'd have e.g. the streams 
{audio, data, subtitle}, you'd still end up with nb_streams = 2 which 
probably isn't what you meant?

Separately, it'd be great if you could, while touching this code, write 
some comments about what's happening here - it's rather non-obvious code 
(especially with the double negations, in clearing the NOHEADER flag, and 
what that flag implies).

// Martin