From 92abc7266b5a7139d85699cce2b2c94c1e4ed2ed Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt Date: Sat, 6 Aug 2022 16:36:46 +0200 Subject: [PATCH] avcodec/vaapi_encode: Use RefStruct pool API, stop abusing AVBuffer API Up until now, the VAAPI encoder uses fake data with the AVBuffer-API: The data pointer does not point to real memory, but is instead just a VABufferID converted to a pointer. This has probably been copied from the VAAPI-hwcontext-API (which presumably does it to avoid allocations). This commit changes this without causing additional allocations by switching to the RefStruct-pool API. This also fixes an unchecked av_buffer_ref(). Signed-off-by: Andreas Rheinhardt --- libavcodec/vaapi_encode.c | 68 ++++++++++++++++----------------------- libavcodec/vaapi_encode.h | 12 ++++--- 2 files changed, 36 insertions(+), 44 deletions(-) diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index e3820956d1..b2f3059643 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -16,11 +16,11 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ -#include "config_components.h" - #include #include +#include "config.h" + #include "libavutil/avassert.h" #include "libavutil/common.h" #include "libavutil/internal.h" @@ -30,6 +30,7 @@ #include "vaapi_encode.h" #include "encode.h" #include "avcodec.h" +#include "refstruct.h" const AVCodecHWConfigInternal *const ff_vaapi_encode_hw_configs[] = { HW_CONFIG_ENCODER_FRAMES(VAAPI, VAAPI), @@ -322,12 +323,12 @@ static int vaapi_encode_issue(AVCodecContext *avctx, pic->recon_surface = (VASurfaceID)(uintptr_t)pic->recon_image->data[3]; av_log(avctx, AV_LOG_DEBUG, "Recon surface is %#x.\n", pic->recon_surface); - pic->output_buffer_ref = av_buffer_pool_get(ctx->output_buffer_pool); + pic->output_buffer_ref = ff_refstruct_pool_get(ctx->output_buffer_pool); if (!pic->output_buffer_ref) { err = AVERROR(ENOMEM); goto fail; } - pic->output_buffer = (VABufferID)(uintptr_t)pic->output_buffer_ref->data; + pic->output_buffer = *pic->output_buffer_ref; av_log(avctx, AV_LOG_DEBUG, "Output buffer is %#x.\n", pic->output_buffer); @@ -658,7 +659,7 @@ fail_at_end: av_freep(&pic->slices); av_freep(&pic->roi); av_frame_free(&pic->recon_image); - av_buffer_unref(&pic->output_buffer_ref); + ff_refstruct_unref(&pic->output_buffer_ref); pic->output_buffer = VA_INVALID_ID; return err; } @@ -780,7 +781,7 @@ static int vaapi_encode_get_coded_data(AVCodecContext *avctx, int ret; if (ctx->coded_buffer_ref) { - output_buffer_prev = (VABufferID)(uintptr_t)ctx->coded_buffer_ref->data; + output_buffer_prev = *ctx->coded_buffer_ref; ret = vaapi_encode_get_coded_buffer_size(avctx, output_buffer_prev); if (ret < 0) goto end; @@ -808,10 +809,8 @@ static int vaapi_encode_get_coded_data(AVCodecContext *avctx, goto end; end: - if (ctx->coded_buffer_ref) { - av_buffer_unref(&ctx->coded_buffer_ref); - } - av_buffer_unref(&pic->output_buffer_ref); + ff_refstruct_unref(&ctx->coded_buffer_ref); + ff_refstruct_unref(&pic->output_buffer_ref); pic->output_buffer = VA_INVALID_ID; return ret; @@ -830,7 +829,7 @@ static int vaapi_encode_output(AVCodecContext *avctx, if (pic->non_independent_frame) { av_assert0(!ctx->coded_buffer_ref); - ctx->coded_buffer_ref = av_buffer_ref(pic->output_buffer_ref); + ctx->coded_buffer_ref = ff_refstruct_ref(pic->output_buffer_ref); if (pic->tail_size) { if (ctx->tail_pkt->size) { @@ -857,7 +856,7 @@ static int vaapi_encode_output(AVCodecContext *avctx, vaapi_encode_set_output_property(avctx, pic, pkt_ptr); end: - av_buffer_unref(&pic->output_buffer_ref); + ff_refstruct_unref(&pic->output_buffer_ref); pic->output_buffer = VA_INVALID_ID; return err; } @@ -872,7 +871,7 @@ static int vaapi_encode_discard(AVCodecContext *avctx, "%"PRId64"/%"PRId64".\n", pic->display_order, pic->encode_order); - av_buffer_unref(&pic->output_buffer_ref); + ff_refstruct_unref(&pic->output_buffer_ref); pic->output_buffer = VA_INVALID_ID; } @@ -2617,28 +2616,25 @@ static av_cold int vaapi_encode_init_roi(AVCodecContext *avctx) return 0; } -static void vaapi_encode_free_output_buffer(void *opaque, - uint8_t *data) +static void vaapi_encode_free_output_buffer(FFRefStructOpaque opaque, + void *obj) { - AVCodecContext *avctx = opaque; + AVCodecContext *avctx = opaque.nc; VAAPIEncodeContext *ctx = avctx->priv_data; - VABufferID buffer_id; - - buffer_id = (VABufferID)(uintptr_t)data; + VABufferID *buffer_id_ref = obj; + VABufferID buffer_id = *buffer_id_ref; vaDestroyBuffer(ctx->hwctx->display, buffer_id); av_log(avctx, AV_LOG_DEBUG, "Freed output buffer %#x\n", buffer_id); } -static AVBufferRef *vaapi_encode_alloc_output_buffer(void *opaque, - size_t size) +static int vaapi_encode_alloc_output_buffer(FFRefStructOpaque opaque, void *obj) { - AVCodecContext *avctx = opaque; + AVCodecContext *avctx = opaque.nc; VAAPIEncodeContext *ctx = avctx->priv_data; - VABufferID buffer_id; + VABufferID *buffer_id = obj; VAStatus vas; - AVBufferRef *ref; // The output buffer size is fixed, so it needs to be large enough // to hold the largest possible compressed frame. We assume here @@ -2647,25 +2643,16 @@ static AVBufferRef *vaapi_encode_alloc_output_buffer(void *opaque, vas = vaCreateBuffer(ctx->hwctx->display, ctx->va_context, VAEncCodedBufferType, 3 * ctx->surface_width * ctx->surface_height + - (1 << 16), 1, 0, &buffer_id); + (1 << 16), 1, 0, buffer_id); if (vas != VA_STATUS_SUCCESS) { av_log(avctx, AV_LOG_ERROR, "Failed to create bitstream " "output buffer: %d (%s).\n", vas, vaErrorStr(vas)); - return NULL; + return AVERROR(ENOMEM); } - av_log(avctx, AV_LOG_DEBUG, "Allocated output buffer %#x\n", buffer_id); + av_log(avctx, AV_LOG_DEBUG, "Allocated output buffer %#x\n", *buffer_id); - ref = av_buffer_create((uint8_t*)(uintptr_t)buffer_id, - sizeof(buffer_id), - &vaapi_encode_free_output_buffer, - avctx, AV_BUFFER_FLAG_READONLY); - if (!ref) { - vaDestroyBuffer(ctx->hwctx->display, buffer_id); - return NULL; - } - - return ref; + return 0; } static av_cold int vaapi_encode_create_recon_frames(AVCodecContext *avctx) @@ -2880,8 +2867,9 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx) } ctx->output_buffer_pool = - av_buffer_pool_init2(sizeof(VABufferID), avctx, - &vaapi_encode_alloc_output_buffer, NULL); + ff_refstruct_pool_alloc_ext(sizeof(VABufferID), 0, avctx, + &vaapi_encode_alloc_output_buffer, NULL, + vaapi_encode_free_output_buffer, NULL); if (!ctx->output_buffer_pool) { err = AVERROR(ENOMEM); goto fail; @@ -2979,7 +2967,7 @@ av_cold int ff_vaapi_encode_close(AVCodecContext *avctx) vaapi_encode_free(avctx, pic); } - av_buffer_pool_uninit(&ctx->output_buffer_pool); + ff_refstruct_pool_uninit(&ctx->output_buffer_pool); if (ctx->va_context != VA_INVALID_ID) { vaDestroyContext(ctx->hwctx->display, ctx->va_context); diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h index d0d6cc2adf..d5d6d5eb1b 100644 --- a/libavcodec/vaapi_encode.h +++ b/libavcodec/vaapi_encode.h @@ -103,7 +103,8 @@ typedef struct VAAPIEncodePicture { int nb_param_buffers; VABufferID *param_buffers; - AVBufferRef *output_buffer_ref; + /* Refcounted via the refstruct-API */ + VABufferID *output_buffer_ref; VABufferID output_buffer; void *priv_data; @@ -275,7 +276,7 @@ typedef struct VAAPIEncodeContext { AVHWFramesContext *recon_frames; // Pool of (reusable) bitstream output buffers. - AVBufferPool *output_buffer_pool; + struct FFRefStructPool *output_buffer_pool; // Global parameters which will be applied at the start of the // sequence (includes rate control parameters below). @@ -383,8 +384,11 @@ typedef struct VAAPIEncodeContext { //void *header_data; //size_t header_data_size; - /** Buffered coded data of a pic if it is an non-independent frame. */ - AVBufferRef *coded_buffer_ref; + /** + * Buffered coded data of a pic if it is an non-independent frame. + * This is a RefStruct reference. + */ + VABufferID *coded_buffer_ref; /** Tail data of a pic, now only used for av1 repeat frame header. */ AVPacket *tail_pkt;