Signed-off-by: James Almer <jamr...@gmail.com> --- This is a proof of concept for a dynamic size buffer pool API.
For the purpose of easy testing and reviewing I replaced the current linked list used to keep a pool of fixed size buffers with the tree based pool that will be used to keep a pool of varying size buffers, instead of adding a new set of functions exclusively for the new API. The final committed work doesn't necessarely have to do the above, as there's no real benefit using a tree when you only need a fixed size buffer pool, other than simplying things. I'm open to suggestions about how to introduce this. Completely separate set of functions and struct names? Sharing the struct and init/uninit functions and only adding a new get() one like in this patch? Any preferences with function/struct naming, for that matter? libavutil/buffer.c | 98 ++++++++++++++++++++++++++++++++++++--------- libavutil/buffer.h | 2 + libavutil/buffer_internal.h | 6 ++- 3 files changed, 85 insertions(+), 21 deletions(-) diff --git a/libavutil/buffer.c b/libavutil/buffer.c index 8d1aa5fa84..9fe5aa9825 100644 --- a/libavutil/buffer.c +++ b/libavutil/buffer.c @@ -251,19 +251,27 @@ AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* (*alloc)(int size)) return pool; } +/* Callback function to free every pooled buffer during uninit, + * to be used with av_tree_enumerate(). */ +static int free_node(void *opaque, void *elem) +{ + BufferPoolEntry *buf = elem; + + buf->free(buf->opaque, buf->data); + av_free(buf); + + return 0; +} + /* * This function gets called when the pool has been uninited and * all the buffers returned to it. */ static void buffer_pool_free(AVBufferPool *pool) { - while (pool->pool) { - BufferPoolEntry *buf = pool->pool; - pool->pool = buf->next; + av_tree_enumerate(pool->root, NULL, NULL, free_node); + av_tree_destroy(pool->root); - buf->free(buf->opaque, buf->data); - av_freep(&buf); - } ff_mutex_destroy(&pool->mutex); if (pool->pool_free) @@ -285,17 +293,29 @@ void av_buffer_pool_uninit(AVBufferPool **ppool) buffer_pool_free(pool); } +/* Callback function to compare two nodes, order them based + * on size, and retrieve them, to be used with av_tree_insert(). */ +static int cmp_insert(const void *key, const void *node) +{ + int ret = FFDIFFSIGN(((const BufferPoolEntry *) key)->size, ((const BufferPoolEntry *) node)->size); + + if (!ret) + ret = FFDIFFSIGN(((const BufferPoolEntry *) key)->data, ((const BufferPoolEntry *) node)->data); + return ret; +} + static void pool_release_buffer(void *opaque, uint8_t *data) { BufferPoolEntry *buf = opaque; AVBufferPool *pool = buf->pool; if(CONFIG_MEMORY_POISONING) - memset(buf->data, FF_MEMORY_POISON, pool->size); + memset(buf->data, FF_MEMORY_POISON, buf->size); ff_mutex_lock(&pool->mutex); - buf->next = pool->pool; - pool->pool = buf; + /* Add the buffer into the pool, using the preallocated + * AVTreeNode stored in buf->node */ + av_tree_insert(&pool->root, buf, cmp_insert, &buf->node); ff_mutex_unlock(&pool->mutex); if (atomic_fetch_add_explicit(&pool->refcount, -1, memory_order_acq_rel) == 1) @@ -304,13 +324,13 @@ static void pool_release_buffer(void *opaque, uint8_t *data) /* allocate a new buffer and override its free() callback so that * it is returned to the pool on free */ -static AVBufferRef *pool_alloc_buffer(AVBufferPool *pool) +static AVBufferRef *pool_alloc_buffer(AVBufferPool *pool, int size) { BufferPoolEntry *buf; AVBufferRef *ret; - ret = pool->alloc2 ? pool->alloc2(pool->opaque, pool->size) : - pool->alloc(pool->size); + ret = pool->alloc2 ? pool->alloc2(pool->opaque, size) : + pool->alloc(size); if (!ret) return NULL; @@ -320,9 +340,17 @@ static AVBufferRef *pool_alloc_buffer(AVBufferPool *pool) return NULL; } + buf->node = av_tree_node_alloc(); + if (!buf->node) { + av_free(buf); + av_buffer_unref(&ret); + return NULL; + } + buf->data = ret->buffer->data; buf->opaque = ret->buffer->opaque; buf->free = ret->buffer->free; + buf->size = size; buf->pool = pool; ret->buffer->opaque = buf; @@ -331,22 +359,49 @@ static AVBufferRef *pool_alloc_buffer(AVBufferPool *pool) return ret; } -AVBufferRef *av_buffer_pool_get(AVBufferPool *pool) +/* Callback function to compare the requested size with the + * size of existing buffers in the pool, to be used with + * av_tree_find(). */ +static int cmp_int(const void *key, const void *node) +{ + return FFDIFFSIGN(*(const int *)key, ((const BufferPoolEntry *) node)->size); +} + +AVBufferRef *av_buffer_pool_get_dyn(AVBufferPool *pool, int size) { AVBufferRef *ret; - BufferPoolEntry *buf; + BufferPoolEntry *buf, *next[2] = { NULL, NULL }; ff_mutex_lock(&pool->mutex); - buf = pool->pool; + /* Find a big enough buffer in the pool. */ + buf = av_tree_find(pool->root, &size, cmp_int, (void **)next); + + if (!buf) + /* If none of the requested size exists, use a bigger one. */ + buf = next[1]; + if (!buf && (buf = next[0])) { + /* If the pool also doesn't have a bigger buffer, but does + * have a smaller one, then replace it with a new buffer of + * the requested size. */ + av_tree_insert(&pool->root, buf, cmp_insert, &buf->node); + buf->free(buf->opaque, buf->data); + av_free(buf->node); + av_freep(&buf); + } + if (buf) { - ret = av_buffer_create(buf->data, pool->size, pool_release_buffer, + ret = av_buffer_create(buf->data, buf->size, pool_release_buffer, buf, 0); if (ret) { - pool->pool = buf->next; - buf->next = NULL; + /* Remove the buffer from the pool. Zero and store the + * AVTreeNode used for it in buf->node so we can use it + * again once the buffer is put back in the pool. */ + av_tree_insert(&pool->root, buf, cmp_insert, &buf->node); + memset(buf->node, 0, av_tree_node_size); + ret->size = size; } } else { - ret = pool_alloc_buffer(pool); + ret = pool_alloc_buffer(pool, size); } ff_mutex_unlock(&pool->mutex); @@ -355,3 +410,8 @@ AVBufferRef *av_buffer_pool_get(AVBufferPool *pool) return ret; } + +AVBufferRef *av_buffer_pool_get(AVBufferPool *pool) +{ + return av_buffer_pool_get_dyn(pool, pool->size); +} diff --git a/libavutil/buffer.h b/libavutil/buffer.h index 73b6bd0b14..244a6de961 100644 --- a/libavutil/buffer.h +++ b/libavutil/buffer.h @@ -284,6 +284,8 @@ void av_buffer_pool_uninit(AVBufferPool **pool); */ AVBufferRef *av_buffer_pool_get(AVBufferPool *pool); +AVBufferRef *av_buffer_pool_get_dyn(AVBufferPool *pool, int size); + /** * @} */ diff --git a/libavutil/buffer_internal.h b/libavutil/buffer_internal.h index 54b67047e5..6672d22516 100644 --- a/libavutil/buffer_internal.h +++ b/libavutil/buffer_internal.h @@ -24,6 +24,7 @@ #include "buffer.h" #include "thread.h" +#include "tree.h" /** * The buffer is always treated as read-only. @@ -61,6 +62,7 @@ struct AVBuffer { typedef struct BufferPoolEntry { uint8_t *data; + int size; /* * Backups of the original opaque/free of the AVBuffer corresponding to @@ -70,12 +72,12 @@ typedef struct BufferPoolEntry { void (*free)(void *opaque, uint8_t *data); AVBufferPool *pool; - struct BufferPoolEntry *next; + struct AVTreeNode *node; } BufferPoolEntry; struct AVBufferPool { AVMutex mutex; - BufferPoolEntry *pool; + struct AVTreeNode *root; /* * This is used to track when the pool is to be freed. -- 2.16.2 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel