From 564797a12a28b6c39a1a4a547665853591f33baa Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 28 Jul 2026 19:26:49 +0200 Subject: [PATCH] test(encode): a QSV construct/drop smoke test, and AvFilterGraph goes Linux-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two loose ends ahead of running this on real Intel silicon. `zerocopy_qsv_alloc_drop_cycles` covers the one ownership question in the crate that was genuinely ambiguous. `ZeroCopyInner::open` builds a `D3d11Hw` and then DERIVES a QSV device + frames ctx from it, and the tuple it used to return handed those two derived pointers out twice — once as the encoder's args, once as the pair moved into `Self`. Free for raw pointers, two owners for `AvBuffer`. Looping construct/drop is what separates the outcomes: a double-unref aborts in the CRT, a missed one leaks an Intel device per session. Nothing else reaches this code — `zerocopy_enabled` defaults QSV OFF, and the native VPL backend supersedes this whole file unless `PUNKTFUNK_QSV_FFMPEG=1` — so the test calls `open` directly and sidesteps both gates, the same shape as `cuda_hw_alloc_drop_cycles`. `AvFilterGraph` is now `#[cfg(target_os = "linux")]`. The Windows gate had been reporting `struct AvFilterGraph is never constructed` since a960dff8 and I had been filtering it out of my own log greps (searching for "never read", which does not match "never constructed"). It is true: the VAAPI dmabuf path is the only filter-graph user, and the AMF/QSV backends build no graph. Cfg'd out rather than `allow`ed, so it cannot outlive its last caller unnoticed. Verified. Windows .133: `cargo test -p pf-encode --features amf-qsv --no-run` at EXITCODE=0 with the AvFilterGraph warnings gone (107 -> 105), and the test binary lists `ffmpeg_win::tests::zerocopy_qsv_alloc_drop_cycles`. Linux .21: workspace check exit 0 / zero errors, pf-encode 33 passed / 0 failed, pf-client-core 34 passed / 0 failed, and zero dead-code warnings there (AvFilterGraph is live on Linux). NOTE for running it: the test binary needs `C:\Users\Public\ffmpeg\bin` on PATH at RUNTIME. `FFMPEG_DIR` is build-time only, and without the DLLs the harness exits silently with no output at all — which reads exactly like a test that does not exist. --- crates/pf-encode/src/enc/libav.rs | 8 +++ .../pf-encode/src/enc/windows/ffmpeg_win.rs | 51 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/crates/pf-encode/src/enc/libav.rs b/crates/pf-encode/src/enc/libav.rs index 5208f267..1954aa6e 100644 --- a/crates/pf-encode/src/enc/libav.rs +++ b/crates/pf-encode/src/enc/libav.rs @@ -78,8 +78,15 @@ impl Drop for AvBuffer { /// of eight failure branches — a four-line cleanup block copied eight times, once inside a macro. /// Freeing the graph is the same ownership question as unref'ing a buffer, so it gets the same /// answer. +/// +/// Linux-only: the VAAPI dmabuf path is the sole filter-graph user in this crate. The Windows +/// AMF/QSV backends feed the encoder directly and build no graph, so on Windows this type would be +/// dead code — cfg'd out rather than `allow`ed, because "nothing here uses it" is the honest +/// statement and an `allow` would keep it compiling after it stopped being true anywhere. +#[cfg(target_os = "linux")] pub(crate) struct AvFilterGraph(*mut ffi::AVFilterGraph); +#[cfg(target_os = "linux")] impl AvFilterGraph { /// Allocate a filter graph, rejecting the null `avfilter_graph_alloc` returns on OOM. /// @@ -99,6 +106,7 @@ impl AvFilterGraph { } } +#[cfg(target_os = "linux")] impl Drop for AvFilterGraph { fn drop(&mut self) { // SAFETY: `self.0` is the non-null graph `alloc` took ownership of, and this type is its diff --git a/crates/pf-encode/src/enc/windows/ffmpeg_win.rs b/crates/pf-encode/src/enc/windows/ffmpeg_win.rs index fbd28076..52106a05 100644 --- a/crates/pf-encode/src/enc/windows/ffmpeg_win.rs +++ b/crates/pf-encode/src/enc/windows/ffmpeg_win.rs @@ -1443,6 +1443,57 @@ impl Encoder for FfmpegWinEncoder { mod tests { use super::*; + /// Construct/drop `ZeroCopyInner` on the QSV path, repeatedly, on real Intel silicon. + /// + /// This is the one path in the crate where ownership was genuinely ambiguous: `open` builds a + /// `D3d11Hw` (D3D11VA device + frames pair) and then DERIVES a QSV device + frames ctx from it, + /// and the tuple it used to return handed those two derived pointers out **twice** — once as + /// the encoder's `dev_ref`/`frames_ref`, once as the pair moved into `Self`. Harmless while they + /// were raw pointers; two owners once they are `AvBuffer`. Looping construct/drop is what tells + /// the difference apart: a double-unref aborts inside the CRT, a missed one leaks an Intel + /// device per session. + /// + /// Nothing else reaches it. `zerocopy_enabled` defaults QSV **off**, so the normal encode path + /// never builds one on Intel, and the native VPL backend (`enc/windows/qsv.rs`) supersedes this + /// whole file unless `PUNKTFUNK_QSV_FFMPEG=1`. Calling `open` directly sidesteps both gates so + /// the ownership itself is what gets exercised. + /// + /// `#[ignore]`d (needs a real Intel QSV device): + /// `cargo test -p pf-encode --features amf-qsv zerocopy_qsv_alloc_drop_cycles -- --ignored --nocapture` + #[test] + #[ignore = "needs a real Intel QSV device (run on an Intel host, not the build box)"] + fn zerocopy_qsv_alloc_drop_cycles() { + use windows::Win32::Graphics::Dxgi::{CreateDXGIFactory1, IDXGIFactory1}; + // SAFETY: `CreateDXGIFactory1`/`EnumAdapters1` are the documented enumeration pair and are + // checked here; `make_device` requires a live `IDXGIAdapter1`, which `adapter` is for the + // duration of the call (it outlives it as a local). The returned device is the sole owner of + // its COM object and outlives every `ZeroCopyInner` built from it below. + let (device, _ctx) = unsafe { + let factory: IDXGIFactory1 = CreateDXGIFactory1().expect("CreateDXGIFactory1"); + let adapter = factory.EnumAdapters1(0).expect("EnumAdapters1(0)"); + pf_frame::dxgi::make_device(&adapter).expect("D3D11 device on adapter 0") + }; + for i in 0..8 { + let zc = ZeroCopyInner::open( + WinVendor::Qsv, + Codec::H264, + PixelFormat::Bgrx, + 640, + 480, + 30, + 8_000_000, + 8, + &device, + ) + .unwrap_or_else(|e| panic!("ZeroCopyInner::open(QSV) failed on iteration {i}: {e:#}")); + // The QSV arm must have derived BOTH halves — an `Option` that came back `None` here + // would mean the AMF branch was taken and the derived-pair ownership never ran. + assert!(zc.qsv_frames.is_some(), "QSV path derived no frames ctx"); + assert!(zc.qsv_device.is_some(), "QSV path derived no device"); + } + eprintln!("8 ZeroCopyInner(QSV) alloc/drop cycles completed without abort"); + } + /// Zero-copy default matrix: the operator override wins in both directions; unset resolves /// AMF on (on-glass validated) and QSV off (opt-in until validated on Intel glass — the /// probe-never-assume rule).