[FFmpeg-devel] [PATCH 1/3] avutil/hwcontext: add av_hwframe_transfer_wait_all()

[FFmpeg-devel] [PATCH 1/3] avutil/hwcontext: add av_hwframe_transfer_wait_all()

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

When hardware device contexts perform asynchronous operations, there
is nobody to clean up after asynchronous commands emitted by
av_hwframe_transfer_data().

In this case, the only way to cleanly uninit without leaving device resources
and memory hanging, is to add some sort of explicit synchronization point on
uninit.

This command adds the public API function for accomplishing this from the point
of view of an av_hwframe_transfer_data() user.
---
 doc/APIchanges                 |  3 +++
 libavutil/hwcontext.c          |  9 +++++++++
 libavutil/hwcontext.h          | 10 ++++++++++
 libavutil/hwcontext_internal.h |  1 +
 libavutil/version.h            |  2 +-
 5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index ac506f4b56..ade4ff1159 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07
 
 API changes, most recent first:
 
+2025-03-xx - xxxxxxxxxx - lavu 59.58.100 - hwcontext.h
+  Add av_hwframe_transfer_wait_all().
+
 2025-02-xx - xxxxxxxxxx - lavu 59.57.100 - log.h
   Add flags AV_LOG_PRINT_TIME and AV_LOG_PRINT_DATETIME.
 
diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
index f06d49c45c..05c77ac7be 100644
--- a/libavutil/hwcontext.c
+++ b/libavutil/hwcontext.c
@@ -492,6 +492,15 @@ int av_hwframe_transfer_data(AVFrame *dst, const AVFrame *src, int flags)
     return 0;
 }
 
+int av_hwframe_transfer_wait_all(AVBufferRef *hwframe_ref, int flags)
+{
+    FFHWFramesContext *ctx = (FFHWFramesContext*) hwframe_ref->data;
+    if (!ctx->hw_type->frames_sync)
+        return 0;
+
+    return ctx->hw_type->frames_sync(&ctx->p);
+}
+
 int av_hwframe_get_buffer(AVBufferRef *hwframe_ref, AVFrame *frame, int flags)
 {
     FFHWFramesContext *ctxi = (FFHWFramesContext*)hwframe_ref->data;
diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
index 96042ba197..7a0d826e61 100644
--- a/libavutil/hwcontext.h
+++ b/libavutil/hwcontext.h
@@ -401,6 +401,16 @@ int av_hwframe_get_buffer(AVBufferRef *hwframe_ctx, AVFrame *frame, int flags);
  */
 int av_hwframe_transfer_data(AVFrame *dst, const AVFrame *src, int flags);
 
+/**
+ * Explicitly wait for all preceding (possibly asynchronous) transfers to be
+ * completed. No-op for synchronous hardware device types.
+ *
+ * @param hwframe_ctx a reference to an AVHWFramesContext
+ * @param flags currently unused, should be set to zero
+ * @return 0 on success, a negative AVERROR error code on failure.
+ */
+int av_hwframe_transfer_wait_all(AVBufferRef *hwframe_ctx, int flags);
+
 enum AVHWFrameTransferDirection {
     /**
      * Transfer the data from the queried hw frame.
diff --git a/libavutil/hwcontext_internal.h b/libavutil/hwcontext_internal.h
index db23579c9e..e1b8c12424 100644
--- a/libavutil/hwcontext_internal.h
+++ b/libavutil/hwcontext_internal.h
@@ -69,6 +69,7 @@ typedef struct HWContextType {
 
     int              (*frames_init)(AVHWFramesContext *ctx);
     void             (*frames_uninit)(AVHWFramesContext *ctx);
+    int              (*frames_sync)(AVHWFramesContext *ctx);
 
     int              (*frames_get_buffer)(AVHWFramesContext *ctx, AVFrame *frame);
     int              (*transfer_get_formats)(AVHWFramesContext *ctx,
diff --git a/libavutil/version.h b/libavutil/version.h
index ee4a36cb17..4b584fd569 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  59
-#define LIBAVUTIL_VERSION_MINOR  57
+#define LIBAVUTIL_VERSION_MINOR  58
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.47.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avfilter/hwupload: wait for all pending transfers on uninit()

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Uploads can be asynchronous, so we need to wait for them on unit to prevent
a cyclic reference from keeping the hwfc alive indefinitely, since there is
no other way for the hwfc to know when prior commands should be cleaned up.

In theory, it might be possible for this to be signalled backwards by
downstream consumers of the corresponding AVFrames, but this is very nontrivial
as there may be multiple, or even no downstream consumers, such as in the
degenerate case of:

 ffmpeg ... -vf hwupload -f null -

Waiting for outstanding transfers to have completed explicitly in uninit
is the only solution that is both robust, simple, and comes without a
performance penalty.
---
 libavfilter/vf_hwupload.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libavfilter/vf_hwupload.c b/libavfilter/vf_hwupload.c
index 6aafac4d4d..3142aabb17 100644
--- a/libavfilter/vf_hwupload.c
+++ b/libavfilter/vf_hwupload.c
@@ -222,6 +222,15 @@ static av_cold void hwupload_uninit(AVFilterContext *avctx)
 {
     HWUploadContext *ctx = avctx->priv;
 
+    /**
+     * Ensure that all outstanding asynchronous uploads are properly completed.
+     * This is to avoid leaking the context, because nobody else will clean up
+     * after the hwfc while there are still pending asynchronous transfers
+     * referencing it.
+     */
+    if (ctx->hwframes_ref)
+        av_hwframe_transfer_wait_all(ctx->hwframes_ref, 0);
+
     av_buffer_unref(&ctx->hwframes_ref);
     av_buffer_unref(&ctx->hwdevice_ref);
 }
-- 
2.47.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] avutil/hwcontext_vulkan: add frames_sync() implementation

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Using a new internal function to wait explicitly for all execution pools
to be done executing their internal commands.
---
 libavutil/hwcontext_vulkan.c | 15 +++++++++++++++
 libavutil/vulkan.c           | 10 ++++++++++
 libavutil/vulkan.h           |  5 +++++
 3 files changed, 30 insertions(+)

diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index 10521ce685..91f2db44cb 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -2702,6 +2702,18 @@ static void vulkan_frames_uninit(AVHWFramesContext *hwfc)
     av_buffer_pool_uninit(&fp->tmp);
 }
 
+static int vulkan_frames_sync(AVHWFramesContext *hwfc)
+{
+    VulkanDevicePriv *p = hwfc->device_ctx->hwctx;
+    VulkanFramesPriv *fp = hwfc->hwctx;
+
+    ff_vk_exec_pool_wait(&p->vkctx, &fp->compute_exec);
+    ff_vk_exec_pool_wait(&p->vkctx, &fp->upload_exec);
+    ff_vk_exec_pool_wait(&p->vkctx, &fp->download_exec);
+
+    return 0;
+}
+
 static int vulkan_frames_init(AVHWFramesContext *hwfc)
 {
     int err;
@@ -4268,6 +4280,8 @@ static int vulkan_transfer_frame(AVHWFramesContext *hwfc,
         ff_vk_exec_wait(&p->vkctx, exec);
         if (!host_mapped)
             err = copy_buffer_data(hwfc, bufs[0], swf, region, planes, 0);
+    } else if (upload) {
+        ff_vk_exec_wait(&p->vkctx, exec);
     }
 
 end:
@@ -4464,6 +4478,7 @@ const HWContextType ff_hwcontext_type_vulkan = {
     .frames_init            = vulkan_frames_init,
     .frames_get_buffer      = vulkan_get_buffer,
     .frames_uninit          = vulkan_frames_uninit,
+    .frames_sync            = vulkan_frames_sync,
 
     .transfer_get_formats   = vulkan_transfer_get_formats,
     .transfer_data_to       = vulkan_transfer_data_to,
diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
index 1d18bf07c1..f4c51c92d6 100644
--- a/libavutil/vulkan.c
+++ b/libavutil/vulkan.c
@@ -230,6 +230,16 @@ AVVulkanDeviceQueueFamily *ff_vk_qf_find(FFVulkanContext *s,
     return NULL;
 }
 
+void ff_vk_exec_pool_wait(FFVulkanContext *s, FFVkExecPool *pool)
+{
+    FFVulkanFunctions *vk = &s->vkfn;
+    for (int i = 0; i < pool->pool_size; i++) {
+        FFVkExecContext *e = &pool->contexts[i];
+        if (e->had_submission)
+            ff_vk_exec_wait(s, e);
+    }
+}
+
 void ff_vk_exec_pool_free(FFVulkanContext *s, FFVkExecPool *pool)
 {
     FFVulkanFunctions *vk = &s->vkfn;
diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
index 617df952c4..7f4522bf16 100644
--- a/libavutil/vulkan.h
+++ b/libavutil/vulkan.h
@@ -408,6 +408,11 @@ int ff_vk_exec_pool_init(FFVulkanContext *s, AVVulkanDeviceQueueFamily *qf,
                          const void *query_create_pnext);
 void ff_vk_exec_pool_free(FFVulkanContext *s, FFVkExecPool *pool);
 
+/**
+ * Wait until all prior submissions have finished.
+ */
+void ff_vk_exec_pool_wait(FFVulkanContext *s, FFVkExecPool *pool);
+
 /**
  * Retrieve an execution pool. Threadsafe.
  */
-- 
2.47.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".