diff mbox

[FFmpeg-devel] avformat: Add max_streams option

Message ID 20161118160315.3441-1-michael@niedermayer.cc
State Accepted
Headers show

Commit Message

Michael Niedermayer Nov. 18, 2016, 4:03 p.m. UTC
This allows user apps to stop OOM due to excessive number of streams
TODO: bump & docs

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/avformat.h      | 7 +++++++
 libavformat/options_table.h | 1 +
 libavformat/utils.c         | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

Comments

wm4 Nov. 18, 2016, 4:17 p.m. UTC | #1
On Fri, 18 Nov 2016 17:03:15 +0100
Michael Niedermayer <michael@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@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.
James Almer Nov. 18, 2016, 4:39 p.m. UTC | #2
On 11/18/2016 1:17 PM, wm4 wrote:
> On Fri, 18 Nov 2016 17:03:15 +0100
> Michael Niedermayer <michael@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@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.

It says it's set through AVOptions.

In any case, we dropped ABI compatibility with libav, so new fields can
have a fixed offset without worrying about a new field popping up above
it on a merge.
Not sure if it can be done right now before a major bump. If it can then
the "NO direct access" warning can be removed.

> 
>> +     */
>> +    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.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Michael Niedermayer Nov. 18, 2016, 5:18 p.m. UTC | #3
On Fri, Nov 18, 2016 at 05:17:10PM +0100, wm4 wrote:
> On Fri, 18 Nov 2016 17:03:15 +0100
> Michael Niedermayer <michael@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@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.

If the previous field cannot be accessed directly due to it being
possibly (re)moved then all following fields must have the same note.

If this note is removed it has to be removed from all the fields in
the struct in git first, i cannot just ommit it in a newly added field


> 
> > +     */
> > +    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.

I agree
People report OOM bugs due this and other things though. (this patch
was the reult of one such report today)
If we can cut the number of such reports down by 50% by limiting
things which are easy to limit during fuzz testing that should make
fuzzing more efficient

[...]
Alexander Strasser Nov. 18, 2016, 5:22 p.m. UTC | #4
Hi Michael!

On 2016-11-18 17:03 +0100, Michael Niedermayer wrote:
> This allows user apps to stop OOM due to excessive number of streams
> TODO: bump & docs
> 
> Signed-off-by: Michael Niedermayer <michael@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)
> +     */
> +    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)

  Maybe I am completely wrong, if so just tell me and sorry for the noise...

  You name the option max_streams, but you compare it to nb_streams after
you divided it by size of a pointer.

  I guess you change it that way so the default stays the same. I hope that
can be achieved in a different way.


  Alexander
Michael Niedermayer Nov. 18, 2016, 5:35 p.m. UTC | #5
On Fri, Nov 18, 2016 at 06:22:06PM +0100, Alexander Strasser wrote:
> Hi Michael!
> 
> On 2016-11-18 17:03 +0100, Michael Niedermayer wrote:
> > This allows user apps to stop OOM due to excessive number of streams
> > TODO: bump & docs
> > 
> > Signed-off-by: Michael Niedermayer <michael@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)
> > +     */
> > +    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)
> 
>   Maybe I am completely wrong, if so just tell me and sorry for the noise...
> 
>   You name the option max_streams, but you compare it to nb_streams after
> you divided it by size of a pointer.
> 
>   I guess you change it that way so the default stays the same. I hope that
> can be achieved in a different way.

changed to:
@@ -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 >= FFMIN(s->max_streams, INT_MAX/sizeof(*streams)))
         return NULL;
     streams = av_realloc_array(s->streams, s->nb_streams + 1, sizeof(*streams));
     if (!streams)

[...]
Andreas Cadhalpun Nov. 18, 2016, 9:51 p.m. UTC | #6
On 18.11.2016 17:03, Michael Niedermayer wrote:
> --- 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},
>  };

Wouldn't it make more sense to default to a saner value like 100 or 1000 or so?
Or are there many use cases for millions of streams per file?

Best regards,
Andreas
Michael Niedermayer Nov. 19, 2016, 12:36 a.m. UTC | #7
On Fri, Nov 18, 2016 at 10:51:33PM +0100, Andreas Cadhalpun wrote:
> On 18.11.2016 17:03, Michael Niedermayer wrote:
> > --- 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},
> >  };
> 
> Wouldn't it make more sense to default to a saner value like 100 or 1000 or so?

was thinking that too but that belongs in a seperate patch


[...]
Michael Niedermayer Dec. 8, 2016, 5:26 p.m. UTC | #8
On Fri, Nov 18, 2016 at 06:35:01PM +0100, Michael Niedermayer wrote:
> On Fri, Nov 18, 2016 at 06:22:06PM +0100, Alexander Strasser wrote:
> > Hi Michael!
> > 
> > On 2016-11-18 17:03 +0100, Michael Niedermayer wrote:
> > > This allows user apps to stop OOM due to excessive number of streams
> > > TODO: bump & docs
> > > 
> > > Signed-off-by: Michael Niedermayer <michael@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)
> > > +     */
> > > +    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)
> > 
> >   Maybe I am completely wrong, if so just tell me and sorry for the noise...
> > 
> >   You name the option max_streams, but you compare it to nb_streams after
> > you divided it by size of a pointer.
> > 
> >   I guess you change it that way so the default stays the same. I hope that
> > can be achieved in a different way.
> 
> changed to:
> @@ -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 >= FFMIN(s->max_streams, INT_MAX/sizeof(*streams)))
>          return NULL;

and applied

[...]
diff mbox

Patch

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)
+     */
+    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)