[FFmpeg-devel] [libav-devel] [PATCH] fifo: add av_fifo_peek2()

[Stefano Sabatini]

On date Thursday 2011-06-30 10:59:12 +0200, Diego Biurrun encoded:
> On Thu, Jun 30, 2011 at 10:11:04AM +0200, Stefano Sabatini wrote:
> > The new functions provides a more generic interface than
> 
> s/functions/function/
> 
> > av_fifo_peek2() for peeking data out of a fifo buffer.
> 
> s/fifo/FIFO/
> 
> > --- a/libavutil/fifo.h
> > +++ b/libavutil/fifo.h
> > @@ -113,4 +113,17 @@ static inline uint8_t av_fifo_peek(AVFifoBuffer *f, int offs)
> > +
> > +/**
> > + * Return a pointer to the data offset by offs stored in the fifo,
> > + * without modifying the fifo itself.
> > + */
> 
> FIFO

Fixed.
 
> Neither the parameters, nor the return value have (Doxygen) documentation.

The return value is documented in the brief.

> But there is interesting stuff that you could document there, namely
> that a NULL pointer will make this function crash.

Yes fixed.

> 
> > +static inline uint8_t *av_fifo_peek2(const AVFifoBuffer *f, int offs)
> 
> const AVFifoBuffer * const f, const int offs would be even more
> const-correct.

Don't know, offs should not necessarily be const as it is passed per
value, * const f looks a bit overkill, unless you have reasons for
keeping that (e.g. avoiding potential warnings).

> > +    uint8_t *ptr = f->rptr + offs;
> > +    if (ptr >= f->end)
> > +        ptr -= f->end - f->buffer;
> > +    return ptr;
> 
> To me this looks like you can still end up outside the FIFO if the
> offset is larger than two times the FIFO size.

Indeed, fixed the logic (which was copypasted from av_fifo_peek()).
-- 
Neglect of duty does not cease, by repetition, to be neglect of duty.
		-- Napoleon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-fifo-add-av_fifo_peek2.patch
Type: text/x-diff
Size: 1566 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110630/83be139e/attachment.bin>