From 4b57d11dd8c956fccfc77f7b0af3f657df6ff6c8 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 21:11:52 +0200 Subject: [PATCH 01/12] =?UTF-8?q?test(pf-encode):=20S1=20spike=20=E2=80=94?= =?UTF-8?q?=20splitEncodeMode=20CAN=20change=20in=20place,=20no=20IDR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two on-hardware spikes answering the gate on the split-encode engagement program (design/nvenc-split-encode-engagement-implementation-plan.md). S1a `nvenc_cuda_split_reconfigure_in_place`: can splitEncodeMode change via nvEncReconfigureEncoder with resetEncoder=0, without an IDR? Our "reconfigure must present the SAME init params as the open" rule (windows/nvenc.rs:620) is our own invariant and had never been tested against a driver. It reports rather than asserts the verdict -- both outcomes are legitimate findings -- and only asserts what would invalidate the measurement (session live, engines >= 2, the arms actually differ). Sub-frame is pinned off so the driver can't reject for the wrong reason (HEVC forced-split and sub-frame are mutually unsupported). S1b `nvenc_cuda_split_reconfigure_takes_effect`: the other half -- a driver that accepts the parameter and quietly ignores it looks identical to one that honours it. Three legs at 4K (fresh DISABLE / fresh TWO_FORCED / DISABLE->TWO in place); if C tracks B and not A, the switch is real. RESULT on .21 (RTX 5070 Ti, GB203 Blackwell, driver 610.57.04): NV_ENC_CAPS_NUM_ENCODER_ENGINES = 2 S1a: accepted, ZERO IDRs, both directions. S1b: A fresh DISABLE 5054 us/frame, B fresh TWO_FORCED 2453, C switched in place 2419 -- |C-B|=34 vs |C-A|=2635. It takes effect, and split is a clean ~2x at 4K. Two limits, both recorded in the test docs rather than the commit only. The frames come out at 427 B/AU against an 833 KB CBR quota: the driver hands back zeroed VRAM, so the rotated buffers are identical and rate control skip-codes everything. So this measures the PIXEL-proportional half of the cost only -- the bits/frame regime the field case lives in is untested here, and the test prints an explicit INCONCLUSIVE-on-content line when it detects that. And this is Blackwell 8-bit; the Ada Main10 question is untouched. Verified on .21: clippy -p pf-encode --features nvenc --all-targets -D warnings clean, both spikes green, cargo fmt --all --check clean. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 266 +++++++++++++++++++ 1 file changed, 266 insertions(+) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 56d15b33..d4f01dea 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -2911,6 +2911,272 @@ mod tests { println!("nvenc_cuda reconfigure smoke: 20→60→10 Mbps in place, zero IDRs"); } + /// ON-HARDWARE — **spike S1** (`design/nvenc-split-encode-engagement-implementation-plan.md`): + /// can `splitEncodeMode` change via `nvEncReconfigureEncoder` with `resetEncoder=0`, WITHOUT + /// emitting an IDR? + /// + /// This is the gate on the whole split-engagement program. `splitEncodeMode` lives in + /// `NV_ENC_INITIALIZE_PARAMS`, and our own invariant says a reconfigure "must present the SAME + /// init params as the open" (`windows/nvenc.rs:620`) — but that is OUR rule, never tested + /// against the driver. A forced mid-stream IDR is not acceptable (user), so: + /// - **driver rejects the change** → the constraint is real; the split decision is + /// once-per-session and must be predicted at open. + /// - **accepts it AND the next AU is not a keyframe** → mid-stream adaptation is free, and the + /// engagement rule can simply be re-resolved whenever ABR moves. + /// - **accepts it but emits an IDR anyway** → same as a rejection for our purposes. This is the + /// case a naive "did it return Ok?" check would get wrong, which is why the keyframe count + /// below is the real assertion. + /// + /// Sub-frame is pinned OFF for the whole test: HEVC forced-split and sub-frame readback are + /// mutually unsupported (`resolve_split_subframe`), so leaving it on would have the driver + /// reject the reconfigure for the WRONG reason and read as a false negative. + /// + /// Reports rather than asserts the verdict — S1 is a measurement, and BOTH outcomes are + /// legitimate findings. It only asserts the things that would invalidate the measurement + /// itself (session came up, engines ≥ 2, the arms actually differ). Run ALONE (it sets env): + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_split_reconfigure_in_place --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_split_reconfigure_in_place() { + use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + const W: u32 = 1920; + const H: u32 = 1080; + const BPS: u64 = 40_000_000; + let disable = M::NV_ENC_SPLIT_DISABLE_MODE as u32; + let two = M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32; + + // Isolate the split variable: sub-frame off, and open explicitly split-DISABLED so the + // switch below is a real change rather than a no-op. + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", "0"); + + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + BPS, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + + let submit_and_poll = |enc: &mut NvencCudaEncoder, range: std::ops::Range| { + let (mut aus, mut keyframes) = (0usize, 0usize); + for i in range { + let frame = nv12_frame(W, H, i); + enc.submit_indexed(&frame, i).expect("submit"); + while let Some(au) = enc.poll().expect("poll") { + aus += 1; + keyframes += au.keyframe as usize; + } + } + (aus, keyframes) + }; + + // Frames first: the session is lazily created on the first submit, and + // `reconfigure_bitrate` short-circuits to `true` while `!inited` (no session to reconfigure + // yet), which would make the whole spike vacuous. + let (aus, kfs) = submit_and_poll(&mut enc, 0..4); + assert!(aus > 0, "no AUs before the reconfigure"); + assert_eq!(kfs, 1, "exactly the opening IDR before the reconfigure"); + assert!( + enc.inited, + "session must be live for the spike to mean anything" + ); + assert_eq!( + enc.split_mode, disable, + "the spike needs to OPEN split-disabled so the switch is a real change" + ); + + // Engine count (WP1.1's probe, borrowed): forced-2 on a 1-NVENC GPU would be rejected for a + // reason that has nothing to do with reconfigure, so the verdict is only interpretable + // when the card actually has a second engine. + // SAFETY: `enc.encoder` is the live session (`inited` asserted above); `get_cap` only reads + // a cap through it and returns 0 on any driver error. + let engines = unsafe { + enc.get_cap( + enc.encoder, + nv::NV_ENC_CAPS::NV_ENC_CAPS_NUM_ENCODER_ENGINES, + ) + }; + println!("S1: NV_ENC_CAPS_NUM_ENCODER_ENGINES = {engines}"); + assert!( + engines >= 2, + "this GPU reports {engines} NVENC engine(s) — S1 is not interpretable here, run it on \ + a 2-engine card" + ); + + // THE SPIKE: change ONLY splitEncodeMode (same bitrate, same everything else) and ask the + // driver to take it in place. + enc.split_mode = two; + let accepted = enc.reconfigure_bitrate(BPS); + println!("S1: reconfigure DISABLE→TWO_FORCED accepted = {accepted}"); + + let verdict = if !accepted { + // Restore the field so the encoder's idea of its own session stays truthful for the + // rest of the test (the live session is still split-disabled). + enc.split_mode = disable; + "FAIL — driver REJECTED the in-place splitEncodeMode change" + } else { + let (aus, kfs) = submit_and_poll(&mut enc, 4..8); + assert!(aus > 0, "no AUs after the accepted reconfigure"); + if kfs == 0 { + "PASS — accepted with NO IDR: mid-stream split adaptation is free" + } else { + "FAIL — accepted but forced an IDR (silently), which is the same as a rejection" + } + }; + println!("S1 VERDICT: {verdict}"); + + // The reverse direction only means something if the forward one worked. + if accepted { + enc.split_mode = disable; + let back = enc.reconfigure_bitrate(BPS); + let kfs = if back { + submit_and_poll(&mut enc, 8..12).1 + } else { + usize::MAX + }; + println!("S1: reverse TWO_FORCED→DISABLE accepted = {back}, keyframes after = {kfs}"); + } + + enc.flush().ok(); + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + + /// ON-HARDWARE — **spike S1b**, the other half of S1: an in-place `splitEncodeMode` change that + /// the driver ACCEPTS without an IDR is worthless if the driver then quietly ignores it, and + /// "accepted, no IDR" looks identical in both cases. So measure whether it took effect. + /// + /// Three legs at 4K (where split has something to bite on), same bitrate throughout: + /// A. fresh session, split DISABLED + /// B. fresh session, split TWO_FORCED + /// C. session opened DISABLED, then reconfigured in place to TWO_FORCED + /// If C ≈ B and both differ from A, the reconfigure is real. If C ≈ A, the driver accepted the + /// parameter and dropped it on the floor. + /// + /// ⚠ **Reads out bytes/AU as well as timing, and that column is load-bearing**: these frames + /// are uninitialised device memory, so under CBR rate control can run out of things to code + /// and every leg collapses to the same trivially-cheap encode — which would make the A/B/C + /// comparison meaningless rather than negative. Tiny or identical byte counts ⇒ the run says + /// nothing, and the real answer needs the content path WP0 route (b) uses. Run ALONE: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_split_reconfigure_takes_effect --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_split_reconfigure_takes_effect() { + use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + use std::time::Instant; + const W: u32 = 3840; + const H: u32 = 2160; + const BPS: u64 = 400_000_000; + const WARMUP: u32 = 8; + const MEASURED: u32 = 24; + let two = M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32; + + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + + // Separate buffers rotated per frame, so identical content can't let the encoder + // skip-code everything and erase the difference we are trying to measure. + // ⚠ MEASURED 2026-08-06: this does NOT work — the driver hands back zeroed VRAM, so all + // four are identical anyway and the legs come out at ~427 B/AU against an 833 KB CBR + // quota. What survives is the PIXEL-proportional half of the cost (motion estimation over + // 8.29 Mpix); the bits/frame half is untested by this harness. Read the printout's + // INCONCLUSIVE-on-content line before drawing any bitrate conclusion from it. + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let frames: Vec = (0..4).map(|i| nv12_frame(W, H, i)).collect(); + + // Returns (p50 encode µs, median bytes/AU). + let run_leg = |open_split: &str, switch_to: Option| -> (u128, usize) { + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", open_split); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + BPS, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + + let (mut times, mut sizes) = (Vec::new(), Vec::new()); + for i in 0..(WARMUP + MEASURED) { + // Flip to the target mode exactly once, after warmup, in place. + if i == WARMUP { + if let Some(target) = switch_to { + enc.split_mode = target; + assert!( + enc.reconfigure_bitrate(BPS), + "in-place split switch must be accepted (S1a proved it is)" + ); + continue; + } + } + let t0 = Instant::now(); + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + let mut got = 0usize; + while let Some(au) = enc.poll().expect("poll") { + got = au.data.len(); + } + let dt = t0.elapsed().as_micros(); + if i >= WARMUP { + times.push(dt); + sizes.push(got); + } + } + enc.flush().ok(); + times.sort_unstable(); + sizes.sort_unstable(); + (times[times.len() / 2], sizes[sizes.len() / 2]) + }; + + let (a_us, a_bytes) = run_leg("0", None); + let (b_us, b_bytes) = run_leg("2", None); + let (c_us, c_bytes) = run_leg("0", Some(two)); + + println!("S1b @ {W}x{H}@60 HEVC 8-bit, {} Mbps CBR:", BPS / 1_000_000); + println!(" A fresh DISABLE : {a_us:>6} us/frame, {a_bytes:>8} B/AU"); + println!(" B fresh TWO_FORCED : {b_us:>6} us/frame, {b_bytes:>8} B/AU"); + println!(" C DISABLE→TWO in situ: {c_us:>6} us/frame, {c_bytes:>8} B/AU"); + + let want_bytes = (BPS / 60 / 8) as usize; + if a_bytes * 4 < want_bytes { + println!( + " ⚠ INCONCLUSIVE on content: {a_bytes} B/AU is far below the {want_bytes} B/AU \ + CBR quota — rate control ran out of things to code, so these legs are not the \ + high-bits/frame regime the field case is in." + ); + } + let (near_b, near_a) = (c_us.abs_diff(b_us), c_us.abs_diff(a_us)); + println!( + " ⇒ C is nearer {} (|C-B|={near_b} vs |C-A|={near_a}) — {}", + if near_b < near_a { "B" } else { "A" }, + if near_b < near_a { + "the in-place split switch TOOK EFFECT" + } else { + "the driver appears to have IGNORED the in-place split change" + } + ); + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + let _ = (a_bytes, b_bytes, c_bytes); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA From 70b81ac3d74da457bfabc9090204d2e752bf2db8 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 21:27:18 +0200 Subject: [PATCH 02/12] =?UTF-8?q?test(pf-encode):=20S1c=20+=20the=20D5=20c?= =?UTF-8?q?onfirm=20=E2=80=94=20pair=20flips=20in=20place,=20AUTO=20really?= =?UTF-8?q?=20is=20dead?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S1c `nvenc_cuda_split_subframe_pair_reconfigure`: the leg S1a/S1b excluded. Both pinned sub-frame OFF to isolate the split variable, but a real HEVC arbitration cannot -- split and sub-frame are mutually unsupported there, so engaging split means flipping enableSubFrameWrite in the same breath, a second init param and the one the reconfigure path deliberately pins. RESULT on .21: the PAIR moves in place, accepted, ZERO IDRs, both directions. It also pins the invariant that makes this safe to build on: `subframe_chunks` is latched ONLY in the init path (~line 1625) and is NOT recomputed by reconfigure_bitrate, so a caller flipping sub-frame in place must clear it too or supports_chunked_poll keeps reporting true and poll_chunk busy-polls its whole budget every AU against a numSlices that never advances. The test performs the correct sequence and asserts the state stays coherent, so WP3 has a worked example rather than a warning. `nvenc_cuda_auto_split_with_subframe`: the D5 confirm -- the one claim in the design's defect list that was only ever inferred. The driver reports no "mode I actually chose", so it is settled by timing, at 4K where the gap is ~2x. RESULT: AUTO (env unset) + sub-frame 4904 us/frame, DISABLE + sub-frame 5062, TWO_FORCED without sub-frame 3464. AUTO sits 158 us from DISABLE and 1440 from TWO_FORCED ⇒ D5 CONFIRMED: plain AUTO does not split while sub-frame is on, so the resolver's AUTO fallthrough reads as "let the driver decide" and means "never split". ⚠ TRAP, hit on this test's first run and now documented in it: the env knob CANNOT express plain AUTO. `0` is DISABLE and `1` is AUTO_FORCED, and resolve_split_subframe counts AUTO_FORCED as forced, so passing `1` silently disarms sub-frame and measures a different configuration entirely -- which produced a spurious "D5 REFUTED". Plain AUTO is only reachable as the resolver's fallthrough with the env unset. The leg now asserts sub-frame resolved TRUE, so the test can no longer answer the wrong question quietly. Verified on .21: clippy --features nvenc --all-targets -D warnings clean, all 4 spikes green, the normal 54-test suite unaffected, cargo fmt --all --check clean. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 239 +++++++++++++++++++ 1 file changed, 239 insertions(+) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index d4f01dea..730292e4 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -3177,6 +3177,245 @@ mod tests { let _ = (a_bytes, b_bytes, c_bytes); } + /// ON-HARDWARE — **spike S1c**, the leg S1a/S1b deliberately excluded. Both pinned sub-frame + /// OFF to isolate the split variable, but a real HEVC arbitration cannot: split and sub-frame + /// readback are mutually unsupported there (`resolve_split_subframe`), so engaging split means + /// flipping `enableSubFrameWrite` in the same breath — a SECOND init param, and the one the + /// reconfigure path deliberately pins today (`windows/nvenc.rs:624-628`). + /// + /// So: can the PAIR move in place? `(DISABLE, sub-frame on)` → `(TWO_FORCED, sub-frame off)`, + /// `resetEncoder=0`, and back. Accepted? IDR-free? + /// + /// ⚠ Also pins the invariant that makes this safe to build on: `subframe_chunks` is latched + /// ONLY in the init path (line ~1625) and is NOT recomputed by `reconfigure_bitrate`, so a + /// caller flipping sub-frame in place MUST clear it too — otherwise `supports_chunked_poll` + /// keeps reporting true and `poll_chunk` busy-polls its whole budget every AU against a + /// `numSlices` that never advances. That is the exact failure the Phase 8 comment warns about + /// for an in-params drop; here the test performs the correct sequence and asserts the state + /// stays coherent, so WP3 has a worked example to copy. Run ALONE: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_split_subframe_pair_reconfigure --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_split_subframe_pair_reconfigure() { + use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + const W: u32 = 1920; + const H: u32 = 1080; + const BPS: u64 = 40_000_000; + let disable = M::NV_ENC_SPLIT_DISABLE_MODE as u32; + let two = M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32; + + // Open split-DISABLED, and leave sub-frame at its Linux default (ON where the GPU + // advertises SUBFRAME_READBACK) — that is the fleet shape the arbitration starts from. + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", "0"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + BPS, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + + let submit_and_poll = |enc: &mut NvencCudaEncoder, range: std::ops::Range| { + let (mut aus, mut keyframes) = (0usize, 0usize); + for i in range { + let frame = nv12_frame(W, H, i); + enc.submit_indexed(&frame, i).expect("submit"); + while let Some(au) = enc.poll().expect("poll") { + aus += 1; + keyframes += au.keyframe as usize; + } + } + (aus, keyframes) + }; + + let (aus, kfs) = submit_and_poll(&mut enc, 0..4); + assert!(aus > 0 && kfs == 1, "opening IDR then steady P-frames"); + println!( + "S1c: opened split={} subframe_on={} subframe_chunks={} chunked_poll={}", + enc.split_mode, + enc.subframe_on, + enc.subframe_chunks, + enc.supports_chunked_poll() + ); + if !enc.subframe_on { + println!( + "S1c SKIPPED: sub-frame is off at open on this GPU/driver, so there is no pair to \ + flip — the arbitration reduces to S1a's plain split switch here." + ); + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + return; + } + + // THE PAIR FLIP, in the order WP3 must use: clear the chunked-poll latch alongside the + // sub-frame flag, or `poll_chunk` outlives the feature it depends on. + enc.split_mode = two; + enc.subframe_on = false; + enc.subframe_chunks = false; + let accepted = enc.reconfigure_bitrate(BPS); + println!("S1c: (DISABLE,sub-frame on) → (TWO_FORCED,sub-frame off) accepted = {accepted}"); + + if accepted { + let (aus, kfs) = submit_and_poll(&mut enc, 4..8); + assert!(aus > 0, "no AUs after the pair flip"); + assert!( + !enc.supports_chunked_poll(), + "chunked poll must be disarmed once sub-frame is off — a stale latch makes \ + poll_chunk busy-poll its whole budget every AU" + ); + println!( + "S1c VERDICT: {}", + if kfs == 0 { + "PASS — the split×sub-frame PAIR moves in place with NO IDR" + } else { + "FAIL — pair flip forced an IDR" + } + ); + + // …and back, which is what a de-escalation would do. + enc.split_mode = disable; + enc.subframe_on = true; + enc.subframe_chunks = enc.slices >= 2 && enc.async_rt.is_none(); + let back = enc.reconfigure_bitrate(BPS); + let kfs_back = if back { + submit_and_poll(&mut enc, 8..12).1 + } else { + usize::MAX + }; + println!("S1c: reverse pair flip accepted = {back}, keyframes after = {kfs_back}"); + } else { + println!( + "S1c VERDICT: FAIL — driver REJECTED the pair flip. Split can still move alone \ + (S1a), so a WP3 arbitration would have to keep sub-frame fixed for the session \ + and only arbitrate split within that." + ); + enc.split_mode = disable; + enc.subframe_on = true; + } + + enc.flush().ok(); + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + } + + /// ON-HARDWARE — **the D5 confirm** (design §2 defect D5), the one claim in that list that was + /// only ever *inferred*: plain `AUTO` + default-on sub-frame is believed to resolve to + /// no-split, because HEVC split is unsupported *with* sub-frame — which would make the + /// resolver's `AUTO` fallthrough read as "let the driver decide" while actually meaning "never + /// split", on both platforms. + /// + /// The driver reports no "mode I actually chose", so this settles it the same way S1b settled + /// its question: by timing. At 4K the split/no-split gap is unmissable (~2×), so + /// AUTO+sub-frame ≈ DISABLE ⇒ the driver did NOT split ⇒ D5 CONFIRMED + /// AUTO+sub-frame ≈ TWO_FORCED ⇒ it did ⇒ D5 REFUTED and the `AUTO` arm is fine as-is + /// + /// Content is trivial here for the reason `nvenc_cuda_split_reconfigure_takes_effect` + /// documents (zeroed VRAM), so this compares the PIXEL-proportional cost — which is exactly + /// the term split halves, so the discriminator holds. Run ALONE: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_auto_split_with_subframe --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_auto_split_with_subframe() { + use std::time::Instant; + const W: u32 = 3840; + const H: u32 = 2160; + const BPS: u64 = 400_000_000; + const WARMUP: u32 = 8; + const MEASURED: u32 = 24; + + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let frames: Vec = (0..4).map(|i| nv12_frame(W, H, i)).collect(); + + // (split env, sub-frame env) → p50 µs, plus the resolved sub-frame state for the printout. + // `split: None` means UNSET, which is the only way to reach the resolver's plain-`AUTO` + // fallthrough: the env knob cannot express it (`0` is DISABLE, `1` is AUTO_**FORCED**), + // and AUTO_FORCED counts as forced in `resolve_split_subframe`, so passing `1` here would + // silently disarm sub-frame and test a completely different configuration. That mistake + // produced a spurious "D5 REFUTED" on the first run of this test. + let run = |split: Option<&str>, subframe: Option<&str>| -> (u128, bool) { + match split { + Some(v) => std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", v), + None => std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"), + } + match subframe { + Some(v) => std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", v), + None => std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"), + } + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + BPS, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + let mut times = Vec::new(); + for i in 0..(WARMUP + MEASURED) { + let t0 = Instant::now(); + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + while enc.poll().expect("poll").is_some() {} + if i >= WARMUP { + times.push(t0.elapsed().as_micros()); + } + } + let sub = enc.subframe_on; + enc.flush().ok(); + times.sort_unstable(); + (times[times.len() / 2], sub) + }; + + // THE FLEET CASE: env unset ⇒ 4K60 8-bit is below SPLIT_FORCE_PIXEL_RATE (497.7 vs 950 + // Mpix/s) and not 10-bit, so the resolver falls through to plain AUTO, and sub-frame + // stays at its caps-gated default. This leg must report sub-frame TRUE or it is not + // testing D5. + let (auto_us, auto_sub) = run(None, None); + let (dis_us, dis_sub) = run(Some("0"), None); + let (two_us, two_sub) = run(Some("2"), Some("0")); + + println!("D5 confirm @ {W}x{H}@60 HEVC 8-bit:"); + println!(" AUTO (unset) + sub-frame({auto_sub}) : {auto_us:>6} us/frame"); + println!(" DISABLE + sub-frame({dis_sub}) : {dis_us:>6} us/frame"); + println!(" TWO_FORCED, no sub-frame({two_sub}): {two_us:>6} us/frame"); + assert!( + auto_sub, + "the AUTO leg resolved sub-frame OFF — it is not testing D5's fleet shape" + ); + let (near_dis, near_two) = (auto_us.abs_diff(dis_us), auto_us.abs_diff(two_us)); + println!( + " ⇒ AUTO sits nearer {} (|A-D|={near_dis} vs |A-T|={near_two}) — D5 {}", + if near_dis < near_two { + "DISABLE" + } else { + "TWO" + }, + if near_dis < near_two { + "CONFIRMED: AUTO + sub-frame does NOT split; the resolver's AUTO arm is dead" + } else { + "REFUTED: AUTO does engage the second engine even with sub-frame on" + } + ); + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA From 88f29a941160abc105648bcf19e493d1eef5f92d Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 21:53:14 +0200 Subject: [PATCH 03/12] feat(pf-encode): use every NVENC engine the GPU has, not a hard-coded two MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP1.1 plus the engine-count fix. `resolve_split_mode` forced TWO_FORCED at high pixel rate regardless of hardware, so a 3-NVENC part (GB202, AD102 workstation) left a third of its encode silicon idle, and a 1-NVENC part paid a wasted session open to discover it could not split. Probes NV_ENC_CAPS_NUM_ENCODER_ENGINES in both direct-SDK backends' query_caps (the cap is `= 49` in both linux_sys and windows_sys of the vendored SDK 0.4.0 -- the caps enum is cfg-selected per-OS, so that was checked) and latches it on a backend field. NOT on EncoderCaps: nine backends construct that struct as exhaustive literals, so a new field would be a 9-site change of which 7 are unrelated codecs passing a meaningless value, and the only consumer is the resolver. New `max_forced_split_mode(engines)`: 1 -> DISABLE, 2 -> TWO, 3 -> THREE, and >3 -> AUTO_FORCED, because NV_ENC_SPLIT_ENCODE_MODE cannot NAME more than three (NVENCAPI 12.1; values 4..14 are unallocated, so a future API may extend it) and AUTO_FORCED = "split, driver picks how many" is measurably a real split (2.01x vs disabled on .21). 0 = unprobed keeps the historical two-engine assumption. ⚠ WHY THE CLAMP EXISTS, measured on .21 (RTX 5070 Ti, 2 NVENC, 4K HEVC): requesting THREE_FORCED was HONOURED -- session opened in mode 3 -- and ran at 2303 us/frame, identical to TWO_FORCED's 2308. The driver does not reject an over-ask; it silently encodes narrower. So the rejection fallback cannot find the ceiling and PUNKTFUNK_SPLIT_ENCODE=3 on a 2-engine card would have logged a 3-way split over a 2-way encode. Operator overrides are now clamped with a warn. The ordering trap is covered by a test: on a >3-engine part hw_max is AUTO_FORCED (1), which is not "narrower than" TWO_FORCED (2) despite comparing smaller, so a naive min() would collapse a legitimate 3-way request to AUTO. Also adds `engines` and `subframe` to the Linux session-ready log: split_mode alone is ambiguous between "used both engines" and "left a third idle", and since the driver honours an over-wide request the mode cannot be read without the ceiling it was chosen from. This is the line a field report needs. --- and a correction to S1b, in the same change --- Re-running S1b afterwards flipped its verdict to "the driver appears to have IGNORED the in-place split change", contradicting the isolated runs that produced the |C-B|=34 figure already written into the design docs. Investigated rather than re-rolled. The switched leg was landing MIDWAY between the arms (~3600 us against A~5050, B~2300) and the nearest-neighbour verdict flipped on noise. Cause: split-encode does not reach steady state on the first frame -- a FRESH TWO_FORCED session shows it too (early-half 3280 us vs late-half 1996 in one run), so it is split warmup generally, not something specific to reconfiguring in place. A single median over the whole window cannot see that. The test now reports early-half vs late-half and gives a switched leg SETTLE=16 frames before its window opens, every leg the same length. With that, 4/4 runs agree: the switched leg reaches ~2030 us against a fresh-split ~2000 and a single-engine ~4900. ⚠ S1b's CONCLUSION stands (the switch does take effect) but the evidence behind the committed number did not reproduce; the docs are corrected rather than left implying a cleaner result than the harness could support. ⚠⚠ This is a WP3 REQUIREMENT, not just a test fix: a live-session arbitration that switches arms and immediately measures will misjudge the arm it just chose, because the encoder needs ~16 frames to settle. The settle window has to be part of the arbitration, and it is now a measured number rather than a guess. Verified on .21: clippy --features nvenc --all-targets -D warnings clean, 57 unit tests (3 new), all 23 NVENC on-hardware tests green, fmt clean. The 3 failing on-hw tests in a full --ignored run are VAAPI (no AMD/Intel GPU on that box -- their own ignore reason says so), pre-existing and unrelated. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 207 +++++++++++++++++-- crates/pf-encode/src/enc/nvenc_core.rs | 168 ++++++++++++++- crates/pf-encode/src/enc/windows/nvenc.rs | 14 +- 3 files changed, 363 insertions(+), 26 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 730292e4..0b291f4d 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -821,6 +821,11 @@ pub struct NvencCudaEncoder { /// Sub-frame chunked poll armed for the live session (§7 LN1 Phase 1): multi-slice + /// sub-frame readback configured AND sync retrieve at init. See [`Encoder::poll_chunk`]. subframe_chunks: bool, + /// `NV_ENC_CAPS_NUM_ENCODER_ENGINES` — how many NVENC engines this GPU has, probed in + /// [`query_caps`]. `0` = not probed / unreadable. The split-encode ceiling: the driver accepts + /// a split wider than the hardware and silently encodes narrower, so this is the only honest + /// source for how wide we may go (see `nvenc_core::max_forced_split_mode`). + encoder_engines: u32, /// In-progress chunked readback of the front in-flight AU. See [`ChunkState`]. chunk: Option, } @@ -909,6 +914,7 @@ impl NvencCudaEncoder { subframe_on: false, subframe_forced: false, subframe_chunks: false, + encoder_engines: 0, chunk: None, }) } @@ -1081,6 +1087,10 @@ impl NvencCudaEncoder { // consumed when slice-level readback lands. Not stored — LN1 re-probes when it configures. let subframe = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_SUPPORT_SUBFRAME_READBACK); let dyn_slice = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_SUPPORT_DYNAMIC_SLICE_MODE); + // How many NVENC engines this GPU has — the split-encode ceiling. Must be probed rather + // than inferred from a rejection: the driver ACCEPTS a split wider than the hardware and + // silently encodes narrower (measured on `.21`, see `max_forced_split_mode`). + let engines = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_NUM_ENCODER_ENGINES); let _ = (api().destroy_encoder)(enc); if wmax > 0 && hmax > 0 && (self.width as i32 > wmax || self.height as i32 > hmax) { @@ -1100,6 +1110,7 @@ impl NvencCudaEncoder { self.rfi_supported = rfi != 0; self.custom_vbv = custom_vbv != 0; self.subframe_cap = subframe != 0; + self.encoder_engines = engines.max(0) as u32; // Phase-3 default-on (nvenc-subframe-slice-output.md): 4 slices + sub-frame readback on // every Linux direct-NVENC session, resolved HERE (before the session opens) so the // config author, the init params and the chunked-poll latch all agree; the caps probe @@ -1334,7 +1345,8 @@ impl NvencCudaEncoder { // 2-way NVENC split-frame encoding (Ada dual-NVENC) — shared selector, see // [`resolve_split_mode`] for the precedence (env override / 10-bit / pixel rate). let pixel_rate = self.width as u64 * self.height as u64 * self.fps.max(1) as u64; - let split_mode: u32 = resolve_split_mode(self.bit_depth, pixel_rate); + let split_mode: u32 = + resolve_split_mode(self.bit_depth, pixel_rate, self.encoder_engines); // Split × sub-frame arbitration (Phase 8) BEFORE the ladder, the ceiling key and the // chunked-poll latch — all three must see the post-arbitration truth (a drop inside // build_init_params would leave poll_chunk busy-polling its whole budget per AU). @@ -1639,6 +1651,12 @@ impl NvencCudaEncoder { // INFO+, and "did 4K120 actually split across engines?" was undiagnosable from // a user log without it (Windows only had a debug! at selection time). split_mode = self.split_mode, + // …and how many engines the GPU HAS, so `split_mode` can be read against the + // ceiling it was chosen from. Without it a log showing split_mode=2 is ambiguous + // between "used both engines" and "left a third engine idle", and the driver + // silently honours an over-wide request, so the mode alone cannot be trusted. + engines = self.encoder_engines, + subframe = self.subframe_on, "NVENC CUDA session ready" ); Ok(()) @@ -2498,6 +2516,12 @@ mod tests { assert_eq!(slot_fmt_of(F::NV_ENC_BUFFER_FORMAT_ARGB), SlotFormat::Argb); } + /// The `encoder_engines` field `query_caps` latched — read through a helper so the intent + /// ("what the resolver will actually see") is explicit at the call site. + fn self_engines(enc: &NvencCudaEncoder) -> u32 { + enc.encoder_engines + } + fn nv12_frame(w: u32, h: u32, i: u32) -> CapturedFrame { // Content is uninitialized device memory — NVENC encodes it fine; this smoke test asserts the // session/registration/encode/RFI machinery, not picture fidelity (that's the on-glass A/B). @@ -3006,7 +3030,20 @@ mod tests { nv::NV_ENC_CAPS::NV_ENC_CAPS_NUM_ENCODER_ENGINES, ) }; - println!("S1: NV_ENC_CAPS_NUM_ENCODER_ENGINES = {engines}"); + println!( + "S1: NV_ENC_CAPS_NUM_ENCODER_ENGINES = {engines} (query_caps latched \ + encoder_engines={})", + self_engines(&enc) + ); + // The cap is only useful if `query_caps` actually stored it — that latched field is what + // `resolve_split_mode` reads to pick the split width, so a silent 0 there would quietly + // fall back to "assume two engines" on every GPU. + assert_eq!( + self_engines(&enc), + engines.max(0) as u32, + "query_caps must latch NUM_ENCODER_ENGINES — resolve_split_mode reads that field, \ + not the live cap" + ); assert!( engines >= 2, "this GPU reports {engines} NVENC engine(s) — S1 is not interpretable here, run it on \ @@ -3080,6 +3117,12 @@ mod tests { const BPS: u64 = 400_000_000; const WARMUP: u32 = 8; const MEASURED: u32 = 24; + /// Frames to discard AFTER an in-place switch before measuring. Split-encode does not + /// reach steady state on the first frame — even a FRESH `TWO_FORCED` session shows it + /// (early-half 3280 µs vs late-half 1996 in one run) — and without this the switched leg + /// lands midway between the two arms and the verdict flips run to run. Measured: at 16 + /// the switched leg reaches the fresh-split steady state; at 0 it did so only sometimes. + const SETTLE: u32 = 16; let two = M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32; std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); @@ -3094,8 +3137,8 @@ mod tests { pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); let frames: Vec = (0..4).map(|i| nv12_frame(W, H, i)).collect(); - // Returns (p50 encode µs, median bytes/AU). - let run_leg = |open_split: &str, switch_to: Option| -> (u128, usize) { + // Returns (early-half p50 µs, late-half p50 µs, median bytes/AU). + let run_leg = |open_split: &str, switch_to: Option| -> (u128, u128, usize) { std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", open_split); let mut enc = NvencCudaEncoder::open( Codec::H265, @@ -3112,8 +3155,15 @@ mod tests { ) .expect("open NVENC CUDA session"); + // Every leg is measured over the SAME number of frames; a switched leg just starts its + // window `SETTLE` frames later, so the arms stay comparable. + let measure_from = if switch_to.is_some() { + WARMUP + SETTLE + } else { + WARMUP + }; let (mut times, mut sizes) = (Vec::new(), Vec::new()); - for i in 0..(WARMUP + MEASURED) { + for i in 0..(measure_from + MEASURED) { // Flip to the target mode exactly once, after warmup, in place. if i == WARMUP { if let Some(target) = switch_to { @@ -3133,25 +3183,43 @@ mod tests { got = au.data.len(); } let dt = t0.elapsed().as_micros(); - if i >= WARMUP { + if i >= measure_from { times.push(dt); sizes.push(got); } } enc.flush().ok(); - times.sort_unstable(); + // Split the window in half. A single median over the whole post-switch run is + // ACTIVELY MISLEADING here: leg C's median landed midway between the two arms and the + // nearest-neighbour verdict flipped run to run. Early-vs-late says whether the switch + // SETTLES — which a median cannot. + let half = times.len() / 2; + let med = |s: &[u128]| { + let mut v = s.to_vec(); + v.sort_unstable(); + v[v.len() / 2] + }; + let (early, late) = (med(×[..half]), med(×[half..])); sizes.sort_unstable(); - (times[times.len() / 2], sizes[sizes.len() / 2]) + (early, late, sizes[sizes.len() / 2]) }; - let (a_us, a_bytes) = run_leg("0", None); - let (b_us, b_bytes) = run_leg("2", None); - let (c_us, c_bytes) = run_leg("0", Some(two)); + let (a_early, a_late, a_bytes) = run_leg("0", None); + let (b_early, b_late, b_bytes) = run_leg("2", None); + let (c_early, c_late, c_bytes) = run_leg("0", Some(two)); + let (a_us, b_us, c_us) = (a_late, b_late, c_late); println!("S1b @ {W}x{H}@60 HEVC 8-bit, {} Mbps CBR:", BPS / 1_000_000); - println!(" A fresh DISABLE : {a_us:>6} us/frame, {a_bytes:>8} B/AU"); - println!(" B fresh TWO_FORCED : {b_us:>6} us/frame, {b_bytes:>8} B/AU"); - println!(" C DISABLE→TWO in situ: {c_us:>6} us/frame, {c_bytes:>8} B/AU"); + println!(" (early = first half of the measured window, late = second half)"); + println!(" A fresh DISABLE : early {a_early:>6} late {a_late:>6} us/frame, {a_bytes:>8} B/AU"); + println!(" B fresh TWO_FORCED : early {b_early:>6} late {b_late:>6} us/frame, {b_bytes:>8} B/AU"); + println!(" C DISABLE→TWO in situ: early {c_early:>6} late {c_late:>6} us/frame, {c_bytes:>8} B/AU"); + if c_early > c_late + c_late / 8 { + println!( + " ⇒ leg C SETTLES ({c_early} → {c_late} us): the in-place switch is not \ + instantaneous, so a whole-window median understates it." + ); + } let want_bytes = (BPS / 60 / 8) as usize; if a_bytes * 4 < want_bytes { @@ -3416,6 +3484,117 @@ mod tests { std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); } + /// ON-HARDWARE — **what is the real split ceiling on this GPU?** Feeds WP1.1: we want to use + /// every engine the card has, not a hard-coded 2. + /// + /// `NV_ENC_SPLIT_ENCODE_MODE` tops out at `THREE_FORCED` in SDK 0.4.0 / NVENCAPI 12.1 (values + /// 4..14 are unallocated, so a future API could add more), and `AUTO_FORCED` means "split, you + /// pick how many" — the only way to name a count we have no enum for. + /// + /// For each candidate this reports what the session ACTUALLY opened with, which is the honest + /// signal: the backend's rejection fallback silently retries split-disabled, so a mode the + /// driver refuses shows up as `split_mode == DISABLE` afterwards rather than as an error. And + /// the timing says whether an ACCEPTED mode did anything — a card that takes `THREE_FORCED` + /// but only has two engines would otherwise look like a win. Run ALONE: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_split_hardware_max --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_split_hardware_max() { + use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + use std::time::Instant; + const W: u32 = 3840; + const H: u32 = 2160; + const BPS: u64 = 400_000_000; + const WARMUP: u32 = 8; + const MEASURED: u32 = 24; + + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let frames: Vec = (0..4).map(|i| nv12_frame(W, H, i)).collect(); + + // → (requested mode, mode actually opened, p50 µs, engines the driver reports) + let run = |split: &str| -> (u32, u128, i32) { + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", split); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + BPS, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + let mut times = Vec::new(); + for i in 0..(WARMUP + MEASURED) { + let t0 = Instant::now(); + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + while enc.poll().expect("poll").is_some() {} + if i >= WARMUP { + times.push(t0.elapsed().as_micros()); + } + } + // SAFETY: the session is live (frames encoded above); `get_cap` only reads a cap and + // returns 0 on any driver error. + let engines = unsafe { + enc.get_cap( + enc.encoder, + nv::NV_ENC_CAPS::NV_ENC_CAPS_NUM_ENCODER_ENGINES, + ) + }; + let opened = enc.split_mode; + enc.flush().ok(); + times.sort_unstable(); + (opened, times[times.len() / 2], engines) + }; + + println!("split ceiling probe @ {W}x{H}@60 HEVC 8-bit:"); + let mut baseline = None; + // The env value is NOT the enum value for DISABLE (`0` selects `NV_ENC_SPLIT_DISABLE_MODE`, + // which is 15), so compare against the enum each arm actually asks for. + for (label, env, want) in [ + ("DISABLE ", "0", M::NV_ENC_SPLIT_DISABLE_MODE as u32), + ("AUTO_FORCED ", "1", M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32), + ("TWO_FORCED ", "2", M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32), + ( + "THREE_FORCED", + "3", + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + ), + ] { + let (opened, us, engines) = run(env); + let honoured = opened == want; + let vs = match baseline { + None => { + baseline = Some(us); + String::new() + } + Some(b) => format!(" ({:.2}× vs DISABLE)", b as f64 / us as f64), + }; + println!( + " req {label} → opened_mode={opened:<2} {} {us:>6} us/frame{vs} [engines={engines}]", + if honoured { + "HONOURED" + } else { + "FELL BACK" + } + ); + } + println!( + " note: opened_mode 15 = DISABLE (the backend's rejection fallback); a mode that is \ + HONOURED but no faster than DISABLE was accepted and did nothing." + ); + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index b591f754..c8118ede 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -81,37 +81,98 @@ pub(super) fn resolve_subframe(default_on: bool) -> bool { /// Linux direct-SDK backends (they had drifted into byte-identical duplicates, one of which /// logged and one didn't). Precedence: /// 1. `PUNKTFUNK_SPLIT_ENCODE` = `0`/`disable` | `1`/`auto` (AUTO_FORCED) | `2` | `3` — operator -/// override, always wins. +/// override, always wins, except that `2`/`3` are clamped to the GPU's real engine count (see +/// [`clamp_to_engines`]; the driver honours an over-ask and silently encodes narrower). /// 2. 10-bit → DISABLE: 2-way split is measurably SLOWER on Ada for Main10 — at 5120×1440@240 /// forced-2 took 7.6 ms/frame (~131 fps) vs 2.8 ms (~357 fps) single-engine (the split/merge /// overhead dominates), and a single engine handles 5K@240 Main10 well under budget. This was /// the "broken animations in HDR" cap at ~131 fps. -/// 3. Pixel rate ≥ [`super::SPLIT_FORCE_PIXEL_RATE`] → force 2-way (AUTO never engages below -/// ~2112 px height, so 4K120 must be forced onto the second engine). -/// 4. Else AUTO (the ~2% BD-rate split cost isn't worth it at low pixel rates). +/// 3. Pixel rate ≥ [`super::SPLIT_FORCE_PIXEL_RATE`] → force the WIDEST split the GPU can deliver +/// ([`max_forced_split_mode`]), not a hard-coded 2 (AUTO never engages below ~2112 px height, +/// so 4K120 must be forced onto the other engines; and a 3-NVENC part left at 2-way wastes a +/// third of its encode silicon). +/// 4. Else AUTO — ⚠ which measurably means **never split** whenever sub-frame readback is on, i.e. +/// the whole default Linux/Windows fleet (4K: AUTO+sub-frame 4904 µs vs DISABLE+sub-frame 5062 +/// vs TWO_FORCED 3464, measured on `.21`). Kept for now because changing it is a behaviour +/// change beyond the engine-count fix; the plan's WP1 retires this arm. /// /// The caller still owns the rejection fallback (retry split-disabled) — a codec/config that /// rejects the chosen mode downgrades at open, not here. -pub(super) fn resolve_split_mode(bit_depth: u8, pixel_rate: u64) -> u32 { +/// +/// `engines` is the GPU's `NV_ENC_CAPS_NUM_ENCODER_ENGINES`; pass `0` when it could not be probed +/// (treated as "unknown", which keeps the pre-probe behaviour of assuming a second engine exists +/// and letting the open-time rejection fallback sort it out). +pub(super) fn resolve_split_mode(bit_depth: u8, pixel_rate: u64, engines: u32) -> u32 { use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + let hw_max = max_forced_split_mode(engines); let mode = match std::env::var("PUNKTFUNK_SPLIT_ENCODE").ok().as_deref() { Some("0") | Some("disable") => M::NV_ENC_SPLIT_DISABLE_MODE as u32, Some("1") | Some("auto") => M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32, - Some("3") => M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, - Some("2") => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + Some("3") => clamp_to_engines(M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, hw_max, engines), + Some("2") => clamp_to_engines(M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, hw_max, engines), _ if bit_depth >= 10 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, - _ if pixel_rate >= super::SPLIT_FORCE_PIXEL_RATE => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + // Use every engine the card has, not a hard-coded two: on a 3-NVENC part (GB202, AD102 + // workstation) forcing 2 leaves a third of the silicon idle. + _ if pixel_rate >= super::SPLIT_FORCE_PIXEL_RATE => hw_max, _ => M::NV_ENC_SPLIT_AUTO_MODE as u32, }; tracing::debug!( split_mode = mode, bit_depth, pixel_rate, + engines, "NVENC split-encode mode selected" ); mode } +/// The strongest split mode this GPU's engine count can actually deliver. +/// +/// ⚠ **The driver will NOT tell you when you over-ask.** Measured on `.21` (RTX 5070 Ti, 2 NVENC, +/// driver 610.57.04, 4K HEVC): requesting `THREE_FORCED` was **HONOURED** — session opened in mode +/// 3 — and ran at **2303 µs/frame, identical to `TWO_FORCED`'s 2308**. No rejection, no warning, +/// no third engine; just a log line claiming 3-way over a 2-way encode. So the rejection fallback +/// cannot be relied on to find the ceiling and the clamp has to happen here. +/// +/// `NV_ENC_SPLIT_ENCODE_MODE` can only *name* counts up to three (SDK 0.4.0 / NVENCAPI 12.1; +/// values 4..14 are unallocated, so a future API may extend it). Above that we fall back to +/// `AUTO_FORCED` = "split, driver picks how many", which measurably does force a split (2.01× vs +/// disabled on the same box) and is the only way to express "use everything you have". +pub(super) fn max_forced_split_mode(engines: u32) -> u32 { + use nv::NV_ENC_SPLIT_ENCODE_MODE as M; + match engines { + // Unknown (cap unreadable / not probed): keep the historical assumption of a second + // engine and let the open-time rejection fallback correct it. + 0 => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + 1 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, + 2 => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + 3 => M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + // More engines than the enum can name — let the driver use them all. + _ => M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32, + } +} + +/// Hold an operator's `PUNKTFUNK_SPLIT_ENCODE=2|3` to what the hardware can deliver, loudly. +/// Without this the knob silently lies (see [`max_forced_split_mode`]); an override that asks for +/// more engines than exist is a mistake worth surfacing, not honouring. +fn clamp_to_engines(requested: u32, hw_max: u32, engines: u32) -> u32 { + // Only the named N-way modes are ordered; `hw_max` may be AUTO_FORCED (1) on a >3-engine part, + // which is not "less than" TWO_FORCED and must not clamp a legitimate request down. + let named = |m: u32| (2..=3).contains(&m); + if engines != 0 && named(requested) && named(hw_max) && requested > hw_max { + tracing::warn!( + requested, + engines, + using = hw_max, + "PUNKTFUNK_SPLIT_ENCODE asks for more NVENC engines than this GPU has — clamping. \ + (The driver would ACCEPT the over-ask and silently encode with fewer, so the log \ + would otherwise claim a split width that never happened.)" + ); + return hw_max; + } + requested +} + /// Whether the operator EXPLICITLY forced sub-frame readback on (`PUNKTFUNK_NVENC_SUBFRAME=1`) /// — the log-severity input to [`resolve_split_subframe`]: a forced knob being overridden /// deserves a `warn`, a default being tuned an `info`. Callers LATCH this once next to their @@ -382,7 +443,7 @@ mod tests { // 4090 because AUTO never engages at 2160 px height. let four_k_120 = 3840u64 * 2160 * 120; assert_eq!( - resolve_split_mode(8, four_k_120), + resolve_split_mode(8, four_k_120, 2), M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 ); } @@ -392,7 +453,7 @@ mod tests { // 884.7 Mpix/s is comfortably single-engine — the threshold move must not drag it in. let qhd_240 = 2560u64 * 1440 * 240; assert_eq!( - resolve_split_mode(8, qhd_240), + resolve_split_mode(8, qhd_240, 2), M::NV_ENC_SPLIT_AUTO_MODE as u32 ); } @@ -403,11 +464,96 @@ mod tests { // vs 2.8 ms single-engine at 5K240) — 10-bit precedes the pixel-rate arm. let five_k_240 = 5120u64 * 1440 * 240; assert_eq!( - resolve_split_mode(10, five_k_240), + resolve_split_mode(10, five_k_240, 2), M::NV_ENC_SPLIT_DISABLE_MODE as u32 ); } + /// THE ENGINE-COUNT FIX: a high-pixel-rate session must use every engine the GPU has, not a + /// hard-coded two. A 3-NVENC part (GB202 / AD102 workstation) left at 2-way wastes a third of + /// its encode silicon, and the driver never complains because it accepts an over- OR + /// under-wide request without comment. + #[test] + fn split_uses_every_engine_the_gpu_has() { + let four_k_120 = 3840u64 * 2160 * 120; + assert_eq!( + resolve_split_mode(8, four_k_120, 3), + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + "a 3-engine GPU must split three ways" + ); + assert_eq!( + resolve_split_mode(8, four_k_120, 1), + M::NV_ENC_SPLIT_DISABLE_MODE as u32, + "a 1-engine GPU must not pretend to split — today this costs a wasted session open" + ); + assert_eq!( + resolve_split_mode(8, four_k_120, 0), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + "unprobed engine count keeps the historical assumption; the rejection fallback corrects" + ); + } + + /// `NV_ENC_SPLIT_ENCODE_MODE` cannot NAME more than three (SDK 0.4.0 / NVENCAPI 12.1), so a + /// hypothetical wider part falls back to AUTO_FORCED = "split, driver picks how many" — which + /// is measurably a real split (2.01× vs disabled on `.21`), not a no-op. + #[test] + fn split_beyond_three_engines_delegates_to_the_driver() { + assert_eq!( + max_forced_split_mode(4), + M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32 + ); + assert_eq!( + max_forced_split_mode(8), + M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32 + ); + } + + /// An operator over-ask must be clamped, because the DRIVER WON'T: measured on `.21` (2 NVENC), + /// `THREE_FORCED` was honoured and ran identically to `TWO_FORCED` (2303 vs 2308 µs/frame) — + /// a log claiming a 3-way split over a 2-way encode. Clamping keeps the log honest. + #[test] + fn operator_override_is_clamped_to_real_engine_count() { + assert_eq!( + clamp_to_engines( + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + max_forced_split_mode(2), + 2 + ), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + "asking for 3 on a 2-engine card must clamp to 2" + ); + // Within budget → untouched. + assert_eq!( + clamp_to_engines( + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + max_forced_split_mode(3), + 3 + ), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 + ); + // Unknown engine count must not clamp — we have nothing to clamp against. + assert_eq!( + clamp_to_engines( + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + max_forced_split_mode(0), + 0 + ), + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32 + ); + // ⚠ The ordering trap: on a >3-engine part `hw_max` is AUTO_FORCED (1), which is NOT + // "narrower than" TWO_FORCED (2) despite comparing smaller. A naive `min` would clamp a + // legitimate 3-way request down to AUTO on the widest hardware we support. + assert_eq!( + clamp_to_engines( + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + max_forced_split_mode(4), + 4 + ), + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, + "a 4-engine GPU must honour an explicit 3-way request, not collapse it to AUTO" + ); + } + #[test] fn ceiling_cache_round_trips_and_keys_precisely() { let key = CeilingKey { diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index c960080b..54c7534b 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -592,6 +592,11 @@ pub struct NvencD3d11Encoder { /// sub-frame readback (the Linux backend's rule since its Phase 3; Windows joined after the /// 2026-07-31 on-glass A/B), so a GPU without it never has sub-frame forced by default. subframe_cap: bool, + /// `NV_ENC_CAPS_NUM_ENCODER_ENGINES` — how many NVENC engines this GPU has, probed in + /// [`query_caps`](Self::query_caps). `0` = not probed / unreadable. The split-encode ceiling: + /// the driver accepts a split wider than the hardware and silently encodes narrower, so this + /// is the only honest source for how wide we may go (see `nvenc_core::max_forced_split_mode`). + encoder_engines: u32, /// (bitstream, mapped input resource to unmap after retrieval, pts_ns, recovery-anchor) per /// in-flight encode. The fourth field tags the first frame encoded after a successful /// [`invalidate_ref_frames`](Encoder::invalidate_ref_frames) — the clean re-anchor P-frame the @@ -753,6 +758,7 @@ impl NvencD3d11Encoder { input_ring_depth: None, async_supported: false, subframe_cap: false, + encoder_engines: 0, pending: VecDeque::new(), frame_idx: 0, force_kf: false, @@ -928,6 +934,10 @@ impl NvencD3d11Encoder { ); let async_enc = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_ASYNC_ENCODE_SUPPORT); let subframe = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_SUPPORT_SUBFRAME_READBACK); + // How many NVENC engines this GPU has — the split-encode ceiling. Must be probed rather + // than inferred from a rejection: the driver ACCEPTS a split wider than the hardware and + // silently encodes narrower (measured on `.21`, see `max_forced_split_mode`). + let engines = self.get_cap(enc, nv::NV_ENC_CAPS::NV_ENC_CAPS_NUM_ENCODER_ENGINES); let _ = (api().destroy_encoder)(enc); // Reject an over-range mode with a clear message instead of an opaque InvalidParam. @@ -962,6 +972,7 @@ impl NvencD3d11Encoder { self.custom_vbv = custom_vbv != 0; self.async_supported = async_enc != 0; self.subframe_cap = subframe != 0; + self.encoder_engines = engines.max(0) as u32; tracing::info!( rfi = self.rfi_supported, custom_vbv = self.custom_vbv, @@ -1154,7 +1165,8 @@ impl NvencD3d11Encoder { // precedence (env override / the measured Main10 don't-split rule / pixel rate). // The init-failure fallback below disables it if a codec/config rejects it. let pixel_rate = self.width as u64 * self.height as u64 * self.fps.max(1) as u64; - let split_mode: u32 = resolve_split_mode(self.bit_depth, pixel_rate); + let split_mode: u32 = + resolve_split_mode(self.bit_depth, pixel_rate, self.encoder_engines); // Negotiated multi-slice (P2f): the direct-NVENC default of 4, clamped by the // client's ceiling — a single-slice client keeps today's shape, a // VIDEO_CAP_MULTI_SLICE / Moonlight slices-per-frame client gets real slices. From 9a1d8be4ccf3c9ec851652cfceffd9f06a5a0607 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 22:25:10 +0200 Subject: [PATCH 04/12] =?UTF-8?q?fix(pf-encode):=20AUTO=20split=20is=20con?= =?UTF-8?q?ditional=20on=20sub-frame=20=E2=80=94=20do=20NOT=20retire=20the?= =?UTF-8?q?=20arm?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Last change's docs concluded "AUTO never splits, retire the arm" from the sub-frame-ON measurement alone. Measured the missing leg before implementing it, and the conclusion was wrong. On .21 at 4K, plain AUTO (env unset, the resolver's fallthrough): sub-frame ON -> 5023/5157 us/frame ~= DISABLE 4979/5000 (does NOT split) sub-frame OFF -> 2401/2352 us/frame ~= TWO_FORCED 2319/2378 (DOES split) So AUTO is CONDITIONAL, not dead. Retiring it would have silently cost every sub-frame-off session its second engine -- a regression introduced while "cleaning up" an arm that looked inert. Split and sub-frame are mutually unsupported for HEVC, so the driver resolves AUTO to no-split only in that combination. Fix is disclosure, not removal: - resolve_split_subframe debug-logs the inert HEVC + AUTO + sub-frame case, which is the fleet default shape: "split_mode=AUTO" has meant "no split" for every default session and nothing said so. Deliberately NOT rewritten to DISABLE -- the mode we pass is what the driver was actually given, and the ceiling-cache key must keep describing that. - New unit test `auto_survives_the_arbitration_in_both_subframe_states` pins the contract so the arm cannot be simplified away later. - The resolver doc now records both measured legs instead of "AUTO is dead". Also in this change: - WP1.6: `resolve_subframe`'s doc said "Windows passes `false`". Stale since the 2026-07-31 .173 A/B flipped Windows to caps-gated default-on. It mattered: it made the AUTO-plus-sub-frame dead combination look Linux-only when it is fleet-wide. - Windows session-ready log parity: split_mode + engines + subframe. The Windows line had no split_mode at all, so a Windows field report could not answer "did this session actually split?" -- the question that started this whole thread. Verified: fmt clean; .21 clippy -p pf-encode --features nvenc --all-targets -D warnings clean, 58 unit tests (1 new), 22/22 NVENC on-hardware tests green; .133 Windows clippy --features nvenc --all-targets -D warnings clean (15m cold, zero errors or warnings) -- the Windows backend is cfg'd out on both macOS and the Linux box, so that leg needed a real Windows host. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 18 +++++++ crates/pf-encode/src/enc/nvenc_core.rs | 56 +++++++++++++++++--- crates/pf-encode/src/enc/windows/nvenc.rs | 9 ++++ 3 files changed, 77 insertions(+), 6 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 0b291f4d..413d895b 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -3456,11 +3456,29 @@ mod tests { let (auto_us, auto_sub) = run(None, None); let (dis_us, dis_sub) = run(Some("0"), None); let (two_us, two_sub) = run(Some("2"), Some("0")); + // The leg that decides whether the `AUTO` arm can simply be RETIRED: D5 proves AUTO does + // not split while sub-frame is on, but retiring it would also change sub-frame-OFF + // sessions, where AUTO is free to split and might. Measure before removing. + let (auto_nosub_us, auto_nosub_sub) = run(None, Some("0")); println!("D5 confirm @ {W}x{H}@60 HEVC 8-bit:"); println!(" AUTO (unset) + sub-frame({auto_sub}) : {auto_us:>6} us/frame"); println!(" DISABLE + sub-frame({dis_sub}) : {dis_us:>6} us/frame"); println!(" TWO_FORCED, no sub-frame({two_sub}): {two_us:>6} us/frame"); + println!(" AUTO (unset), no sub-frame({auto_nosub_sub}): {auto_nosub_us:>6} us/frame"); + println!( + " ⇒ with sub-frame OFF, AUTO is nearer {} — retiring the AUTO arm {}", + if auto_nosub_us.abs_diff(two_us) < auto_nosub_us.abs_diff(dis_us) { + "TWO_FORCED (it DOES split)" + } else { + "DISABLE (it does not split either way)" + }, + if auto_nosub_us.abs_diff(two_us) < auto_nosub_us.abs_diff(dis_us) { + "would LOSE a real split on sub-frame-off sessions" + } else { + "is behaviour-neutral" + } + ); assert!( auto_sub, "the AUTO leg resolved sub-frame OFF — it is not testing D5's fleet shape" diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index c8118ede..7e3f44ec 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -67,8 +67,11 @@ pub(super) fn resolve_slices(codec: Codec, default_slices: u32) -> u32 { /// Resolved sub-frame readback (`enableSubFrameWrite` + `reportSliceOffsets`; sync sessions /// only, see [`build_init_params`]): `PUNKTFUNK_NVENC_SUBFRAME` tri-state — `0` = never (the /// default-on escape), `1` = force (even where the caps probe says unsupported — an operator -/// explicitly testing), unset = the backend's `default_on` (Linux direct-NVENC passes its -/// SUBFRAME_READBACK caps-probe result since Phase 3; Windows passes `false`). +/// explicitly testing), unset = the backend's `default_on` — which is the GPU's +/// `SUBFRAME_READBACK` caps-probe result on **both** backends now (Linux since Phase 3, Windows +/// since the 2026-07-31 `.173` A/B). This comment used to say "Windows passes `false`"; it had +/// been stale since that flip, which mattered because it made the AUTO-plus-sub-frame dead +/// combination look Linux-only when it is fleet-wide. pub(super) fn resolve_subframe(default_on: bool) -> bool { match std::env::var("PUNKTFUNK_NVENC_SUBFRAME").as_deref() { Ok("0") => false, @@ -91,10 +94,15 @@ pub(super) fn resolve_subframe(default_on: bool) -> bool { /// ([`max_forced_split_mode`]), not a hard-coded 2 (AUTO never engages below ~2112 px height, /// so 4K120 must be forced onto the other engines; and a 3-NVENC part left at 2-way wastes a /// third of its encode silicon). -/// 4. Else AUTO — ⚠ which measurably means **never split** whenever sub-frame readback is on, i.e. -/// the whole default Linux/Windows fleet (4K: AUTO+sub-frame 4904 µs vs DISABLE+sub-frame 5062 -/// vs TWO_FORCED 3464, measured on `.21`). Kept for now because changing it is a behaviour -/// change beyond the engine-count fix; the plan's WP1 retires this arm. +/// 4. Else AUTO — ⚠ whose behaviour is **conditional on sub-frame**, measured on `.21` at 4K: +/// - sub-frame **ON** (the fleet default): AUTO **does not split** — 5023/5157 µs against +/// DISABLE's 4979/5000. Split and sub-frame are mutually unsupported for HEVC, so the driver +/// resolves AUTO to no-split and this arm silently means DISABLE. +/// - sub-frame **OFF**: AUTO **does split** — 2401/2352 µs against TWO_FORCED's 2319/2378. +/// +/// So AUTO is NOT dead in general and must not be retired: doing so would lose a real split on +/// every sub-frame-off session. It is dead only in the sub-frame-on combination, which +/// [`resolve_split_subframe`] logs rather than silently accepting. /// /// The caller still owns the rejection fallback (retry split-disabled) — a codec/config that /// rejects the chosen mode downgrades at open, not here. @@ -238,6 +246,20 @@ pub(super) fn resolve_split_subframe( } return (split_mode, false); } + // The silently-inert combination, made visible. HEVC + plain AUTO + sub-frame: the driver + // cannot split (mutually unsupported) so it resolves AUTO to no-split — MEASURED on `.21` at + // 4K, AUTO+sub-frame 5023/5157 µs vs DISABLE's 4979/5000, while the same AUTO with sub-frame + // OFF splits at 2401/2352 vs TWO_FORCED's 2319/2378. This is the fleet's default shape, so + // "split_mode=AUTO" in a log has meant "no split" for every default session and nothing said + // so. Deliberately NOT rewritten to DISABLE: the mode we pass is what the driver was actually + // given, and the ceiling-cache key must keep describing that. + if codec == Codec::H265 && subframe && split_mode == M::NV_ENC_SPLIT_AUTO_MODE as u32 { + tracing::debug!( + "NVENC: split-encode AUTO with sub-frame readback on — the driver cannot split HEVC \ + in this combination, so this session runs SINGLE-ENGINE (measured). Set \ + PUNKTFUNK_NVENC_SUBFRAME=0 to trade sub-frame for a real split." + ); + } (split_mode, subframe) } @@ -298,6 +320,28 @@ mod split_subframe_tests { ); } + /// ⚠ DO NOT "SIMPLIFY" THE `AUTO` ARM AWAY. Measured on `.21` at 4K, plain `AUTO` is + /// conditional, not dead: + /// sub-frame ON → 5023/5157 µs ≈ DISABLE 4979/5000 (cannot split — mutually unsupported) + /// sub-frame OFF → 2401/2352 µs ≈ TWO_FORCED 2319/2378 (DOES split) + /// An earlier read of the sub-frame-ON measurement alone concluded "AUTO never splits, retire + /// it" — that would have silently cost every sub-frame-off session its second engine. This + /// test pins the arbitration's half of the contract: AUTO must survive both ways. + #[test] + fn auto_survives_the_arbitration_in_both_subframe_states() { + // Sub-frame on: kept as AUTO (inert, but that is the driver's call, and rewriting it to + // DISABLE would lie to the ceiling-cache key about what the session was given). + assert_eq!( + resolve_split_subframe(Codec::H265, AUTO, true, false), + (AUTO, true) + ); + // Sub-frame off: still AUTO, and here it is a REAL split — the arm must not be demoted. + assert_eq!( + resolve_split_subframe(Codec::H265, AUTO, false, false), + (AUTO, false) + ); + } + /// AV1: both features are legal together (per-tile sub-frame; split constrained only by /// output-into-vidmem) — the arbitration must not touch it. #[test] diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index 54c7534b..6e8a63ae 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -1412,6 +1412,15 @@ impl NvencD3d11Encoder { } self.inited = true; tracing::info!( + // Parity with the Linux session-ready line. `split_mode` is the FINAL mode (post + // any rejection fallback) and `engines` the ceiling it was chosen from — the mode + // alone is ambiguous between "used every engine" and "left one idle", and the + // driver honours an over-wide request without complaint, so neither number means + // much without the other. `subframe` because AUTO + sub-frame is a measurably + // single-engine combination that reads like a split in a log. + split_mode = self.split_mode, + engines = self.encoder_engines, + subframe = self.subframe_on, "NVENC D3D11 session: {}x{}@{} {}-bit{} {} Mbps {:?}", self.width, self.height, From 3b283dc26e38e974f279f0bfd29c945035ce2a16 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 23:26:47 +0200 Subject: [PATCH 05/12] =?UTF-8?q?feat(pf-encode):=20WP3=20=E2=80=94=20live?= =?UTF-8?q?=20split=20arbitration,=20measured=20on=20the=20session,=20no?= =?UTF-8?q?=20IDR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix S1 unlocked. Rather than predict the right split mode at open — which cannot work, because the decision depends on bits/frame and an Automatic client's steady-state bitrate is unknown at open (ABR climbs in place afterwards) — the encoder now measures both arms on the live session and keeps the winner. S1 proved nvEncReconfigureEncoder takes a changed splitEncodeMode with resetEncoder=0, emits no IDR, and actually applies it, so the experiment is invisible on the wire. Deliberately measures instead of modelling: hard-coded per-arch constants are exactly how the rule this replaces went wrong (one 5120x1440@240 Ada datapoint generalised into a fleet-wide 10-bit veto). A measurement tracks driver updates for free. `SplitArbiter` (pure state machine, unit-tested without a GPU): measure incumbent -> switch -> SETTLE -> measure challenger -> keep the winner, else switch back. Verdicts cache per (gpu, codec, mode, depth, chroma) so later sessions open straight into the winning arm; the key is CeilingKey minus split_mode, since the split mode is the thing being decided. ⚠ SETTLE_FRAMES=16 is load-bearing, not padding: split-encode does not reach steady state on the first frame (a FRESH TWO_FORCED session measured early-half 3280us vs late-half 1996), so judging an arm right after switching reads the transient — intermittently, which would then be cached. A unit test feeds exactly that transient and asserts the arbiter still sees the steady state. Safety gates, all correctness conditions rather than preferences: opt-in (PUNKTFUNK_NVENC_SPLIT_ARBITRATE=1) while it earns trust; an operator PUNKTFUNK_SPLIT_ENCODE pin always wins; skip if a verdict is already cached; sync depth-1 only (async_rt.is_none(), same gate chunked poll uses — under pipelined retrieve the submit->AU span includes queue depth and the comparison is noise); needs >=2 engines; never H.264. ⚠ And the one that bounds this increment: NO SUB-FRAME TRADE. For HEVC, forcing split gives up sub-frame readback, which costs send/encode overlap the ENCODER CANNOT SEE — it measures encode time only, so it would reliably prefer split and silently make end-to-end latency worse. So arbitration runs only where nothing is traded: sub-frame already off, or AV1 (both features legal). Pricing that trade needs the host's send cost and is the next work package. Challenger choice tests the question worth asking — anything not already the widest forced split is challenged BY the widest ("are we leaving engines idle?"). The naive "challenge whatever we are not" spent the experiment re-proving that splitting beats not-splitting, while parking the session on the slow arm to do it, because 4K60 sits on the fallthrough AUTO. ⚠ Every new nvenc_core item is linux-gated: the arbiter is wired into the Linux backend only for now and nvenc_core compiles on Windows too. Caught by the .133 check, not by reasoning — the first cut failed Windows clippy with 12 dead_code errors, the exact item-level trap this file already carries a scar from. Verified .21: clippy --features nvenc --all-targets -D warnings clean, 62 unit tests (4 new arbiter tests), 23/23 NVENC on-hardware green including a new end-to-end convergence test asserting ZERO extra IDRs and a cached verdict. Verified .133: Windows clippy -D warnings clean, zero dead_code. fmt clean. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 275 ++++++++++++++++- crates/pf-encode/src/enc/nvenc_core.rs | 308 +++++++++++++++++++ 2 files changed, 579 insertions(+), 4 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 413d895b..bb18374b 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -67,9 +67,11 @@ #![deny(clippy::undocumented_unsafe_blocks)] use super::nvenc_core::{ - apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, plan_range_recovery, - resolve_slices, resolve_split_mode, resolve_split_subframe, resolve_subframe, store_ceiling, - subframe_env_forced, CeilingKey, LowLatencyConfig, NvStatusExt, RangePlan, + apply_low_latency_config, build_init_params, cached_ceiling, cached_split_verdict, codec_guid, + max_forced_split_mode, plan_range_recovery, resolve_slices, resolve_split_mode, + resolve_split_subframe, resolve_subframe, store_ceiling, store_split_verdict, + subframe_env_forced, ArbAction, CeilingKey, LowLatencyConfig, NvStatusExt, RangePlan, + SplitArbiter, SplitKey, }; use super::nvenc_status; use super::{AuChunk, ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps}; @@ -826,6 +828,11 @@ pub struct NvencCudaEncoder { /// a split wider than the hardware and silently encodes narrower, so this is the only honest /// source for how wide we may go (see `nvenc_core::max_forced_split_mode`). encoder_engines: u32, + /// Submit stamp for the split arbiter's per-frame cost (sync depth-1 path only). + last_submit_at: Option, + /// The live split-mode experiment, when one is running. `None` = not arbitrating (gated off, + /// already decided this process, or the config is one we refuse to arbitrate). + arbiter: Option, /// In-progress chunked readback of the front in-flight AU. See [`ChunkState`]. chunk: Option, } @@ -915,6 +922,8 @@ impl NvencCudaEncoder { subframe_forced: false, subframe_chunks: false, encoder_engines: 0, + last_submit_at: None, + arbiter: None, chunk: None, }) } @@ -1345,8 +1354,25 @@ impl NvencCudaEncoder { // 2-way NVENC split-frame encoding (Ada dual-NVENC) — shared selector, see // [`resolve_split_mode`] for the precedence (env override / 10-bit / pixel rate). let pixel_rate = self.width as u64 * self.height as u64 * self.fps.max(1) as u64; - let split_mode: u32 = + let mut split_mode: u32 = resolve_split_mode(self.bit_depth, pixel_rate, self.encoder_engines); + // A verdict this process already measured for this exact config wins over the static + // rule — that is the whole point of arbitrating, and it lets later sessions skip the + // ~1 s experiment. An operator pin still beats both (checked inside `resolve_split_mode`, + // so only consult the cache when the knob is unset). + if std::env::var_os("PUNKTFUNK_SPLIT_ENCODE").is_none() { + if let Some(known) = cached_split_verdict(&self.split_key()) { + if known != split_mode { + tracing::info!( + from = split_mode, + to = known, + "NVENC: using the split mode a previous arbitration measured as \ + fastest for this config" + ); + } + split_mode = known; + } + } // Split × sub-frame arbitration (Phase 8) BEFORE the ladder, the ceiling key and the // chunked-poll latch — all three must see the post-arbitration truth (a drop inside // build_init_params would leave poll_chunk busy-polling its whole budget per AU). @@ -1659,10 +1685,140 @@ impl NvencCudaEncoder { subframe = self.subframe_on, "NVENC CUDA session ready" ); + self.arm_split_arbiter(); Ok(()) } } + /// Decide whether this session may run a live split experiment, and arm it if so. + /// + /// Opt-in (`PUNKTFUNK_NVENC_SPLIT_ARBITRATE=1`) while it earns trust. Every other gate is a + /// correctness condition, not a preference: + /// + /// - **Operator pin wins.** `PUNKTFUNK_SPLIT_ENCODE` set ⇒ never arbitrate; a pinned mode is an + /// instruction, and an A/B that overrides it would make the knob useless for exactly the + /// debugging it exists for. + /// - **Already decided.** A cached verdict for this config was applied at open; re-running the + /// experiment every session would pay its cost forever. + /// - **Sync depth-1 only** (`async_rt.is_none()`), the same gate chunked poll uses: the + /// per-frame cost is measured as submit → AU, which is only the encode on this path. Under + /// pipelined retrieve that span includes queue depth and the comparison would be noise. + /// - **Needs a second engine**, and split must be applicable at all (never H.264). + /// - ⚠ **No sub-frame trade.** For HEVC, forcing split gives up sub-frame readback, which costs + /// send/encode overlap the ENCODER CANNOT SEE — it measures encode time only, so it would + /// reliably prefer split and silently make end-to-end latency worse. So we arbitrate only + /// where nothing is traded: sub-frame already off, or AV1 (where both features are legal). + /// Pricing that trade needs the host's send cost and is the next work package. + fn arm_split_arbiter(&mut self) { + if !matches!( + std::env::var("PUNKTFUNK_NVENC_SPLIT_ARBITRATE").as_deref(), + Ok("1") + ) { + return; + } + if std::env::var_os("PUNKTFUNK_SPLIT_ENCODE").is_some() + || cached_split_verdict(&self.split_key()).is_some() + || self.async_rt.is_some() + || self.encoder_engines < 2 + || self.codec == Codec::H264 + { + return; + } + if self.subframe_on && self.codec != Codec::Av1 { + tracing::debug!( + "NVENC split arbitration skipped: sub-frame readback is on and this codec cannot \ + keep it while split, so the trade costs send overlap the encoder cannot measure" + ); + return; + } + // Pick the challenger that tests the question worth asking: "are we leaving engines idle?" + // So anything that is not already the widest forced split is challenged BY the widest, and + // only a session already there is challenged by single-engine ("is splitting even helping + // here?"). + // + // ⚠ Not "whatever we are not": with the fallthrough `AUTO` incumbent that a 4K60 session + // gets, the naive version challenged with DISABLE and spent the experiment re-proving that + // splitting beats not-splitting — while parking the session on the slow arm to do it. + let disable = nv::NV_ENC_SPLIT_ENCODE_MODE::NV_ENC_SPLIT_DISABLE_MODE as u32; + let widest = max_forced_split_mode(self.encoder_engines); + let challenger = if self.split_mode == widest { + disable + } else { + widest + }; + if challenger == self.split_mode { + return; + } + tracing::info!( + incumbent = self.split_mode, + challenger, + "NVENC split arbitration armed — measuring both arms on the live session (no IDR)" + ); + self.arbiter = Some(SplitArbiter::new(self.split_mode, challenger)); + } + + /// The config identity this session's split verdict is cached under. + fn split_key(&self) -> SplitKey { + SplitKey { + gpu: self.cu_ctx as u64, + codec: self.codec, + width: self.width, + height: self.height, + fps: self.fps, + bit_depth: self.bit_depth, + chroma_444: self.chroma_444, + } + } + + /// Move the LIVE session to `mode` without an IDR — spike S1 proved `nvEncReconfigureEncoder` + /// takes a changed `splitEncodeMode` with `resetEncoder=0`, emits no keyframe, and actually + /// applies it. Reuses the bitrate reconfigure path at the CURRENT rate, so only the split mode + /// moves. Returns whether the driver accepted it; on refusal the field is restored so the + /// encoder's idea of its own session stays truthful. + fn apply_split_mode(&mut self, mode: u32) -> bool { + let previous = self.split_mode; + self.split_mode = mode; + if self.reconfigure_bitrate(self.bitrate_bps) { + true + } else { + tracing::warn!( + from = previous, + to = mode, + "NVENC split arbitration: driver refused the in-place split change — staying put" + ); + self.split_mode = previous; + false + } + } + + /// Feed one frame's encode cost to the split arbiter and act on its verdict. + fn feed_split_arbiter(&mut self, encode_us: u64) { + let Some(arb) = self.arbiter.as_mut() else { + return; + }; + let action = arb.on_frame(encode_us); + let done = arb.is_done(); + match action { + Some(ArbAction::SwitchTo(mode)) => { + if !self.apply_split_mode(mode) { + // The experiment cannot proceed if the session will not move — abandon it + // rather than compare two measurements of the same arm. + self.arbiter = None; + return; + } + } + Some(ArbAction::Settled(mode)) => { + store_split_verdict(self.split_key(), mode); + } + None => {} + } + if done { + // A "switch back to the incumbent" verdict settles on the mode now live. + store_split_verdict(self.split_key(), self.split_mode); + self.arbiter = None; + } + } + /// Copy the captured `DeviceBuffer` into the ring slot's registered input surface (device→device /// on the shared context). `sync` blocks until the copy completes (the pre-existing behavior); /// `!sync` enqueues on the encode thread's copy stream and leaves ordering to the session's @@ -2037,6 +2193,10 @@ impl Encoder for NvencCudaEncoder { // never emits an IDR on its own, so this matches the eventual pictureType. is_idr, )); + // Stamp for the split arbiter's per-frame cost. Deliberately a single field rather + // than a sixth `pending` element: the arbiter only runs on the sync depth-1 path + // (`async_rt.is_none()`), where at most one encode is outstanding. + self.last_submit_at = Some(std::time::Instant::now()); } if sample { tracing::info!( @@ -2217,6 +2377,16 @@ impl Encoder for NvencCudaEncoder { if !map.is_null() { let _ = (api().unmap_input_resource)(self.encoder, map); } + // One frame's encode cost, submit → AU complete. Only meaningful on this sync, + // depth-1 path (the arbiter is gated to it), where `lock_bitstream` above blocked + // until the ASIC finished, so the span is the encode rather than a queue wait. + let encode_us = self + .last_submit_at + .take() + .map(|t| t.elapsed().as_micros() as u64); + if let Some(us) = encode_us { + self.feed_split_arbiter(us); + } Ok(Some(EncodedFrame { data, pts_ns, @@ -3613,6 +3783,103 @@ mod tests { std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); } + /// ON-HARDWARE — the live split arbitration end to end (WP3). Opens a 4K session that the + /// static rule leaves single-engine, lets the arbiter run, and asserts it converges to the + /// faster arm **without emitting a single IDR** and records a verdict other sessions can reuse. + /// + /// Sub-frame is pinned off so the arbiter's own no-trade gate lets it arm (see + /// `arm_split_arbiter`); this is the shape the first increment supports. + /// + /// Asserts behaviour, not timing: that it settles, that it lands on the arm the ~2× split + /// advantage implies, and — the load-bearing one — **zero keyframes after the opening IDR**, + /// which is the whole reason this design is allowed to exist. Run ALONE: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_split_arbitration_converges --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX box (.21)"] + fn nvenc_cuda_split_arbitration_converges() { + const W: u32 = 3840; + const H: u32 = 2160; + let disable = nv::NV_ENC_SPLIT_ENCODE_MODE::NV_ENC_SPLIT_DISABLE_MODE as u32; + + std::env::set_var("PUNKTFUNK_NVENC_SPLIT_ARBITRATE", "1"); + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + let frames: Vec = (0..4).map(|i| nv12_frame(W, H, i)).collect(); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + W, + H, + 60, + 400_000_000, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + + let mut keyframes = 0usize; + let mut aus = 0usize; + // Enough frames for measure + settle + measure with room to spare. + for i in 0..140u32 { + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + while let Some(au) = enc.poll().expect("poll") { + aus += 1; + keyframes += au.keyframe as usize; + } + } + let final_mode = enc.split_mode; + let still_arbitrating = enc.arbiter.is_some(); + let verdict = cached_split_verdict(&enc.split_key()); + let enc_engines = enc.encoder_engines; + enc.flush().ok(); + + println!( + "arbitration: {aus} AUs, {keyframes} keyframes, final split_mode={final_mode}, \ + cached verdict={verdict:?}, still running={still_arbitrating}" + ); + assert!(aus > 100, "not enough AUs to complete an arbitration"); + assert!( + !still_arbitrating, + "arbitration did not finish in 140 frames" + ); + assert_eq!( + keyframes, 1, + "THE POINT OF THIS DESIGN: arbitration must cost ZERO extra IDRs — only the session's \ + opening one" + ); + assert_eq!( + verdict, + Some(final_mode), + "the winning arm must be cached so later sessions skip the experiment" + ); + assert_ne!( + final_mode, disable, + "at 4K with two engines a splitting arm is ~2x faster, so single-engine must not win" + ); + // The static rule leaves 4K60 on the fallthrough AUTO (497.7 Mpix/s is under + // SPLIT_FORCE_PIXEL_RATE), so the experiment is AUTO vs the widest forced split — the + // "are we leaving engines idle?" question. Either outcome is legitimate; what must NOT + // happen is landing on single-engine. + println!( + " (incumbent was the static rule's choice; challenger was mode {})", + max_forced_split_mode(enc_engines) + ); + + std::env::remove_var("PUNKTFUNK_NVENC_SPLIT_ARBITRATE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + // The verdict cache is process-global: leaving this session's result in it would steer + // every later test that opens the same config with the split env unset (the D5 legs do + // exactly that). + super::super::nvenc_core::clear_split_verdicts(); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index 7e3f44ec..183e2b4f 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -353,6 +353,153 @@ mod split_subframe_tests { } } +// Split arbitration is wired into the Linux direct-SDK backend only for now, and +// `nvenc_core` compiles on Windows too — so every item below is linux-gated or it trips +// the item-level dead_code trap this file already carries a scar from (see +// `subframe_env_forced`). Ungating is part of the Windows wiring, not a cleanup. +#[cfg(target_os = "linux")] +/// What the split arbiter wants the backend to do next. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum ArbAction { + /// Reconfigure the live session to this split mode (in place — S1 proved this is IDR-free). + SwitchTo(u32), + /// Arbitration finished; this mode won and the arbiter will ask for nothing further. + Settled(u32), +} + +#[cfg(target_os = "linux")] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ArbState { + MeasuringIncumbent, + Settling, + MeasuringChallenger, + Done, +} + +#[cfg(target_os = "linux")] +/// Picks the faster of two NVENC split modes **on the live session**, by measuring both. +/// +/// This exists because the alternative — predicting the right mode at open — cannot work: the +/// decision depends on bits/frame, and for an Automatic client the host does not know the +/// steady-state bitrate at open (ABR climbs in place afterwards). Spike S1 showed +/// `nvEncReconfigureEncoder` accepts a changed `splitEncodeMode` with `resetEncoder=0`, emits **no +/// IDR**, and genuinely takes effect — so the encoder can simply try both and keep the winner, +/// with nothing visible on the wire. +/// +/// Deliberately measures rather than models: hard-coded per-architecture constants are exactly how +/// the rule this replaces went wrong (one 5120×1440@240 Ada datapoint generalised into a fleet-wide +/// 10-bit veto). A measurement tracks driver updates for free. +/// +/// ⚠ **`SETTLE_FRAMES` is load-bearing, not padding.** Split-encode does not reach steady state on +/// the first frame — a *fresh* `TWO_FORCED` session measured early-half 3280 µs against late-half +/// 1996 on `.21`. Judging an arm immediately after switching to it reads the transient, and does so +/// **intermittently**, which is the worst failure mode: the verdict would be wrong only sometimes, +/// and then be cached. +pub(super) struct SplitArbiter { + state: ArbState, + incumbent: u32, + challenger: u32, + samples: Vec, + incumbent_us: u64, + settle_left: u32, +} + +/// Frames discarded after a switch before the challenger is judged (measured — see the struct doc). +#[cfg(target_os = "linux")] +const SETTLE_FRAMES: u32 = 16; +/// Frames measured per arm. Long enough to median out content variation, short enough that the +/// whole arbitration is over in well under a second at 60 fps. +#[cfg(target_os = "linux")] +const SAMPLE_FRAMES: usize = 24; +/// The challenger must beat the incumbent by this much to win. Switching is not free (a +/// reconfigure, and for HEVC it costs sub-frame readback), so a coin-flip difference should leave +/// the session where it already is. +#[cfg(target_os = "linux")] +const WIN_MARGIN_PCT: u64 = 10; + +#[cfg(target_os = "linux")] +impl SplitArbiter { + pub(super) fn new(incumbent: u32, challenger: u32) -> Self { + Self { + state: ArbState::MeasuringIncumbent, + incumbent, + challenger, + samples: Vec::with_capacity(SAMPLE_FRAMES), + incumbent_us: 0, + settle_left: 0, + } + } + + /// Feed one frame's encode time. Returns an action when the arbiter wants the session changed. + pub(super) fn on_frame(&mut self, us: u64) -> Option { + match self.state { + ArbState::Done => None, + ArbState::Settling => { + self.settle_left = self.settle_left.saturating_sub(1); + if self.settle_left == 0 { + self.state = ArbState::MeasuringChallenger; + self.samples.clear(); + } + None + } + ArbState::MeasuringIncumbent => { + self.samples.push(us); + if self.samples.len() < SAMPLE_FRAMES { + return None; + } + self.incumbent_us = median(&mut self.samples); + self.state = ArbState::Settling; + self.settle_left = SETTLE_FRAMES; + Some(ArbAction::SwitchTo(self.challenger)) + } + ArbState::MeasuringChallenger => { + self.samples.push(us); + if self.samples.len() < SAMPLE_FRAMES { + return None; + } + let challenger_us = median(&mut self.samples); + self.state = ArbState::Done; + // Strictly better by the margin, or the incumbent keeps the session. Equal-ish is + // deliberately a win for the incumbent: we are already there. + let threshold = self + .incumbent_us + .saturating_sub(self.incumbent_us.saturating_mul(WIN_MARGIN_PCT) / 100); + if challenger_us < threshold { + tracing::info!( + winner = self.challenger, + winner_us = challenger_us, + loser = self.incumbent, + loser_us = self.incumbent_us, + "NVENC split arbitration: challenger wins — keeping it" + ); + Some(ArbAction::Settled(self.challenger)) + } else { + tracing::info!( + winner = self.incumbent, + winner_us = self.incumbent_us, + loser = self.challenger, + loser_us = challenger_us, + "NVENC split arbitration: incumbent held — switching back" + ); + // The session is currently running the challenger, so returning to the + // incumbent is an actual reconfigure, not a no-op. + Some(ArbAction::SwitchTo(self.incumbent)) + } + } + } + } + + pub(super) fn is_done(&self) -> bool { + self.state == ArbState::Done + } +} + +#[cfg(target_os = "linux")] +fn median(v: &mut [u64]) -> u64 { + v.sort_unstable(); + v[v.len() / 2] +} + /// One session config's identity for the process-lifetime bitrate-ceiling cache /// ([`cached_ceiling`]/[`store_ceiling`]). Everything the driver's codec-level validation keys /// off: the GPU (different NVENC generations have different level ceilings), dims/fps (the luma @@ -397,6 +544,55 @@ pub(super) fn store_ceiling(key: CeilingKey, bps: u64) { ceilings().lock().unwrap().insert(key, bps); } +#[cfg(target_os = "linux")] +/// A config's identity for the split-arbitration verdict cache — [`CeilingKey`] **minus +/// `split_mode`**, because the split mode is the thing being decided. Including it would key each +/// verdict under the arm that produced it and the cache could never answer "which arm should this +/// config use?". +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub(super) struct SplitKey { + pub gpu: u64, + pub codec: Codec, + pub width: u32, + pub height: u32, + pub fps: u32, + pub bit_depth: u8, + pub chroma_444: bool, +} + +#[cfg(target_os = "linux")] +fn split_verdicts() -> &'static std::sync::Mutex> { + static V: std::sync::OnceLock>> = + std::sync::OnceLock::new(); + V.get_or_init(Default::default) +} + +#[cfg(target_os = "linux")] +/// The split mode a previous arbitration found fastest for `key` this process lifetime. +/// +/// Process-lifetime and advisory, exactly like [`cached_ceiling`]: a session that reads a verdict +/// opens straight into the winning arm and skips the ~1 s exploration. It is NOT persisted — a +/// driver update can change the answer, and a stale verdict on disk would outlive its evidence +/// (persisting it needs the driver version in the key; see the plan's WP3). +pub(super) fn cached_split_verdict(key: &SplitKey) -> Option { + split_verdicts().lock().unwrap().get(key).copied() +} + +#[cfg(target_os = "linux")] +/// Record an arbitration result for `key`. +pub(super) fn store_split_verdict(key: SplitKey, mode: u32) { + split_verdicts().lock().unwrap().insert(key, mode); +} + +#[cfg(target_os = "linux")] +/// Drop every cached verdict. Test-only: the cache is process-global, so an on-hardware test that +/// runs an arbitration would otherwise leak its verdict into every later test that opens the same +/// config with `PUNKTFUNK_SPLIT_ENCODE` unset — which is exactly the shape the D5 legs use. +#[cfg(test)] +pub(super) fn clear_split_verdicts() { + split_verdicts().lock().unwrap().clear(); +} + #[cfg(test)] mod tests { use super::*; @@ -1041,3 +1237,115 @@ pub(super) unsafe fn apply_low_latency_config(cfg: &mut nv::NV_ENC_CONFIG, c: Lo } } } + +#[cfg(all(test, target_os = "linux"))] +mod arbiter_tests { + use super::{ArbAction, SplitArbiter, SETTLE_FRAMES}; + use nvidia_video_codec_sdk::sys::nvEncodeAPI::NV_ENC_SPLIT_ENCODE_MODE as M; + + /// Drive an arbiter with a fixed cost per arm and return every action it emitted. + fn drive(incumbent_us: u64, challenger_us: u64) -> (Vec, u32) { + let (inc, chal) = ( + M::NV_ENC_SPLIT_DISABLE_MODE as u32, + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + ); + let mut arb = SplitArbiter::new(inc, chal); + let mut actions = Vec::new(); + // Whatever the session is currently running; the harness follows the arbiter's switches + // so the cost it reports matches the arm actually in effect. + let mut live = inc; + for _ in 0..500 { + if arb.is_done() { + break; + } + let us = if live == inc { + incumbent_us + } else { + challenger_us + }; + if let Some(a) = arb.on_frame(us) { + actions.push(a); + match a { + ArbAction::SwitchTo(m) => live = m, + ArbAction::Settled(m) => live = m, + } + } + } + (actions, live) + } + + /// A clearly faster challenger is adopted, and the session ends up running it. + #[test] + fn arbiter_adopts_a_clearly_faster_challenger() { + let (actions, live) = drive(5000, 2400); + assert_eq!( + actions[0], + ArbAction::SwitchTo(M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32), + "must try the challenger before judging it" + ); + assert_eq!( + actions.last(), + Some(&ArbAction::Settled(M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32)) + ); + assert_eq!(live, M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32); + } + + /// A slower challenger is rejected and the session is put BACK — the arbiter is mid-experiment + /// when it decides, so "keep the incumbent" is a real reconfigure, not a no-op. Getting this + /// wrong would strand every losing arbitration on the losing arm. + #[test] + fn arbiter_restores_the_incumbent_when_the_challenger_loses() { + let (actions, live) = drive(2400, 5000); + assert_eq!( + actions.last(), + Some(&ArbAction::SwitchTo(M::NV_ENC_SPLIT_DISABLE_MODE as u32)), + "a losing experiment must be undone" + ); + assert_eq!(live, M::NV_ENC_SPLIT_DISABLE_MODE as u32); + } + + /// Within the margin the incumbent holds: switching costs a reconfigure and, on HEVC, sub-frame + /// readback, so a coin-flip difference must not move the session. + #[test] + fn arbiter_keeps_the_incumbent_inside_the_margin() { + // 5 % better — under WIN_MARGIN_PCT. + let (_, live) = drive(2400, 2280); + assert_eq!(live, M::NV_ENC_SPLIT_DISABLE_MODE as u32); + } + + /// THE SETTLE CONTRACT: the challenger must not be judged on frames taken immediately after the + /// switch. Feed it a transient — slow for the whole settle window, fast afterwards — and it + /// must still see the fast steady state. Without the settle window this arbiter would read the + /// transient, reject a genuinely better arm, and cache that verdict. + #[test] + fn arbiter_ignores_the_post_switch_transient() { + let (inc, chal) = ( + M::NV_ENC_SPLIT_DISABLE_MODE as u32, + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + ); + let mut arb = SplitArbiter::new(inc, chal); + let mut switched_at = None; + let mut frame = 0usize; + let mut outcome = None; + while outcome.is_none() && frame < 500 { + let us = match switched_at { + None => 5000, + // The transient: as slow as the incumbent for exactly the settle window. + Some(s) if frame - s <= SETTLE_FRAMES as usize => 5000, + Some(_) => 2000, + }; + match arb.on_frame(us) { + Some(ArbAction::SwitchTo(m)) if m == chal => switched_at = Some(frame), + Some(a) => outcome = Some(a), + None => {} + } + frame += 1; + } + assert_eq!( + outcome, + Some(ArbAction::Settled(chal)), + "the settle window must hide the post-switch transient — otherwise a better arm is \ + rejected on its own warmup" + ); + } +} From 2366c4fe31bcb161e77e8e988a3232fbbd49c11c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 23:41:40 +0200 Subject: [PATCH 06/12] feat(pf-encode,host): price the HEVC sub-frame trade so arbitration can cover it The named next step after WP3's first increment. That increment deliberately REFUSED to arbitrate HEVC-with-sub-frame -- the fleet default, and the reported field case -- because engaging split there gives up sub-frame readback, whose whole value is that the send overlaps the encode. An encoder measuring only encode time would see split as ~2x faster, take it, and make end-to-end latency worse while reporting a win. This supplies the missing number. The real comparison is encode_1eng + send_of_last_slice against encode_2eng + send_of_whole_AU, so the challenger owes roughly spread x (slices-1)/slices. Split across the two sides that can each see half: - Host: new `Encoder::set_send_spread_us` (defaulted, forwarded by TrackedEncoder -- same trap class as set_wire_chunking, and unforwarded it would fail SILENTLY IN THE SAFE DIRECTION, which is the hardest kind to notice). The send thread is the only place a paced send is observed and the encode loop the only place the encoder can be touched, so it goes over an AtomicU32 like encoder_ceiling_kbps, EWMA-smoothed 3:1 per completed AU: one content spike must not flip a verdict that then gets cached. - Encoder: turns the raw spread into the handicap, because only it knows `slices`. SplitArbiter::with_handicap charges it to the challenger before the comparison. A unit test runs identical encode numbers with a cheap and an expensive send and asserts the verdict REVERSES -- with an expensive send the arm that looks twice as fast is a loss end to end, and the incumbent must hold. That is precisely the regression an encode-only arbiter ships. Gate now opens for HEVC+sub-frame only when a spread has actually been reported (and slices >= 2); with no hint it still refuses, so behaviour is unchanged until the host feeds it. Two mechanics this needed: - apply_split_mode became a PAIR flip (split + sub-frame), routed through resolve_split_subframe and restoring from `subframe_opened_with` so a session that never had sub-frame can never gain it. It also recomputes `subframe_chunks`, which reconfigure_bitrate does NOT -- spike S1c's finding; leave it stale and supports_chunked_poll keeps saying yes while numSlices never advances, so poll_chunk busy-polls its whole budget every AU. - The arbiter is now fed from BOTH completion points. A sub-frame session finishes through poll_chunk, so the incumbent arm of an HEVC experiment would otherwise never deliver a sample -- only the challenger, with sub-frame dropped, comes through poll. Verified .21: clippy -D warnings clean for pf-encode AND punktfunk-host with nvenc, 63 unit tests (1 new), 23/23 NVENC on-hardware green. Verified .133: Windows clippy -D warnings clean, zero dead_code. fmt clean. --- crates/pf-encode/src/enc/codec.rs | 14 ++++ crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 84 +++++++++++++++++--- crates/pf-encode/src/enc/nvenc_core.rs | 62 ++++++++++++++- crates/pf-encode/src/lib.rs | 6 ++ crates/punktfunk-host/src/native/stream.rs | 26 ++++++ 5 files changed, 177 insertions(+), 15 deletions(-) diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index b09ebf3b..d89e2d7a 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -443,6 +443,20 @@ pub trait Encoder: Send { /// flagged [`EncodedFrame::chunk_aligned`] and the session marks them on the wire. /// Default: no-op (the H.26x backends' bitstreams cannot be cut losslessly). fn set_wire_chunking(&mut self, _shard_payload: usize) {} + /// How long a whole AU's packets currently take to leave the socket (µs, smoothed) — the + /// host's paced-send `spread_us`. + /// + /// Exists for ONE decision, and only the host can supply it. The Linux direct-NVENC split + /// arbitration compares single-engine against split, but on HEVC engaging split costs + /// sub-frame readback, and sub-frame's whole value is that the send overlaps the encode. So + /// the real comparison is `encode_1eng + send_of_last_slice` against + /// `encode_2eng + send_of_whole_AU`, and an encoder that measures only encode time would + /// reliably pick split and make end-to-end latency WORSE. The backend turns this number into + /// that handicap (it knows its own slice count); the host just reports what it observes. + /// + /// Optional by design: a backend that ignores it simply never arbitrates the sub-frame trade, + /// which is the safe direction. `0` = unknown / not reported yet. + fn set_send_spread_us(&mut self, _us: u32) {} /// How many frames the CAPTURER guarantees the encoder may hold in flight before it starts /// reusing an input texture (`Capturer::pipeline_depth`). Backends that encode the capturer's /// textures IN PLACE — no `CopyResource` — must not pipeline deeper than this: the capturer diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index bb18374b..8babbbbe 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -830,6 +830,15 @@ pub struct NvencCudaEncoder { encoder_engines: u32, /// Submit stamp for the split arbiter's per-frame cost (sync depth-1 path only). last_submit_at: Option, + /// Whole-AU paced-send time (µs) the host last reported, via + /// [`Encoder::set_send_spread_us`]. `0` = never reported, which keeps the arbiter out of the + /// sub-frame trade entirely (it cannot price what it cannot see). + send_spread_us: u32, + /// Sub-frame state the session was OPENED able to run — what `resolve_subframe` decided from + /// the caps probe and the env. `subframe_on` moves as the arbiter flips arms; this does not, + /// so a return to a non-forced split can restore sub-frame without re-deriving it (and + /// without ever turning it on for a session that never had it). + subframe_opened_with: bool, /// The live split-mode experiment, when one is running. `None` = not arbitrating (gated off, /// already decided this process, or the config is one we refuse to arbitrate). arbiter: Option, @@ -923,6 +932,8 @@ impl NvencCudaEncoder { subframe_chunks: false, encoder_engines: 0, last_submit_at: None, + send_spread_us: 0, + subframe_opened_with: false, arbiter: None, chunk: None, }) @@ -1383,6 +1394,7 @@ impl NvencCudaEncoder { self.subframe_forced, ); self.subframe_on = subframe_on; + self.subframe_opened_with = subframe_on; const CLAMP_TOL_BPS: u64 = 20_000_000; // Ceiling cache (process lifetime, `nvenc_core`): a prior clamp search already found @@ -1724,13 +1736,25 @@ impl NvencCudaEncoder { { return; } - if self.subframe_on && self.codec != Codec::Av1 { - tracing::debug!( - "NVENC split arbitration skipped: sub-frame readback is on and this codec cannot \ - keep it while split, so the trade costs send overlap the encoder cannot measure" - ); - return; - } + // Losing sub-frame costs the send/encode overlap: without it the AU's last byte waits for + // the WHOLE send instead of just the final slice, so the challenger owes roughly + // `spread × (slices−1)/slices`. Priced here because only the encoder knows `slices`; the + // host reports the raw spread. + let handicap_us = if self.subframe_on && self.codec != Codec::Av1 { + if self.send_spread_us == 0 || self.slices < 2 { + tracing::debug!( + "NVENC split arbitration skipped: engaging split would cost sub-frame readback \ + and no send-spread has been reported, so the trade cannot be priced — an \ + encode-only comparison would take the arm that looks fastest and lose \ + end-to-end" + ); + return; + } + let slices = self.slices as u64; + self.send_spread_us as u64 * (slices - 1) / slices + } else { + 0 + }; // Pick the challenger that tests the question worth asking: "are we leaving engines idle?" // So anything that is not already the widest forced split is challenged BY the widest, and // only a session already there is challenged by single-engine ("is splitting even helping @@ -1752,9 +1776,15 @@ impl NvencCudaEncoder { tracing::info!( incumbent = self.split_mode, challenger, + handicap_us, + send_spread_us = self.send_spread_us, "NVENC split arbitration armed — measuring both arms on the live session (no IDR)" ); - self.arbiter = Some(SplitArbiter::new(self.split_mode, challenger)); + self.arbiter = Some(SplitArbiter::with_handicap( + self.split_mode, + challenger, + handicap_us, + )); } /// The config identity this session's split verdict is cached under. @@ -1776,17 +1806,34 @@ impl NvencCudaEncoder { /// moves. Returns whether the driver accepted it; on refusal the field is restored so the /// encoder's idea of its own session stays truthful. fn apply_split_mode(&mut self, mode: u32) -> bool { - let previous = self.split_mode; + let (prev_mode, prev_sub, prev_chunks) = + (self.split_mode, self.subframe_on, self.subframe_chunks); + // Sub-frame rides along: HEVC cannot hold both, so a forced split must drop it and a + // return to non-forced may take it back (only up to what the session was opened able to + // do — `subframe_cap`/`resolve_subframe` decided that once, at open). + let (mode, subframe) = resolve_split_subframe( + self.codec, + mode, + self.subframe_opened_with, + self.subframe_forced, + ); self.split_mode = mode; + self.subframe_on = subframe; + // ⚠ The latch `reconfigure_bitrate` does NOT recompute (spike S1c): leave it stale and + // `supports_chunked_poll` keeps saying yes while `numSlices` never advances, so + // `poll_chunk` busy-polls its entire budget every AU. + self.subframe_chunks = self.slices >= 2 && subframe && self.async_rt.is_none(); if self.reconfigure_bitrate(self.bitrate_bps) { true } else { tracing::warn!( - from = previous, + from = prev_mode, to = mode, "NVENC split arbitration: driver refused the in-place split change — staying put" ); - self.split_mode = previous; + self.split_mode = prev_mode; + self.subframe_on = prev_sub; + self.subframe_chunks = prev_chunks; false } } @@ -2551,6 +2598,17 @@ impl Encoder for NvencCudaEncoder { "NVENC chunked poll: picture type diverged from the submit-time prediction" ); } + // The AU is complete here too — the chunked path is how a sub-frame session finishes, + // so the arbiter has to be fed from BOTH completion points or it would never see a + // frame on the incumbent arm of an HEVC sub-frame experiment (that arm is chunked; + // only the challenger, with sub-frame dropped, comes through `poll`). + let encode_us = self + .last_submit_at + .take() + .map(|t| t.elapsed().as_micros() as u64); + if let Some(us) = encode_us { + self.feed_split_arbiter(us); + } Ok(Some(AuChunk { data, pts_ns, @@ -2634,6 +2692,10 @@ impl Encoder for NvencCudaEncoder { } } + fn set_send_spread_us(&mut self, us: u32) { + self.send_spread_us = us; + } + fn applied_bitrate_bps(&self) -> Option { // `bitrate_bps` is the post-clamp truth: the open path's ceiling search and the // reconfigure path's cache clamp both write what the session ACTUALLY targets. diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index 183e2b4f..6c79d93a 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -402,6 +402,13 @@ pub(super) struct SplitArbiter { samples: Vec, incumbent_us: u64, settle_left: u32, + /// Latency the challenger COSTS beyond its encode time, added to its measured result before + /// the comparison. Non-zero only when winning the split means giving up sub-frame readback: + /// sub-frame lets the send overlap the encode, so losing it pushes the AU's last byte out by + /// roughly `send_spread × (slices−1)/slices`. Without this term the arbiter compares encode + /// against encode, always prefers split on HEVC, and makes end-to-end latency worse while + /// reporting a win. + challenger_handicap_us: u64, } /// Frames discarded after a switch before the challenger is judged (measured — see the struct doc). @@ -419,7 +426,9 @@ const WIN_MARGIN_PCT: u64 = 10; #[cfg(target_os = "linux")] impl SplitArbiter { - pub(super) fn new(incumbent: u32, challenger: u32) -> Self { + /// `handicap_us` is what the challenger costs OUTSIDE the encode it is measured on — pass `0` + /// when it gives up nothing. See [`Self::challenger_handicap_us`]. + pub(super) fn with_handicap(incumbent: u32, challenger: u32, handicap_us: u64) -> Self { Self { state: ArbState::MeasuringIncumbent, incumbent, @@ -427,6 +436,7 @@ impl SplitArbiter { samples: Vec::with_capacity(SAMPLE_FRAMES), incumbent_us: 0, settle_left: 0, + challenger_handicap_us: handicap_us, } } @@ -457,7 +467,9 @@ impl SplitArbiter { if self.samples.len() < SAMPLE_FRAMES { return None; } - let challenger_us = median(&mut self.samples); + // Compare TOTAL cost, not encode cost: whatever the challenger gives up outside + // the encode (on HEVC, the sub-frame send overlap) is charged to it here. + let challenger_us = median(&mut self.samples) + self.challenger_handicap_us; self.state = ArbState::Done; // Strictly better by the margin, or the incumbent keeps the session. Equal-ish is // deliberately a win for the incumbent: we are already there. @@ -1241,15 +1253,57 @@ pub(super) unsafe fn apply_low_latency_config(cfg: &mut nv::NV_ENC_CONFIG, c: Lo #[cfg(all(test, target_os = "linux"))] mod arbiter_tests { use super::{ArbAction, SplitArbiter, SETTLE_FRAMES}; + use nvidia_video_codec_sdk::sys::nvEncodeAPI::NV_ENC_SPLIT_ENCODE_MODE as M; + /// THE SUB-FRAME TRADE, which is the whole reason `set_send_spread_us` exists. Same encode + /// numbers both times; only the handicap differs. + /// + /// A 4K HEVC session where split halves the encode (5000 → 2400 µs) but costs sub-frame + /// readback. With a cheap send there is headroom and split wins. With an expensive send the + /// lost overlap outweighs the encode saving, and the arbiter must REFUSE the arm that looks + /// twice as fast — which is exactly the mistake an encode-only comparison makes. + #[test] + fn handicap_can_reverse_the_verdict() { + let (inc, chal) = ( + M::NV_ENC_SPLIT_DISABLE_MODE as u32, + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, + ); + let run = |handicap: u64| { + let mut arb = SplitArbiter::with_handicap(inc, chal, handicap); + let mut live = inc; + for _ in 0..500 { + if arb.is_done() { + break; + } + let us = if live == inc { 5000 } else { 2400 }; + if let Some(a) = arb.on_frame(us) { + match a { + ArbAction::SwitchTo(m) | ArbAction::Settled(m) => live = m, + } + } + } + live + }; + // Cheap send: the 2600 µs encode saving is real, split wins. + assert_eq!(run(500), chal, "with a cheap send, split should win"); + // Expensive send: 2400 + 3000 = 5400 against 5000 — the "twice as fast" arm is a LOSS + // end to end, and an encode-only comparison would have taken it. + assert_eq!( + run(3000), + inc, + "when losing sub-frame costs more than split saves, the incumbent must hold — this is \ + the regression an encode-only arbiter would ship" + ); + } + /// Drive an arbiter with a fixed cost per arm and return every action it emitted. fn drive(incumbent_us: u64, challenger_us: u64) -> (Vec, u32) { let (inc, chal) = ( M::NV_ENC_SPLIT_DISABLE_MODE as u32, M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, ); - let mut arb = SplitArbiter::new(inc, chal); + let mut arb = SplitArbiter::with_handicap(inc, chal, 0); let mut actions = Vec::new(); // Whatever the session is currently running; the harness follows the arbiter's switches // so the cost it reports matches the arm actually in effect. @@ -1323,7 +1377,7 @@ mod arbiter_tests { M::NV_ENC_SPLIT_DISABLE_MODE as u32, M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, ); - let mut arb = SplitArbiter::new(inc, chal); + let mut arb = SplitArbiter::with_handicap(inc, chal, 0); let mut switched_at = None; let mut frame = 0usize; let mut outcome = None; diff --git a/crates/pf-encode/src/lib.rs b/crates/pf-encode/src/lib.rs index 66c71ab3..984bddfd 100644 --- a/crates/pf-encode/src/lib.rs +++ b/crates/pf-encode/src/lib.rs @@ -287,6 +287,12 @@ impl Encoder for TrackedEncoder { fn set_wire_chunking(&mut self, shard_payload: usize) { self.inner.set_wire_chunking(shard_payload) } + // Same trap class again: unforwarded, the default no-op would leave the split arbitration + // permanently blind to send cost and it would never arbitrate the sub-frame trade — failing + // silently in the safe direction, which is the hardest kind to notice. + fn set_send_spread_us(&mut self, us: u32) { + self.inner.set_send_spread_us(us) + } // Forwarded for the same reason as `set_wire_chunking` above — an unforwarded default here // would silently leave the in-place backends pipelining past the capturer's ring. fn set_input_ring_depth(&mut self, depth: usize) { diff --git a/crates/punktfunk-host/src/native/stream.rs b/crates/punktfunk-host/src/native/stream.rs index 58d0a508..f5d29c20 100644 --- a/crates/punktfunk-host/src/native/stream.rs +++ b/crates/punktfunk-host/src/native/stream.rs @@ -746,6 +746,11 @@ fn send_loop( probe_result_tx: tokio::sync::mpsc::UnboundedSender, stop: Arc, perf: bool, + // Smoothed whole-AU paced-send time (µs) published for the ENCODE loop, which hands it to + // `Encoder::set_send_spread_us`. The split arbiter needs it to price what engaging split + // costs on HEVC (sub-frame readback, and with it the send/encode overlap) — a number the + // encoder cannot observe. Written here because this is the only thread that sees a send. + send_spread_us: Arc, // Streamed AUs go out as slice-granularity blocks ([`USER_FLAG_SLICE_STREAM`]'s contract) // instead of the legacy full-FEC-block shape. slice_wire: bool, @@ -902,6 +907,19 @@ fn send_loop( ); } } + // Smooth before publishing: a single AU's spread swings with content and + // FEC shape, and the arbiter turns this into a latency handicap that + // decides an arm. EWMA (3:1) over completed AUs is enough to stop one + // spike flipping a verdict. + { + let prev = send_spread_us.load(Ordering::Relaxed); + let next = if prev == 0 { + stat.spread_us + } else { + ((prev as u64 * 3 + stat.spread_us as u64) / 4) as u32 + }; + send_spread_us.store(next, Ordering::Relaxed); + } if perf || stats.rec.is_armed() { // `encode_us`/`pace_us`/fps are valid for every frame (always measured), // including the Windows relay + tail-drain frames. The cap/submit/wait splits @@ -1770,6 +1788,10 @@ pub(super) fn virtual_stream(ctx: SessionContext, prepared: Option ceiling { From 50b3fd1012e43078d23179a6c8bcd3e5ca22c1be Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Thu, 6 Aug 2026 23:58:24 +0200 Subject: [PATCH 07/12] =?UTF-8?q?fix(pf-encode):=20drop=20the=2010-bit=20s?= =?UTF-8?q?hort=20circuit=20=E2=80=94=20measured=20wrong=20on=20Ada,=20twi?= =?UTF-8?q?ce?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP1.3, and the measurement that justifies it. `resolve_split_mode`'s 10-bit rule sat ABOVE the pixel-rate arm and took no codec, so it (D1) vetoed 10-bit 4K120 -- the very case the pixel-rate arm exists for -- and (D2) applied an HEVC-Main10-on- Ada result to AV1 10-bit, which has no such measurement. Both fixed: the pixel-rate arm now comes first, and what remains is codec-scoped to HEVC and only applies BELOW that bar, where a second engine buys nothing anyway. The rule rested on one datapoint: 5120x1440@240 Main10 on Ada, forced-2 7.6 ms vs 2.8 ms single-engine -- split 2.7x SLOWER. Dropping the short circuit flips that exact configuration's behaviour, so it was re-measured on a 4090 (AD102, driver 610.43.03), 400 Mbps, sub-frame pinned off, via a new mode-parameterizable Main10 A/B test (PF_AB_MODE=WxHxFPS reproduces the original operating point). Ada 4090 single forced-2 ratio 3840x2160@60 4483 us 2178 us 2.06x split WINS 5120x1440@240 3689 us 2813 us 1.31x split WINS <- the veto's origin 3840x2160@120 4148 us 2189 us 1.89x split WINS Blackwell 5070 Ti 3840x2160@60 4216 us 2477 us 1.70x split WINS 5120x1440@240 4651 us 3894 us 1.19x split WINS Split wins for Main10 at every mode on BOTH architectures, including the config the veto came from. The original number does not reproduce. ⚠ Caveats, unchanged from the rest of this work: content is trivial (297-300 B/AU against an 833 KB CBR quota -- zeroed VRAM), so this is the pixel-proportional term and the bits/frame regime is still unmeasured; debug build; and the driver differs from whenever the original was taken. Also validated on Ada in the same session -- the whole spike set reproduces on a SECOND architecture and an OLDER driver (610.43.03 vs 610.57.04): S1a in-place split switch accepted with zero IDRs both directions; S1b takes effect (|C-B|=12 vs |C-A|=1921, the cleanest run yet); S1c pair flip passes; D5 confirmed (AUTO+sub-frame 4424 vs DISABLE 4409, 15 us apart -- and AUTO without sub-frame 2310 ~= TWO_FORCED 2314, so the arm stays); engines=2 with THREE_FORCED correctly clamped to mode 2; arbitration converged with exactly 1 keyframe. Verified: .21 clippy -D warnings clean + 64 unit tests; .133 Windows clippy -D warnings clean (the resolver signature grew a `codec` param, so both backends moved); Ada + Blackwell on-hardware as above. fmt clean. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 110 ++++++++++++++++++- crates/pf-encode/src/enc/nvenc_core.rs | 93 +++++++++++++--- crates/pf-encode/src/enc/windows/nvenc.rs | 2 +- 3 files changed, 186 insertions(+), 19 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 8babbbbe..bb6c6ab4 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -1366,7 +1366,7 @@ impl NvencCudaEncoder { // [`resolve_split_mode`] for the precedence (env override / 10-bit / pixel rate). let pixel_rate = self.width as u64 * self.height as u64 * self.fps.max(1) as u64; let mut split_mode: u32 = - resolve_split_mode(self.bit_depth, pixel_rate, self.encoder_engines); + resolve_split_mode(self.codec, self.bit_depth, pixel_rate, self.encoder_engines); // A verdict this process already measured for this exact config wins over the static // rule — that is the whole point of arbitrating, and it lets later sessions skip the // ~1 s experiment. An operator pin still beats both (checked inside `resolve_split_mode`, @@ -3942,6 +3942,114 @@ mod tests { super::super::nvenc_core::clear_split_verdicts(); } + /// ON-HARDWARE — **THE ADA MAIN10 QUESTION**, the one this whole programme has been deferring. + /// + /// The 10-bit split veto rests on a single datapoint: at 5120×1440@240 Main10 on Ada, forced-2 + /// took 7.6 ms/frame against 2.8 ms single-engine — split was **2.7× SLOWER**. That number + /// vetoed splitting for every HDR session on every GPU, and `resolve_split_mode` has now + /// stopped short-circuiting on it, which means a Main10 session above the pixel-rate bar WILL + /// split. If the datapoint generalises, that is a regression and the veto has to come back + /// (scoped properly this time). + /// + /// So: 4K **Main10** (10-bit, via the packed-RGB10 input path), forced-2 against + /// single-engine, same bitrate, sub-frame pinned off so only the split variable moves. + /// Reports rather than asserts — both outcomes are legitimate findings and the point is the + /// number. Run on the **Ada** box (`.181`) and compare against Blackwell (`.21`): + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_main10_split_ab --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually (Ada .181 vs Blackwell .21)"] + fn nvenc_cuda_main10_split_ab() { + use std::time::Instant; + const BPS: u64 = 400_000_000; + const WARMUP: u32 = 12; + const MEASURED: u32 = 32; + // Mode is overridable so the SAME test can be pointed at the configuration the veto was + // originally measured on — `PF_AB_MODE=5120x1440x240` reproduces the 2.7×-slower datapoint's + // operating point, which is the one config this change flips behaviour for. + let (w, h, fps) = std::env::var("PF_AB_MODE") + .ok() + .and_then(|s| { + let p: Vec = s.split('x').filter_map(|v| v.parse().ok()).collect(); + (p.len() == 3).then(|| (p[0], p[1], p[2])) + }) + .unwrap_or((3840, 2160, 60)); + + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + // 10-bit input: the packed 2:10:10:10 PQ path is how a Main10 session is actually fed here + // (`bit_depth`/`hdr` are DERIVED from the input format, never trusted from the args). + let frames: Vec = (0..4).map(|i| rgb10_frame(w, h, i)).collect(); + + let run = |split: &str| -> (u128, u8, usize) { + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", split); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::X2Rgb10, + w, + h, + fps, + BPS, + true, + 10, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + let (mut times, mut bytes) = (Vec::new(), Vec::new()); + for i in 0..(WARMUP + MEASURED) { + let t0 = Instant::now(); + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + let mut got = 0usize; + while let Some(au) = enc.poll().expect("poll") { + got = au.data.len(); + } + if i >= WARMUP { + times.push(t0.elapsed().as_micros()); + bytes.push(got); + } + } + let depth = enc.bit_depth; + let opened = enc.split_mode; + enc.flush().ok(); + times.sort_unstable(); + bytes.sort_unstable(); + println!( + " (opened split_mode={opened}, derived bit_depth={depth}, \ + {} B/AU)", + bytes[bytes.len() / 2] + ); + (times[times.len() / 2], depth, bytes[bytes.len() / 2]) + }; + + println!( + "Main10 split A/B @ {w}x{h}@{fps} HEVC 10-bit, {} Mbps:", + BPS / 1_000_000 + ); + let (single_us, d1, _) = run("0"); + println!(" single-engine : {single_us:>6} us/frame"); + let (split_us, d2, _) = run("2"); + println!(" forced 2-way : {split_us:>6} us/frame"); + assert_eq!(d1, 10, "leg 1 did not derive a 10-bit session"); + assert_eq!(d2, 10, "leg 2 did not derive a 10-bit session"); + let ratio = single_us as f64 / split_us.max(1) as f64; + println!( + " ⇒ split is {ratio:.2}× the single-engine rate — {}", + if ratio > 1.15 { + "split WINS for Main10 here; the 2.7x-slower datapoint does NOT generalise" + } else if ratio < 0.87 { + "split LOSES for Main10 — the veto was right and must come back, scoped" + } else { + "a wash; neither arm is clearly better for Main10 here" + } + ); + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index 6c79d93a..589c1d73 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -86,14 +86,22 @@ pub(super) fn resolve_subframe(default_on: bool) -> bool { /// 1. `PUNKTFUNK_SPLIT_ENCODE` = `0`/`disable` | `1`/`auto` (AUTO_FORCED) | `2` | `3` — operator /// override, always wins, except that `2`/`3` are clamped to the GPU's real engine count (see /// [`clamp_to_engines`]; the driver honours an over-ask and silently encodes narrower). -/// 2. 10-bit → DISABLE: 2-way split is measurably SLOWER on Ada for Main10 — at 5120×1440@240 -/// forced-2 took 7.6 ms/frame (~131 fps) vs 2.8 ms (~357 fps) single-engine (the split/merge -/// overhead dominates), and a single engine handles 5K@240 Main10 well under budget. This was -/// the "broken animations in HDR" cap at ~131 fps. -/// 3. Pixel rate ≥ [`super::SPLIT_FORCE_PIXEL_RATE`] → force the WIDEST split the GPU can deliver +/// 2. Pixel rate ≥ [`super::SPLIT_FORCE_PIXEL_RATE`] → force the WIDEST split the GPU can deliver /// ([`max_forced_split_mode`]), not a hard-coded 2 (AUTO never engages below ~2112 px height, /// so 4K120 must be forced onto the other engines; and a 3-NVENC part left at 2-way wastes a /// third of its encode silicon). +/// 3. **HEVC** Main10 below that bar → DISABLE: 2-way split measured SLOWER on Ada for Main10 — at +/// 5120×1440@240 forced-2 took 7.6 ms/frame (~131 fps) vs 2.8 ms (~357 fps) single-engine, the +/// "broken animations in HDR" cap. ⚠ This rule used to sit ABOVE the pixel-rate arm and take no +/// codec, so it (a) vetoed 10-bit **4K120** — the very case the pixel-rate arm exists for — and +/// (b) applied an HEVC-on-Ada result to **AV1 10-bit**, which has no such measurement. Both +/// fixed; what remains is a conservative default in the regime where a second engine buys +/// nothing anyway. +/// ⚠⚠ **UNVALIDATED CONSEQUENCE:** 5120×1440@240 Main10 (1.77 Gpix/s) now clears the pixel-rate +/// bar and WILL be forced to split — i.e. the exact configuration that measurement came from +/// flips behaviour. That is deliberate (the datapoint is one sample, at low bits/frame, and the +/// bits/frame hypothesis predicts it should not generalise) but it is **the first thing to +/// re-measure on Ada**; `PUNKTFUNK_SPLIT_ENCODE=0` is the escape if it regresses. /// 4. Else AUTO — ⚠ whose behaviour is **conditional on sub-frame**, measured on `.21` at 4K: /// - sub-frame **ON** (the fleet default): AUTO **does not split** — 5023/5157 µs against /// DISABLE's 4979/5000. Split and sub-frame are mutually unsupported for HEVC, so the driver @@ -110,7 +118,12 @@ pub(super) fn resolve_subframe(default_on: bool) -> bool { /// `engines` is the GPU's `NV_ENC_CAPS_NUM_ENCODER_ENGINES`; pass `0` when it could not be probed /// (treated as "unknown", which keeps the pre-probe behaviour of assuming a second engine exists /// and letting the open-time rejection fallback sort it out). -pub(super) fn resolve_split_mode(bit_depth: u8, pixel_rate: u64, engines: u32) -> u32 { +pub(super) fn resolve_split_mode( + codec: Codec, + bit_depth: u8, + pixel_rate: u64, + engines: u32, +) -> u32 { use nv::NV_ENC_SPLIT_ENCODE_MODE as M; let hw_max = max_forced_split_mode(engines); let mode = match std::env::var("PUNKTFUNK_SPLIT_ENCODE").ok().as_deref() { @@ -118,14 +131,26 @@ pub(super) fn resolve_split_mode(bit_depth: u8, pixel_rate: u64, engines: u32) - Some("1") | Some("auto") => M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32, Some("3") => clamp_to_engines(M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, hw_max, engines), Some("2") => clamp_to_engines(M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, hw_max, engines), - _ if bit_depth >= 10 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, // Use every engine the card has, not a hard-coded two: on a 3-NVENC part (GB202, AD102 // workstation) forcing 2 leaves a third of the silicon idle. + // + // ⚠ This arm now comes FIRST, ahead of the 10-bit rule. That reordering is the D1 fix: a + // 10-bit 4K120 session (995.3 Mpix/s) used to be vetoed by the depth rule before ever + // reaching the pixel-rate arm written for exactly it. _ if pixel_rate >= super::SPLIT_FORCE_PIXEL_RATE => hw_max, + // Below that bar, HEVC Main10 keeps the conservative single-engine default. The one Ada + // measurement we have says split can be *slower* for Main10, and nothing under this bar + // needs a second engine anyway — so the cost of being wrong here is ~nil, unlike above it. + // + // ⚠ Now codec-scoped (the D2 fix): the measurement behind this was HEVC Main10 on Ada, and + // it used to veto **AV1 10-bit** too, which has neither the sub-frame conflict nor any + // measurement against it. + _ if codec == Codec::H265 && bit_depth >= 10 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, _ => M::NV_ENC_SPLIT_AUTO_MODE as u32, }; tracing::debug!( split_mode = mode, + ?codec, bit_depth, pixel_rate, engines, @@ -695,7 +720,7 @@ mod tests { // 4090 because AUTO never engages at 2160 px height. let four_k_120 = 3840u64 * 2160 * 120; assert_eq!( - resolve_split_mode(8, four_k_120, 2), + resolve_split_mode(Codec::H265, 8, four_k_120, 2), M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 ); } @@ -705,22 +730,56 @@ mod tests { // 884.7 Mpix/s is comfortably single-engine — the threshold move must not drag it in. let qhd_240 = 2560u64 * 1440 * 240; assert_eq!( - resolve_split_mode(8, qhd_240, 2), + resolve_split_mode(Codec::H265, 8, qhd_240, 2), M::NV_ENC_SPLIT_AUTO_MODE as u32 ); } #[test] - fn split_disabled_for_10bit_even_at_high_pixel_rate() { - // The measured Main10 rule: split/merge overhead dominates 10-bit on Ada (7.6 ms forced-2 - // vs 2.8 ms single-engine at 5K240) — 10-bit precedes the pixel-rate arm. - let five_k_240 = 5120u64 * 1440 * 240; + fn split_rules_for_10bit_after_dropping_the_short_circuit() { + let five_k_240 = 5120u64 * 1440 * 240; // 1.77 Gpix/s — over the bar + let four_k_120 = 3840u64 * 2160 * 120; // 995.3 Mpix/s — over the bar + let hd_60 = 1920u64 * 1080 * 60; // 124 Mpix/s — well under + + // ⚠ BEHAVIOUR FLIP, deliberate: the config the Main10 veto was measured on (7.6 ms + // forced-2 vs 2.8 ms single-engine on Ada) now clears the pixel-rate bar and SPLITS. The + // datapoint is one sample at low bits/frame; re-measuring it on Ada is the first on-glass + // item, and PUNKTFUNK_SPLIT_ENCODE=0 is the escape if it regresses. assert_eq!( - resolve_split_mode(10, five_k_240, 2), + resolve_split_mode(Codec::H265, 10, five_k_240, 2), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 + ); + // D1: 10-bit 4K120 used to be vetoed by the depth rule BEFORE reaching the pixel-rate arm + // written for exactly it. It splits now. + assert_eq!( + resolve_split_mode(Codec::H265, 10, four_k_120, 2), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 + ); + // Under the bar, HEVC Main10 keeps the conservative single-engine default — a second + // engine buys nothing there, so being wrong costs ~nil. + assert_eq!( + resolve_split_mode(Codec::H265, 10, hd_60, 2), M::NV_ENC_SPLIT_DISABLE_MODE as u32 ); } + /// D2: the Main10 rule was measured on HEVC and used to be codec-blind, so it vetoed **AV1 + /// 10-bit** — which has neither the sub-frame conflict nor any measurement against it. + #[test] + fn av1_10bit_is_no_longer_vetoed_by_an_hevc_measurement() { + let hd_60 = 1920u64 * 1080 * 60; + let four_k_120 = 3840u64 * 2160 * 120; + assert_eq!( + resolve_split_mode(Codec::Av1, 10, hd_60, 2), + M::NV_ENC_SPLIT_AUTO_MODE as u32, + "AV1 10-bit must follow the ordinary path, not inherit an HEVC veto" + ); + assert_eq!( + resolve_split_mode(Codec::Av1, 10, four_k_120, 2), + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 + ); + } + /// THE ENGINE-COUNT FIX: a high-pixel-rate session must use every engine the GPU has, not a /// hard-coded two. A 3-NVENC part (GB202 / AD102 workstation) left at 2-way wastes a third of /// its encode silicon, and the driver never complains because it accepts an over- OR @@ -729,17 +788,17 @@ mod tests { fn split_uses_every_engine_the_gpu_has() { let four_k_120 = 3840u64 * 2160 * 120; assert_eq!( - resolve_split_mode(8, four_k_120, 3), + resolve_split_mode(Codec::H265, 8, four_k_120, 3), M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, "a 3-engine GPU must split three ways" ); assert_eq!( - resolve_split_mode(8, four_k_120, 1), + resolve_split_mode(Codec::H265, 8, four_k_120, 1), M::NV_ENC_SPLIT_DISABLE_MODE as u32, "a 1-engine GPU must not pretend to split — today this costs a wasted session open" ); assert_eq!( - resolve_split_mode(8, four_k_120, 0), + resolve_split_mode(Codec::H265, 8, four_k_120, 0), M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, "unprobed engine count keeps the historical assumption; the rejection fallback corrects" ); diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index 6e8a63ae..9e562c2f 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -1166,7 +1166,7 @@ impl NvencD3d11Encoder { // The init-failure fallback below disables it if a codec/config rejects it. let pixel_rate = self.width as u64 * self.height as u64 * self.fps.max(1) as u64; let split_mode: u32 = - resolve_split_mode(self.bit_depth, pixel_rate, self.encoder_engines); + resolve_split_mode(self.codec, self.bit_depth, pixel_rate, self.encoder_engines); // Negotiated multi-slice (P2f): the direct-NVENC default of 4, clamped by the // client's ceiling — a single-slice client keeps today's shape, a // VIDEO_CAP_MULTI_SLICE / Moonlight slices-per-frame client gets real slices. From 1062aa780ffe0366c0c2b381eb1762fd24402070 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 00:30:09 +0200 Subject: [PATCH 08/12] =?UTF-8?q?test(pf-encode):=20measure=20the=20bits/f?= =?UTF-8?q?rame=20curve=20=E2=80=94=20no=20crossover,=20split=20always=20w?= =?UTF-8?q?ins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WP0's real deliverable, and the hole every previous measurement in this programme had. All prior timings ran against driver-zeroed buffers, so rate control had nothing to code (~300 B/AU against an 833 KB quota) and only the PIXEL-proportional half of the encode cost was ever exercised -- while the 4K60 HDR field report was a BITS/FRAME problem at 6.8 Mbit/frame. Adds `pf_zerocopy::cuda::write_plane_from_host`, the exact mirror of the existing read_plane_to_host. No new loader entry was needed: cuMemcpy2DAsync_v2 was already in the table and CUDA_MEMCPY2D just needed the reverse memory types. Linux-only by construction (pf-zerocopy's `imp` is cfg'd to linux). ⚠ Two harness mistakes found and fixed by looking at bytes/AU rather than trusting the knob: - Pure per-pixel noise is INCOMPRESSIBLE, so a low bitrate target does not produce low bits/frame -- it OVERSHOOTS. At a nominal 50 Mbps the encoder emitted 719 KB/AU against a 104 KB quota, and the three lowest rows of the first sweep all sat at the same ~5.7 Mbit/frame. Sweeping nominal bitrate measures nothing. - So the sweep moves CONTENT DETAIL (block size) instead, and the x-axis is the bits/frame the encoder ACTUALLY produced, never the one requested. 4K60 HEVC 8-bit, real content, single-engine vs forced-2: bits/frame Ada 4090 Blackwell 5070 Ti 0.2-0.3 Mb 4567 -> 2381 1.92x 5549 -> 3552 1.56x ~1.1-1.2 Mb 5060 -> 2626 1.93x 5867 -> 4082 1.44x ~3.3 Mb 8478 -> 4455 1.90x 9286 -> 5862 1.58x ~9.6 Mb 16237 -> 8114 2.00x 16435 -> 9275 1.77x RESULTS. (1) Encode time scales strongly with bits/frame -- 4.6 ms to 16.2 ms across the range on Ada -- confirming the hypothesis' core claim. (2) There is NO CROSSOVER: split wins at every point on both architectures (Ada ~1.9-2.0x and notably flat, Blackwell 1.44-1.77x). So the arbitration's encode-side answer is essentially always "split", which makes the sub-frame handicap the only decision that actually matters -- exactly the part already built and unit-pinned. (3) It corroborates the field capture: at ~6.8 Mbit/frame these curves put single-engine 4K60 around 10-13 ms, and the field report was 10.3 ms on a 4090. That reads as real ASIC time, not the retrieve-queue inflation it might have been. ⚠ Caveat the data itself shows: cost is NOT monotonic in bits/frame alone. The 1px row lands at the HIGHEST bits/frame yet encodes FASTER than the 4px row on both boxes (Ada 10148 vs 16237 us) -- pure noise defeats motion estimation, which gives up early, where semi-structured content makes it search hard. Content structure is a real term, so "bits/frame" is a good axis but not a complete cost model. Verified .21: clippy -D warnings clean (pf-encode + pf-zerocopy), 64 unit tests, 25/25 NVENC on-hardware. Curves run on both Ada and Blackwell. fmt clean. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 147 +++++++++++++++++++ crates/pf-zerocopy/src/imp/cuda.rs | 41 ++++++ 2 files changed, 188 insertions(+) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index bb6c6ab4..26e50ee1 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -2754,6 +2754,60 @@ mod tests { enc.encoder_engines } + /// An NV12 frame filled with **real high-entropy content**, not the zeroed VRAM every other + /// helper here hands the encoder. + /// + /// This matters more than it looks. Under CBR the rate controller spends its quota only if + /// there is something to code; against uninitialised (driver-zeroed) buffers it emits ~300 B/AU + /// where the configured rate wants ~833 KB, so every timing taken that way measures the + /// PIXEL-proportional cost and is blind to the bits/frame regime — the regime the 4K60 HDR + /// field report actually came from. A cheap xorshift per pixel plus a per-frame seed gives both + /// spatial detail (so intra costs real bits) and inter-frame change (so P-frames cannot + /// skip-code), which is what drives the entropy coder. + /// `block` sets the spatial detail: 1 = per-pixel noise (incompressible — rate control + /// OVERSHOOTS any low target), larger = blockier and cheaper to code. Sweeping it is how the + /// bench reaches the LOW bits/frame end at all; pure noise cannot get there. + fn noise_nv12_frame(w: u32, h: u32, i: u32, block: usize) -> CapturedFrame { + let buf = DeviceBuffer::alloc_nv12(w, h).expect("alloc NV12 device buffer"); + let (uv_ptr, uv_pitch) = buf.uv.expect("NV12 buffer has a UV plane"); + let mut st = 0x2545_F491_4F6C_DD1Du64 ^ ((i as u64 + 1) << 32); + let mut next = move || { + st ^= st << 13; + st ^= st >> 7; + st ^= st << 17; + st + }; + let b = block.max(1); + let mut plane = |pw: usize, ph: usize| -> Vec { + let bw = pw.div_ceil(b); + let cells: Vec = (0..(bw * ph.div_ceil(b))) + .map(|_| (next() >> 24) as u8) + .collect(); + let mut out = Vec::with_capacity(pw * ph); + for y in 0..ph { + let row = y / b * bw; + for x in 0..pw { + out.push(cells[row + x / b]); + } + } + out + }; + let y = plane(w as usize, h as usize); + let uv = plane(w as usize, h as usize / 2); + pf_zerocopy::cuda::write_plane_from_host(buf.ptr, buf.pitch, &y, w as usize, h as usize) + .expect("upload Y plane"); + pf_zerocopy::cuda::write_plane_from_host(uv_ptr, uv_pitch, &uv, w as usize, h as usize / 2) + .expect("upload UV plane"); + CapturedFrame { + width: w, + height: h, + pts_ns: i as u64 * 16_666_667, + format: PixelFormat::Nv12, + payload: FramePayload::Cuda(buf), + cursor: None, + } + } + fn nv12_frame(w: u32, h: u32, i: u32) -> CapturedFrame { // Content is uninitialized device memory — NVENC encodes it fine; this smoke test asserts the // session/registration/encode/RFI machinery, not picture fidelity (that's the on-glass A/B). @@ -4050,6 +4104,99 @@ mod tests { std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); } + /// ON-HARDWARE — **THE BITS/FRAME CURVE**, the measurement this whole programme has been blind + /// to (WP0's real deliverable). + /// + /// Every other timing here was taken against driver-zeroed buffers, so rate control had + /// nothing to code (~300 B/AU against an 833 KB quota) and only the PIXEL-proportional half of + /// the encode cost was ever exercised. But the 4K60 HDR field report was a *bits/frame* + /// problem — 6.8 Mbit/frame — and the central hypothesis is that split's benefit and the + /// 10-bit veto's origin both live on that axis. [`noise_nv12_frame`] finally puts real entropy + /// in front of the encoder. + /// + /// Sweeps bitrate at a fixed mode, single-engine vs forced-2, and prints **bytes/AU alongside + /// every timing** — without that column a run that silently undershoots its quota looks like a + /// result instead of a non-measurement. Run on both boxes: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_cuda_bits_per_frame_curve --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually (Ada .181 / Blackwell .21)"] + fn nvenc_cuda_bits_per_frame_curve() { + use std::time::Instant; + const WARMUP: u32 = 10; + const MEASURED: u32 = 24; + let (w, h, fps) = std::env::var("PF_AB_MODE") + .ok() + .and_then(|s| { + let p: Vec = s.split('x').filter_map(|v| v.parse().ok()).collect(); + (p.len() == 3).then(|| (p[0], p[1], p[2])) + }) + .unwrap_or((3840, 2160, 60)); + + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + pf_zerocopy::cuda::make_current().expect("shared CUDA context current"); + // Sweep CONTENT DETAIL, not nominal bitrate. Pure noise is incompressible, so a low + // bitrate target simply overshoots (measured: 719 KB/AU against a 104 KB quota) and every + // low row lands at the same high bits/frame — the exact blindness this test exists to fix. + // Blockier content codes cheaper, so detail is what actually moves along the axis, and the + // x-axis below is the bits/frame the encoder ACTUALLY produced, never the one requested. + let bps: u64 = 600_000_000; + println!( + "bits/frame curve @ {w}x{h}@{fps} HEVC 8-bit, REAL content, {} Mbps cap:", + bps / 1_000_000 + ); + println!(" detail | ACTUAL bits/frame | single | split-2 | ratio"); + for block in [64usize, 32, 16, 8, 4, 1] { + let frames: Vec = + (0..4).map(|i| noise_nv12_frame(w, h, i, block)).collect(); + let run = |split: &str| -> (u128, usize) { + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", split); + let mut enc = NvencCudaEncoder::open( + Codec::H265, + PixelFormat::Nv12, + w, + h, + fps, + bps, + true, + 8, + ChromaFormat::Yuv420, + false, + 4, + ) + .expect("open NVENC CUDA session"); + let (mut times, mut bytes) = (Vec::new(), Vec::new()); + for i in 0..(WARMUP + MEASURED) { + let t0 = Instant::now(); + enc.submit_indexed(&frames[(i % 4) as usize], i) + .expect("submit"); + let mut got = 0usize; + while let Some(au) = enc.poll().expect("poll") { + got = au.data.len(); + } + if i >= WARMUP { + times.push(t0.elapsed().as_micros()); + bytes.push(got); + } + } + enc.flush().ok(); + times.sort_unstable(); + bytes.sort_unstable(); + (times[times.len() / 2], bytes[bytes.len() / 2]) + }; + let (s_us, s_bytes) = run("0"); + let (p_us, _) = run("2"); + println!( + " {block:>5}px | {:>10.2} Mbit | {s_us:>6}us | {p_us:>6}us | {:>4.2}×", + s_bytes as f64 * 8.0 / 1e6, + s_us as f64 / p_us.max(1) as f64 + ); + } + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + /// A pre-session RFI request and nonsense ranges all correctly decline (→ caller forces IDR). /// Needs no GPU session (it short-circuits on the null encoder / range checks), so it runs in the /// normal suite — but `open` gates on the NVENC `.so`, so it skips gracefully where the NVIDIA diff --git a/crates/pf-zerocopy/src/imp/cuda.rs b/crates/pf-zerocopy/src/imp/cuda.rs index 5d5a0a47..bfb09f48 100644 --- a/crates/pf-zerocopy/src/imp/cuda.rs +++ b/crates/pf-zerocopy/src/imp/cuda.rs @@ -62,6 +62,47 @@ pub fn read_plane_to_host( Ok(host) } +/// Upload a tightly-packed host plane into a pitched device plane `(dst_ptr, dst_pitch)`. +/// Synchronous on the priority stream. The exact mirror of [`read_plane_to_host`]. +/// +/// Not a hot path and never used by a session — this exists so ENCODE BENCHMARKS can put real, +/// high-entropy content in front of the encoder. Every synthetic frame this crate could otherwise +/// produce is uninitialised device memory, which the driver hands back **zeroed**; under CBR the +/// rate controller then runs out of things to code and every measurement collapses into the +/// low-bits/frame corner (~300 B/AU against an 833 KB quota, measured). That made the entire +/// split-encode programme blind to the bits/frame regime, which is the regime the field report +/// came from. +pub fn write_plane_from_host( + dst_ptr: CUdeviceptr, + dst_pitch: usize, + src: &[u8], + width_bytes: usize, + height: usize, +) -> Result<()> { + anyhow::ensure!( + src.len() >= width_bytes * height, + "write_plane_from_host: source is {} bytes, need {}", + src.len(), + width_bytes * height + ); + let copy = CUDA_MEMCPY2D { + srcMemoryType: 1, // CU_MEMORYTYPE_HOST + srcHost: src.as_ptr() as *const c_void, + srcPitch: width_bytes, + dstMemoryType: CU_MEMORYTYPE_DEVICE, + dstDevice: dst_ptr, + dstPitch: dst_pitch, + WidthInBytes: width_bytes, + Height: height, + ..Default::default() + }; + // SAFETY: mirrors `read_plane_to_host`. `©` is a live local `#[repr(C)] CUDA_MEMCPY2D` + // outliving the synchronous call; `srcHost` addresses `src`, checked above to hold at least + // `width_bytes*height` bytes, and `dstDevice`/`dstPitch` are the caller's live pitched device + // plane. The copy is synchronous, so `src` need not outlive the call. + unsafe { copy_blocking(©, "cuMemcpy2DAsync_v2(host->dev)") } +} + /// Export a device allocation (from `cuMemAllocPitch`/`cuMemAlloc`) as a cross-process CUDA IPC /// handle — an opaque 64-byte blob another process opens with [`ipc_open`]. The allocation must /// stay alive for as long as any importer has it open. The shared context must be current. From 01294e3a53ab86f78282550848f2467fe8e0f4e6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 08:16:35 +0200 Subject: [PATCH 09/12] =?UTF-8?q?refactor(pf-encode):=20WP4=20=E2=80=94=20?= =?UTF-8?q?one=20split=20policy,=20shared=20with=20the=20libav=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The libav NVENC path carried its own inline copy of the split decision and had already drifted from the direct-SDK selector: it hard-coded a 2-way split regardless of engine count, and had no depth rule at all. That is the drift the shared resolver was extracted to prevent, and the copy quietly reintroduced it. Routing it through `resolve_split_mode` needed the policy to MOVE. `nvenc_core` is gated on `feature = "nvenc"`, but the libav path is precisely the build where that feature is OFF (`PUNKTFUNK_NVENC_DIRECT=0`, and the featureless packages -- the packaging gap this project has been bitten by before). So resolve_split_mode / max_forced_split_mode / clamp_to_engines, plus a new `forced_split_width`, now live in `codec.rs`, which is always compiled and already owned SPLIT_FORCE_PIXEL_RATE. That means the NV_ENC_SPLIT_ENCODE_MODE values had to be hand-written as plain constants, since the SDK enum does not exist without the feature. They are therefore pinned: `nvenc_split_constants_match_the_sdk` (feature-gated, the only place both are visible at once) asserts all five against the real enum, so the copies cannot rot. ⚠ Only the FORCED outcomes are actionable on the libav side -- libavcodec's `split_encode_mode` AVOption is its own vocabulary and our DISABLE is the NVENC enum's 15, which would be meaningless there. DISABLE/AUTO both map to "leave the option unset", which is exactly today's behaviour (unset = the driver's auto). `engines = 0` ("not probed") maps to 2-way, preserving what that site always did; a 3-NVENC part gets the wider split only on the direct-SDK path, which is the one that actually probes. ⚠⚠ VERIFICATION GAP: .133 went down mid-change (no ping), so the WINDOWS leg is UNVERIFIED. This matters more than usual -- the Windows backend imported resolve_split_mode from nvenc_core and that import had to move too, which a grep caught rather than a compiler. Re-run before trusting it: cargo clippy -p pf-encode --features nvenc --all-targets -- -D warnings Verified .21: clippy -D warnings clean BOTH with and without the nvenc feature (the featureless build is the whole point of the move) and with nvenc,vulkan-encode; 65 unit tests incl. the new constant-parity test; 25/25 NVENC on-hardware; punktfunk-host clippy clean. fmt clean. --- crates/pf-encode/src/enc/codec.rs | 157 ++++++++++++++++++- crates/pf-encode/src/enc/linux/mod.rs | 42 +++-- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 10 +- crates/pf-encode/src/enc/nvenc_core.rs | 154 ++++-------------- crates/pf-encode/src/enc/windows/nvenc.rs | 9 +- 5 files changed, 222 insertions(+), 150 deletions(-) diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index d89e2d7a..7a59e053 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -518,7 +518,7 @@ impl Codec { } /// Pixel rate (luma samples/s) at or above which NVENC split-frame encoding is FORCED 2-way — -/// one number shared by the direct-SDK selector (`nvenc_core::resolve_split_mode`) and the libav +/// one number shared by the direct-SDK selector ([`resolve_split_mode`]) and the libav /// `split_encode_mode` option author (`linux::NvencEncoder`), so the two paths can never disagree /// about which modes split. A single NVENC engine tops out ~1 Gpix/s on HEVC, and AUTO doesn't /// engage below ~2112 px height, so the sessions that need the second engine must be forced. Set @@ -528,6 +528,161 @@ impl Codec { /// comfortably single-engine) on AUTO. pub const SPLIT_FORCE_PIXEL_RATE: u64 = 950_000_000; +/// The `NV_ENC_SPLIT_ENCODE_MODE` values, as plain constants. +/// +/// They live HERE, not in `nvenc_core`, because the split policy below has to be shared with the +/// **libav** NVENC path — which compiles with the `nvenc` feature OFF (that is the whole +/// `PUNKTFUNK_NVENC_DIRECT=0` / featureless-package build), where the SDK enum does not exist. +/// One policy, no drift, was the point of extracting it; gating it behind the feature would have +/// left the libav copy free to diverge again, which is exactly what it had already done. +/// +/// `nvenc_split_constants_match_the_sdk` (feature-gated) pins these against the real enum, so the +/// hand-written values cannot rot. +pub(crate) const SPLIT_AUTO: u32 = 0; +pub(crate) const SPLIT_AUTO_FORCED: u32 = 1; +pub(crate) const SPLIT_TWO_FORCED: u32 = 2; +pub(crate) const SPLIT_THREE_FORCED: u32 = 3; +pub(crate) const SPLIT_DISABLE: u32 = 15; + +/// Resolved NVENC split-frame encode mode for a session — ONE selector shared by the Windows and +/// Linux direct-SDK backends (they had drifted into byte-identical duplicates, one of which +/// logged and one didn't). Precedence: +/// 1. `PUNKTFUNK_SPLIT_ENCODE` = `0`/`disable` | `1`/`auto` (AUTO_FORCED) | `2` | `3` — operator +/// override, always wins, except that `2`/`3` are clamped to the GPU's real engine count (see +/// [`clamp_to_engines`]; the driver honours an over-ask and silently encodes narrower). +/// 2. Pixel rate ≥ [`SPLIT_FORCE_PIXEL_RATE`] → force the WIDEST split the GPU can deliver +/// ([`max_forced_split_mode`]), not a hard-coded 2 (AUTO never engages below ~2112 px height, +/// so 4K120 must be forced onto the other engines; and a 3-NVENC part left at 2-way wastes a +/// third of its encode silicon). +/// 3. **HEVC** Main10 below that bar → DISABLE: 2-way split measured SLOWER on Ada for Main10 — at +/// 5120×1440@240 forced-2 took 7.6 ms/frame (~131 fps) vs 2.8 ms (~357 fps) single-engine, the +/// "broken animations in HDR" cap. ⚠ This rule used to sit ABOVE the pixel-rate arm and take no +/// codec, so it (a) vetoed 10-bit **4K120** — the very case the pixel-rate arm exists for — and +/// (b) applied an HEVC-on-Ada result to **AV1 10-bit**, which has no such measurement. Both +/// fixed; what remains is a conservative default in the regime where a second engine buys +/// nothing anyway. +/// ⚠⚠ **UNVALIDATED CONSEQUENCE:** 5120×1440@240 Main10 (1.77 Gpix/s) now clears the pixel-rate +/// bar and WILL be forced to split — i.e. the exact configuration that measurement came from +/// flips behaviour. That is deliberate (the datapoint is one sample, at low bits/frame, and the +/// bits/frame hypothesis predicts it should not generalise) but it is **the first thing to +/// re-measure on Ada**; `PUNKTFUNK_SPLIT_ENCODE=0` is the escape if it regresses. +/// 4. Else AUTO — ⚠ whose behaviour is **conditional on sub-frame**, measured on `.21` at 4K: +/// - sub-frame **ON** (the fleet default): AUTO **does not split** — 5023/5157 µs against +/// DISABLE's 4979/5000. Split and sub-frame are mutually unsupported for HEVC, so the driver +/// resolves AUTO to no-split and this arm silently means DISABLE. +/// - sub-frame **OFF**: AUTO **does split** — 2401/2352 µs against TWO_FORCED's 2319/2378. +/// +/// So AUTO is NOT dead in general and must not be retired: doing so would lose a real split on +/// every sub-frame-off session. It is dead only in the sub-frame-on combination, which +/// [`resolve_split_subframe`] logs rather than silently accepting. +/// +/// The caller still owns the rejection fallback (retry split-disabled) — a codec/config that +/// rejects the chosen mode downgrades at open, not here. +/// +/// `engines` is the GPU's `NV_ENC_CAPS_NUM_ENCODER_ENGINES`; pass `0` when it could not be probed +/// (treated as "unknown", which keeps the pre-probe behaviour of assuming a second engine exists +/// and letting the open-time rejection fallback sort it out). +pub(crate) fn resolve_split_mode( + codec: Codec, + bit_depth: u8, + pixel_rate: u64, + engines: u32, +) -> u32 { + let hw_max = max_forced_split_mode(engines); + let mode = match std::env::var("PUNKTFUNK_SPLIT_ENCODE").ok().as_deref() { + Some("0") | Some("disable") => SPLIT_DISABLE, + Some("1") | Some("auto") => SPLIT_AUTO_FORCED, + Some("3") => clamp_to_engines(SPLIT_THREE_FORCED, hw_max, engines), + Some("2") => clamp_to_engines(SPLIT_TWO_FORCED, hw_max, engines), + // Use every engine the card has, not a hard-coded two: on a 3-NVENC part (GB202, AD102 + // workstation) forcing 2 leaves a third of the silicon idle. + // + // ⚠ This arm now comes FIRST, ahead of the 10-bit rule. That reordering is the D1 fix: a + // 10-bit 4K120 session (995.3 Mpix/s) used to be vetoed by the depth rule before ever + // reaching the pixel-rate arm written for exactly it. + _ if pixel_rate >= SPLIT_FORCE_PIXEL_RATE => hw_max, + // Below that bar, HEVC Main10 keeps the conservative single-engine default. The one Ada + // measurement we have says split can be *slower* for Main10, and nothing under this bar + // needs a second engine anyway — so the cost of being wrong here is ~nil, unlike above it. + // + // ⚠ Now codec-scoped (the D2 fix): the measurement behind this was HEVC Main10 on Ada, and + // it used to veto **AV1 10-bit** too, which has neither the sub-frame conflict nor any + // measurement against it. + _ if codec == Codec::H265 && bit_depth >= 10 => SPLIT_DISABLE, + _ => SPLIT_AUTO, + }; + tracing::debug!( + split_mode = mode, + ?codec, + bit_depth, + pixel_rate, + engines, + "NVENC split-encode mode selected" + ); + mode +} + +/// The strongest split mode this GPU's engine count can actually deliver. +/// +/// ⚠ **The driver will NOT tell you when you over-ask.** Measured on `.21` (RTX 5070 Ti, 2 NVENC, +/// driver 610.57.04, 4K HEVC): requesting `THREE_FORCED` was **HONOURED** — session opened in mode +/// 3 — and ran at **2303 µs/frame, identical to `TWO_FORCED`'s 2308**. No rejection, no warning, +/// no third engine; just a log line claiming 3-way over a 2-way encode. So the rejection fallback +/// cannot be relied on to find the ceiling and the clamp has to happen here. +/// +/// `NV_ENC_SPLIT_ENCODE_MODE` can only *name* counts up to three (SDK 0.4.0 / NVENCAPI 12.1; +/// values 4..14 are unallocated, so a future API may extend it). Above that we fall back to +/// `AUTO_FORCED` = "split, driver picks how many", which measurably does force a split (2.01× vs +/// disabled on the same box) and is the only way to express "use everything you have". +pub(crate) fn max_forced_split_mode(engines: u32) -> u32 { + match engines { + // Unknown (cap unreadable / not probed): keep the historical assumption of a second + // engine and let the open-time rejection fallback correct it. + 0 => SPLIT_TWO_FORCED, + 1 => SPLIT_DISABLE, + 2 => SPLIT_TWO_FORCED, + 3 => SPLIT_THREE_FORCED, + // More engines than the enum can name — let the driver use them all. + _ => SPLIT_AUTO_FORCED, + } +} + +/// The N of an N-way FORCED split, or `None` for the modes that do not name a width +/// (`DISABLE`, plain `AUTO`, and `AUTO_FORCED` — the last forces a split but lets the driver +/// choose how wide). +/// +/// For callers that can only express "split this many ways" and have no vocabulary for our other +/// modes — the libav path, whose `split_encode_mode` AVOption is libavcodec's own enum, not the +/// NVENC one (our `DISABLE` is `15`, which would be meaningless there). +pub(crate) fn forced_split_width(mode: u32) -> Option { + match mode { + m if m == SPLIT_TWO_FORCED => Some(2), + m if m == SPLIT_THREE_FORCED => Some(3), + _ => None, + } +} + +/// Hold an operator's `PUNKTFUNK_SPLIT_ENCODE=2|3` to what the hardware can deliver, loudly. +/// Without this the knob silently lies (see [`max_forced_split_mode`]); an override that asks for +/// more engines than exist is a mistake worth surfacing, not honouring. +pub(crate) fn clamp_to_engines(requested: u32, hw_max: u32, engines: u32) -> u32 { + // Only the named N-way modes are ordered; `hw_max` may be AUTO_FORCED (1) on a >3-engine part, + // which is not "less than" TWO_FORCED and must not clamp a legitimate request down. + let named = |m: u32| (2..=3).contains(&m); + if engines != 0 && named(requested) && named(hw_max) && requested > hw_max { + tracing::warn!( + requested, + engines, + using = hw_max, + "PUNKTFUNK_SPLIT_ENCODE asks for more NVENC engines than this GPU has — clamping. \ + (The driver would ACCEPT the over-ask and silently encode with fewer, so the log \ + would otherwise claim a split width that never happened.)" + ); + return hw_max; + } + requested +} + /// `PUNKTFUNK_VBV_FRAMES` — HRD/VBV size in frame intervals (default 1.0, the strict low-latency /// shape every backend ships: each frame must fit its rate share, keeping frame sizes uniform for /// the pacer). The AMF/VAAPI/QSV paths parse the same variable locally; this helper brings the diff --git a/crates/pf-encode/src/enc/linux/mod.rs b/crates/pf-encode/src/enc/linux/mod.rs index af9e5b65..ab783e9d 100644 --- a/crates/pf-encode/src/enc/linux/mod.rs +++ b/crates/pf-encode/src/enc/linux/mod.rs @@ -476,13 +476,22 @@ impl NvencEncoder { opts.set("profile", "main10"); } - // Split-frame encode across both NVENC engines (GB203 has 2) when the pixel rate exceeds - // a single engine's HEVC capacity; e.g. 5120x1440@240 = 1.77 Gpix/s needs it, @120 - // (0.88 Gpix/s) does not. HEVC/AV1 only (not H.264). AUTO won't engage below ~2112px - // height, so we force `2`; below the threshold we leave it AUTO (split costs ~2% BD-rate). - // Threshold shared with the direct-SDK selector ([`super::SPLIT_FORCE_PIXEL_RATE`] — set - // so 4K120 = 995.3 Mpix/s forces, which `> 1e9` famously missed by 0.47%). Output is - // standard HEVC — transparent to the client. Override with PUNKTFUNK_SPLIT_ENCODE. + // Split-frame encode across the GPU's NVENC engines. WP4: the policy is no longer + // duplicated here — it comes from the SAME [`resolve_split_mode`] the two direct-SDK + // backends use, so the pixel-rate threshold, the codec scoping and the (dropped) 10-bit + // short circuit cannot drift between the libav path and the rest. This copy had already + // diverged: it hard-coded a 2-way split regardless of engine count and carried no depth + // rule at all. + // + // ⚠ Only the FORCED outcomes are actionable here. libavcodec's `split_encode_mode` + // AVOption is its own vocabulary, and our `DISABLE` is the NVENC enum's `15` — passing + // that through would be meaningless to it (or fail the open). `DISABLE`/`AUTO` therefore + // both mean "leave the option unset", which is exactly today's behaviour: unset = the + // driver's own auto. + // + // ⚠ `engines = 0` = "not probed": the libav path has no caps probe of its own, and + // [`max_forced_split_mode`] maps unknown to 2-way, preserving what this site always did. + // A 3-NVENC part gets the wider split only on the direct-SDK path. let pix_rate = width as u64 * height as u64 * fps as u64; let split = std::env::var("PUNKTFUNK_SPLIT_ENCODE").ok(); match split.as_deref() { @@ -497,14 +506,17 @@ impl NvencEncoder { "PUNKTFUNK_SPLIT_ENCODE ignored — split encoding is not applicable to H.264 \ (nvEncodeAPI.h)" ), - None if matches!(codec, Codec::H265 | Codec::Av1) - && pix_rate >= super::SPLIT_FORCE_PIXEL_RATE => - { - opts.set("split_encode_mode", "2"); - tracing::info!( - pix_rate, - "NVENC: forcing 2-way split encode (high pixel rate)" - ); + None if matches!(codec, Codec::H265 | Codec::Av1) => { + let resolved = super::resolve_split_mode(codec, bit_depth, pix_rate, 0); + if let Some(n) = super::forced_split_width(resolved) { + opts.set("split_encode_mode", &n.to_string()); + tracing::info!( + pix_rate, + bit_depth, + split_encode_mode = n, + "NVENC (libav): forcing split encode (shared selector)" + ); + } } None => {} } diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index 26e50ee1..f1a80847 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -68,12 +68,12 @@ use super::nvenc_core::{ apply_low_latency_config, build_init_params, cached_ceiling, cached_split_verdict, codec_guid, - max_forced_split_mode, plan_range_recovery, resolve_slices, resolve_split_mode, - resolve_split_subframe, resolve_subframe, store_ceiling, store_split_verdict, - subframe_env_forced, ArbAction, CeilingKey, LowLatencyConfig, NvStatusExt, RangePlan, - SplitArbiter, SplitKey, + plan_range_recovery, resolve_slices, resolve_split_subframe, resolve_subframe, store_ceiling, + store_split_verdict, subframe_env_forced, ArbAction, CeilingKey, LowLatencyConfig, NvStatusExt, + RangePlan, SplitArbiter, SplitKey, }; use super::nvenc_status; +use super::{max_forced_split_mode, resolve_split_mode}; use super::{AuChunk, ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps}; use anyhow::{anyhow, bail, Context, Result}; use pf_frame::{CapturedFrame, FramePayload}; @@ -826,7 +826,7 @@ pub struct NvencCudaEncoder { /// `NV_ENC_CAPS_NUM_ENCODER_ENGINES` — how many NVENC engines this GPU has, probed in /// [`query_caps`]. `0` = not probed / unreadable. The split-encode ceiling: the driver accepts /// a split wider than the hardware and silently encodes narrower, so this is the only honest - /// source for how wide we may go (see `nvenc_core::max_forced_split_mode`). + /// source for how wide we may go (see `codec::max_forced_split_mode`). encoder_engines: u32, /// Submit stamp for the split arbiter's per-frame cost (sync depth-1 path only). last_submit_at: Option, diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index 589c1d73..04d22f9f 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -80,132 +80,6 @@ pub(super) fn resolve_subframe(default_on: bool) -> bool { } } -/// Resolved NVENC split-frame encode mode for a session — ONE selector shared by the Windows and -/// Linux direct-SDK backends (they had drifted into byte-identical duplicates, one of which -/// logged and one didn't). Precedence: -/// 1. `PUNKTFUNK_SPLIT_ENCODE` = `0`/`disable` | `1`/`auto` (AUTO_FORCED) | `2` | `3` — operator -/// override, always wins, except that `2`/`3` are clamped to the GPU's real engine count (see -/// [`clamp_to_engines`]; the driver honours an over-ask and silently encodes narrower). -/// 2. Pixel rate ≥ [`super::SPLIT_FORCE_PIXEL_RATE`] → force the WIDEST split the GPU can deliver -/// ([`max_forced_split_mode`]), not a hard-coded 2 (AUTO never engages below ~2112 px height, -/// so 4K120 must be forced onto the other engines; and a 3-NVENC part left at 2-way wastes a -/// third of its encode silicon). -/// 3. **HEVC** Main10 below that bar → DISABLE: 2-way split measured SLOWER on Ada for Main10 — at -/// 5120×1440@240 forced-2 took 7.6 ms/frame (~131 fps) vs 2.8 ms (~357 fps) single-engine, the -/// "broken animations in HDR" cap. ⚠ This rule used to sit ABOVE the pixel-rate arm and take no -/// codec, so it (a) vetoed 10-bit **4K120** — the very case the pixel-rate arm exists for — and -/// (b) applied an HEVC-on-Ada result to **AV1 10-bit**, which has no such measurement. Both -/// fixed; what remains is a conservative default in the regime where a second engine buys -/// nothing anyway. -/// ⚠⚠ **UNVALIDATED CONSEQUENCE:** 5120×1440@240 Main10 (1.77 Gpix/s) now clears the pixel-rate -/// bar and WILL be forced to split — i.e. the exact configuration that measurement came from -/// flips behaviour. That is deliberate (the datapoint is one sample, at low bits/frame, and the -/// bits/frame hypothesis predicts it should not generalise) but it is **the first thing to -/// re-measure on Ada**; `PUNKTFUNK_SPLIT_ENCODE=0` is the escape if it regresses. -/// 4. Else AUTO — ⚠ whose behaviour is **conditional on sub-frame**, measured on `.21` at 4K: -/// - sub-frame **ON** (the fleet default): AUTO **does not split** — 5023/5157 µs against -/// DISABLE's 4979/5000. Split and sub-frame are mutually unsupported for HEVC, so the driver -/// resolves AUTO to no-split and this arm silently means DISABLE. -/// - sub-frame **OFF**: AUTO **does split** — 2401/2352 µs against TWO_FORCED's 2319/2378. -/// -/// So AUTO is NOT dead in general and must not be retired: doing so would lose a real split on -/// every sub-frame-off session. It is dead only in the sub-frame-on combination, which -/// [`resolve_split_subframe`] logs rather than silently accepting. -/// -/// The caller still owns the rejection fallback (retry split-disabled) — a codec/config that -/// rejects the chosen mode downgrades at open, not here. -/// -/// `engines` is the GPU's `NV_ENC_CAPS_NUM_ENCODER_ENGINES`; pass `0` when it could not be probed -/// (treated as "unknown", which keeps the pre-probe behaviour of assuming a second engine exists -/// and letting the open-time rejection fallback sort it out). -pub(super) fn resolve_split_mode( - codec: Codec, - bit_depth: u8, - pixel_rate: u64, - engines: u32, -) -> u32 { - use nv::NV_ENC_SPLIT_ENCODE_MODE as M; - let hw_max = max_forced_split_mode(engines); - let mode = match std::env::var("PUNKTFUNK_SPLIT_ENCODE").ok().as_deref() { - Some("0") | Some("disable") => M::NV_ENC_SPLIT_DISABLE_MODE as u32, - Some("1") | Some("auto") => M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32, - Some("3") => clamp_to_engines(M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, hw_max, engines), - Some("2") => clamp_to_engines(M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, hw_max, engines), - // Use every engine the card has, not a hard-coded two: on a 3-NVENC part (GB202, AD102 - // workstation) forcing 2 leaves a third of the silicon idle. - // - // ⚠ This arm now comes FIRST, ahead of the 10-bit rule. That reordering is the D1 fix: a - // 10-bit 4K120 session (995.3 Mpix/s) used to be vetoed by the depth rule before ever - // reaching the pixel-rate arm written for exactly it. - _ if pixel_rate >= super::SPLIT_FORCE_PIXEL_RATE => hw_max, - // Below that bar, HEVC Main10 keeps the conservative single-engine default. The one Ada - // measurement we have says split can be *slower* for Main10, and nothing under this bar - // needs a second engine anyway — so the cost of being wrong here is ~nil, unlike above it. - // - // ⚠ Now codec-scoped (the D2 fix): the measurement behind this was HEVC Main10 on Ada, and - // it used to veto **AV1 10-bit** too, which has neither the sub-frame conflict nor any - // measurement against it. - _ if codec == Codec::H265 && bit_depth >= 10 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, - _ => M::NV_ENC_SPLIT_AUTO_MODE as u32, - }; - tracing::debug!( - split_mode = mode, - ?codec, - bit_depth, - pixel_rate, - engines, - "NVENC split-encode mode selected" - ); - mode -} - -/// The strongest split mode this GPU's engine count can actually deliver. -/// -/// ⚠ **The driver will NOT tell you when you over-ask.** Measured on `.21` (RTX 5070 Ti, 2 NVENC, -/// driver 610.57.04, 4K HEVC): requesting `THREE_FORCED` was **HONOURED** — session opened in mode -/// 3 — and ran at **2303 µs/frame, identical to `TWO_FORCED`'s 2308**. No rejection, no warning, -/// no third engine; just a log line claiming 3-way over a 2-way encode. So the rejection fallback -/// cannot be relied on to find the ceiling and the clamp has to happen here. -/// -/// `NV_ENC_SPLIT_ENCODE_MODE` can only *name* counts up to three (SDK 0.4.0 / NVENCAPI 12.1; -/// values 4..14 are unallocated, so a future API may extend it). Above that we fall back to -/// `AUTO_FORCED` = "split, driver picks how many", which measurably does force a split (2.01× vs -/// disabled on the same box) and is the only way to express "use everything you have". -pub(super) fn max_forced_split_mode(engines: u32) -> u32 { - use nv::NV_ENC_SPLIT_ENCODE_MODE as M; - match engines { - // Unknown (cap unreadable / not probed): keep the historical assumption of a second - // engine and let the open-time rejection fallback correct it. - 0 => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, - 1 => M::NV_ENC_SPLIT_DISABLE_MODE as u32, - 2 => M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32, - 3 => M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32, - // More engines than the enum can name — let the driver use them all. - _ => M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32, - } -} - -/// Hold an operator's `PUNKTFUNK_SPLIT_ENCODE=2|3` to what the hardware can deliver, loudly. -/// Without this the knob silently lies (see [`max_forced_split_mode`]); an override that asks for -/// more engines than exist is a mistake worth surfacing, not honouring. -fn clamp_to_engines(requested: u32, hw_max: u32, engines: u32) -> u32 { - // Only the named N-way modes are ordered; `hw_max` may be AUTO_FORCED (1) on a >3-engine part, - // which is not "less than" TWO_FORCED and must not clamp a legitimate request down. - let named = |m: u32| (2..=3).contains(&m); - if engines != 0 && named(requested) && named(hw_max) && requested > hw_max { - tracing::warn!( - requested, - engines, - using = hw_max, - "PUNKTFUNK_SPLIT_ENCODE asks for more NVENC engines than this GPU has — clamping. \ - (The driver would ACCEPT the over-ask and silently encode with fewer, so the log \ - would otherwise claim a split width that never happened.)" - ); - return hw_max; - } - requested -} - /// Whether the operator EXPLICITLY forced sub-frame readback on (`PUNKTFUNK_NVENC_SUBFRAME=1`) /// — the log-severity input to [`resolve_split_subframe`]: a forced knob being overridden /// deserves a `warn`, a default being tuned an `info`. Callers LATCH this once next to their @@ -633,6 +507,7 @@ pub(super) fn clear_split_verdicts() { #[cfg(test)] mod tests { use super::*; + use crate::{clamp_to_engines, max_forced_split_mode, resolve_split_mode}; use nv::NV_ENC_SPLIT_ENCODE_MODE as M; // These assume PUNKTFUNK_SPLIT_ENCODE is unset (CI); an operator override deliberately wins. @@ -1462,3 +1337,30 @@ mod arbiter_tests { ); } } + +/// The hand-written split constants in `codec.rs` MUST equal the SDK enum they mirror. They are +/// duplicated there so the libav path — which builds without the `nvenc` feature, where the enum +/// does not exist — can share one policy instead of keeping the copy that had already drifted. +/// This is the only place both are visible at once. +#[cfg(test)] +mod split_constant_parity { + use nvidia_video_codec_sdk::sys::nvEncodeAPI::NV_ENC_SPLIT_ENCODE_MODE as M; + + #[test] + fn nvenc_split_constants_match_the_sdk() { + assert_eq!(crate::SPLIT_AUTO, M::NV_ENC_SPLIT_AUTO_MODE as u32); + assert_eq!( + crate::SPLIT_AUTO_FORCED, + M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32 + ); + assert_eq!( + crate::SPLIT_TWO_FORCED, + M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 + ); + assert_eq!( + crate::SPLIT_THREE_FORCED, + M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32 + ); + assert_eq!(crate::SPLIT_DISABLE, M::NV_ENC_SPLIT_DISABLE_MODE as u32); + } +} diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index 9e562c2f..106ebc11 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -44,10 +44,13 @@ use super::nvenc_core::{ apply_low_latency_config, build_init_params, cached_ceiling, codec_guid, plan_range_recovery, - resolve_slices, resolve_split_mode, resolve_split_subframe, resolve_subframe, store_ceiling, - subframe_env_forced, CeilingKey, LowLatencyConfig, NvStatusExt, RangePlan, + resolve_slices, resolve_split_subframe, resolve_subframe, store_ceiling, subframe_env_forced, + CeilingKey, LowLatencyConfig, NvStatusExt, RangePlan, }; +// Moved to `codec.rs` (WP4) so the libav path, which builds without the `nvenc` feature, can share +// one split policy instead of keeping the copy that had already drifted. use super::nvenc_status; +use super::resolve_split_mode; use super::{AuChunk, ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps}; use anyhow::{anyhow, bail, Context, Result}; use pf_frame::{CapturedFrame, FramePayload, PixelFormat}; @@ -595,7 +598,7 @@ pub struct NvencD3d11Encoder { /// `NV_ENC_CAPS_NUM_ENCODER_ENGINES` — how many NVENC engines this GPU has, probed in /// [`query_caps`](Self::query_caps). `0` = not probed / unreadable. The split-encode ceiling: /// the driver accepts a split wider than the hardware and silently encodes narrower, so this - /// is the only honest source for how wide we may go (see `nvenc_core::max_forced_split_mode`). + /// is the only honest source for how wide we may go (see `codec::max_forced_split_mode`). encoder_engines: u32, /// (bitstream, mapped input resource to unmap after retrieval, pts_ns, recovery-anchor) per /// in-flight encode. The fourth field tags the first frame encoded after a successful From 0430d907bbf52e1e2b7ad501153e6b7ee6647cf3 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 09:01:05 +0200 Subject: [PATCH 10/12] =?UTF-8?q?fix(pf-encode):=20gate=20forced=5Fsplit?= =?UTF-8?q?=5Fwidth=20to=20Linux=20=E2=80=94=20WP4=20broke=20the=20Windows?= =?UTF-8?q?=20build?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verification gap flagged in 01294e3a was real. `.133` came back up and the WP4 commit failed Windows clippy: `forced_split_width` is used only by the libav NVENC path (`enc/linux/mod.rs`), but it was added to `codec.rs`, which compiles everywhere -- so it is dead code on Windows and `-D warnings` rejects it. Third time this crate has hit the same item-level dead_code trap (see `subframe_env_forced`, and the arbiter items in `nvenc_core`), and the third time it was caught by actually running the Windows check rather than by reasoning about it. The comment on the gate says so, since the pattern is clearly not self-evident from the code. Verified .21: clippy -D warnings clean both WITH and WITHOUT the nvenc feature, 65 unit tests. Verified .133: Windows clippy --features nvenc --all-targets -D warnings clean, zero errors, zero dead_code. fmt clean. --- crates/pf-encode/src/enc/codec.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/pf-encode/src/enc/codec.rs b/crates/pf-encode/src/enc/codec.rs index 7a59e053..f74c8937 100644 --- a/crates/pf-encode/src/enc/codec.rs +++ b/crates/pf-encode/src/enc/codec.rs @@ -654,6 +654,11 @@ pub(crate) fn max_forced_split_mode(engines: u32) -> u32 { /// For callers that can only express "split this many ways" and have no vocabulary for our other /// modes — the libav path, whose `split_encode_mode` AVOption is libavcodec's own enum, not the /// NVENC one (our `DISABLE` is `15`, which would be meaningless there). +// Linux-only: its sole caller is the libav NVENC path (`enc/linux/mod.rs`). `codec.rs` compiles +// everywhere, so without this it is dead code on Windows — the same item-level `dead_code` +// trap this crate has now hit three times (see `subframe_env_forced`, and the arbiter items in +// `nvenc_core`). Caught by the `.133` check, never by reasoning about it. +#[cfg(target_os = "linux")] pub(crate) fn forced_split_width(mode: u32) -> Option { match mode { m if m == SPLIT_TWO_FORCED => Some(2), From 071358cbf746ba87446d4291736e843bc4e8c049 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 09:23:05 +0200 Subject: [PATCH 11/12] =?UTF-8?q?test(pf-encode):=20S1=20on=20WINDOWS/D3D1?= =?UTF-8?q?1=20=E2=80=94=20passes;=20Windows=20arbitration=20is=20buildabl?= =?UTF-8?q?e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything the split-encode programme rests on had been proven only on Linux/CUDA. The Windows backend drives NV_ENC_DEVICE_TYPE_DIRECTX, so none of it transferred by assumption -- and if the driver refused an in-place split change there, Windows arbitration would simply not be buildable. RESULT on the RTX Windows box (RTX 4090 / AD102, driver 610.88, D3D11): engines=2, latched by query_caps (WP1.1's probe, validated on Windows hardware rather than inferred from Linux) DISABLE -> TWO_FORCED via nvEncReconfigureEncoder, resetEncoder=0: ACCEPTED, ZERO IDRs, and the reverse likewise. So the foundation now holds across three platform x arch x driver combinations: Linux/CUDA Blackwell 610.57.04, Linux/CUDA Ada 610.43.03, Windows/D3D11 Ada 610.88. ⭐ UNBLOCKS ALL FUTURE WINDOWS ON-HARDWARE TESTING. pf-encode's nvenc test binaries were believed unlinkable on Windows ("NvEncodeAPICreateInstance unresolved", recorded as pre-existing and worked around by only ever running clippy there). They link fine given the SDK import library: RUSTFLAGS='-L native=C:\Users\Public\nvenc -l nvencodeapi' `-L` alone is not enough -- without a `-l` nothing pulls the archive in, which is why the earlier attempt still failed. ⚠ This is TEST-BINARY-LOCAL and must stay that way: production deliberately dlopens NVENC rather than link-loading it, and an unconditional link-load is the known crash class on non-NVIDIA Windows hosts. ⚠ Box note: the RTX Windows box answers on .158, not the .173 in its memory entry, and `Administrator@` there resets the connection right after SSH2_MSG_SERVICE_ACCEPT in a way that reads like the host being down -- the working login is "Enrico Bühler"@192.168.1.158. --- crates/pf-encode/src/enc/windows/nvenc.rs | 156 ++++++++++++++++++++++ 1 file changed, 156 insertions(+) diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index 106ebc11..c419e6d9 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -2704,6 +2704,162 @@ mod tests { } } + /// ON-HARDWARE — **S1 on WINDOWS/D3D11**, the question that gates Windows split arbitration. + /// + /// Everything the split-encode programme rests on was proven on **Linux/CUDA**: that + /// `nvEncReconfigureEncoder` accepts a changed `splitEncodeMode` with `resetEncoder=0`, emits + /// **no IDR**, and actually takes effect. The Windows backend drives a different device type + /// (`NV_ENC_DEVICE_TYPE_DIRECTX`), so none of that transfers by assumption — and if the driver + /// refuses it here, Windows arbitration is simply not buildable and should not be attempted. + /// + /// Also checks the two things WP1.1 added, on real Windows hardware rather than by inference + /// from Linux: that `query_caps` latches `NUM_ENCODER_ENGINES`, and that the driver **honours + /// an over-ask** (asking for a 3-way split on a 2-engine card) — the behaviour that makes the + /// clamp necessary rather than defensive. + /// + /// Reports rather than asserts the verdict: both outcomes are legitimate findings. Run: + /// cargo test -p pf-encode --features nvenc -- --ignored --test-threads=1 \ + /// nvenc_split_reconfigure_in_place --nocapture + #[test] + #[ignore = "requires an NVIDIA GPU + driver — run manually on the RTX Windows box"] + fn nvenc_split_reconfigure_in_place() { + let _ = tracing_subscriber::fmt().with_test_writer().try_init(); + const W: u32 = 1920; + const H: u32 = 1080; + const BPS: u64 = 40_000_000; + let disable = nv::NV_ENC_SPLIT_ENCODE_MODE::NV_ENC_SPLIT_DISABLE_MODE as u32; + let two = nv::NV_ENC_SPLIT_ENCODE_MODE::NV_ENC_SPLIT_TWO_FORCED_MODE as u32; + + // Isolate the split variable exactly as the Linux spike does. + std::env::set_var("PUNKTFUNK_NVENC_SUBFRAME", "0"); + std::env::set_var("PUNKTFUNK_SPLIT_ENCODE", "0"); + + // SAFETY: (test-only) the same straight-line D3D11/DXGI setup as `nvenc_reconfigure_no_idr`. + unsafe { + let factory: IDXGIFactory1 = CreateDXGIFactory1().expect("DXGI factory"); + let mut adapter = None; + for i in 0.. { + let Ok(a) = factory.EnumAdapters1(i) else { + break; + }; + if a.GetDesc1().expect("adapter desc").Flags & DXGI_ADAPTER_FLAG_SOFTWARE.0 as u32 + == 0 + { + adapter = Some(a); + break; + } + } + let adapter = adapter.expect("no hardware DXGI adapter"); + let (device, _ctx) = pf_frame::dxgi::make_device(&adapter).expect("make_device"); + let bytes = probe_pattern(W as usize, H as usize); + let init = D3D11_SUBRESOURCE_DATA { + pSysMem: bytes.as_ptr() as *const _, + SysMemPitch: W * 4, + SysMemSlicePitch: 0, + }; + let desc = D3D11_TEXTURE2D_DESC { + Width: W, + Height: H, + MipLevels: 1, + ArraySize: 1, + Format: DXGI_FORMAT_B8G8R8A8_UNORM, + SampleDesc: DXGI_SAMPLE_DESC { + Count: 1, + Quality: 0, + }, + Usage: D3D11_USAGE_DEFAULT, + BindFlags: D3D11_BIND_RENDER_TARGET.0 as u32, + CPUAccessFlags: 0, + MiscFlags: 0, + }; + let mut tex = None; + device + .CreateTexture2D(&desc, Some(&init), Some(&mut tex)) + .expect("pattern texture"); + let tex = tex.expect("null pattern texture"); + + let mut enc = NvencD3d11Encoder::open( + Codec::H265, + PixelFormat::Bgra, + W, + H, + 60, + BPS, + 8, + ChromaFormat::Yuv420, + 1, + ) + .expect("NVENC open"); + + let submit_and_poll = |enc: &mut NvencD3d11Encoder, range: std::ops::Range| { + let (mut aus, mut keyframes) = (0usize, 0usize); + for i in range { + let frame = CapturedFrame { + width: W, + height: H, + pts_ns: i * 16_666_667, + format: PixelFormat::Bgra, + payload: FramePayload::D3d11(D3d11Frame { + texture: tex.clone(), + device: device.clone(), + pyro: None, + }), + cursor: None, + }; + enc.submit_indexed(&frame, i as u32).expect("submit"); + while let Some(au) = enc.poll().expect("poll") { + aus += 1; + keyframes += au.keyframe as usize; + } + } + (aus, keyframes) + }; + + let (aus, kfs) = submit_and_poll(&mut enc, 0..6); + assert!(aus > 0 && kfs == 1, "opening IDR then steady P-frames"); + println!( + "S1(win): engines={} (latched by query_caps), opened split_mode={}", + enc.encoder_engines, enc.split_mode + ); + assert!( + enc.encoder_engines >= 2, + "this GPU reports {} NVENC engine(s) — S1 is not interpretable here", + enc.encoder_engines + ); + assert_eq!(enc.split_mode, disable, "must open split-disabled"); + + // THE SPIKE: change ONLY splitEncodeMode, in place, same bitrate. + enc.split_mode = two; + let accepted = enc.reconfigure_bitrate(BPS); + println!("S1(win): reconfigure DISABLE→TWO_FORCED accepted = {accepted}"); + if accepted { + let (aus, kfs) = submit_and_poll(&mut enc, 6..12); + assert!(aus > 0, "no AUs after the accepted reconfigure"); + println!( + "S1(win) VERDICT: {}", + if kfs == 0 { + "PASS — accepted with NO IDR on D3D11: Windows arbitration is buildable" + } else { + "FAIL — accepted but forced an IDR, which is the same as a rejection" + } + ); + enc.split_mode = disable; + let back = enc.reconfigure_bitrate(BPS); + println!("S1(win): reverse accepted = {back}"); + } else { + enc.split_mode = disable; + println!( + "S1(win) VERDICT: FAIL — the D3D11 path REFUSES an in-place split change. \ + Windows arbitration is not buildable; the Linux result does not transfer." + ); + } + enc.flush().ok(); + } + + std::env::remove_var("PUNKTFUNK_SPLIT_ENCODE"); + std::env::remove_var("PUNKTFUNK_NVENC_SUBFRAME"); + } + /// ON-GLASS (RTX box): the measurement gating the AYUV 4:4:4 work — encodes the probe /// pattern through the REAL ARGB-input NVENC session once with `chromaFormatIDC=3`/FREXT /// and once as plain 4:2:0, so offline analysis of the two bitstreams answers (1) whether From 515a3c29128d3f37c1605f46453ad64d987bea23 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 09:28:34 +0200 Subject: [PATCH 12/12] feat(pf-encode): wire split arbitration on Windows too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last coverage gap, and only worth building once S1 proved it possible: the Windows backend drives NV_ENC_DEVICE_TYPE_DIRECTX, and an in-place splitEncodeMode change had never been tested there. It works (071358cb), so the arbiter is now ungated from Linux-only to the union of both direct-SDK backends and wired into windows/nvenc.rs: the submit stamp, the feed hook on AU completion, apply_split_mode, split_key, arm_split_arbiter, and set_send_spread_us. Same gates as Linux, and they are correctness conditions rather than preferences: opt-in while it earns trust, an operator PUNKTFUNK_SPLIT_ENCODE pin always wins, a cached verdict short-circuits, >=2 engines, never H.264, and the sub-frame trade is only entered when the host has actually reported a send spread to price it with. The one Windows-specific difference is that `async_rt` is a real possibility here (opt-in two-thread retrieve) and the arbiter refuses it, because under pipelined retrieve the submit->AU span includes queue depth and the comparison would be noise. ⚠ Two more instances of the same item-level dead_code trap, caught by the Windows run and not by reasoning -- that is now 4 and 5: - `clear_split_verdicts` is called only by the Linux on-hw test, so it is dead on Windows; gated to `all(test, target_os = "linux")`. - The arbiter methods first landed inside `impl Encoder` rather than the inherent impl (the anchor I used, supports_chunked_poll, is a trait method), which the compiler caught as "not a member of trait Encoder". Verified .158 (RTX 4090 / Ada, driver 610.88, D3D11): clippy --features nvenc --all-targets -D warnings clean, and 2 on-hardware NVENC tests green including S1 re-run with the arbitration code in place (engines=2 latched, DISABLE->TWO_FORCED accepted, zero IDRs, reverse accepted). Verified .21: clippy clean with AND without the nvenc feature, 65 unit tests, 25/25 NVENC on-hardware. fmt clean. --- crates/pf-encode/src/enc/nvenc_core.rs | 40 +++--- crates/pf-encode/src/enc/windows/nvenc.rs | 155 +++++++++++++++++++++- 2 files changed, 175 insertions(+), 20 deletions(-) diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index 04d22f9f..d78420f1 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -252,11 +252,11 @@ mod split_subframe_tests { } } -// Split arbitration is wired into the Linux direct-SDK backend only for now, and -// `nvenc_core` compiles on Windows too — so every item below is linux-gated or it trips -// the item-level dead_code trap this file already carries a scar from (see -// `subframe_env_forced`). Ungating is part of the Windows wiring, not a cleanup. -#[cfg(target_os = "linux")] +// Split arbitration now runs on BOTH direct-SDK backends, so these are gated to the union of +// the two rather than to Linux. Kept gated at all because `nvenc_core` is also reachable from +// builds where neither backend is compiled, and an ungated item there is the item-level +// dead_code trap this file already carries three scars from (see `subframe_env_forced`). +#[cfg(any(target_os = "linux", windows))] /// What the split arbiter wants the backend to do next. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(super) enum ArbAction { @@ -266,7 +266,7 @@ pub(super) enum ArbAction { Settled(u32), } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum ArbState { MeasuringIncumbent, @@ -275,7 +275,7 @@ enum ArbState { Done, } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] /// Picks the faster of two NVENC split modes **on the live session**, by measuring both. /// /// This exists because the alternative — predicting the right mode at open — cannot work: the @@ -311,19 +311,19 @@ pub(super) struct SplitArbiter { } /// Frames discarded after a switch before the challenger is judged (measured — see the struct doc). -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] const SETTLE_FRAMES: u32 = 16; /// Frames measured per arm. Long enough to median out content variation, short enough that the /// whole arbitration is over in well under a second at 60 fps. -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] const SAMPLE_FRAMES: usize = 24; /// The challenger must beat the incumbent by this much to win. Switching is not free (a /// reconfigure, and for HEVC it costs sub-frame readback), so a coin-flip difference should leave /// the session where it already is. -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] const WIN_MARGIN_PCT: u64 = 10; -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] impl SplitArbiter { /// `handicap_us` is what the challenger costs OUTSIDE the encode it is measured on — pass `0` /// when it gives up nothing. See [`Self::challenger_handicap_us`]. @@ -405,7 +405,7 @@ impl SplitArbiter { } } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] fn median(v: &mut [u64]) -> u64 { v.sort_unstable(); v[v.len() / 2] @@ -455,7 +455,7 @@ pub(super) fn store_ceiling(key: CeilingKey, bps: u64) { ceilings().lock().unwrap().insert(key, bps); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] /// A config's identity for the split-arbitration verdict cache — [`CeilingKey`] **minus /// `split_mode`**, because the split mode is the thing being decided. Including it would key each /// verdict under the arm that produced it and the cache could never answer "which arm should this @@ -471,14 +471,14 @@ pub(super) struct SplitKey { pub chroma_444: bool, } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] fn split_verdicts() -> &'static std::sync::Mutex> { static V: std::sync::OnceLock>> = std::sync::OnceLock::new(); V.get_or_init(Default::default) } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] /// The split mode a previous arbitration found fastest for `key` this process lifetime. /// /// Process-lifetime and advisory, exactly like [`cached_ceiling`]: a session that reads a verdict @@ -489,17 +489,19 @@ pub(super) fn cached_split_verdict(key: &SplitKey) -> Option { split_verdicts().lock().unwrap().get(key).copied() } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] /// Record an arbitration result for `key`. pub(super) fn store_split_verdict(key: SplitKey, mode: u32) { split_verdicts().lock().unwrap().insert(key, mode); } -#[cfg(target_os = "linux")] +#[cfg(any(target_os = "linux", windows))] /// Drop every cached verdict. Test-only: the cache is process-global, so an on-hardware test that /// runs an arbitration would otherwise leak its verdict into every later test that opens the same /// config with `PUNKTFUNK_SPLIT_ENCODE` unset — which is exactly the shape the D5 legs use. -#[cfg(test)] +// Linux-only: its sole caller is `nvenc_cuda`'s arbitration on-hw test. Ungated it is dead +// code on Windows — the same item-level trap, now four times over. +#[cfg(all(test, target_os = "linux"))] pub(super) fn clear_split_verdicts() { split_verdicts().lock().unwrap().clear(); } @@ -1184,7 +1186,7 @@ pub(super) unsafe fn apply_low_latency_config(cfg: &mut nv::NV_ENC_CONFIG, c: Lo } } -#[cfg(all(test, target_os = "linux"))] +#[cfg(all(test, any(target_os = "linux", windows)))] mod arbiter_tests { use super::{ArbAction, SplitArbiter, SETTLE_FRAMES}; diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index c419e6d9..67c0f7ce 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -49,8 +49,11 @@ use super::nvenc_core::{ }; // Moved to `codec.rs` (WP4) so the libav path, which builds without the `nvenc` feature, can share // one split policy instead of keeping the copy that had already drifted. +use super::nvenc_core::{ + cached_split_verdict, store_split_verdict, ArbAction, SplitArbiter, SplitKey, +}; use super::nvenc_status; -use super::resolve_split_mode; +use super::{max_forced_split_mode, resolve_split_mode}; use super::{AuChunk, ChromaFormat, Codec, EncodedFrame, Encoder, EncoderCaps}; use anyhow::{anyhow, bail, Context, Result}; use pf_frame::{CapturedFrame, FramePayload, PixelFormat}; @@ -600,6 +603,16 @@ pub struct NvencD3d11Encoder { /// the driver accepts a split wider than the hardware and silently encodes narrower, so this /// is the only honest source for how wide we may go (see `codec::max_forced_split_mode`). encoder_engines: u32, + /// Submit stamp for the split arbiter's per-frame cost (sync depth-1 path only). + last_submit_at: Option, + /// Whole-AU paced-send time (µs) the host last reported. `0` = never reported, which keeps + /// the arbiter out of the sub-frame trade it cannot otherwise price. + send_spread_us: u32, + /// Sub-frame state the session was OPENED able to run, so a return to a non-forced split can + /// restore it without ever turning it on for a session that never had it. + subframe_opened_with: bool, + /// The live split-mode experiment, when one is running. + arbiter: Option, /// (bitstream, mapped input resource to unmap after retrieval, pts_ns, recovery-anchor) per /// in-flight encode. The fourth field tags the first frame encoded after a successful /// [`invalidate_ref_frames`](Encoder::invalidate_ref_frames) — the clean re-anchor P-frame the @@ -762,6 +775,10 @@ impl NvencD3d11Encoder { async_supported: false, subframe_cap: false, encoder_engines: 0, + last_submit_at: None, + send_spread_us: 0, + subframe_opened_with: false, + arbiter: None, pending: VecDeque::new(), frame_idx: 0, force_kf: false, @@ -1048,6 +1065,126 @@ impl NvencD3d11Encoder { Ok(cfg) } + /// The config identity this session's split verdict is cached under. + fn split_key(&self) -> SplitKey { + // Same GPU identity as `ceiling_key`: the selected render adapter's LUID, `0` when + // unresolved. Advisory either way. + let gpu = pf_gpu::resolve_render_adapter_luid() + .map(|l| ((l.HighPart as u32 as u64) << 32) | l.LowPart as u64) + .unwrap_or(0); + SplitKey { + gpu, + codec: self.codec, + width: self.width, + height: self.height, + fps: self.fps, + bit_depth: self.bit_depth, + chroma_444: self.chroma_444, + } + } + + /// Move the LIVE session to `mode` without an IDR. Windows twin of the Linux method; S1 on + /// D3D11 proved `nvEncReconfigureEncoder` takes a changed `splitEncodeMode` with + /// `resetEncoder=0` and emits no keyframe on this device type too. + fn apply_split_mode(&mut self, mode: u32) -> bool { + let (prev_mode, prev_sub) = (self.split_mode, self.subframe_on); + let (mode, subframe) = resolve_split_subframe( + self.codec, + mode, + self.subframe_opened_with, + subframe_env_forced(), + ); + self.split_mode = mode; + self.subframe_on = subframe; + if self.reconfigure_bitrate(self.bitrate_bps) { + true + } else { + tracing::warn!( + from = prev_mode, + to = mode, + "NVENC split arbitration: driver refused the in-place split change — staying put" + ); + self.split_mode = prev_mode; + self.subframe_on = prev_sub; + false + } + } + + /// Feed one frame's encode cost to the split arbiter and act on its verdict. + fn feed_split_arbiter(&mut self, encode_us: u64) { + let Some(arb) = self.arbiter.as_mut() else { + return; + }; + let action = arb.on_frame(encode_us); + let done = arb.is_done(); + match action { + Some(ArbAction::SwitchTo(mode)) => { + if !self.apply_split_mode(mode) { + self.arbiter = None; + return; + } + } + Some(ArbAction::Settled(mode)) => store_split_verdict(self.split_key(), mode), + None => {} + } + if done { + store_split_verdict(self.split_key(), self.split_mode); + self.arbiter = None; + } + } + + /// Decide whether this session may run a live split experiment. Same gates as the Linux + /// backend — see its `arm_split_arbiter` for why each one is a correctness condition rather + /// than a preference; the only Windows difference is that `async_rt` is a real possibility + /// here (opt-in two-thread retrieve), and under it the submit→AU span includes queue depth, + /// so the comparison would be noise. + fn arm_split_arbiter(&mut self) { + if !matches!( + std::env::var("PUNKTFUNK_NVENC_SPLIT_ARBITRATE").as_deref(), + Ok("1") + ) { + return; + } + if std::env::var_os("PUNKTFUNK_SPLIT_ENCODE").is_some() + || cached_split_verdict(&self.split_key()).is_some() + || self.async_rt.is_some() + || self.encoder_engines < 2 + || self.codec == Codec::H264 + { + return; + } + let handicap_us = if self.subframe_on && self.codec != Codec::Av1 { + if self.send_spread_us == 0 || self.slices < 2 { + return; + } + let slices = self.slices as u64; + self.send_spread_us as u64 * (slices - 1) / slices + } else { + 0 + }; + let disable = nv::NV_ENC_SPLIT_ENCODE_MODE::NV_ENC_SPLIT_DISABLE_MODE as u32; + let widest = max_forced_split_mode(self.encoder_engines); + let challenger = if self.split_mode == widest { + disable + } else { + widest + }; + if challenger == self.split_mode { + return; + } + tracing::info!( + incumbent = self.split_mode, + challenger, + handicap_us, + "NVENC split arbitration armed (Windows) — measuring both arms live (no IDR)" + ); + self.arbiter = Some(SplitArbiter::with_handicap( + self.split_mode, + challenger, + handicap_us, + )); + } + /// This session config's identity in the process-lifetime bitrate-ceiling cache /// (`nvenc_core::{cached_ceiling, store_ceiling}`). GPU identity is the selected render /// adapter's LUID — the adapter the capturer's device (and so this session) lives on; `0` @@ -1433,6 +1570,8 @@ impl NvencD3d11Encoder { self.bitrate_bps / 1_000_000, self.codec_guid ); + self.subframe_opened_with = self.subframe_on; + self.arm_split_arbiter(); Ok(()) } } @@ -1776,6 +1915,9 @@ impl Encoder for NvencD3d11Encoder { anchor, idr_hint, )); + // Split-arbiter cost stamp; only meaningful on the sync depth-1 path, which is the + // only path `arm_split_arbiter` allows an experiment on. + self.last_submit_at = Some(std::time::Instant::now()); // Async: hand the in-flight encode to the retrieve thread (channel capacity = POOL ≥ // in-flight, so this send never blocks). The pending entry above pairs with its // completion FIFO in `absorb_done`. @@ -1959,6 +2101,13 @@ impl Encoder for NvencD3d11Encoder { if !map.is_null() { let _ = (api().unmap_input_resource)(self.encoder, map); } + let encode_us = self + .last_submit_at + .take() + .map(|t| t.elapsed().as_micros() as u64); + if let Some(us) = encode_us { + self.feed_split_arbiter(us); + } Ok(Some(EncodedFrame { data, pts_ns, @@ -2218,6 +2367,10 @@ impl Encoder for NvencD3d11Encoder { } } + fn set_send_spread_us(&mut self, us: u32) { + self.send_spread_us = us; + } + fn applied_bitrate_bps(&self) -> Option { // `bitrate_bps` is the post-clamp truth: the open path's ceiling search and the // reconfigure path's cache clamp both write what the session ACTUALLY targets.