[FFmpeg-cvslog] avcodec/vaapi_encode: Use RefStruct pool API, stop abusing AVBuffer API

Andreas Rheinhardt git at videolan.org
Wed Nov 1 22:02:50 EET 2023


ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt at outlook.com> | Sat Aug  6 16:36:46 2022 +0200| [92abc7266b5a7139d85699cce2b2c94c1e4ed2ed] | committer: Andreas Rheinhardt

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 <andreas.rheinhardt at outlook.com>

> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=92abc7266b5a7139d85699cce2b2c94c1e4ed2ed
---

 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 <inttypes.h>
 #include <string.h>
 
+#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;



More information about the ffmpeg-cvslog mailing list