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

[James Almer <jamrial@gmail.com>]

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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".