diff mbox series

[FFmpeg-devel,08/35] lavu/fifo: add new FIFO read/peek functions

Message ID 20220111204610.14262-8-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/35] lavu/fifo: disallow overly large fifo sizes | 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

Anton Khirnov Jan. 11, 2022, 8:45 p.m. UTC
As for writing, use separate functions for reading to a buffer and a
callback. Allow the callbacks to limit the amount of data read,
similarly to what is done for writing.

Consistently use size_t for sizes.
---
 doc/APIchanges   |  3 ++-
 libavutil/fifo.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
 libavutil/fifo.h | 60 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Jan. 13, 2022, 5:41 p.m. UTC | #1
Anton Khirnov:
> As for writing, use separate functions for reading to a buffer and a
> callback. Allow the callbacks to limit the amount of data read,
> similarly to what is done for writing.
> 
> Consistently use size_t for sizes.
> ---
>  doc/APIchanges   |  3 ++-
>  libavutil/fifo.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/fifo.h | 60 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 130 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 0b179c30e5..f64759d69d 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -19,7 +19,8 @@ API changes, most recent first:
>    Operations on FIFOs created with this function on these elements
>    rather than bytes.
>    Add 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_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().
>  
>  2022-01-xx - xxxxxxxxxx - lavu fifo.h
>    Access to all AVFifoBuffer members is deprecated. The struct will
> diff --git a/libavutil/fifo.c b/libavutil/fifo.c
> index 1d94fff457..ea944bc936 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -265,6 +265,74 @@ int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb,
>      return fifo_write_common(f, NULL, nb_elems, read_cb, opaque);
>  }
>  
> +static int fifo_peek_common(const AVFifoBuffer *f, uint8_t *buf, size_t *nb_elems,
> +                            size_t offset, AVFifoCB write_cb, void *opaque)
> +{
> +    const FifoBuffer *fb = (FifoBuffer*)f;
> +    size_t       to_read = *nb_elems;
> +    size_t      offset_r = fb->offset_r;
> +    int              ret = 0;
> +
> +    if (offset > av_fifo_can_read(f) ||
> +        to_read > av_fifo_can_read(f) - offset) {

You are calling av_fifo_can_read() multiple times.

> +        *nb_elems = 0;
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (offset_r >= fb->nb_elems - offset)
> +        offset_r -= fb->nb_elems - offset;
> +    else
> +        offset_r += offset;
> +
> +    do {

Shouldn't this be a while loop?

> +        size_t    len = FFMIN(fb->nb_elems - offset_r, to_read);
> +        uint8_t *rptr = f->buffer + offset_r * fb->elem_size;
> +
> +        if (write_cb) {
> +            ret = write_cb(opaque, rptr, &len);
> +            if (ret < 0 || len == 0)
> +                break;
> +        } else {
> +            memcpy(buf, rptr, len * fb->elem_size);
> +            buf += len * fb->elem_size;
> +        }
> +        offset_r += len;
> +        if (offset_r >= fb->nb_elems)
> +            offset_r = 0;
> +        to_read -= len;
> +    } while (to_read > 0);
> +
> +    *nb_elems -= to_read;
> +
> +    return ret;
> +}
> +
> +int av_fifo_read(AVFifoBuffer *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);

In contrast to the current av_fifo_generic_read() the callback will
still see the non-drained FIFO if the callback is called multiple times.
I don't know whether this slight behaviour change can cause problems
when updating.

> +    return ret;
> +}
> +
> +int av_fifo_read_to_cb(AVFifoBuffer *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(AVFifoBuffer *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(AVFifoBuffer *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);
> +}
> +
>  int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size)
>  {
>      FifoBuffer *fb = (FifoBuffer*)f;
> diff --git a/libavutil/fifo.h b/libavutil/fifo.h
> index ac1245ff39..c7be5e8f7d 100644
> --- a/libavutil/fifo.h
> +++ b/libavutil/fifo.h
> @@ -189,6 +189,66 @@ int av_fifo_write(AVFifoBuffer *f, const void *buf, size_t nb_elems);
>  int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb,
>                            void *opaque, size_t *nb_elems);
>  
> +/**
> + * Read data from a FIFO.
> + *
> + * @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
> + *
> + * @return a non-negative number on success, a negative error code on failure
> + */
> +int av_fifo_read(AVFifoBuffer *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(AVFifoBuffer *f, AVFifoCB write_cb,
> +                       void *opaque, size_t *nb_elems);
> +
> +/**
> + * Read data from a FIFO without modifying FIFO state.
> + *
> + * @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; 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(AVFifoBuffer *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(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque,

Ok, seems like we differ on our naming: I'd call this a read_cb, because
it reads from the fifo, but it of course also writes, so it seems fine
given that you are consistent.

> +                       size_t *nb_elems, size_t offset);
> +
>  /**
>   * Discard the specified amount of data from an AVFifoBuffer.
>   * @param size number of elements to discard, MUST NOT be larger than
>
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 0b179c30e5..f64759d69d 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -19,7 +19,8 @@  API changes, most recent first:
   Operations on FIFOs created with this function on these elements
   rather than bytes.
   Add 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_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().
 
 2022-01-xx - xxxxxxxxxx - lavu fifo.h
   Access to all AVFifoBuffer members is deprecated. The struct will
diff --git a/libavutil/fifo.c b/libavutil/fifo.c
index 1d94fff457..ea944bc936 100644
--- a/libavutil/fifo.c
+++ b/libavutil/fifo.c
@@ -265,6 +265,74 @@  int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb,
     return fifo_write_common(f, NULL, nb_elems, read_cb, opaque);
 }
 
+static int fifo_peek_common(const AVFifoBuffer *f, uint8_t *buf, size_t *nb_elems,
+                            size_t offset, AVFifoCB write_cb, void *opaque)
+{
+    const FifoBuffer *fb = (FifoBuffer*)f;
+    size_t       to_read = *nb_elems;
+    size_t      offset_r = fb->offset_r;
+    int              ret = 0;
+
+    if (offset > av_fifo_can_read(f) ||
+        to_read > av_fifo_can_read(f) - offset) {
+        *nb_elems = 0;
+        return AVERROR(EINVAL);
+    }
+
+    if (offset_r >= fb->nb_elems - offset)
+        offset_r -= fb->nb_elems - offset;
+    else
+        offset_r += offset;
+
+    do {
+        size_t    len = FFMIN(fb->nb_elems - offset_r, to_read);
+        uint8_t *rptr = f->buffer + offset_r * fb->elem_size;
+
+        if (write_cb) {
+            ret = write_cb(opaque, rptr, &len);
+            if (ret < 0 || len == 0)
+                break;
+        } else {
+            memcpy(buf, rptr, len * fb->elem_size);
+            buf += len * fb->elem_size;
+        }
+        offset_r += len;
+        if (offset_r >= fb->nb_elems)
+            offset_r = 0;
+        to_read -= len;
+    } while (to_read > 0);
+
+    *nb_elems -= to_read;
+
+    return ret;
+}
+
+int av_fifo_read(AVFifoBuffer *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(AVFifoBuffer *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(AVFifoBuffer *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(AVFifoBuffer *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);
+}
+
 int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size)
 {
     FifoBuffer *fb = (FifoBuffer*)f;
diff --git a/libavutil/fifo.h b/libavutil/fifo.h
index ac1245ff39..c7be5e8f7d 100644
--- a/libavutil/fifo.h
+++ b/libavutil/fifo.h
@@ -189,6 +189,66 @@  int av_fifo_write(AVFifoBuffer *f, const void *buf, size_t nb_elems);
 int av_fifo_write_from_cb(AVFifoBuffer *f, AVFifoCB read_cb,
                           void *opaque, size_t *nb_elems);
 
+/**
+ * Read data from a FIFO.
+ *
+ * @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
+ *
+ * @return a non-negative number on success, a negative error code on failure
+ */
+int av_fifo_read(AVFifoBuffer *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(AVFifoBuffer *f, AVFifoCB write_cb,
+                       void *opaque, size_t *nb_elems);
+
+/**
+ * Read data from a FIFO without modifying FIFO state.
+ *
+ * @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; 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(AVFifoBuffer *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(AVFifoBuffer *f, AVFifoCB write_cb, void *opaque,
+                       size_t *nb_elems, size_t offset);
+
 /**
  * Discard the specified amount of data from an AVFifoBuffer.
  * @param size number of elements to discard, MUST NOT be larger than