[FFmpeg-devel] [PATCH] [RFC][WIP] avutil/buffer: add add a dynamnic size buffer pool API
James Almer
jamrial at gmail.com
Fri Mar 16 20:21:41 EET 2018
Signed-off-by: James Almer <jamrial at 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
More information about the ffmpeg-devel
mailing list