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

[James Almer]

On 11/18/2016 1:17 PM, wm4 wrote:
> 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.

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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>