[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