[FFmpeg-devel] [PATCH 43/49] avcodec/qsv: Use RefStruct API for memory id (mids) array

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Mon Oct 2 21:13:32 EEST 2023


Avoids allocations and therefore error checks and cleanup code;
also avoids indirections.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
---
Would be nice if someone could test these additional patches;
I only ensured that they compile properly.

Here is current branch (there will be merge conflicts with
vaapi_encode.c and ffv1dec.c when using the patches sent to the ML):
https://github.com/mkver/FFmpeg/commits/refstruct9

 libavcodec/qsv.c          | 55 +++++++++++++++------------------------
 libavcodec/qsv_internal.h | 11 ++++----
 libavcodec/qsvdec.c       |  3 ++-
 libavcodec/qsvenc.c       |  3 ++-
 4 files changed, 31 insertions(+), 41 deletions(-)

diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index 7563625627..4f639e33f5 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -34,6 +34,7 @@
 
 #include "avcodec.h"
 #include "qsv_internal.h"
+#include "refstruct.h"
 
 #define MFX_IMPL_VIA_MASK(impl) (0x0f00 & (impl))
 #define QSV_HAVE_USER_PLUGIN    !QSV_ONEVPL
@@ -741,20 +742,19 @@ int ff_qsv_init_internal_session(AVCodecContext *avctx, QSVSession *qs,
     return 0;
 }
 
-static void mids_buf_free(void *opaque, uint8_t *data)
+static void mids_buf_free(FFRefStructOpaque opaque, void *obj)
 {
-    AVBufferRef *hw_frames_ref = opaque;
+    AVBufferRef *hw_frames_ref = opaque.nc;
     av_buffer_unref(&hw_frames_ref);
-    av_freep(&data);
 }
 
-static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
+static QSVMid *qsv_create_mids(AVBufferRef *hw_frames_ref)
 {
     AVHWFramesContext    *frames_ctx = (AVHWFramesContext*)hw_frames_ref->data;
     AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;
     int                  nb_surfaces = frames_hwctx->nb_surfaces;
 
-    AVBufferRef *mids_buf, *hw_frames_ref1;
+    AVBufferRef *hw_frames_ref1;
     QSVMid *mids;
     int i;
 
@@ -762,35 +762,27 @@ static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
     if (!hw_frames_ref1)
         return NULL;
 
-    mids = av_calloc(nb_surfaces, sizeof(*mids));
+    mids = ff_refstruct_alloc_ext(nb_surfaces * sizeof(*mids), 0,
+                                  hw_frames_ref1, mids_buf_free);
     if (!mids) {
         av_buffer_unref(&hw_frames_ref1);
         return NULL;
     }
 
-    mids_buf = av_buffer_create((uint8_t*)mids, nb_surfaces * sizeof(*mids),
-                                mids_buf_free, hw_frames_ref1, 0);
-    if (!mids_buf) {
-        av_buffer_unref(&hw_frames_ref1);
-        av_freep(&mids);
-        return NULL;
-    }
-
     for (i = 0; i < nb_surfaces; i++) {
         QSVMid *mid = &mids[i];
         mid->handle_pair   = (mfxHDLPair*)frames_hwctx->surfaces[i].Data.MemId;
         mid->hw_frames_ref = hw_frames_ref1;
     }
 
-    return mids_buf;
+    return mids;
 }
 
 static int qsv_setup_mids(mfxFrameAllocResponse *resp, AVBufferRef *hw_frames_ref,
-                          AVBufferRef *mids_buf)
+                          QSVMid *mids)
 {
     AVHWFramesContext    *frames_ctx = (AVHWFramesContext*)hw_frames_ref->data;
     AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;
-    QSVMid                     *mids = (QSVMid*)mids_buf->data;
     int                  nb_surfaces = frames_hwctx->nb_surfaces;
     int i;
 
@@ -811,12 +803,7 @@ static int qsv_setup_mids(mfxFrameAllocResponse *resp, AVBufferRef *hw_frames_re
         return AVERROR(ENOMEM);
     }
 
-    resp->mids[resp->NumFrameActual + 1] = av_buffer_ref(mids_buf);
-    if (!resp->mids[resp->NumFrameActual + 1]) {
-        av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual]);
-        av_freep(&resp->mids);
-        return AVERROR(ENOMEM);
-    }
+    resp->mids[resp->NumFrameActual + 1] = ff_refstruct_ref(mids);
 
     return 0;
 }
@@ -850,7 +837,7 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
             return MFX_ERR_UNSUPPORTED;
         }
 
-        ret = qsv_setup_mids(resp, ctx->hw_frames_ctx, ctx->mids_buf);
+        ret = qsv_setup_mids(resp, ctx->hw_frames_ctx, ctx->mids);
         if (ret < 0) {
             av_log(ctx->logctx, AV_LOG_ERROR,
                    "Error filling an external frame allocation request\n");
@@ -861,7 +848,8 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
         AVHWFramesContext *ext_frames_ctx = (AVHWFramesContext*)ctx->hw_frames_ctx->data;
         mfxFrameInfo      *i  = &req->Info;
 
-        AVBufferRef *frames_ref, *mids_buf;
+        AVBufferRef *frames_ref;
+        QSVMid *mids;
         AVHWFramesContext *frames_ctx;
         AVQSVFramesContext *frames_hwctx;
 
@@ -889,14 +877,14 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
             return MFX_ERR_MEMORY_ALLOC;
         }
 
-        mids_buf = qsv_create_mids(frames_ref);
-        if (!mids_buf) {
+        mids = qsv_create_mids(frames_ref);
+        if (!mids) {
             av_buffer_unref(&frames_ref);
             return MFX_ERR_MEMORY_ALLOC;
         }
 
-        ret = qsv_setup_mids(resp, frames_ref, mids_buf);
-        av_buffer_unref(&mids_buf);
+        ret = qsv_setup_mids(resp, frames_ref, mids);
+        ff_refstruct_unref(&mids);
         av_buffer_unref(&frames_ref);
         if (ret < 0) {
             av_log(ctx->logctx, AV_LOG_ERROR,
@@ -913,7 +901,7 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
 static mfxStatus qsv_frame_free(mfxHDL pthis, mfxFrameAllocResponse *resp)
 {
     av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual]);
-    av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual + 1]);
+    ff_refstruct_unref(&resp->mids[resp->NumFrameActual + 1]);
     av_freep(&resp->mids);
     return MFX_ERR_NONE;
 }
@@ -1105,11 +1093,10 @@ int ff_qsv_init_session_frames(AVCodecContext *avctx, mfxSession *psession,
         qsv_frames_ctx->logctx = avctx;
 
         /* allocate the memory ids for the external frames */
-        av_buffer_unref(&qsv_frames_ctx->mids_buf);
-        qsv_frames_ctx->mids_buf = qsv_create_mids(qsv_frames_ctx->hw_frames_ctx);
-        if (!qsv_frames_ctx->mids_buf)
+        ff_refstruct_unref(&qsv_frames_ctx->mids);
+        qsv_frames_ctx->mids = qsv_create_mids(qsv_frames_ctx->hw_frames_ctx);
+        if (!qsv_frames_ctx->mids)
             return AVERROR(ENOMEM);
-        qsv_frames_ctx->mids    = (QSVMid*)qsv_frames_ctx->mids_buf->data;
         qsv_frames_ctx->nb_mids = frames_hwctx->nb_surfaces;
 
         err = MFXVideoCORE_SetFrameAllocator(session, &frame_allocator);
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index c2d301b4a2..d970cd20f0 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -115,11 +115,12 @@ typedef struct QSVFramesContext {
     AVBufferRef *hw_frames_ctx;
     void *logctx;
 
-    /* The memory ids for the external frames.
-     * Refcounted, since we need one reference owned by the QSVFramesContext
-     * (i.e. by the encoder/decoder) and another one given to the MFX session
-     * from the frame allocator. */
-    AVBufferRef *mids_buf;
+    /**
+     * The memory ids for the external frames.
+     * Refcounted (via the RefStruct API), since we need one reference
+     * owned by the QSVFramesContext (i.e. by the encoder/decoder) and
+     * another one given to the MFX session from the frame allocator.
+     */
     QSVMid *mids;
     int  nb_mids;
 } QSVFramesContext;
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index da700f25e9..e53d320d0d 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -50,6 +50,7 @@
 #include "hwconfig.h"
 #include "qsv.h"
 #include "qsv_internal.h"
+#include "refstruct.h"
 
 #if QSV_ONEVPL
 #include <mfxdispatcher.h>
@@ -875,7 +876,7 @@ static void qsv_decode_close_qsvcontext(QSVContext *q)
     ff_qsv_close_internal_session(&q->internal_qs);
 
     av_buffer_unref(&q->frames_ctx.hw_frames_ctx);
-    av_buffer_unref(&q->frames_ctx.mids_buf);
+    ff_refstruct_unref(&q->frames_ctx.mids);
     av_buffer_pool_uninit(&q->pool);
 }
 
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index a0144b0760..8a607ee5e7 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -41,6 +41,7 @@
 #include "qsv.h"
 #include "qsv_internal.h"
 #include "qsvenc.h"
+#include "refstruct.h"
 
 struct profile_names {
     mfxU16 profile;
@@ -2622,7 +2623,7 @@ int ff_qsv_enc_close(AVCodecContext *avctx, QSVEncContext *q)
     ff_qsv_close_internal_session(&q->internal_qs);
 
     av_buffer_unref(&q->frames_ctx.hw_frames_ctx);
-    av_buffer_unref(&q->frames_ctx.mids_buf);
+    ff_refstruct_unref(&q->frames_ctx.mids);
 
     cur = q->work_frames;
     while (cur) {
-- 
2.34.1



More information about the ffmpeg-devel mailing list