From patchwork Mon Jun 1 16:35:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 20060 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 726D844A747 for ; Mon, 1 Jun 2020 19:36:24 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5245D68A47E; Mon, 1 Jun 2020 19:36:24 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk1-f196.google.com (mail-qk1-f196.google.com [209.85.222.196]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A037D687F52 for ; Mon, 1 Jun 2020 19:36:17 +0300 (EEST) Received: by mail-qk1-f196.google.com with SMTP id b27so9646304qka.4 for ; Mon, 01 Jun 2020 09:36:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:mime-version :content-transfer-encoding; bh=nvnvT3eQMw2L+WLOSUhTa4uPb5lecF4+rLJ+rg5pHnc=; b=AY9K92/KR+vp6PRloCfUBZC85EpD+eaROtkXgi82jKFyWmK44wj3fCuIIpgpoew5BZ Rk/mWZko+sc/dqzZYFUfOxkyciFhk9r0eReHrYlsii7MDbXrWKRpN/2STSHiQoLKZGYm uFzi5vTONknfkrankJI2PqAJ6p8KlekGCgO9cRU4+bKFeq8XYZZq6taEl5fX/je3dQw6 XefLUEUiu0FASMTvYaCBhS8oSkET68kG53fH3I99A4dQyp/doLHqAyaehFan0fZS3S5C 3uZqW8d4kYBn2muUlEPjcjDPWmYdyPhjl3Rijbwj0yW56JcdwtxkDWzwtUAgYLbT5Owa IJpA== 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:mime-version :content-transfer-encoding; bh=nvnvT3eQMw2L+WLOSUhTa4uPb5lecF4+rLJ+rg5pHnc=; b=bQP4P3DGyQ7RkqGSeCbFh9XpaAvgQStKHk4XhJP8XflIqDPzlciiZSvw4pC5Lqojnn UsuYnGlbuBjGqRDnFiiBWg1soDBmR7yDbuCznsGxHj7TLih0ZON7kMgDOJqkbnz5Cff6 1yqE0YuB27FawjFZunkbJQEjGXuhC3HelvU+00FTICeUtSGW/AyoOaHznjwrixX+/jDT 4VooWLGZU3VL1ya0MiMrPsx0WeGTHaEzzdKnWbgQIRmu+MMDWHWfNaaCjSnWqIHfxmtc KOB/RZWsn9hYDf8/S8ciVzLVnz6z/NQhgsB5qP9GqBp0XAxB1uZ735Em8ZuBoV/jhyjr RGoQ== X-Gm-Message-State: AOAM531vW6QUejXHziuvU8Doya52Jv9Igpd4mrtgcRHwVRAYBR5Y1TG1 8WkEA3zBlkVTZuo5Ap7TKg4I/4bU X-Google-Smtp-Source: ABdhPJxwsL0r30W3u+g6QTjPzGvQRbILGGyTiNCiW2jBusdN3BaTXi95O02qIrICTfMN4huPjepnxQ== X-Received: by 2002:ae9:eb83:: with SMTP id b125mr22174495qkg.85.1591029375638; Mon, 01 Jun 2020 09:36:15 -0700 (PDT) Received: from localhost.localdomain ([181.23.72.208]) by smtp.gmail.com with ESMTPSA id k6sm15153214qte.52.2020.06.01.09.36.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 Jun 2020 09:36:14 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Mon, 1 Jun 2020 13:35:39 -0300 Message-Id: <20200601163540.13179-1-jamrial@gmail.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PoC][PATCH 1/2] avutil/buffer: add av_buffer_create2() to allow AVBufferRef to store complex structures 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Signed-off-by: James Almer --- This is an attempt at solving the annoying constrain of only supporting flat arrays within any given AVBufferRef. It introduces a new function to create buffers that accepts two new callback functions, one to allocate a new buffer when a new writable reference needs to be created, and one to copy data to it. In the default scenario, the alloc and copy callbacks simply call av_buffer_alloc() and memcpy() respectively, which is the current behavior of treating the buffer as a flat array. In a more complex scenario, the real benefit comes from the copy callback, which will let a custom implementation set up the new buffer how it pleases, including handling pointers within the complex struct it may be storing. Patch 2/2 is an example implementation of this. libavutil/buffer.c | 39 ++++++++++++++++++++++++++++++----- libavutil/buffer.h | 41 +++++++++++++++++++++++++++++++++++++ libavutil/buffer_internal.h | 10 +++++++++ 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/libavutil/buffer.c b/libavutil/buffer.c index 7ff6adc2ec..b048e168e8 100644 --- a/libavutil/buffer.c +++ b/libavutil/buffer.c @@ -26,9 +26,12 @@ #include "mem.h" #include "thread.h" -AVBufferRef *av_buffer_create(uint8_t *data, int size, - void (*free)(void *opaque, uint8_t *data), - void *opaque, int flags) +AVBufferRef *av_buffer_create2(uint8_t *data, int size, + AVBufferRef* (*alloc)(void *opaque, int size), + int (*copy)(void *opaque, AVBufferRef *dst, + const uint8_t *src, int size), + void (*free)(void *opaque, uint8_t *data), + void *opaque, int flags) { AVBufferRef *ref = NULL; AVBuffer *buf = NULL; @@ -39,6 +42,8 @@ AVBufferRef *av_buffer_create(uint8_t *data, int size, buf->data = data; buf->size = size; + buf->alloc = alloc ? alloc : av_buffer_default_alloc; + buf->copy = copy ? copy : av_buffer_default_copy; buf->free = free ? free : av_buffer_default_free; buf->opaque = opaque; @@ -59,11 +64,29 @@ AVBufferRef *av_buffer_create(uint8_t *data, int size, return ref; } +AVBufferRef *av_buffer_create(uint8_t *data, int size, + void (*free)(void *opaque, uint8_t *data), + void *opaque, int flags) +{ + return av_buffer_create2(data, size, NULL, NULL, free, opaque, flags); +} + void av_buffer_default_free(void *opaque, uint8_t *data) { av_free(data); } +AVBufferRef *av_buffer_default_alloc(void *opaque, int size) +{ + return av_buffer_alloc(size); +} + +int av_buffer_default_copy(void *opaque, AVBufferRef *dst, const uint8_t *src, int size) +{ + memcpy(dst->data, src, size); + return 0; +} + AVBufferRef *av_buffer_alloc(int size) { AVBufferRef *ret = NULL; @@ -151,15 +174,21 @@ int av_buffer_get_ref_count(const AVBufferRef *buf) int av_buffer_make_writable(AVBufferRef **pbuf) { AVBufferRef *newbuf, *buf = *pbuf; + AVBuffer *b = buf->buffer; + int ret; if (av_buffer_is_writable(buf)) return 0; - newbuf = av_buffer_alloc(buf->size); + newbuf = b->alloc(b->opaque, buf->size); if (!newbuf) return AVERROR(ENOMEM); - memcpy(newbuf->data, buf->data, buf->size); + ret = b->copy(b->opaque, newbuf, buf->data, buf->size); + if (ret < 0) { + av_buffer_unref(&newbuf); + return ret; + } buffer_replace(pbuf, &newbuf); diff --git a/libavutil/buffer.h b/libavutil/buffer.h index e0f94314f4..375e04034a 100644 --- a/libavutil/buffer.h +++ b/libavutil/buffer.h @@ -131,6 +131,47 @@ AVBufferRef *av_buffer_create(uint8_t *data, int size, void (*free)(void *opaque, uint8_t *data), void *opaque, int flags); +/** + * Create an AVBuffer from an existing array. + * + * If this function is successful, data is owned by the AVBuffer. The caller may + * only access data through the returned AVBufferRef and references derived from + * it. + * If this function fails, data is left untouched. + * @param data data array + * @param size size of data in bytes + * @param alloc a callback for allocating a new buffer when a new writable + * reference for this buffer is created + * @param copy a callback for copying this buffer's data into the newly + * allocated buffer by the alloc callback + * @param free a callback for freeing this buffer's data + * @param opaque parameter to be got for processing or passed to alloc/copy/free + * @param flags a combination of AV_BUFFER_FLAG_* + * + * @return an AVBufferRef referring to data on success, NULL on failure. + */ +AVBufferRef *av_buffer_create2(uint8_t *data, int size, + AVBufferRef* (*alloc)(void *opaque, int size), + int (*copy)(void *opaque, AVBufferRef *dst, + const uint8_t *src, int size), + void (*free)(void *opaque, uint8_t *data), + void *opaque, int flags); + +/** + * Default alloc callback, which calls av_buffer_alloc() and returns the + * newly allocated buffer. + * This function is meant to be passed to av_buffer_create2() or + * av_buffer_pool_init2(), not called directly. + */ +AVBufferRef *av_buffer_default_alloc(void *opaque, int size); + +/** + * Default copy callback, which copies the data pointed by src to dst. + * This function is meant to be passed to av_buffer_create2(), not called + * directly. + */ +int av_buffer_default_copy(void *opaque, AVBufferRef *dst, const uint8_t *src, int size); + /** * Default free callback, which calls av_free() on the buffer data. * This function is meant to be passed to av_buffer_create(), not called diff --git a/libavutil/buffer_internal.h b/libavutil/buffer_internal.h index 70d2615a06..27fcb5f015 100644 --- a/libavutil/buffer_internal.h +++ b/libavutil/buffer_internal.h @@ -39,6 +39,16 @@ struct AVBuffer { */ atomic_uint refcount; + /** + * a callback to allocate a new writable buffer + */ + AVBufferRef* (*alloc)(void *opaque, int size); + + /** + * a callback to copy the data into a newly allocated writable buffer + */ + int (*copy)(void *opaque, AVBufferRef *dst, const uint8_t *src, int size); + /** * a callback for freeing the data */ From patchwork Mon Jun 1 16:35:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Almer X-Patchwork-Id: 20061 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 687A844A747 for ; Mon, 1 Jun 2020 19:36:26 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4D31E68A0C3; Mon, 1 Jun 2020 19:36:26 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-qk1-f195.google.com (mail-qk1-f195.google.com [209.85.222.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 02AED687F52 for ; Mon, 1 Jun 2020 19:36:19 +0300 (EEST) Received: by mail-qk1-f195.google.com with SMTP id w3so9629326qkb.6 for ; Mon, 01 Jun 2020 09:36:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references:mime-version :content-transfer-encoding; bh=cQsY8ZhRMrrhVk9nbnKYg39wM9fqDq1U6TzMhDJvYqI=; b=nSaZNvKO2sH30xUX2B0ZlXOP7vDlDxd1cxWWU9T+WEHzu4QS+8buUXafTZlTUykM2B y6/3d7FATgoQdjySaVSUU98Wfe5os0Og+hkOYmlhocfxW3F+WwjRTsfVPap1P9T/XW5Z owCIXsXvTCFN3W5wvzamiQC968BVm/JJQlANmT8zCZqk7miHSrvxZziQPX9qUOrz1KRw OzcSjHj/TChpGgMHLahch3wjXNT/g9dDyHS4G2n7XSf7+DDFBp1+9KoK8FcEqUKYD/F0 Xs9b1kIDMCotOwGBh0Q2hL0rhQ51zxzqr2hdxpUo4Qw/MkARJitruxtAI9saGGM4ijC7 3o6A== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=cQsY8ZhRMrrhVk9nbnKYg39wM9fqDq1U6TzMhDJvYqI=; b=f3EsSCbkz9IrUHmz4SzRlVtpjnGizhdtNUSyjsjD2KKCjzMYCXG7XSd0bUj46CzlVK 3j3XVvZeZoDHwlnq/I3oq2PAyXDqrxt2Hr6k+OkQAOtrF/N6EmeFe/xH3ambXDkWd1mn yp3X8A4uDDi84ekBf0fzEMhowW00KfabRP14CqIC8WP8tArvVD+WfUataNAA5Nftv3Ek YeoZvsZcq5S+ctUO81yXqsbSFf9+5kxwIBjumYAjMDFGV4zGQyo6+vIaYMnBnT14GLwO kxK+jDfBJ50PrSCWGPITiW3FJkCXb6JldbRNaK/80S9tz8SALYnHFN2uASR2l/sI1x+R dl+A== X-Gm-Message-State: AOAM532NhXH63fgVmJUJ8Lm4SlqysuQTQRLnOLXWYn0cbNh4lO4F604k BVgITRkc82B+1/FkHb0BXjzy7plP X-Google-Smtp-Source: ABdhPJxKkm2GR17r+8P3oEipIAcKaZsaYPJy7lCTF9hX2+0gzVGfxsw2mXRs4zOXmstvpyZoiSoETQ== X-Received: by 2002:a37:6851:: with SMTP id d78mr20532266qkc.86.1591029377208; Mon, 01 Jun 2020 09:36:17 -0700 (PDT) Received: from localhost.localdomain ([181.23.72.208]) by smtp.gmail.com with ESMTPSA id k6sm15153214qte.52.2020.06.01.09.36.15 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 01 Jun 2020 09:36:16 -0700 (PDT) From: James Almer To: ffmpeg-devel@ffmpeg.org Date: Mon, 1 Jun 2020 13:35:40 -0300 Message-Id: <20200601163540.13179-2-jamrial@gmail.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200601163540.13179-1-jamrial@gmail.com> References: <20200601163540.13179-1-jamrial@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PoC][PATCH 2/2] avutil/video_enc_params: make use of the complex AVBufferRef allocator 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 Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Signed-off-by: James Almer --- As mentioned in Patch 1/2, this is an example for the complex structure handling in AVBufferRef. It doesn't need to be applied, even if it could (Despite being public, it's new enough that we can change it if we do it now). libavutil/video_enc_params.c | 37 ++++++++++++++++++-- libavutil/video_enc_params.h | 67 ++++++++++++++++++------------------ 2 files changed, 68 insertions(+), 36 deletions(-) diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c index c46c0f1dc6..9e64ad4d59 100644 --- a/libavutil/video_enc_params.c +++ b/libavutil/video_enc_params.c @@ -32,6 +32,9 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type, AVVideoEncParams *par; size_t size; + // Ensure blocks is the last field in the struct + av_assert0(offsetof(AVVideoEncParams, blocks) == sizeof(*par) - sizeof(AVVideoBlockParams*)); + size = sizeof(*par); if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) @@ -45,7 +48,7 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type, par->type = type; par->nb_blocks = nb_blocks; par->block_size = sizeof(AVVideoBlockParams); - par->blocks_offset = sizeof(*par); + par->blocks = par->nb_blocks ? (AVVideoBlockParams *)&par[1] : NULL; if (out_size) *out_size = size; @@ -53,6 +56,35 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type, return par; } +static int copy_callback(void *opaque, AVBufferRef *dst, const uint8_t *src, int size) +{ + AVVideoEncParams *par_dst = (AVVideoEncParams *)dst->data; + AVVideoEncParams *par_src = (AVVideoEncParams *)src; + + memcpy(par_dst, par_src, offsetof(AVVideoEncParams, blocks)); + par_dst->blocks = par_src->nb_blocks ? (AVVideoBlockParams *)&par_dst[1] : NULL; + if (par_dst->blocks) + memcpy(par_dst->blocks, par_src->blocks, par_src->nb_blocks * par_src->block_size); + + return 0; +} + +static AVBufferRef *alloc_callback(void *opaque, int size) +{ + AVBufferRef *buf; + uint8_t *par; + + par = av_malloc(size); + if (!par) + return NULL; + buf = av_buffer_create2(par, size, alloc_callback, + copy_callback, NULL, NULL, 0); + if (!buf) + av_free(par); + + return buf; +} + AVVideoEncParams* av_video_enc_params_create_side_data(AVFrame *frame, enum AVVideoEncParamsType type, unsigned int nb_blocks) @@ -64,7 +96,8 @@ av_video_enc_params_create_side_data(AVFrame *frame, enum AVVideoEncParamsType t par = av_video_enc_params_alloc(type, nb_blocks, &size); if (!par) return NULL; - buf = av_buffer_create((uint8_t *)par, size, NULL, NULL, 0); + buf = av_buffer_create2((uint8_t *)par, size, alloc_callback, + copy_callback, NULL, NULL, 0); if (!buf) { av_freep(&par); return NULL; diff --git a/libavutil/video_enc_params.h b/libavutil/video_enc_params.h index 43fa443154..b443dc50d9 100644 --- a/libavutil/video_enc_params.h +++ b/libavutil/video_enc_params.h @@ -57,6 +57,33 @@ enum AVVideoEncParamsType { AV_VIDEO_ENC_PARAMS_H264, }; +/** + * Data structure for storing block-level encoding information. + * It is allocated as a part of AVVideoEncParams and should be retrieved with + * av_video_enc_params_block(). + * + * sizeof(AVVideoBlockParams) is not a part of the ABI and new fields may be + * added to it. + */ +typedef struct AVVideoBlockParams { + /** + * Distance in luma pixels from the top-left corner of the visible frame + * to the top-left corner of the block. + * Can be negative if top/right padding is present on the coded frame. + */ + int src_x, src_y; + /** + * Width and height of the block in luma pixels. + */ + int w, h; + + /** + * Difference between this block's final quantization parameter and the + * corresponding per-frame value. + */ + int32_t delta_qp; +} AVVideoBlockParams; + /** * Video encoding parameters for a given frame. This struct is allocated along * with an optional array of per-block AVVideoBlockParams descriptors. @@ -67,15 +94,9 @@ typedef struct AVVideoEncParams { * Number of blocks in the array. * * May be 0, in which case no per-block information is present. In this case - * the values of blocks_offset / block_size are unspecified and should not - * be accessed. + * the value of block_size is unspecified and should not be accessed. */ unsigned int nb_blocks; - /** - * Offset in bytes from the beginning of this structure at which the array - * of blocks starts. - */ - size_t blocks_offset; /* * Size of each block in bytes. May not match sizeof(AVVideoBlockParams). */ @@ -99,34 +120,13 @@ typedef struct AVVideoEncParams { * plane (first index) and AC/DC coefficients (second index). */ int32_t delta_qp[4][2]; -} AVVideoEncParams; -/** - * Data structure for storing block-level encoding information. - * It is allocated as a part of AVVideoEncParams and should be retrieved with - * av_video_enc_params_block(). - * - * sizeof(AVVideoBlockParams) is not a part of the ABI and new fields may be - * added to it. - */ -typedef struct AVVideoBlockParams { /** - * Distance in luma pixels from the top-left corner of the visible frame - * to the top-left corner of the block. - * Can be negative if top/right padding is present on the coded frame. + * The arary of blocks, if any was allocated. Should only be accessed using + * av_video_enc_params_block(). */ - int src_x, src_y; - /** - * Width and height of the block in luma pixels. - */ - int w, h; - - /** - * Difference between this block's final quantization parameter and the - * corresponding per-frame value. - */ - int32_t delta_qp; -} AVVideoBlockParams; + AVVideoBlockParams *blocks; +} AVVideoEncParams; /* * Get the block at the specified {@code idx}. Must be between 0 and nb_blocks. @@ -135,8 +135,7 @@ static av_always_inline AVVideoBlockParams* av_video_enc_params_block(AVVideoEncParams *par, unsigned int idx) { av_assert0(idx < par->nb_blocks); - return (AVVideoBlockParams *)((uint8_t *)par + par->blocks_offset + - idx * par->block_size); + return (AVVideoBlockParams *)&par->blocks[idx]; } /**