On Mon, 28 Jan 2013 17:39:34 +0100, Reinhard Tartler <[email protected]> wrote:
> 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.
> 

Ok, added a note about this to av_buffer_alloc() locally.

> 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.

No, it's not. Calling init multiple times will simply give you multiple
completely independent buffer pools.

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to