[FFmpeg-devel] [PATCH] Fix resource leaks in vsrc_gfxcapture (PR #20543)

[FFmpeg-devel] [PATCH] Fix resource leaks in vsrc_gfxcapture (PR #20543)

[Timo Rothenpieler via ffmpeg-devel]

PR #20543 opened by Timo Rothenpieler (BtbN)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20543
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20543.patch

The first 3 commits aren't strictly fixes for existing issues, but just code cleanup and general improvements found while hunting down the actual leaks.
Using the free threaded pools fixes mysterious internal heap resource leaks on dll unload/RoUninit.
And actually properly ref-counting the Callback-Helper allows the callback handlers to be freed, fixing several leaks that went completely unnoticed.

Lastly, it keeps the cbdata object alive past the return of the helper function it's in, cause re-allocating and freeing it and the mutex/condvar in it can be quite expensive depending on the threading backend used.
winpthreads uses 5 WinAPI objects to represent one condvar, it seems.

This fixes #20537


>From e0ea041e2ec0da49425572d89ffab2030616eabe Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 18:31:11 +0200
Subject: [PATCH 1/6] avfilter/vsrc_gfxcapture: don't pass pointer to ComPtr

While it does appear to work fine, with all the operator overloads, it
at least has potential for surprises, so pass it by reference instead.
---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index 9562863d7f..b037dc89aa 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -436,7 +436,7 @@ static int wgc_setup_gfxcapture_capture(AVFilterContext *avctx)
     return 0;
 }
 
-static int wgc_try_get_next_frame(AVFilterContext *avctx, ComPtr<IDirect3D11CaptureFrame> *capture_frame)
+static int wgc_try_get_next_frame(AVFilterContext *avctx, ComPtr<IDirect3D11CaptureFrame> &capture_frame)
 {
     GfxCaptureContext *cctx = CCTX(avctx->priv);
     GfxCaptureContextCpp *ctx = cctx->ctx;
@@ -447,11 +447,11 @@ static int wgc_try_get_next_frame(AVFilterContext *avctx, ComPtr<IDirect3D11Capt
     ComPtr<ID3D11Texture2D> frame_texture;
     SizeInt32 frame_size = { 0, 0 };
 
-    CHECK_HR_RET(wgctx->frame_pool->TryGetNextFrame(capture_frame->ReleaseAndGetAddressOf()));
-    if (!capture_frame->Get())
+    CHECK_HR_RET(wgctx->frame_pool->TryGetNextFrame(&capture_frame));
+    if (!capture_frame)
         return AVERROR(EAGAIN);
 
-    CHECK_HR_RET(capture_frame->Get()->get_ContentSize(&frame_size));
+    CHECK_HR_RET(capture_frame->get_ContentSize(&frame_size));
     if (frame_size.Width != wgctx->cap_size.Width || frame_size.Height != wgctx->cap_size.Height) {
         av_log(avctx, AV_LOG_VERBOSE, "Capture size changed to %dx%d\n", frame_size.Width, frame_size.Height);
 
@@ -1394,7 +1394,7 @@ static int process_frame_if_exists(AVFilterLink *outlink)
         ComPtr<ID3D11Texture2D> frame_texture;
         TimeSpan frame_time = { 0 };
 
-        ret = wgc_try_get_next_frame(avctx, &capture_frame);
+        ret = wgc_try_get_next_frame(avctx, capture_frame);
         if (ret < 0)
             return ret;
 
-- 
2.49.1


>From ade3b9566e7235401a07bf5dbdce475b86d63f23 Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 19:12:53 +0200
Subject: [PATCH 2/6] avfilter/vsrc_gfxcapture: fix re-using ret variable from
 outside of lambda scope

---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index b037dc89aa..ade14d297f 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -1394,9 +1394,9 @@ static int process_frame_if_exists(AVFilterLink *outlink)
         ComPtr<ID3D11Texture2D> frame_texture;
         TimeSpan frame_time = { 0 };
 
-        ret = wgc_try_get_next_frame(avctx, capture_frame);
-        if (ret < 0)
-            return ret;
+        int res = wgc_try_get_next_frame(avctx, capture_frame);
+        if (res < 0)
+            return res;
 
         CHECK_HR_RET(capture_frame->get_SystemRelativeTime(&frame_time));
 
-- 
2.49.1


>From e588d8e110c5b61422d1dfb37d84a35efc89132b Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 19:13:29 +0200
Subject: [PATCH 3/6] avfilter/vsrc_gfxcapture: stop capture session before
 initializing capture thread shutdown

It might have things going on in the background that still need cleaned
up, and this gives it a chance to enqueue them on the dispatch queue.
---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index ade14d297f..47c3b2db88 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -229,7 +229,7 @@ static void wgc_stop_capture_session(AVFilterContext *avctx) noexcept
     if (wgctx->capture_session) {
         ComPtr<IClosable> closable;
         if (SUCCEEDED(wgctx->capture_session.As(&closable))) {
-            closable->Close();
+            CHECK_HR_LOG(closable->Close());
         } else {
             av_log(avctx, AV_LOG_ERROR, "Failed to get capture session IClosable interface\n");
         }
@@ -243,6 +243,11 @@ static void wgc_stop_capture_session(AVFilterContext *avctx) noexcept
             av_log(avctx, AV_LOG_ERROR, "Failed to get frame pool IClosable interface\n");
         }
     }
+
+    wgctx->capture_session.Reset();
+    wgctx->frame_pool.Reset();
+    wgctx->capture_item.Reset();
+    wgctx->d3d_device.Reset();
 }
 
 static int wgc_calculate_client_area(AVFilterContext *avctx)
@@ -557,6 +562,9 @@ static int wgc_thread_worker(AVFilterContext *avctx)
 
         if (!msg.hwnd && msg.message == WM_WGC_THREAD_SHUTDOWN) {
             av_log(avctx, AV_LOG_DEBUG, "Initializing WGC thread shutdown\n");
+
+            wgc_stop_capture_session(avctx);
+
             if (FAILED(wgctx->dispatcher_queue_controller->ShutdownQueueAsync(&async))) {
                 av_log(avctx, AV_LOG_ERROR, "Failed to shutdown dispatcher queue\n");
                 return AVERROR_EXTERNAL;
-- 
2.49.1


>From 076ac041907f9ddbe3d6b95b00f7bdd0de2b671e Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 20:23:37 +0200
Subject: [PATCH 4/6] avfilter/vsrc_gfxcapture: use free threaded capture frame
 pool

Apparently, using a normal frame pool in a multithreaded environment
leads to strange resource leaks on shutdown, which vanish when using a
free threaded pool.
---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index 47c3b2db88..62b1a94802 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -327,7 +327,7 @@ static int wgc_setup_gfxcapture_session(AVFilterContext *avctx)
     std::unique_ptr<GfxCaptureContextWgc> &wgctx = ctx->wgc;
     int ret;
 
-    ComPtr<IDirect3D11CaptureFramePoolStatics> frame_pool_statics;
+    ComPtr<IDirect3D11CaptureFramePoolStatics2> frame_pool_statics;
     ComPtr<ID3D11Device> d3d11_device = ctx->device_hwctx->device;
     ComPtr<ID3D10Multithread> d3d10_multithread;
     ComPtr<IDXGIDevice> dxgi_device;
@@ -350,8 +350,8 @@ static int wgc_setup_gfxcapture_session(AVFilterContext *avctx)
     CHECK_HR_RET(d3d11_device.As(&dxgi_device));
     CHECK_HR_RET(ctx->fn.CreateDirect3D11DeviceFromDXGIDevice(dxgi_device.Get(), &wgctx->d3d_device));
 
-    CHECK_HR_RET(get_activation_factory<IDirect3D11CaptureFramePoolStatics>(ctx, RuntimeClass_Windows_Graphics_Capture_Direct3D11CaptureFramePool, &frame_pool_statics));
-    CHECK_HR_RET(frame_pool_statics->Create(wgctx->d3d_device.Get(), fmt, CAPTURE_POOL_SIZE, wgctx->cap_size, &wgctx->frame_pool));
+    CHECK_HR_RET(get_activation_factory<IDirect3D11CaptureFramePoolStatics2>(ctx, RuntimeClass_Windows_Graphics_Capture_Direct3D11CaptureFramePool, &frame_pool_statics));
+    CHECK_HR_RET(frame_pool_statics->CreateFreeThreaded(wgctx->d3d_device.Get(), fmt, CAPTURE_POOL_SIZE, wgctx->cap_size, &wgctx->frame_pool));
     CHECK_HR_RET(wgctx->frame_pool->CreateCaptureSession(wgctx->capture_item.Get(), &wgctx->capture_session));
 
     if (SUCCEEDED(wgctx->capture_session.As(&session2))) {
-- 
2.49.1


>From 01c85376323b126fd789ca942561e64541b368a2 Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 21:31:56 +0200
Subject: [PATCH 5/6] avfilter/vsrc_gfxcapture: fix leaking all callback
 handlers

Fixes #20537
---
 libavfilter/vsrc_gfxcapture_winrt.hpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.hpp b/libavfilter/vsrc_gfxcapture_winrt.hpp
index a02e768c8f..ba1979b1c1 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.hpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.hpp
@@ -159,9 +159,9 @@ private:
 template<class Iface, typename... Args, typename F>
 static Microsoft::WRL::ComPtr<Iface> create_cb_handler(F&& cb_func)
 {
-    return Microsoft::WRL::ComPtr<Iface>(
-        new FFTypedCBHandler<Iface, F, Args...>(std::forward<F>(cb_func))
-    );
+    Microsoft::WRL::ComPtr<Iface> res;
+    res.Attach(new FFTypedCBHandler<Iface, F, Args...>(std::forward<F>(cb_func)));
+    return res;
 }
 
 /******************************************
-- 
2.49.1


>From 663854d2ade7106b3a1a5014d9f462740365f66a Mon Sep 17 00:00:00 2001
From: Timo Rothenpieler <timo@rothenpieler.org>
Date: Wed, 17 Sep 2025 22:03:28 +0200
Subject: [PATCH 6/6] avfilter/vsrc_gfxcapture: keep cbdata object alive

Depending on the threading backend the stdlib uses, creating a
mutex/condvar can be quite expensive.
So keep this object alive in the ctx, on which we synchronize via the
uninit mutex anyway.
---
 libavfilter/vsrc_gfxcapture_winrt.cpp | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vsrc_gfxcapture_winrt.cpp b/libavfilter/vsrc_gfxcapture_winrt.cpp
index 62b1a94802..c492b90257 100644
--- a/libavfilter/vsrc_gfxcapture_winrt.cpp
+++ b/libavfilter/vsrc_gfxcapture_winrt.cpp
@@ -149,6 +149,7 @@ struct GfxCaptureContextCpp {
     volatile int wgc_thread_init_res { INT_MAX };
     std::recursive_mutex wgc_thread_uninit_mutex;
     volatile int wgc_thread_res { 0 };
+    std::shared_ptr<void> wgc_thread_cb_data;
 
     HWND capture_hwnd { nullptr };
     HMONITOR capture_hmonitor { nullptr };
@@ -716,11 +717,16 @@ static int run_on_wgc_thread(AVFilterContext *avctx, F &&cb)
     struct CBData {
         std::mutex mutex;
         std::condition_variable cond;
-        bool done { false };
-        bool cancel { false };
-        int ret { AVERROR_BUG };
+        bool done;
+        bool cancel;
+        int ret;
     };
-    auto cbdata = std::make_shared<CBData>();
+    auto cbdata = ctx->wgc_thread_cb_data ?
+                  std::static_pointer_cast<CBData>(ctx->wgc_thread_cb_data) :
+                  std::make_shared<CBData>();
+
+    cbdata->done = cbdata->cancel = false;
+    cbdata->ret = AVERROR_BUG;
 
     boolean res = 0;
     CHECK_HR_RET(wgctx->dispatcher_queue->TryEnqueue(
-- 
2.49.1

_______________________________________________
ffmpeg-devel mailing list -- ffmpeg-devel@ffmpeg.org
To unsubscribe send an email to ffmpeg-devel-leave@ffmpeg.org