[FFmpeg-devel] [PATCH 03/35] lavu/fifo: introduce the notion of element size
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Thu Jan 13 18:50:46 EET 2022
Anton Khirnov:
> Many AVFifoBuffer users operate on fixed-size elements (e.g. pointers),
> but the current FIFO API deals exclusively in bytes, requiring extra
> complexity in all these callers.
>
> Add a new AVFifoBuffer constructor creating a FIFO with an element size
> that may be larger than a byte. All operations on such a FIFO then
> operate on complete elements.
> ---
> doc/APIchanges | 6 ++
> libavutil/fifo.c | 194 ++++++++++++++++++++++++++++++--------------
> libavutil/fifo.h | 53 +++++++++---
> libavutil/version.h | 2 +-
> 4 files changed, 179 insertions(+), 76 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 21fa02ae9d..5646cf2278 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,12 @@ libavutil: 2021-04-27
>
> API changes, most recent first:
>
> +2022-01-xx - xxxxxxxxxx - lavu 57.19.100 - fifo.h
> + Add av_fifo_alloc2(), which allows setting a FIFO element size.
> + Operations on FIFOs created with this function on these elements
> + rather than bytes.
> + Add av_fifo_elem_size().
> +
> 2022-01-xx - xxxxxxxxxx - lavu fifo.h
> Access to all AVFifoBuffer members is deprecated. The struct will
> become an incomplete type in a future major libavutil version.
> diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> index aaade01333..fc7c93470f 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -38,20 +38,28 @@ typedef struct CTX_STRUCT_NAME {
> // These fields must match then contents of AVFifoBuffer in fifo.h
> // until FF_API_FIFO_PUBLIC is removed
> uint8_t *buffer;
> +#if FF_API_FIFO_PUBLIC
> uint8_t *rptr, *wptr, *end;
> uint32_t rndx, wndx;
> +#endif
> /////////////////////////////////////////
> +
> + size_t elem_size, nb_elems;
> + size_t offset_r, offset_w;
> + // distinguishes the ambigous situation offset_r == offset_w
ambiguous
> + int is_empty;
> } FifoBuffer;
>
> -AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
> +AVFifoBuffer *av_fifo_alloc2(size_t nb_elems, size_t elem_size,
> + unsigned int flags)
> {
> FifoBuffer *f;
> void *buffer;
>
> - if (nmemb > FIFO_SIZE_MAX / size)
> + if (nb_elems > FIFO_SIZE_MAX / elem_size)
> return NULL;
>
> - buffer = av_realloc_array(NULL, nmemb, size);
> + buffer = av_realloc_array(NULL, nb_elems, elem_size);
> if (!buffer)
> return NULL;
> f = av_mallocz(sizeof(*f));
> @@ -60,11 +68,24 @@ AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
> return NULL;
> }
> f->buffer = buffer;
> - f->end = f->buffer + nmemb * size;
> +#if FF_API_FIFO_PUBLIC
> + f->end = f->buffer + nb_elems * elem_size;
> +#endif
> +
> + f->nb_elems = nb_elems;
> + f->elem_size = elem_size;
> +
> av_fifo_reset((AVFifoBuffer*)f);
> return (AVFifoBuffer*)f;
> }
>
> +AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
> +{
> + if (nmemb > SIZE_MAX / size)
> + return NULL;
> + return av_fifo_alloc2(nmemb * size, 1, 0);
> +}
> +
> AVFifoBuffer *av_fifo_alloc(unsigned int size)
> {
> return av_fifo_alloc_array(size, 1);
> @@ -88,67 +109,85 @@ void av_fifo_freep(AVFifoBuffer **f)
>
> void av_fifo_reset(AVFifoBuffer *f)
> {
> + FifoBuffer *fb = (FifoBuffer*)f;
> +
> + fb->offset_r = 0;
> + fb->offset_w = 0;
> + fb->is_empty = 1;
> +#if FF_API_FIFO_PUBLIC
> f->wptr = f->rptr = f->buffer;
> f->wndx = f->rndx = 0;
> +#endif
> +}
> +
> +size_t av_fifo_elem_size(const AVFifoBuffer *f)
> +{
> + const FifoBuffer *fb = (const FifoBuffer*)f;
> + return fb->elem_size;
> }
>
> int av_fifo_size(const AVFifoBuffer *f)
> {
> - return (uint32_t)(f->wndx - f->rndx);
> + const FifoBuffer *fb = (const FifoBuffer*)f;
> + if (fb->offset_w <= fb->offset_r && !fb->is_empty)
> + return fb->nb_elems - fb->offset_r + fb->offset_w;
> + return fb->offset_w - fb->offset_r;
> }
>
> int av_fifo_space(const AVFifoBuffer *f)
> {
> - return f->end - f->buffer - av_fifo_size(f);
> + const FifoBuffer *fb = (const FifoBuffer*)f;
> + return fb->nb_elems - av_fifo_size(f);
> }
>
> int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size)
> {
> - unsigned int old_size = f->end - f->buffer;
> + FifoBuffer *fb = (FifoBuffer*)f;
>
> if (new_size > FIFO_SIZE_MAX)
> return AVERROR(EINVAL);
>
> - if (old_size < new_size) {
> - size_t offset_r = f->rptr - f->buffer;
> - size_t offset_w = f->wptr - f->buffer;
> + if (fb->nb_elems < new_size) {
> uint8_t *tmp;
>
> - tmp = av_realloc(f->buffer, new_size);
> + tmp = av_realloc_array(f->buffer, new_size, fb->elem_size);
> if (!tmp)
> return AVERROR(ENOMEM);
>
> // move the data from the beginning of the ring buffer
> // to the newly allocated space
> - // the second condition distinguishes full vs empty fifo
> - if (offset_w <= offset_r && av_fifo_size(f)) {
> - const size_t copy = FFMIN(new_size - old_size, offset_w);
> - memcpy(tmp + old_size, tmp, copy);
> - if (copy < offset_w) {
> - memmove(tmp, tmp + copy , offset_w - copy);
> - offset_w -= copy;
> + if (fb->offset_w <= fb->offset_r && !fb->is_empty) {
> + const size_t copy = FFMIN(new_size - fb->nb_elems, fb->offset_w);
> + memcpy(tmp + fb->nb_elems * fb->elem_size, tmp, copy * fb->elem_size);
> + if (copy < fb->offset_w) {
> + memmove(tmp, tmp + copy * fb->elem_size,
> + (fb->offset_w - copy) * fb->elem_size);
> + fb->offset_w -= copy;
> } else
> - offset_w = old_size + copy;
> + fb->offset_w = fb->nb_elems + copy;
> }
>
> f->buffer = tmp;
> +#if FF_API_FIFO_PUBLIC
> f->end = f->buffer + new_size;
> - f->rptr = f->buffer + offset_r;
> - f->wptr = f->buffer + offset_w;
> + f->rptr = f->buffer + fb->offset_r * fb->elem_size;
> + f->wptr = f->buffer + fb->offset_w * fb->elem_size;
> +#endif
> + fb->nb_elems = new_size;
> }
> return 0;
> }
>
> 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)
> + FifoBuffer *fb = (FifoBuffer*)f;
> +
> + if (size > UINT_MAX - av_fifo_size(f))
> return AVERROR(EINVAL);
>
> size += av_fifo_size(f);
> -
> - if (old_size < size)
> - return av_fifo_realloc2(f, FFMAX(size, 2*old_size));
> + if (fb->nb_elems < size)
> + return av_fifo_realloc2(f, FFMAX(size, 2 * fb->nb_elems));
> return 0;
> }
>
> @@ -157,62 +196,78 @@ int av_fifo_grow(AVFifoBuffer *f, unsigned int size)
> int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size,
> int (*func)(void *, void *, int))
> {
> + FifoBuffer *fb = (FifoBuffer*)f;
> int total = size;
> + size_t offset_w = fb->offset_w;
> +#if FF_API_FIFO_PUBLIC
> uint32_t wndx= f->wndx;
> - uint8_t *wptr= f->wptr;
> +#endif
>
> if (size > av_fifo_space(f))
> return AVERROR(ENOSPC);
>
> do {
> - int len = FFMIN(f->end - wptr, size);
> + size_t len = FFMIN(fb->nb_elems - offset_w, size);
> + uint8_t *wptr = f->buffer + offset_w * fb->elem_size;
> +
> if (func) {
> - len = func(src, wptr, len);
> - if (len <= 0)
> + int ret = func(src, wptr, len);
> + if (ret <= 0)
> break;
> + len = ret;
> } else {
> - memcpy(wptr, src, len);
> - src = (uint8_t *)src + len;
> + memcpy(wptr, src, len * fb->elem_size);
> + src = (uint8_t *)src + len * fb->elem_size;
> }
> - wptr += len;
> - if (wptr >= f->end)
> - wptr = f->buffer;
> + offset_w += len;
> + if (offset_w >= fb->nb_elems)
> + offset_w = 0;
> +#if FF_API_FIFO_PUBLIC
> wndx += len;
> +#endif
> size -= len;
> } while (size > 0);
> +#if FF_API_FIFO_PUBLIC
> f->wndx= wndx;
> - f->wptr= wptr;
> + f->wptr= f->buffer + offset_w * fb->elem_size;
> +#endif
> + fb->offset_w = offset_w;
> +
> + if (total - size > 0)
size < total
> + fb->is_empty = 0;
> +
> return total - size;
> }
>
> int av_fifo_generic_peek_at(AVFifoBuffer *f, void *dest, int offset, int buf_size, void (*func)(void*, void*, int))
> {
> - uint8_t *rptr = f->rptr;
> + FifoBuffer *fb = (FifoBuffer*)f;
> + size_t offset_r = fb->offset_r;
>
> if (offset < 0 || buf_size > av_fifo_size(f) - offset)
> return AVERROR(EINVAL);
>
> - if (offset >= f->end - rptr)
> - rptr += offset - (f->end - f->buffer);
> + if (offset >= fb->nb_elems - offset_r)
> + offset_r -= fb->nb_elems - offset;
> else
> - rptr += offset;
> + offset_r += offset;
>
> while (buf_size > 0) {
> + uint8_t *rptr = f->buffer + offset_r * fb->elem_size;
> int len;
>
> - if (rptr >= f->end)
> - rptr -= f->end - f->buffer;
> -
> - len = FFMIN(f->end - rptr, buf_size);
> + len = FFMIN(fb->nb_elems - offset_r, buf_size);
> if (func)
> func(dest, rptr, len);
> else {
> - memcpy(dest, rptr, len);
> - dest = (uint8_t *)dest + len;
> + memcpy(dest, rptr, len * fb->elem_size);
> + dest = (uint8_t *)dest + len * fb->elem_size;
> }
>
> buf_size -= len;
> - rptr += len;
> + offset_r += len;
> + if (offset_r >= fb->nb_elems)
> + offset_r -= fb->nb_elems;
> }
>
> return 0;
> @@ -221,22 +276,24 @@ int av_fifo_generic_peek_at(AVFifoBuffer *f, void *dest, int offset, int buf_siz
> int av_fifo_generic_peek(AVFifoBuffer *f, void *dest, int buf_size,
> void (*func)(void *, void *, int))
> {
> - uint8_t *rptr = f->rptr;
> + FifoBuffer *fb = (FifoBuffer*)f;
> + size_t offset_r = fb->offset_r;
>
> if (buf_size > av_fifo_size(f))
> return AVERROR(EINVAL);
>
> do {
> - int len = FFMIN(f->end - rptr, buf_size);
> + uint8_t *rptr = f->buffer + offset_r * fb->elem_size;
> + int len = FFMIN(fb->nb_elems - offset_r, buf_size);
> if (func)
> func(dest, rptr, len);
> else {
> - memcpy(dest, rptr, len);
> - dest = (uint8_t *)dest + len;
> + memcpy(dest, rptr, len * fb->elem_size);
> + dest = (uint8_t *)dest + len * fb->elem_size;
> }
> - rptr += len;
> - if (rptr >= f->end)
> - rptr -= f->end - f->buffer;
> + offset_r += len;
> + if (offset_r >= fb->nb_elems)
> + offset_r -= fb->nb_elems;
> buf_size -= len;
> } while (buf_size > 0);
>
> @@ -246,16 +303,19 @@ int av_fifo_generic_peek(AVFifoBuffer *f, void *dest, int buf_size,
> int av_fifo_generic_read(AVFifoBuffer *f, void *dest, int buf_size,
> void (*func)(void *, void *, int))
> {
> + FifoBuffer *fb = (FifoBuffer*)f;
> +
> if (buf_size > av_fifo_size(f))
> return AVERROR(EINVAL);
>
> do {
> - int len = FFMIN(f->end - f->rptr, buf_size);
> + uint8_t *rptr = f->buffer + fb->offset_r * fb->elem_size;
> + int len = FFMIN(fb->nb_elems - fb->offset_r, buf_size);
> if (func)
> - func(dest, f->rptr, len);
> + func(dest, rptr, len);
> else {
> - memcpy(dest, f->rptr, len);
> - dest = (uint8_t *)dest + len;
> + memcpy(dest, rptr, len * fb->elem_size);
> + dest = (uint8_t *)dest + len * fb->elem_size;
> }
> av_fifo_drain(f, len);
> buf_size -= len;
> @@ -266,9 +326,19 @@ int av_fifo_generic_read(AVFifoBuffer *f, void *dest, int buf_size,
> /** Discard data from the FIFO. */
> void av_fifo_drain(AVFifoBuffer *f, int size)
> {
> - av_assert2(av_fifo_size(f) >= size);
> - f->rptr += size;
> - if (f->rptr >= f->end)
> - f->rptr -= f->end - f->buffer;
> + FifoBuffer *fb = (FifoBuffer*)f;
> + const size_t cur_size = av_fifo_size(f);
> +
> + av_assert2(cur_size >= size);
> + if (cur_size == size)
> + fb->is_empty = 1;
> +
> + if (fb->offset_r >= fb->nb_elems - size)
> + fb->offset_r -= fb->nb_elems - size;
> + else
> + fb->offset_r += size;
> +#if FF_API_FIFO_PUBLIC
> + f->rptr = f->buffer + fb->offset_r * fb->elem_size;
> f->rndx += size;
> +#endif
> }
> diff --git a/libavutil/fifo.h b/libavutil/fifo.h
> index ca4e7fe060..22362c0239 100644
> --- a/libavutil/fifo.h
> +++ b/libavutil/fifo.h
> @@ -48,6 +48,9 @@ AVFifoBuffer;
> * Initialize an AVFifoBuffer.
> * @param size of FIFO
> * @return AVFifoBuffer or NULL in case of memory allocation failure
> + *
> + * @note the element size of the allocated FIFO will be 1, i.e. all operations
> + * will be in bytes
> */
> AVFifoBuffer *av_fifo_alloc(unsigned int size);
>
> @@ -56,9 +59,26 @@ AVFifoBuffer *av_fifo_alloc(unsigned int size);
> * @param nmemb number of elements
> * @param size size of the single element
> * @return AVFifoBuffer or NULL in case of memory allocation failure
> + *
> + * @note the element size of the allocated FIFO will be 1, i.e. all operations
> + * will be in bytes
> */
> AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size);
>
> +/**
> + * Allocate and initialize an AVFifoBuffer with a given element size.
> + *
> + * @param f pointer to the newly-allocated FIFO will be written here on success
> + * @param nb_elems initial number of elements that can be stored in the FIFO
> + * @param elem_size Size in bytes of a single element. Further operations on
> + * the returned FIFO will implicitly use this element size.
> + * @param flags currently unused, must be 0
> + *
> + * @return newly-allocated AVFifoBuffer on success, a negative error code on failure
> + */
> +AVFifoBuffer *av_fifo_alloc2(size_t elems, size_t elem_size,
> + unsigned int flags);
> +
> /**
> * Free an AVFifoBuffer.
> * @param f AVFifoBuffer to free
> @@ -71,6 +91,12 @@ void av_fifo_free(AVFifoBuffer *f);
> */
> void av_fifo_freep(AVFifoBuffer **f);
>
> +/**
> + * @return Element size for FIFO operations. This element size is set at
> + * FIFO allocation and remains constant during its lifetime
> + */
> +size_t av_fifo_elem_size(const AVFifoBuffer *f);
> +
> /**
> * Reset the AVFifoBuffer to the state right after av_fifo_alloc, in particular it is emptied.
> * @param f AVFifoBuffer to reset
> @@ -78,16 +104,16 @@ void av_fifo_freep(AVFifoBuffer **f);
> void av_fifo_reset(AVFifoBuffer *f);
>
> /**
> - * Return the amount of data in bytes in the AVFifoBuffer, that is the
> - * amount of data you can read from it.
> + * Return the amount of data in the AVFifoBuffer, that is the amount of data
> + * (in elements, as returned by av_fifo_elem_size()) you can read from it.
> * @param f AVFifoBuffer to read from
> * @return size
> */
> int av_fifo_size(const AVFifoBuffer *f);
>
> /**
> - * Return the amount of space in bytes in the AVFifoBuffer, that is the
> - * amount of data you can write into it.
> + * Return the amount of space in the AVFifoBuffer, that is the amount of data
> + * (in elements, as returned by av_fifo_elem_size()) you can write into it.
> * @param f AVFifoBuffer to write into
> * @return size
> */
> @@ -97,8 +123,8 @@ int av_fifo_space(const AVFifoBuffer *f);
> * Feed data at specific position from an AVFifoBuffer to a user-supplied callback.
> * Similar as av_fifo_gereric_read but without discarding data.
> * @param f AVFifoBuffer to read from
> - * @param offset offset from current read position
> - * @param buf_size number of bytes to read
> + * @param offset offset in elements from current read position
> + * @param buf_size number of elements to read
> * @param func generic read function
> * @param dest data destination
> *
> @@ -110,7 +136,7 @@ int av_fifo_generic_peek_at(AVFifoBuffer *f, void *dest, int offset, int buf_siz
> * Feed data from an AVFifoBuffer to a user-supplied callback.
> * Similar as av_fifo_gereric_read but without discarding data.
> * @param f AVFifoBuffer to read from
> - * @param buf_size number of bytes to read
> + * @param buf_size number of elements to read
> * @param func generic read function
> * @param dest data destination
> *
> @@ -121,7 +147,7 @@ int av_fifo_generic_peek(AVFifoBuffer *f, void *dest, int buf_size, void (*func)
> /**
> * Feed data from an AVFifoBuffer to a user-supplied callback.
> * @param f AVFifoBuffer to read from
> - * @param buf_size number of bytes to read
> + * @param buf_size number of elements to read
> * @param func generic read function
> * @param dest data destination
> *
> @@ -134,13 +160,13 @@ int av_fifo_generic_read(AVFifoBuffer *f, void *dest, int buf_size, void (*func)
> * @param f AVFifoBuffer to write to
> * @param src data source; non-const since it may be used as a
> * modifiable context by the function defined in func
> - * @param size number of bytes to write
> + * @param size number of elements to write
> * @param func generic write function; the first parameter is src,
> * the second is dest_buf, the third is dest_buf_size.
> * func must return the number of bytes written to dest_buf, or <= 0 to
> * indicate no more data available to write.
> * If func is NULL, src is interpreted as a simple byte array for source data.
> - * @return the number of bytes written to the FIFO or a negative error code on failure
> + * @return the number of elements written to the FIFO or a negative error code on failure
> */
> int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size, int (*func)(void*, void*, int));
>
> @@ -149,7 +175,7 @@ int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size, int (*func)(void
> * In case of reallocation failure, the old FIFO is kept unchanged.
> *
> * @param f AVFifoBuffer to resize
> - * @param size new AVFifoBuffer size in bytes
> + * @param size new AVFifoBuffer size in elements
> * @return <0 for failure, >=0 otherwise
> */
> int av_fifo_realloc2(AVFifoBuffer *f, unsigned int size);
> @@ -160,7 +186,8 @@ int av_fifo_realloc2(AVFifoBuffer *f, unsigned int size);
> * The new fifo size may be larger than the requested size.
> *
> * @param f AVFifoBuffer to resize
> - * @param additional_space the amount of space in bytes to allocate in addition to av_fifo_size()
> + * @param additional_space the amount of space in elements to allocate in
> + * addition to av_fifo_size()
> * @return <0 for failure, >=0 otherwise
> */
> int av_fifo_grow(AVFifoBuffer *f, unsigned int additional_space);
> @@ -168,7 +195,7 @@ int av_fifo_grow(AVFifoBuffer *f, unsigned int additional_space);
> /**
> * Read and discard the specified amount of data from an AVFifoBuffer.
> * @param f AVFifoBuffer to read from
> - * @param size amount of data to read in bytes
> + * @param size amount of data to read in elements
> */
> void av_fifo_drain(AVFifoBuffer *f, int size);
>
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 7c031f547e..180ebd5223 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 57
> -#define LIBAVUTIL_VERSION_MINOR 18
> +#define LIBAVUTIL_VERSION_MINOR 19
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>
You should document that av_fifo_peek2() must not be used with a fifo
returned by av_fifo_alloc2(). (It has not been ported to use elements
instead of bytes and its documentation just claims "The used buffer size
can be checked with av_fifo_size()" which is not wrong but does not
really mention that this now returns the element count.)
More information about the ffmpeg-devel
mailing list