[FFmpeg-devel] [PATCH] fifo: add av_fifo_grow()

[Alexander Strasser]

Michael Niedermayer wrote:
> On Mon, May 14, 2012 at 11:28:11PM +0200, Alexander Strasser wrote:
> > Hi Michael,
> > 
> > Michael Niedermayer wrote:
> > [...]
> > > diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> > [...]
> > > +int av_fifo_grow(AVFifoBuffer *f, unsigned int size)
> > > +{
> > > +    unsigned int old_size = f->end - f->buffer;
> > > +    if(size + (unsigned)av_fifo_size(f) < size)
> > > +        return AVERROR(EINVAL);
> > > +
> > > +    size += av_fifo_size(f);
> > > +
> > > +    if (old_size < size)
> > > +        return av_fifo_realloc2(f, FFMAX(size, 2*size));
> > > +    return 0;
> > > +}
> > 
> >   I am no means an expert concerning the AVFifoBuffer implementation,
> > but it looks like some of this is already done in av_fifo_realloc2
> > and does not need to be duplicated in here.
> > 
> >   Where am I missing the point? :(
> 
> for(i=0; i<N; i++)
>     av_fifo_realloc2(i) is slow
> 
> for(i=0; i<N; i++)
>     av_fifo_grow(i) is fast 
> 
> why not fix av_fifo_realloc2()?
> it could break some code using it that has specific expectations on
> the resulting fifo size

   You are right. I actually meant something else but just discovered
that I misread the code.

> > >  // src must NOT be const as it can be a context for func that may need updating (like a pointer or byte counter)
> > >  int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size, int (*func)(void*, void*, int))
> > >  {
> > > diff --git a/libavutil/fifo.h b/libavutil/fifo.h
> > > index 22a9aa5..eabeec3 100644
> > > --- a/libavutil/fifo.h
> > > +++ b/libavutil/fifo.h
> > > @@ -103,6 +103,18 @@ int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size, int (*func)(void
> > >  int av_fifo_realloc2(AVFifoBuffer *f, unsigned int size);
> > >  
> > >  /**
> > > + * Enlarge an AVFifoBuffer.
> > > + * In case of reallocation failure, the old FIFO is kept unchanged.
> > > + * The new fifo size may be larger than the requested size.
> > > + *
> > > + * @param f AVFifoBuffer to resize
> > > + * @param additional_space the additional space in addition to av_fifo_size()
> > > + *        which the user wants.
> > 
> >   This could profit from a less awkward wording more in line with the
> > other AVFifoBuffer docs. What do think of
> > "the amount of space in bytes to allocate in addition to av_fifo_size()"
> > ?
> 
> locally fixed

  Thanks.

  Alexander