[FFmpeg-devel] [PATCH] avformat: Add max_streams option

[wm4]

On Fri, 18 Nov 2016 17:03:15 +0100
Michael Niedermayer <michael at niedermayer.cc> wrote:

> This allows user apps to stop OOM due to excessive number of streams
> TODO: bump & docs
> 
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavformat/avformat.h      | 7 +++++++
>  libavformat/options_table.h | 1 +
>  libavformat/utils.c         | 2 +-
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index f9f4d72..c81a916 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1899,6 +1899,13 @@ typedef struct AVFormatContext {
>       * - decoding: set by user through AVOptions (NO direct access)
>       */
>      char *protocol_blacklist;
> +
> +    /**
> +     * The maximum number of streams.
> +     * - encoding: unused
> +     * - decoding: set by user through AVOptions (NO direct access)

Can we stop this. It makes no sense at all to document something like
it's public API, and then not allowing access to it. Besides, "NO
direct access" doesn't even make clear _how_ to access it.

> +     */
> +    int max_streams;
>  } AVFormatContext;
>  
>  int av_format_get_probe_score(const AVFormatContext *s);
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index 9d61d5a..d5448e5 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -105,6 +105,7 @@ static const AVOption avformat_options[] = {
>  {"format_whitelist", "List of demuxers that are allowed to be used", OFFSET(format_whitelist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
>  {"protocol_whitelist", "List of protocols that are allowed to be used", OFFSET(protocol_whitelist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
>  {"protocol_blacklist", "List of protocols that are not allowed to be used", OFFSET(protocol_blacklist), AV_OPT_TYPE_STRING, { .str = NULL },  CHAR_MIN, CHAR_MAX, D },
> +{"max_streams", "maximum number of streams", OFFSET(max_streams), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 0, INT_MAX, D },
>  {NULL},
>  };
>  
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 5664646..ded0b52 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4210,7 +4210,7 @@ AVStream *avformat_new_stream(AVFormatContext *s, const AVCodec *c)
>      int i;
>      AVStream **streams;
>  
> -    if (s->nb_streams >= INT_MAX/sizeof(*streams))
> +    if (s->nb_streams >= s->max_streams/sizeof(*streams))
>          return NULL;
>      streams = av_realloc_array(s->streams, s->nb_streams + 1, sizeof(*streams));
>      if (!streams)

I wonder how this option makes sense? There are a lot of other things
that are limited by available memory only. If this is supposed to be
some sort of DoS protection it's not effective.