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

[Stefano Sabatini]

On date Monday 2011-07-04 16:42:04 +0200, Diego Biurrun encoded:
> On Mon, Jul 04, 2011 at 04:13:31PM +0200, Stefano Sabatini wrote:
> > 
> > Updated with changes:
> > 
> > * extended documentation, clarify the meaning of offs and of the returned
> >   pointer
> > * fix a bug (tested this time)
> > * keep unchanged the av_fifo_peek() code for avoiding performance
> >   regressions
> > 
> > >From 0e9763f0dc6f0ca5de4de3f055a37c566c755cb4 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Wed, 29 Jun 2011 17:30:23 +0200
> > Subject: [PATCH] fifo: add av_fifo_peek2()
> > 
> > The new function provides a more generic interface than the one of by
> > av_fifo_peek() for peeking at a FIFO buffer data.
> 
> s/than the one by//

Fixed (at least I hope).

> > --- a/libavutil/fifo.h
> > +++ b/libavutil/fifo.h
> > @@ -113,4 +113,25 @@ static inline uint8_t av_fifo_peek(AVFifoBuffer *f, int offs)
> > +
> > +/**
> > + * Return a pointer to the data offset by offs stored in a FIFO
> > + * buffer. The FIFO buffer is not modified.
> 
> Return a pointer to the data stored in a FIFO buffer at a certain offset.
> The FIFO buffer is not modified.

Changed.
 
> > + * The FIFO buffer is treated like a circular buffer, so the returned
> > + * value always points to the allocated buffer area.
> > + *
> > + * @param pointer to the the FIFO buffer to peek at, must be non-NULL
> 
> f
> 
> This should have generated a new doxygen warning.

Fixed.
 
> > + * @param offs an offset in bytes
> > + */
> > +static inline uint8_t *av_fifo_peek2(const AVFifoBuffer *f, int offs)
> > +{
> > +    uint8_t *ptr = f->rptr + offs;
> > +    if (ptr >= f->end)
> > +        ptr = f->buffer + (ptr - f->end) % (f->end - f->buffer);
> > +    else if (ptr < f->buffer)
> > +        ptr = f->buffer + (f->end - ptr) % (f->end - f->buffer);
> > +    return ptr;
> 

> alternative:
> 
>   uint8_t *ptr = f->rptr + offs;
>   if (ptr >= f->end)
>       ptr = ptr - f->end;

uhm this requires a cast (ptr - ptr => int)

>   else if (ptr < f->buffer)
>       ptr = f->end - ptr;
>   return f->buffer + ptr % (f->end - f->buffer);

I kept the old version here for avoiding ugly casts and avoid
obfuscating temporaries.

> 
> > --- a/libavutil/Makefile
> > +++ b/libavutil/Makefile
> > @@ -77,7 +77,7 @@ OBJS-$(ARCH_ARM) += arm/cpu.o
> >  
> > -TESTPROGS = adler32 aes base64 cpu crc des eval lls md5 pca sha tree
> > +TESTPROGS = adler32 aes base64 cpu crc des eval fifo lls md5 pca sha tree
> >  TESTPROGS-$(HAVE_LZO1X_999_COMPRESS) += lzo
> 
> Nowadays making FATE tests out of these is all the rage.

Uh.. fate test added.

> > --- a/libavutil/fifo.c
> > +++ b/libavutil/fifo.c
> > @@ -127,3 +127,35 @@ void av_fifo_drain(AVFifoBuffer *f, int size)
> > +
> > +#ifdef TEST
> > +
> > +#undef printf
> > +
> > +int main(void)
> > +{
> > +    /* create a fifo buffer */
> 
> FIFO
> 
> > +    /* peek at fifo */
> 
> FIFO
> 
> > +    for (i = -50; i < 50; i++) {
> > +        int *v = (int *)av_fifo_peek2(fifo, i*sizeof(int *));
> > +        printf("%d: %d\n", i, *v);
> 

> This can be done without an ugly cast.

Uhm, this fixes a warning here.
-- 
Sir, it's very possible this asteroid is not stable.
		-- C3P0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fifo-add-av_fifo_peek2.patch
Type: text/x-diff
Size: 1402 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110705/0278f47c/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-fifo-add-fifo-API-test-program-and-fate-test.patch
Type: text/x-diff
Size: 3387 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110705/0278f47c/attachment-0001.bin>