On Wed, 14 Mar 2018 14:45:33 -0300 James Almer <jamr...@gmail.com> wrote:
> On 3/14/2018 1:41 PM, wm4 wrote: > > On Wed, 14 Mar 2018 13:30:28 -0300 > > James Almer <jamr...@gmail.com> wrote: > > > >> On 3/14/2018 12:51 PM, wm4 wrote: > >>> On Wed, 14 Mar 2018 12:30:04 -0300 > >>> James Almer <jamr...@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 <jamr...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel