On Mon, Jan 28, 2013 at 5:20 PM, Anton Khirnov <[email protected]> wrote:
>
> On Mon, 28 Jan 2013 11:21:17 +0100, Reinhard Tartler <[email protected]>
> wrote:
>> On Mon, Jan 28, 2013 at 7:53 AM, Anton Khirnov <[email protected]> wrote:
>> > +/* remove the whole buffer list from the pool and return it */
>> > +static BufferPoolEntry *get_pool(AVBufferPool *pool)
>> > +{
>> > + BufferPoolEntry *cur, *last = NULL;
>> > +
>> > + cur = avpriv_atomic_ptr_cas((void * volatile *)&pool->pool, NULL,
>> > NULL);
>> > + if (!cur)
>> > + return NULL;
>> > +
>> > + while (cur != last) {
>> > + FFSWAP(BufferPoolEntry*, cur, last);
>> > + cur = avpriv_atomic_ptr_cas((void * volatile *)&pool->pool, last,
>> > NULL);
>> > + }
>>
>> I think a do {} while() would make this look a bit less weird.
>
> Fixed
>
>> > +/**
>> > + * @defgroup lavu_bufferpool AVBufferPool
>> > + * @ingroup lavu_data
>> > + *
>> > + * @{
>> > + * AVBufferPool is an API for a lock-free thread-safe pool of AVBuffers.
>> > + *
>> > + * Frequently allocating and freeing large buffers may be slow.
>> > AVBufferPool is
>> > + * meant to solve this in cases when the caller needs a set of buffers of
>> > the
>> > + * same size (the most obvious use case being buffers for raw video or
>> > audio
>> > + * frames).
>> > + *
>> > + * At the beginning, the user must call av_buffer_pool_init() to create
>> > the
>> > + * buffer pool. Then whenever a buffer is needed, call
>> > av_buffer_alloc_pool() to
>> > + * get a reference to a new buffer, similar to av_buffer_alloc(). This new
>> > + * reference works in all aspects the same way as the one created by
>> > + * av_buffer_alloc(). However, when the last reference to this buffer is
>> > + * unreferenced, it is returned to the pool instead of being freed and
>> > will be
>> > + * reused for subsequent av_buffer_alloc_pool() calls.
>> > + *
>> > + * When the caller is done with the pool and no longer needs to allocate
>> > any new
>> > + * buffers, av_buffer_pool_uninit() must be called to mark the pool as
>> > freeable.
>> > + * Once all the buffers are released, it will automatically be freed.
>> > + *
>> > + * Allocating and releasing buffers with this API is thread-safe as long
>> > as
>> > + * either the default alloc callback is used, or the user-supplied one is
>> > + * thread-safe.
>> > + */
>> > +
>> > +/**
>> > + * The buffer pool. This structure is opaque and not meant to be accessed
>> > + * directly. It is allocated with av_buffer_pool_init() and freed with
>> > + * av_buffer_pool_uninit().
>> > + */
>> > +typedef struct AVBufferPool AVBufferPool;
>> > +
>> > +/**
>> > + * Allocate and initialize a buffer pool.
>> > + *
>> > + * @param size size of each buffer in this pool
>> > + * @param alloc a function that will be used to allocate new buffers when
>> > the
>> > + * pool is empty. May be NULL, then the default allocator will be used
>> > + * (av_buffer_alloc()).
>> > + * @return newly created buffer pool on success, NULL on error.
>> > + */
>> > +AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int
>> > size));
>> > +
>> > +/**
>> > + * Mark the pool as being available for freeing. It will actually be
>> > freed only
>> > + * once all the allocated buffers associated with the pool are released.
>> > Thus it
>> > + * is safe to call this function while some of the allocated buffers are
>> > still
>> > + * in use.
>> > + *
>> > + * @param pool pointer to the pool to be freed. It will be set to NULL.
>> > + * @see av_buffer_pool_can_uninit()
>> > + */
>> > +void av_buffer_pool_uninit(AVBufferPool **pool);
>> > +
>> > +/**
>> > + * Allocate a new AVBuffer, reusing an old buffer from the pool when
>> > available.
>> > + *
>> > + * @return a reference to the new buffer on success, NULL on error.
>> > + */
>> > +AVBufferRef *av_buffer_alloc_pool(AVBufferPool *pool);
>> > +
>> > +/**
>> > + * Same as av_buffer_allocz_pool(), except the data in the returned
>> > buffer will
>> > + * be initialized to zero.
>> > + */
>> > +AVBufferRef *av_buffer_allocz_pool(AVBufferPool *pool);
>> > +
>> > +/**
>> > + * @}
>> > + */
>>
>> The documentation should elaborate a bit more what methods support
>> what kind of concurrency situation. Which of them may be called from
>> different threads simultanously, and which not?
>>
>
> Wel...the whole API is just 3.5 functions (alloc/allocz are the same thing
> with
> a different flavor). init and uninit it only makes sense to call once, so the
> question of concurrency does not even arise. alloc is thread-safe, as the
> documentation explicitly says. So I don't see what more could be said here.
The general description does, but not the documentation for the actual methods.
AFAIUI, calling init multiple times is not only pointless, but also
harmful. I would prefer either the documentation to state this, or the
implementation to cover such misbehavior.
I will not fight for both points, as said, these things caught my eye.
--
regards,
Reinhard
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
