[FFmpeg-devel] [HACK] Remove MAX_STREAMS usages

[Aurelien Jacobs]

On Sun, Oct 03, 2010 at 09:09:56PM +0200, Aurelien Jacobs wrote:
> On Thu, Sep 30, 2010 at 12:07:42AM +0200, Aurelien Jacobs wrote:
> > 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 ?
> 
> Ping ? I will apply soon if nobody comments.

Applied.

Aurel