From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id D49E846973 for ; Wed, 17 Sep 2025 20:20:34 +0000 (UTC) Authentication-Results: ffbox; dkim=fail (body hash mismatch (got b'KhPJpgf6N7jW9742FkAkK0NWACK4YlvFcLdxosvv8cw=', expected b'pmplhdntQmFeAUURRgQzOc/a5LtobHSMEE+jwEVMPgI=')) header.d=ffmpeg.org header.i=@ffmpeg.org header.a=rsa-sha256 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ffmpeg.org; i=@ffmpeg.org; q=dns/txt; s=mail; t=1758140418; h=mime-version : to : date : message-id : reply-to : subject : list-id : list-archive : list-archive : list-help : list-owner : list-post : list-subscribe : list-unsubscribe : from : cc : content-type : content-transfer-encoding : from; bh=KhPJpgf6N7jW9742FkAkK0NWACK4YlvFcLdxosvv8cw=; b=dv7YOdTNRiUKqbkbJZOWJkC+RU6S8GVjOQ6BtJiVV+g7s8Ce111HpY9PrRQ8rlq5YZCXX s+NFHynstB0KvOjJMKbQEUf8b+7pCsLgIr3v9jvE/7zjuUbkVUjOR9XfzjiNbUDFcs9tSlj rwCIITr/4qiNizwppFvbUzTV2iINiwj65j+lr3DnBtjsQupJxFxMC7RYvprvjsEhD6dN8NX lvkZdGuD+N+lDed6dWZ4GzlUJkGt4jkojQyqgemImznVR1R16cKL7xzAqUuVVMI36ZABw5p +RUfTVSSTpirH3zmb9vcjnpJo2zX7s4+3HTSbPSeCxjR8DeVOpUJmfqq7Scw== Received: from [172.19.0.4] (unknown [172.19.0.4]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 78DF268E7EF; Wed, 17 Sep 2025 23:20:18 +0300 (EEST) ARC-Seal: i=1; cv=none; a=rsa-sha256; d=ffmpeg.org; s=arc; t=1758140415; b=kCBpnBA+U/W1eauPvX+oOvtzV4YruBiLf718TnBwZEc+ZBCrf9bZIFaMMMkq376V6FTQW +0ReC6aPAGWS4E1U88sxUPQCShr+9Ajx8bvTBoREtItG9VRY/IqGcVMrriQTvEhyvEzxKz4 VAml5sUdCIzqNKhSpcWhjZqRQDXp/thyp/+H08Y3usj8cvxxza9zfBRqgmiXthCtmRQjYPx ywyGjuYi1fMysgm9AUE4r9ZKwlD/DQk3H4MqRZzC4lYn0AENlBaps0UBOBXxjRenNWxXzfL 6byu4mzOmKzPt5sCTw964WM8lD1/njwkmVIg+XArDGso9VhF1gBTn4fn4K7g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=ffmpeg.org; s=arc; t=1758140415; h=from : sender : reply-to : subject : date : message-id : to : cc : mime-version : content-type : content-transfer-encoding : content-id : content-description : resent-date : resent-from : resent-sender : resent-to : resent-cc : resent-message-id : in-reply-to : references : list-id : list-help : list-unsubscribe : list-subscribe : list-post : list-owner : list-archive; bh=ze8/Yk/sKWoMS/SF77aJ06J3qlI6/1q5ZQnmn+nkO7Y=; b=XV8a+eD2IgocDdATMlzgB6ROPns1umwV037En+p3UfkLr2To+hy+uc1bE5qu7r514I1ro 0EC913vrwAuDd5ei1jf1YbXwQ0iXPxEMPtgGE29PNVIH/HAZtuYULtk174fNbr9xvHN/6sR bBtJraJh9fkxqu1A1HNRM+MOIFYQ+8JburXKkx03LOA5c1g4mn6fmPh8k7lgdtWrzR9ZC/a QyYlclq0YTAQxuKri5Bc3dfSZJk+x+q3D+qvtj02tn84HS1yjFJqAnIMRlUNUpTd/HdxfS3 /ypQFssV5QUYH2dw+B//9Ekk2LT1KrNz81BG0fG5j+wA57dLr509Ykq8u3Vg== ARC-Authentication-Results: i=1; ffmpeg.org; dkim=pass header.d=ffmpeg.org header.i=@ffmpeg.org; arc=none; dmarc=none Authentication-Results: ffmpeg.org; dkim=pass header.d=ffmpeg.org header.i=@ffmpeg.org; arc=none (Message is not ARC signed); dmarc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ffmpeg.org; i=@ffmpeg.org; q=dns/txt; s=mail; t=1758140404; h=content-type : mime-version : content-transfer-encoding : from : to : reply-to : subject : date : from; bh=pmplhdntQmFeAUURRgQzOc/a5LtobHSMEE+jwEVMPgI=; b=2I1xY6e5cJvrGDNBxLyv887QC41jts3CbzlFokZHvsNmSn2fDoOsNij3E/dPFeR3PAFIv BU07hHZt0lmqh0RTUpXcATr2/z83/51+/CFQQhh+iS/h83q1vA6n9XwQjWfY3Bs14vQ1hth fUbLVnjx6jku740SJdqlRCAw2foNM7hRBy8HTAtwbJhp42ofSJP8OhMEMcOGY1+aC8yBFFd oHVhCY3SVFzI4FvA04OieDnuD8cAHehyX7lYj5/yIGub2yK8tNe9tgILHLbcjdQbisu5iFM Q2j9ooczCF2JnsR2qwHynWxJqlNrk4TDWXZxbIPEgkC2/vigNKapTI4/ncCg== Received: from ed19c606a818 (code.ffmpeg.org [188.245.149.3]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id EB39968D449 for ; Wed, 17 Sep 2025 23:20:03 +0300 (EEST) MIME-Version: 1.0 To: ffmpeg-devel@ffmpeg.org Date: Wed, 17 Sep 2025 20:20:03 -0000 Message-ID: <175814040433.25.16545521889251544433@463a07221176> Message-ID-Hash: OJY3LIDDVL4WPV6CBKRIWLH33ODNJULQ X-Message-ID-Hash: OJY3LIDDVL4WPV6CBKRIWLH33ODNJULQ X-MailFrom: code@ffmpeg.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; header-match-ffmpeg-devel.ffmpeg.org-0; header-match-ffmpeg-devel.ffmpeg.org-1; header-match-ffmpeg-devel.ffmpeg.org-2; header-match-ffmpeg-devel.ffmpeg.org-3; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list Reply-To: FFmpeg development discussions and patches Subject: [FFmpeg-devel] [PATCH] Fix resource leaks in vsrc_gfxcapture (PR #20543) List-Id: FFmpeg development discussions and patches Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: From: Timo Rothenpieler via ffmpeg-devel Cc: Timo Rothenpieler Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Archived-At: List-Archive: List-Post: 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 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 *capture_frame) +static int wgc_try_get_next_frame(AVFilterContext *avctx, ComPtr &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 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 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 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 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 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 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 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 &wgctx = ctx->wgc; int ret; - ComPtr frame_pool_statics; + ComPtr frame_pool_statics; ComPtr d3d11_device = ctx->device_hwctx->device; ComPtr d3d10_multithread; ComPtr 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(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(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 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 static Microsoft::WRL::ComPtr create_cb_handler(F&& cb_func) { - return Microsoft::WRL::ComPtr( - new FFTypedCBHandler(std::forward(cb_func)) - ); + Microsoft::WRL::ComPtr res; + res.Attach(new FFTypedCBHandler(std::forward(cb_func))); + return res; } /****************************************** -- 2.49.1 >>From 663854d2ade7106b3a1a5014d9f462740365f66a Mon Sep 17 00:00:00 2001 From: Timo Rothenpieler 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 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(); + auto cbdata = ctx->wgc_thread_cb_data ? + std::static_pointer_cast(ctx->wgc_thread_cb_data) : + std::make_shared(); + + 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