diff mbox series

[FFmpeg-devel,05/35] lavu/fifo: add a new FIFO grow function

Message ID 20220111204610.14262-5-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel,01/35] lavu/fifo: disallow overly large fifo sizes | expand

Commit Message

Anton Khirnov Jan. 11, 2022, 8:45 p.m. UTC
Consistently use size_t for sizes.

Unlike av_fifo_grow(), which addds to the currently used size, this
function adds to the allocated size.

No new function is provided for a generic realloc, since the current code
only supports increasing the FIFO size.
---
 doc/APIchanges   |  3 ++-
 libavutil/fifo.c | 61 +++++++++++++++++++++++++++---------------------
 libavutil/fifo.h | 14 +++++++++++
 3 files changed, 50 insertions(+), 28 deletions(-)

Comments

James Almer Jan. 13, 2022, 5:04 p.m. UTC | #1
On 1/11/2022 5:45 PM, Anton Khirnov wrote:
> Consistently use size_t for sizes.
> 
> Unlike av_fifo_grow(), which addds to the currently used size, this
> function adds to the allocated size.
> 
> No new function is provided for a generic realloc, since the current code
> only supports increasing the FIFO size.
> ---
>   doc/APIchanges   |  3 ++-
>   libavutil/fifo.c | 61 +++++++++++++++++++++++++++---------------------
>   libavutil/fifo.h | 14 +++++++++++
>   3 files changed, 50 insertions(+), 28 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 9400c5147a..98eae55719 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -18,7 +18,8 @@ API changes, most recent first:
>     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(), av_fifo_can_read(), av_fifo_can_write().
> +  Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(),
> +  av_fifo_grow2().
>   
>   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 8cde2c20e1..28d6d1b2e6 100644
> --- a/libavutil/fifo.c
> +++ b/libavutil/fifo.c
> @@ -150,41 +150,48 @@ int av_fifo_space(const AVFifoBuffer *f)
>       return av_fifo_can_write(f);
>   }
>   
> -int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size)
> +int av_fifo_grow2(AVFifoBuffer *f, size_t inc)
>   {
>       FifoBuffer *fb = (FifoBuffer*)f;
> +    uint8_t *tmp;
>   
> -    if (new_size > FIFO_SIZE_MAX)
> +    if (inc > FIFO_SIZE_MAX - fb->nb_elems)
>           return AVERROR(EINVAL);
>   
> -    if (fb->nb_elems < new_size) {
> -        uint8_t *tmp;
> -
> -        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
> -        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
> -                fb->offset_w = fb->nb_elems + copy;
> -        }
> +    tmp = av_realloc_array(f->buffer, fb->nb_elems + inc, fb->elem_size);

This sounds like a good opportunity to introduce av_fast_realloc_array() 
in mem.h, with FifoBuffer being a private struct where you can safely 
store the required buffer size field.

av_realloc_array() can be slow in an auto-grow scenario if there's a lot 
of buffering before stuff starts being drained.

> +    if (!tmp)
> +        return AVERROR(ENOMEM);
> +
> +    // move the data from the beginning of the ring buffer
> +    // to the newly allocated space
> +    if (fb->offset_w <= fb->offset_r && !fb->is_empty) {
> +        const size_t copy = FFMIN(inc, 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
> +            fb->offset_w = fb->nb_elems + copy;
> +    }
> +
> +    f->buffer     = tmp;
> +    fb->nb_elems += inc;
>   
> -        f->buffer = tmp;
>   #if FF_API_FIFO_PUBLIC
> -        f->end    = f->buffer + new_size;
> -        f->rptr   = f->buffer + fb->offset_r * fb->elem_size;
> -        f->wptr   = f->buffer + fb->offset_w * fb->elem_size;
> +    f->end    = f->buffer + fb->nb_elems * fb->elem_size;
> +    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_realloc2(AVFifoBuffer *f, unsigned int new_size)
> +{
> +    FifoBuffer *fb = (FifoBuffer*)f;
> +    if (fb->nb_elems < new_size)
> +        return av_fifo_grow2(f, new_size - fb->nb_elems);
>       return 0;
>   }
>   
> diff --git a/libavutil/fifo.h b/libavutil/fifo.h
> index 9e78082b3b..375d0d133b 100644
> --- a/libavutil/fifo.h
> +++ b/libavutil/fifo.h
> @@ -129,6 +129,20 @@ size_t av_fifo_can_read(const AVFifoBuffer *f);
>    */
>   size_t av_fifo_can_write(const AVFifoBuffer *f);
>   
> +/**
> + * Enlarge an AVFifoBuffer.
> + *
> + * 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 AVFifoBuffer 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(AVFifoBuffer *f, size_t inc);
> +
>   /**
>    * Feed data at specific position from an AVFifoBuffer to a user-supplied callback.
>    * Similar as av_fifo_gereric_read but without discarding data.
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index 9400c5147a..98eae55719 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -18,7 +18,8 @@  API changes, most recent first:
   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(), av_fifo_can_read(), av_fifo_can_write().
+  Add av_fifo_elem_size(), av_fifo_can_read(), av_fifo_can_write(),
+  av_fifo_grow2().
 
 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 8cde2c20e1..28d6d1b2e6 100644
--- a/libavutil/fifo.c
+++ b/libavutil/fifo.c
@@ -150,41 +150,48 @@  int av_fifo_space(const AVFifoBuffer *f)
     return av_fifo_can_write(f);
 }
 
-int av_fifo_realloc2(AVFifoBuffer *f, unsigned int new_size)
+int av_fifo_grow2(AVFifoBuffer *f, size_t inc)
 {
     FifoBuffer *fb = (FifoBuffer*)f;
+    uint8_t *tmp;
 
-    if (new_size > FIFO_SIZE_MAX)
+    if (inc > FIFO_SIZE_MAX - fb->nb_elems)
         return AVERROR(EINVAL);
 
-    if (fb->nb_elems < new_size) {
-        uint8_t *tmp;
-
-        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
-        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
-                fb->offset_w = fb->nb_elems + copy;
-        }
+    tmp = av_realloc_array(f->buffer, fb->nb_elems + inc, fb->elem_size);
+    if (!tmp)
+        return AVERROR(ENOMEM);
+
+    // move the data from the beginning of the ring buffer
+    // to the newly allocated space
+    if (fb->offset_w <= fb->offset_r && !fb->is_empty) {
+        const size_t copy = FFMIN(inc, 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
+            fb->offset_w = fb->nb_elems + copy;
+    }
+
+    f->buffer     = tmp;
+    fb->nb_elems += inc;
 
-        f->buffer = tmp;
 #if FF_API_FIFO_PUBLIC
-        f->end    = f->buffer + new_size;
-        f->rptr   = f->buffer + fb->offset_r * fb->elem_size;
-        f->wptr   = f->buffer + fb->offset_w * fb->elem_size;
+    f->end    = f->buffer + fb->nb_elems * fb->elem_size;
+    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_realloc2(AVFifoBuffer *f, unsigned int new_size)
+{
+    FifoBuffer *fb = (FifoBuffer*)f;
+    if (fb->nb_elems < new_size)
+        return av_fifo_grow2(f, new_size - fb->nb_elems);
     return 0;
 }
 
diff --git a/libavutil/fifo.h b/libavutil/fifo.h
index 9e78082b3b..375d0d133b 100644
--- a/libavutil/fifo.h
+++ b/libavutil/fifo.h
@@ -129,6 +129,20 @@  size_t av_fifo_can_read(const AVFifoBuffer *f);
  */
 size_t av_fifo_can_write(const AVFifoBuffer *f);
 
+/**
+ * Enlarge an AVFifoBuffer.
+ *
+ * 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 AVFifoBuffer 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(AVFifoBuffer *f, size_t inc);
+
 /**
  * Feed data at specific position from an AVFifoBuffer to a user-supplied callback.
  * Similar as av_fifo_gereric_read but without discarding data.