diff mbox series

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

Message ID AM7PR03MB666084522FC18188307F9F3B8F5E9@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Headers show
Series New FIFO API | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/make_aarch64_jetson success Make finished
andriy/make_fate_aarch64_jetson success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 24, 2022, 2:45 p.m. UTC
From: Anton Khirnov <anton@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(-)

Comments

Andreas Rheinhardt Feb. 5, 2022, 7:55 a.m. UTC | #1
Andreas Rheinhardt:
> From: Anton Khirnov <anton@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;
> +}
> +
diff mbox series

Patch

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);
+    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;
+}
+
+size_t av_fifo_elem_size(const AVFifo *f)
+{
+    return f->elem_size;
+}
+
+size_t av_fifo_can_read(const AVFifo *f)
+{
+    if (f->offset_w <= f->offset_r && !f->is_empty)
+        return f->nb_elems - f->offset_r + f->offset_w;
+    return f->offset_w - f->offset_r;
+}
+
+size_t av_fifo_can_write(const AVFifo *f)
+{
+    return f->nb_elems - av_fifo_can_read(f);
+}
+
+int av_fifo_grow2(AVFifo *f, size_t inc)
+{
+    uint8_t *tmp;
+
+    if (inc > SIZE_MAX - f->nb_elems)
+        return AVERROR(EINVAL);
+
+    tmp = av_realloc_array(f->buffer, f->nb_elems + inc, f->elem_size);
+    if (!tmp)
+        return AVERROR(ENOMEM);
+    f->buffer = tmp;
+
+    // move the data from the beginning of the ring buffer
+    // to the newly allocated space
+    if (f->offset_w <= f->offset_r && !f->is_empty) {
+        const size_t copy = FFMIN(inc, f->offset_w);
+        memcpy(tmp + f->nb_elems * f->elem_size, tmp, copy * f->elem_size);
+        if (copy < f->offset_w) {
+            memmove(tmp, tmp + copy * f->elem_size,
+                    (f->offset_w - copy) * f->elem_size);
+            f->offset_w -= copy;
+        } else
+            f->offset_w = f->nb_elems + copy;
+    }
+
+    f->nb_elems += inc;
+
+    return 0;
+}
+
+static int fifo_write_common(AVFifo *f, const uint8_t *buf, size_t *nb_elems,
+                             AVFifoCB read_cb, void *opaque)
+{
+    size_t to_write = *nb_elems;
+    size_t offset_w = f->offset_w;
+    int         ret = 0;
+
+    if (to_write > av_fifo_can_write(f))
+        return AVERROR(ENOSPC);
+
+    while (to_write > 0) {
+        size_t    len = FFMIN(f->nb_elems - offset_w, to_write);
+        uint8_t *wptr = f->buffer + offset_w * f->elem_size;
+
+        if (read_cb) {
+            ret = read_cb(opaque, wptr, &len);
+            if (ret < 0 || len == 0)
+                break;
+        } else {
+            memcpy(wptr, buf, len * f->elem_size);
+            buf += len * f->elem_size;
+        }
+        offset_w += len;
+        if (offset_w >= f->nb_elems)
+            offset_w = 0;
+        to_write -= len;
+    }
+    f->offset_w = offset_w;
+
+    if (*nb_elems != to_write)
+        f->is_empty = 0;
+    *nb_elems -= to_write;
+
+    return ret;
+}
+
+int av_fifo_write(AVFifo *f, const void *buf, size_t nb_elems)
+{
+    return fifo_write_common(f, buf, &nb_elems, NULL, NULL);
+}
+
+int av_fifo_write_from_cb(AVFifo *f, AVFifoCB read_cb,
+                          void *opaque, size_t *nb_elems)
+{
+    return fifo_write_common(f, NULL, nb_elems, read_cb, opaque);
+}
+
+static int fifo_peek_common(const AVFifo *f, uint8_t *buf, size_t *nb_elems,
+                            size_t offset, AVFifoCB write_cb, void *opaque)
+{
+    size_t  to_read = *nb_elems;
+    size_t offset_r = f->offset_r;
+    size_t can_read = av_fifo_can_read(f);
+    int         ret = 0;
+
+    if (offset > can_read || to_read > can_read - offset) {
+        *nb_elems = 0;
+        return AVERROR(EINVAL);
+    }
+
+    if (offset_r >= f->nb_elems - offset)
+        offset_r -= f->nb_elems - offset;
+    else
+        offset_r += offset;
+
+    while (to_read > 0) {
+        size_t    len = FFMIN(f->nb_elems - offset_r, to_read);
+        uint8_t *rptr = f->buffer + offset_r * f->elem_size;
+
+        if (write_cb) {
+            ret = write_cb(opaque, rptr, &len);
+            if (ret < 0 || len == 0)
+                break;
+        } else {
+            memcpy(buf, rptr, len * f->elem_size);
+            buf += len * f->elem_size;
+        }
+        offset_r += len;
+        if (offset_r >= f->nb_elems)
+            offset_r = 0;
+        to_read -= len;
+    }
+
+    *nb_elems -= to_read;
+
+    return ret;
+}
+
+int av_fifo_read(AVFifo *f, void *buf, size_t nb_elems)
+{
+    int ret = fifo_peek_common(f, buf, &nb_elems, 0, NULL, NULL);
+    av_fifo_drain2(f, nb_elems);
+    return ret;
+}
+
+int av_fifo_read_to_cb(AVFifo *f, AVFifoCB write_cb,
+                       void *opaque, size_t *nb_elems)
+{
+    int ret = fifo_peek_common(f, NULL, nb_elems, 0, write_cb, opaque);
+    av_fifo_drain2(f, *nb_elems);
+    return ret;
+}
+
+int av_fifo_peek(AVFifo *f, void *buf, size_t nb_elems, size_t offset)
+{
+    return fifo_peek_common(f, buf, &nb_elems, offset, NULL, NULL);
+}
+
+int av_fifo_peek_to_cb(AVFifo *f, AVFifoCB write_cb, void *opaque,
+                       size_t *nb_elems, size_t offset)
+{
+    return fifo_peek_common(f, NULL, nb_elems, offset, write_cb, opaque);
+}
+
+void av_fifo_drain2(AVFifo *f, size_t size)
+{
+    const size_t cur_size = av_fifo_can_read(f);
+
+    av_assert0(cur_size >= size);
+    if (cur_size == size)
+        f->is_empty = 1;
+
+    if (f->offset_r >= f->nb_elems - size)
+        f->offset_r -= f->nb_elems - size;
+    else
+        f->offset_r += size;
+}
+
+void av_fifo_reset2(AVFifo *f)
+{
+    f->offset_r = f->offset_w = 0;
+    f->is_empty = 1;
+}
+
+void av_fifo_freep2(AVFifo **f)
+{
+    if (*f) {
+        av_freep(&(*f)->buffer);
+        av_freep(f);
+    }
+}
+
+
 #define OLD_FIFO_SIZE_MAX (size_t)FFMIN3(INT_MAX, UINT32_MAX, SIZE_MAX)
 
 AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size)
diff --git a/libavutil/fifo.h b/libavutil/fifo.h
index f4fd291e59..f455022e3c 100644
--- a/libavutil/fifo.h
+++ b/libavutil/fifo.h
@@ -28,6 +28,185 @@ 
 #include "avutil.h"
 #include "attributes.h"
 
+typedef struct AVFifo AVFifo;
+
+/**
+ * Callback for writing or reading from a FIFO, passed to (and invoked from) the
+ * av_fifo_*_cb() functions. It may be invoked multiple times from a single
+ * av_fifo_*_cb() call and may process less data than the maximum size indicated
+ * by nb_elems.
+ *
+ * @param opaque the opaque pointer provided to the av_fifo_*_cb() function
+ * @param buf the buffer for reading or writing the data, depending on which
+ *            av_fifo_*_cb function is called
+ * @param nb_elems On entry contains the maximum number of elements that can be
+ *                 read from / written into buf. On success, the callback should
+ *                 update it to contain the number of elements actually written.
+ *
+ * @return 0 on success, a negative error code on failure (will be returned from
+ *         the invoking av_fifo_*_cb() function)
+ */
+typedef int AVFifoCB(void *opaque, void *buf, size_t *nb_elems);
+
+/**
+ * Allocate and initialize an AVFifo with a given element size.
+ *
+ * @param 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 AVFifo on success, a negative error code on failure
+ */
+AVFifo *av_fifo_alloc2(size_t elems, size_t elem_size,
+                       unsigned int flags);
+
+/**
+ * @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 AVFifo *f);
+
+/**
+ * @return number of elements available for reading from the given FIFO.
+ */
+size_t av_fifo_can_read(const AVFifo *f);
+
+/**
+ * @return number of elements that can be written into the given FIFO.
+ */
+size_t av_fifo_can_write(const AVFifo *f);
+
+/**
+ * Enlarge an AVFifo.
+ *
+ * On success, the FIFO will be large enough to hold exactly
+ * inc + av_fifo_can_read() + av_fifo_can_write()
+ * elements. In case of failure, the old FIFO is kept unchanged.
+ *
+ * @param f AVFifo to resize
+ * @param inc number of elements to allocate for, in addition to the current
+ *            allocated size
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_grow2(AVFifo *f, size_t inc);
+
+/**
+ * Write data into a FIFO.
+ *
+ * In case nb_elems > av_fifo_can_write(f), nothing is written and an error
+ * is returned.
+ *
+ * @param f the FIFO buffer
+ * @param buf Data to be written. nb_elems * av_fifo_elem_size(f) bytes will be
+ *            read from buf on success.
+ * @param nb_elems number of elements to write into FIFO
+ *
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_write(AVFifo *f, const void *buf, size_t nb_elems);
+
+/**
+ * Write data from a user-provided callback into a FIFO.
+ *
+ * @param f the FIFO buffer
+ * @param read_cb Callback supplying the data to the FIFO. May be called
+ *                multiple times.
+ * @param opaque opaque user data to be provided to read_cb
+ * @param nb_elems Should point to the maximum number of elements that can be
+ *                 written. Will be updated to contain the number of elements
+ *                 actually written.
+ *
+ * @return non-negative number on success, a negative error code on failure
+ */
+int av_fifo_write_from_cb(AVFifo *f, AVFifoCB read_cb,
+                          void *opaque, size_t *nb_elems);
+
+/**
+ * Read data from a FIFO.
+ *
+ * In case nb_elems > av_fifo_can_read(f), nothing is read and an error
+ * is returned.
+ *
+ * @param f the FIFO buffer
+ * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes
+ *            will be written into buf on success.
+ * @param nb_elems number of elements to read from FIFO
+ *
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_read(AVFifo *f, void *buf, size_t nb_elems);
+
+/**
+ * Feed data from a FIFO into a user-provided callback.
+ *
+ * @param f the FIFO buffer
+ * @param write_cb Callback the data will be supplied to. May be called
+ *                 multiple times.
+ * @param opaque opaque user data to be provided to write_cb
+ * @param nb_elems Should point to the maximum number of elements that can be
+ *                 read. Will be updated to contain the total number of elements
+ *                 actually sent to the callback.
+ *
+ * @return non-negative number on success, a negative error code on failure
+ */
+int av_fifo_read_to_cb(AVFifo *f, AVFifoCB write_cb,
+                       void *opaque, size_t *nb_elems);
+
+/**
+ * Read data from a FIFO without modifying FIFO state.
+ *
+ * Returns an error if an attempt is made to peek to nonexistent elements
+ * (i.e. if offset + nb_elems is larger than av_fifo_can_read(f)).
+ *
+ * @param f the FIFO buffer
+ * @param buf Buffer to store the data. nb_elems * av_fifo_elem_size(f) bytes
+ *            will be written into buf.
+ * @param nb_elems number of elements to read from FIFO
+ * @param offset number of initial elements to skip.
+ *
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_peek(AVFifo *f, void *buf, size_t nb_elems, size_t offset);
+
+/**
+ * Feed data from a FIFO into a user-provided callback.
+ *
+ * @param f the FIFO buffer
+ * @param write_cb Callback the data will be supplied to. May be called
+ *                 multiple times.
+ * @param opaque opaque user data to be provided to write_cb
+ * @param nb_elems Should point to the maximum number of elements that can be
+ *                 read. Will be updated to contain the total number of elements
+ *                 actually sent to the callback.
+ * @param offset number of initial elements to skip; offset + *nb_elems must not
+ *               be larger than av_fifo_can_read(f).
+ *
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_peek_to_cb(AVFifo *f, AVFifoCB write_cb, void *opaque,
+                       size_t *nb_elems, size_t offset);
+
+/**
+ * Discard the specified amount of data from an AVFifo.
+ * @param size number of elements to discard, MUST NOT be larger than
+ *             av_fifo_can_read(f)
+ */
+void av_fifo_drain2(AVFifo *f, size_t size);
+
+/*
+ * Empty the AVFifo.
+ * @param f AVFifo to reset
+ */
+void av_fifo_reset2(AVFifo *f);
+
+/**
+ * Free an AVFifo and reset pointer to NULL.
+ * @param f Pointer to an AVFifo to free. *f == NULL is allowed.
+ */
+void av_fifo_freep2(AVFifo **f);
+
+
 typedef struct AVFifoBuffer {
     uint8_t *buffer;
     uint8_t *rptr, *wptr, *end;
diff --git a/libavutil/version.h b/libavutil/version.h
index 953aac9d94..331b8f6ea9 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, \