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

wm4 nfxjfg at googlemail.com
Wed Mar 14 20:59:39 EET 2018


On Wed, 14 Mar 2018 14:45:33 -0300
James Almer <jamrial at gmail.com> wrote:

> On 3/14/2018 1:41 PM, wm4 wrote:
> > On Wed, 14 Mar 2018 13:30:28 -0300
> > James Almer <jamrial at gmail.com> wrote:
> >   
> >> On 3/14/2018 12:51 PM, wm4 wrote:  
> >>> 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.    
> >>
> >> No, that's a good idea, and it would come in handy in a lot more places
> >> than this fast_alloc API. But it's probably not trivial to implement,
> >> and would explain why the existing buffer pool API is for fixed sizes only.
> >>
> >> In any case, do you think it's better to try and implement your
> >> suggestion instead of this patch? You're right that it's a matter of
> >> creating one extra reference and this function would probably stop being
> >> "fast".  
> > 
> > Depends what's even the point of the new API (i.e. where it's used).  
> 
> My idea is to use it in place of av_fast_malloc() on code were an
> AVBufferRef would come in handy. Namely h2645_parse, so the escaped NALu
> buffers can be reused without the need of a bunch of memcpy.
> In such cases the references created may be gone by the time
> av_buffer_fast_alloc() is called again, so no new buffer would have to
> be allocated, but there's no warranty of that.
> 
> A dynamic size buffer pool however has a lot more use cases across the
> codebase.
> 
> > A pool should actually be relatively easy to implement: just resize the
> > pool if the new buffer exceeds the pool's size. But then it might waste
> > memory if all later elements are much smaller.  
> 
> That's probably easy to implement but i guess not too efficient, as you
> said. A better implementation would be a true dynamic size pool where
> each time you request a buffer the API looks at all those in the pool
> for one big enough and returns it if available, and if not, instead of
> allocating another buffer that will eventually make it to the pool, it
> replaces the until then biggest one.
> A function can also be added to resize all the buffers in the pool to
> make sure they are all within an arbitrary limit. Or simply a way to
> limit the pool right during init().

Yeah, but that's called malloc(), heh. Or at least it's approximating
some sort of generic purpose memory allocator.


More information about the ffmpeg-devel mailing list