On date Thursday 2011-06-30 14:56:37 +0200, Michael Niedermayer encoded: > On Thu, Jun 30, 2011 at 12:41:55PM +0200, Stefano Sabatini wrote: > > 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 > > > fifo.h | 24 +++++++++++++++++++++--- > > 1 file changed, 21 insertions(+), 3 deletions(-) > > 18713c5dbe8b3aa107e58aab73c828e5664cb307 0004-fifo-add-av_fifo_peek2.patch > > From 8388c50d94084ca3a795b8f7e1cc364f47789010 Mon Sep 17 00:00:00 2001 > > From: Stefano Sabatini <[email protected]> > > 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. > > --- > > libavutil/fifo.h | 24 +++++++++++++++++++++--- > > 1 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/libavutil/fifo.h b/libavutil/fifo.h > > index 999d0bf..ed09e4e 100644 > > --- a/libavutil/fifo.h > > +++ b/libavutil/fifo.h > > @@ -106,11 +106,29 @@ int av_fifo_realloc2(AVFifoBuffer *f, unsigned int > > size); > > */ > > void av_fifo_drain(AVFifoBuffer *f, int size); > > > > -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, without modifying the FIFO buffer itself. > > + * > > + * @param pointer to the the FIFO buffer to peek at, must be non-NULL > > + * @param offs an offset in bytes > > + */ > > We also should say something about the validity of the returned ptr +1 > +2 ...
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 I'm also attaching a small test program, which I used for testing here (not necessarily to be committed, just for showing the API usage).
>From 0e9763f0dc6f0ca5de4de3f055a37c566c755cb4 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini <[email protected]> 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. --- libavutil/fifo.h | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/libavutil/fifo.h b/libavutil/fifo.h index 999d0bf..0ef0e83 100644 --- a/libavutil/fifo.h +++ b/libavutil/fifo.h @@ -113,4 +113,25 @@ static inline uint8_t av_fifo_peek(AVFifoBuffer *f, int offs) ptr -= f->end - f->buffer; return *ptr; } + +/** + * Return a pointer to the data offset by offs stored in a FIFO + * buffer. The FIFO buffer is not modified. + * + * 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 + * @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; +} + #endif /* AVUTIL_FIFO_H */ -- 1.7.2.5
>From 074c4bc4b097a05d1e89b56ffa31d1e309f7282a Mon Sep 17 00:00:00 2001 From: Stefano Sabatini <[email protected]> Date: Mon, 4 Jul 2011 15:37:01 +0200 Subject: [PATCH] fifo: add fifo API test program --- libavutil/Makefile | 2 +- libavutil/fifo.c | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletions(-) diff --git a/libavutil/Makefile b/libavutil/Makefile index ad2a3ee..fd60eb2 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -77,7 +77,7 @@ OBJS-$(ARCH_ARM) += arm/cpu.o OBJS-$(ARCH_PPC) += ppc/cpu.o OBJS-$(ARCH_X86) += x86/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 DIRS = arm bfin sh4 x86 diff --git a/libavutil/fifo.c b/libavutil/fifo.c index d101989..a992ab3 100644 --- a/libavutil/fifo.c +++ b/libavutil/fifo.c @@ -127,3 +127,35 @@ void av_fifo_drain(AVFifoBuffer *f, int size) f->rptr -= f->end - f->buffer; f->rndx += size; } + +#ifdef TEST + +#undef printf + +int main(void) +{ + /* create a fifo buffer */ + AVFifoBuffer *fifo = av_fifo_alloc(13 * sizeof(int)); + int i, j; + + /* fill data */ + for (i = 0; av_fifo_space(fifo) >= sizeof(int); i++) + av_fifo_generic_write(fifo, &i, sizeof(int), NULL); + + /* peek at fifo */ + for (i = -50; i < 50; i++) { + int *v = (int *)av_fifo_peek2(fifo, i*sizeof(int *)); + printf("%d: %d\n", i, *v); + } + + /* read data */ + for (i = 0; av_fifo_size(fifo) >= sizeof(int); i++) { + av_fifo_generic_read(fifo, &j, sizeof(int), NULL); + printf("%d ", j); + } + printf("\n"); + + return 0; +} + +#endif -- 1.7.2.5
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
