fix(pf-encode): AUTO split is conditional on sub-frame — do NOT retire the arm
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.
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user