[FFmpeg-devel] [HACK] Remove MAX_STREAMS usages

[Aurelien Jacobs]

On Fri, Sep 03, 2010 at 10:31:40AM +0200, Michael Niedermayer wrote:
> On Thu, Aug 12, 2010 at 10:13:20PM +0200, Aurelien Jacobs wrote:
> >  utils.c |   27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > ef7b009d45960c34528c5657bd47da27ea317151  max_stream.diff
> > commit 6056ed1b62ad76e16234ef5ef1a4a7c78d68f148
> > Author: Aurelien Jacobs <aurel at gnuage.org>
> > Date:   Thu Aug 12 18:18:26 2010 +0200
> > 
> >     dynamically use nb_streams instead of static use of MAX_STREAMS
> > 
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index faad812..4bde019 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -1871,10 +1871,13 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
> >      AVPacket pkt1, *pkt = &pkt1;
> >      AVStream *st;
> >      int read_size, i, ret;
> > -    int64_t end_time, start_time[MAX_STREAMS];
> > +    int64_t end_time, *start_time;
> >      int64_t filesize, offset, duration;
> >      int retry=0;
> >  
> > +    if (!(start_time = av_malloc(ic->nb_streams * sizeof(*start_time))))
> > +        return;
> > +
> >      ic->cur_st = NULL;
> >  
> >      /* flush packet queue */
> > @@ -1935,6 +1938,7 @@ static void av_estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset
> >      }while(   end_time==AV_NOPTS_VALUE
> >             && filesize > (DURATION_MAX_READ_SIZE<<retry)
> >             && ++retry <= DURATION_MAX_RETRY);
> > +    av_free(start_time);
> >  
> >      fill_all_stream_timings(ic);
> 
> Before this chnage AVFMTCTX_NOHEADER dox should be clarified to say that
> streams may only be added from the calling thread and not a background
> thread otherwise this is a race and heap overflow.

OK. First patch document this.

> i guess none of our demuxers adds streams from background threads, but
> this too needs to be checked (mostly audio/video grabing demuxers seem
> like they could by odd design do this in principle)

I've checked and havn't found any.

And for extra safety, I've made a local copy of ic->nb_streams at the
begining of the function and used this local copy to make sure it can't
increase during the function execution.

> > @@ -2161,13 +2165,18 @@ int av_find_stream_info(AVFormatContext *ic)
> >      AVStream *st;
> >      AVPacket pkt1, *pkt;
> >      int64_t old_offset = url_ftell(ic->pb);
> > +    int nb_streams = ic->nb_streams;
> >      struct {
> >          int64_t last_dts;
> >          int64_t duration_gcd;
> >          int duration_count;
> >          double duration_error[MAX_STD_TIMEBASES];
> >          int64_t codec_info_duration;
> > -    } info[MAX_STREAMS] = {{0}};
> > +    } *info, *tmp_info;
> > +
> > +    info = av_mallocz(ic->nb_streams * sizeof(*info));
> > +    if (!info)
> > +        return AVERROR(ENOMEM);
> >  
> >      for(i=0;i<ic->nb_streams;i++) {
> >          st = ic->streams[i];
> > @@ -2198,7 +2207,7 @@ int av_find_stream_info(AVFormatContext *ic)
> >          }
> >      }
> >  
> > -    for(i=0;i<MAX_STREAMS;i++){
> > +    for(i=0;i<ic->nb_streams;i++){
> >          info[i].last_dts= AV_NOPTS_VALUE;
> >      }
> >  
> > @@ -2264,8 +2273,19 @@ int av_find_stream_info(AVFormatContext *ic)
> >              break;
> >          }
> >  
> > +        if (ic->nb_streams > nb_streams) {
> > +            if (!(tmp_info = av_realloc(info, ic->nb_streams*sizeof(*info)))) {
> 
> missing checks for integer overflows and possibly not just here
> that is unless the stream adding code limits to a small enough number AND
> this limiting code is well documented to why it is needed and cant just be
> removed

Added interger overflow check before each alloc.

> also the path over EAGAIN will at least crash if it adds more streams

Now I made sure that we do the realloc before the 'continue'.

Is this new version OK ?

Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: noheader_doc.diff
Type: text/x-diff
Size: 643 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100930/b0307351/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: max_stream.diff
Type: text/x-diff
Size: 4128 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100930/b0307351/attachment-0001.diff>