From patchwork Fri Mar 16 18:21:41 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 8008 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.2.1.70 with SMTP id c67csp1076642jad; Fri, 16 Mar 2018 11:22:21 -0700 (PDT) X-Google-Smtp-Source: AG47ELsEMyrxirjJft6b6zPwc29hB1AszcnTD94aYVK3MuLd+jnlVASZKrj/WZTpGRK34bDmnYTT X-Received: by 10.223.208.132 with SMTP id y4mr2420226wrh.238.1521224541554; Fri, 16 Mar 2018 11:22:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521224541; cv=none; d=google.com; s=arc-20160816; b=eW1UJBd+ijbMGYMKhYtog25LPGRJypDaLDv+0N39hrx+++R54Y1y8NsjfTV3OxzJeA BWVRXAKFmLgb1WPgDhuo2miWugwbU2NUSPEA26C3HDpbfaxZuJ1KD/TtQcg20TTDokAl cvuQ1jA5KXNhvL2iev80cxeEhc6+2njTxdurldmTkENrNr5elzIaQSA3XXpWJ80b6N5Y pPlCGRLpfh7K/28MDlLry57ZWZpCgndfMRNbaeon6ww9+MxNfLW9n2m7T+13LwD7zee8 YwsyG0HDPH7Vo5xdPhGaareg9R89mmKpdqGIOR4MWtP8LSfmVoyb9v6c2vbTT4c20fBT x3KQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:message-id:date:to:from:dkim-signature :delivered-to:arc-authentication-results; bh=3gbsK+VI27BxzKAp3iPSrLwZ+VdP5RMLvmCLt7WCpOc=; b=iONejzYWUxlG7isnxukOy60djkS7Q0ui0zM3d5OJFbZA6VkEx4ylMEzk9X8dzBnR3W Qu0ZCHoza9Iyhqu1ujiF4PQQ1jeo+eHDT9U+UXIofdPRcDTIDdqaB0iczVeiRYNliNia NJuuOFzOStRqrMCTl+VFOdWXIKPNM5x4beDH8iGiOrF0zLq1Mhn4qtbHW+1JcNhTWKyT cfDAJDkfCfY8yGbf4kzHuyBn2NKYpcpwvXRpjO6WOEL7xHACRdZWZNai64IHFC6aPw1r sh6uwRoIcQC0cJARuAojbuybTHUn7L43U4jjOUypRo9av2d3d/Pt0DeROw2OdQZM3pA0 e2KA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=iCmKHr3V; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id 131si4601725wmt.225.2018.03.16.11.22.21; Fri, 16 Mar 2018 11:22:21 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@gmail.com header.s=20161025 header.b=iCmKHr3V; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 45D47689E8B; Fri, 16 Mar 2018 20:22:07 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qt0-f193.google.com (mail-qt0-f193.google.com [209.85.216.193]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 59339689E46 for ; Fri, 16 Mar 2018 20:22:00 +0200 (EET) Received: by mail-qt0-f193.google.com with SMTP id h4so6042035qtn.13 for ; Fri, 16 Mar 2018 11:22:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=B+i/+BYfIoq6Wyh2NNBIqMBaOut1xq4t166IejYhikE=; b=iCmKHr3VNaz5QaxZs3HEzvrDUGyFqwffxFYP+TjRbZoaTEheu774m9X8xINcgkwY+E SCjUBoQgYyiaTl8fkniNrISdi3YU9Fiqjll1nLFYh6KAxDr9wyOXulcMhe9Kz+MksHqf XHIekEUYbneeOZ1URyZolrJeGBdffqLc6l2aWM7kISSjoMkmOLxMtdhiGp7NrvvymxWx 356xFdaT1hOkXWCJF/dwmrSVgAi/+VF+uZCJRVgzXly113T5w1oyf1c/8h4UuAj7xfrh F+ES7aM1+6ccCTYAzIW3t8FtZI0s5lpU5mg+tfwjWcZMCqNWcJ6uDLRtUdSc5nqnqsH/ V0mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=B+i/+BYfIoq6Wyh2NNBIqMBaOut1xq4t166IejYhikE=; b=pD7PM6eY36V/PyBPn3zMIC7bWryH+wsCK0+bi7vmLZpKQNlzFpKLNZ3xuHmEgpJbQ4 wlCyOq9JqGGfk0yS2bF2guGoVKxxvAhBSVwBykwT8s1J6IJGBWt+WgWKxgjCYg03ER3d fD4zQrlQ16YjrdxwFzoz5O4Mbbr4mpgwNW+Go1fvLOlXlNWxpHUGXApSttLPXQxSR4Dj lK0t0RWcdmqRP2IlhZyK8Nq+pFsmR3R80U2/j7tBpfVyxjJH3eVGHylatOJjPXWjSQLF UvoNJKtzsYcCee67/6+OQPvODPzd2+FcXR1ZAAewzhOPAVkyGJlqXz0aTStJVVKmP2zy L9uQ== X-Gm-Message-State: AElRT7FPftKXVwM0l8z40AruGiNn2q+b5aNsf/It4a9FkyPbqIjzjX9h uypyBX06tdcU+IEdJSFl99khCA== X-Received: by 10.200.46.83 with SMTP id s19mr4163586qta.153.1521224531438; Fri, 16 Mar 2018 11:22:11 -0700 (PDT) Received: from localhost.localdomain ([190.188.171.140]) by smtp.gmail.com with ESMTPSA id s31sm6239226qts.42.2018.03.16.11.22.10 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Mar 2018 11:22:10 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Fri, 16 Mar 2018 15:21:41 -0300 Message-Id: <20180316182141.10156-1-jamrial@gmail.com> X-Mailer: git-send-email 2.16.2 Subject: [FFmpeg-devel] [PATCH] [RFC][WIP] avutil/buffer: add add a dynamnic size buffer pool API X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches MIME-Version: 1.0 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Signed-off-by: James Almer --- 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.