[FFmpeg-devel] [PATCH v2] avutil/buffer: add av_buffer_fast_alloc()

[wm4]

On Wed, 14 Mar 2018 12:30:04 -0300
James Almer <jamrial at gmail.com> wrote:

> Same use case as av_fast_malloc(). If the buffer passed to it is
> writable and big enough it will be reused, otherwise a new one will
> be allocated instead.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> TODO: Changelog and APIChanges entries, version bump.
> 
>  libavutil/buffer.c | 33 +++++++++++++++++++++++++++++++++
>  libavutil/buffer.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 75 insertions(+)
> 
> diff --git a/libavutil/buffer.c b/libavutil/buffer.c
> index 8d1aa5fa84..26e015d7ee 100644
> --- a/libavutil/buffer.c
> +++ b/libavutil/buffer.c
> @@ -215,6 +215,39 @@ int av_buffer_realloc(AVBufferRef **pbuf, int size)
>      return 0;
>  }
>  
> +static inline int buffer_fast_alloc(AVBufferRef **pbuf, int size,
> +                                    AVBufferRef* (*buffer_alloc)(int size))
> +{
> +    AVBufferRef *buf = *pbuf;
> +
> +    if (buf && av_buffer_is_writable(buf)
> +            && buf->data == buf->buffer->data
> +            && size <= buf->buffer->size) {
> +        buf->size = FFMAX(0, size);
> +        return 0;
> +    }
> +
> +    av_buffer_unref(pbuf);
> +
> +    buf = buffer_alloc(size);
> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +
> +    *pbuf = buf;
> +
> +    return 0;
> +}
> +
> +int av_buffer_fast_alloc(AVBufferRef **pbuf, int size)
> +{
> +    return buffer_fast_alloc(pbuf, size, av_buffer_alloc);
> +}
> +
> +int av_buffer_fast_allocz(AVBufferRef **pbuf, int size)
> +{
> +    return buffer_fast_alloc(pbuf, size, av_buffer_allocz);
> +}
> +
>  AVBufferPool *av_buffer_pool_init2(int size, void *opaque,
>                                     AVBufferRef* (*alloc)(void *opaque, int size),
>                                     void (*pool_free)(void *opaque))
> diff --git a/libavutil/buffer.h b/libavutil/buffer.h
> index 73b6bd0b14..1166017d22 100644
> --- a/libavutil/buffer.h
> +++ b/libavutil/buffer.h
> @@ -197,6 +197,48 @@ int av_buffer_make_writable(AVBufferRef **buf);
>   */
>  int av_buffer_realloc(AVBufferRef **buf, int size);
>  
> +/**
> + * Allocate a buffer, reusing the given one if writable and large enough.
> + *
> + * @code{.c}
> + * AVBufferRef *buf = ...;
> + * int ret = av_buffer_fast_alloc(&buf, size);
> + * if (ret < 0) {
> + *     // Allocation failed; buf already freed
> + *     return ret;
> + * }
> + * @endcode
> + *
> + * @param buf  A buffer reference. *buf may be NULL. On success, a new buffer
> + *             reference will be written in its place. On failure, it will be
> + *             unreferenced and set to NULL.
> + * @param size Required buffer size.
> + *
> + * @return 0 on success, a negative AVERROR on failure.
> + *
> + * @see av_buffer_realloc()
> + * @see av_buffer_fast_allocz()
> + */
> +int av_buffer_fast_alloc(AVBufferRef **buf, int size);
> +
> +/**
> + * Allocate and clear a buffer, reusing the given one if writable and large
> + * enough.
> + *
> + * Like av_buffer_fast_alloc(), but all newly allocated space is initially
> + * cleared. Reused buffer is not cleared.
> + *
> + * @param buf  A buffer reference. *buf may be NULL. On success, a new buffer
> + *             reference will be written in its place. On failure, it will be
> + *             unreferenced and set to NULL.
> + * @param size Required buffer size.
> + *
> + * @return 0 on success, a negative AVERROR on failure.
> + *
> + * @see av_buffer_fast_alloc()
> + */
> +int av_buffer_fast_allocz(AVBufferRef **buf, int size);
> +
>  /**
>   * @}
>   */

LGTM

Also some comment on the side: I think what you'd actually want is some
sort of flexible buffer pool. E.g. consider the allocated AVBufferRef
gets used and buffered by the consumer for 1 frame. Then the
AVBufferRef will always be not-writable, and will always be newly
allocated, and this optimization via fast_alloc does not help. With a
buffer pool, it would allocate a second AVBufferRef, but then always
reuse it. Anyway, I might be overthinking it.