[FFmpeg-devel] [PATCH v2 03/31] lavu/fifo: Add new AVFifo API based upon the notion of element size

[Andreas Rheinhardt]

Andreas Rheinhardt:
> From: Anton Khirnov <anton at khirnov.net>
> 
> 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 AVFifo API 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.
> 
> This API does not reuse AVFifoBuffer and its API at all, but instead uses
> an opaque struct called AVFifo. The AVFifoBuffer API will be deprecated
> in a future commit once all of its users have been switched to the new
> API.
> 
> Not reusing AVFifoBuffer also allowed to use the full range of size_t
> from the beginning.
> ---
>  doc/APIchanges      |   9 ++
>  libavutil/fifo.c    | 224 ++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/fifo.h    | 179 +++++++++++++++++++++++++++++++++++
>  libavutil/version.h |   2 +-
>  4 files changed, 413 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 8df0364e4c..57a9df9bef 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,15 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2022-01-xx - xxxxxxxxxx - lavu 57.19.100 - fifo.h
> +  Add a new FIFO API, which allows setting a FIFO element size.
> +  This API operates on these elements rather than on bytes.
> +  Add av_fifo_alloc2(), av_fifo_elem_size(), av_fifo_can_read(),
> +  av_fifo_can_write(), av_fifo_grow2(), av_fifo_drain2(), av_fifo_write(),
> +  av_fifo_write_from_cb(), av_fifo_read(), av_fifo_read_to_cb(),
> +  av_fifo_peek(), av_fifo_peek_to_cb(), av_fifo_drain2(), av_fifo_reset2(),
> +  av_fifo_freep2().
> +
>  2022-01-04 - 78dc21b123e - lavu 57.16.100 - frame.h
>    Add AV_FRAME_DATA_DOVI_METADATA.
>  
> diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> index 55621f0dca..0e0d84258f 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -26,6 +26,230 @@
>  #include "common.h"
>  #include "fifo.h"
>  
> +struct AVFifo {
> +    uint8_t *buffer;
> +
> +    size_t elem_size, nb_elems;
> +    size_t offset_r, offset_w;
> +    // distinguishes the ambiguous situation offset_r == offset_w
> +    int    is_empty;
> +};
> +
> +AVFifo *av_fifo_alloc2(size_t nb_elems, size_t elem_size,
> +                       unsigned int flags)
> +{
> +    AVFifo *f;
> +    void *buffer;
> +
> +    if (!elem_size)
> +        return NULL;
> +
> +    buffer = av_realloc_array(NULL, nb_elems, elem_size);

After Anton pointed out that it is ill-defined what av_realloc_array()
does in case one requests zero elements (it allocates a single byte,
although it is documented to free the buffer provided to it) this has
been changed to only allocate anything in case a positive number of
elements has been requested:

+    void *buffer = NULL;

+

+    if (!elem_size)

+        return NULL;

+

+    if (nb_elems) {

+        buffer = av_realloc_array(NULL, nb_elems, elem_size);

+        if (!buffer)

+            return NULL;

+    }

+    f = av_mallocz(sizeof(*f));

+    if (!f) {

+        av_free(buffer);

+        return NULL;

+    }


I intend to apply this set with this change and his change requested in
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/292349.html
applied (and with another trivial change in qsvdec necessitated by
8ca06a8148) tomorrow unless there are objections.

> +    if (!buffer)
> +        return NULL;
> +    f = av_mallocz(sizeof(*f));
> +    if (!f) {
> +        av_free(buffer);
> +        return NULL;
> +    }
> +    f->buffer    = buffer;
> +    f->nb_elems  = nb_elems;
> +    f->elem_size = elem_size;
> +    f->is_empty  = 1;
> +
> +    return f;
> +}
> +