fix(encode): NVENC partial-init session leak + three backend-parity gaps

All four found in the pf-encode quality sweep and verified against source.

- NVENC partial-init leak (BOTH platforms, high): `init_session` publishes
  `self.encoder` — and on Windows charges LIVE_SESSION_UNITS — *before* its
  remaining fallible steps (bitstream buffers; on Linux also the input-surface
  alloc and `register_resource`). A failure there left a live session with
  `inited == false`, and every guard on the re-init path keys off `inited`, so
  the next submit skipped teardown and overwrote `self.encoder`: the session
  leaked permanently toward the driver's per-process cap, and its budget units
  never returned, progressively starving parallel-display admission. `teardown`
  already keys off `encoder.is_null()` rather than `inited`, so it cleans up
  exactly this half-built state — it just was never called. Now invoked on the
  `init_session` error path on both platforms.

- `can_encode_10bit` asked the wrong backend (medium): it resolved via
  `linux_auto_is_vaapi`, which ignores `encoder_pref`, while `can_encode_444`
  and `open_video` honour it. On a host that forces a backend (e.g.
  `encoder_pref = "vaapi"` on an NVIDIA box) the probe answered for NVENC while
  the session opened VAAPI, so the negotiated bit depth — and the HDR/SDR colour
  label derived from it — described a backend that never ran. Now uses the same
  `linux_zero_copy_is_vaapi` mirror, and `linux_auto_is_vaapi` carries a warning
  that it resolves the `auto` case only and is not a dispatch mirror.

- Linux software arm ignored SW_BITRATE_CEIL (low): the Windows arm clamped
  openh264 to 100 Mbps, the Linux arm passed the full negotiated rate. The
  constant is now module-scope so both arms share one value.

- QSV/AMF env-parity (low): `PUNKTFUNK_IR_PERIOD_FRAMES` was a no-op on QSV
  despite the comment claiming parity with AMF, and `PUNKTFUNK_NO_QSV_LTR` /
  `PUNKTFUNK_INTRA_REFRESH` had dropped AMF's `trim()` and `yes`/`on` spellings,
  so a value with stray whitespace silently did nothing on Intel while the same
  value worked on AMD.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-19 20:43:16 +02:00
parent ab679a56e7
commit 7cde80ce84
4 changed files with 71 additions and 11 deletions
+12 -1
View File
@@ -1078,7 +1078,18 @@ impl Encoder for NvencCudaEncoder {
// 4:4:4 honesty: engage FREXT only on a genuine YUV444 input; a subsampled NV12/RGB input // 4:4:4 honesty: engage FREXT only on a genuine YUV444 input; a subsampled NV12/RGB input
// can't reconstruct full chroma, so clear the flag so `caps().chroma_444` is truthful. // can't reconstruct full chroma, so clear the flag so `caps().chroma_444` is truthful.
self.chroma_444 = self.chroma_444 && buf.yuv444; self.chroma_444 = self.chroma_444 && buf.yuv444;
self.init_session()?; // `init_session` publishes `self.encoder` before its remaining fallible steps (bitstream
// buffers, input-surface alloc, `register_resource`), so a failure there leaves a live
// session with `inited == false`. Every guard on the re-init path keys off `inited`, so
// without this the next submit would skip teardown and overwrite `self.encoder`, leaking
// the session and its registered input surfaces permanently. `teardown` keys off
// `encoder.is_null()`, not `inited`, so it cleans up exactly this half-built state.
if let Err(e) = self.init_session() {
// SAFETY: the encode thread owns the session and a failed init leaves nothing
// mid-encode to race with.
unsafe { self.teardown() };
return Err(e);
}
} else { } else {
// Steady state: the copy helpers need the shared context current on this thread. // Steady state: the copy helpers need the shared context current on this thread.
cuda::make_current().context("cuCtxSetCurrent (encode thread)")?; cuda::make_current().context("cuCtxSetCurrent (encode thread)")?;
+13 -1
View File
@@ -1135,7 +1135,19 @@ impl Encoder for NvencD3d11Encoder {
self.chroma_444 = false; self.chroma_444 = false;
} }
let device = frame.device.clone(); let device = frame.device.clone();
self.init_session(&device)?; // `init_session` publishes `self.encoder` (and charges LIVE_SESSION_UNITS) BEFORE its
// last fallible steps, so a failure there leaves a live session with `inited == false`.
// Every guard on the re-init path keys off `inited`, so without this the next submit
// would skip teardown and overwrite `self.encoder` — leaking the session permanently
// (toward the driver's per-process cap) along with its session-budget units.
// `teardown` keys off `encoder.is_null()`, not `inited`, so it cleans up exactly this
// half-built state and is a no-op when nothing was opened.
if let Err(e) = self.init_session(&device) {
// SAFETY: same contract as the teardown above — the encode thread owns the session,
// and a failed init leaves nothing mid-encode to race with.
unsafe { self.teardown() };
return Err(e);
}
self.init_device = dev_raw; self.init_device = dev_raw;
} }
// The session's opening frame — NVENC emits it as an IDR regardless of pic flags, so the // The session's opening frame — NVENC emits it as an IDR regardless of pic flags, so the
+18 -4
View File
@@ -146,7 +146,12 @@ const NUM_LTR_SLOTS: usize = 2;
/// `PUNKTFUNK_NO_QSV_LTR` — defeat switch for the LTR-RFI path (parity with /// `PUNKTFUNK_NO_QSV_LTR` — defeat switch for the LTR-RFI path (parity with
/// `PUNKTFUNK_NO_AMF_LTR`); loss recovery then always falls back to IDR. /// `PUNKTFUNK_NO_AMF_LTR`); loss recovery then always falls back to IDR.
fn ltr_disabled() -> bool { fn ltr_disabled() -> bool {
std::env::var("PUNKTFUNK_NO_QSV_LTR").is_ok_and(|v| v == "1" || v.eq_ignore_ascii_case("true")) // Same accepted spellings as AMF's `ltr_disabled` — this had dropped the `trim()` and the
// `yes`/`on` forms, so a value with stray whitespace (easy to produce with `set VAR=1 `)
// silently left LTR enabled on Intel while the identical value worked on AMD.
std::env::var("PUNKTFUNK_NO_QSV_LTR")
.map(|v| matches!(v.trim(), "1" | "true" | "yes" | "on"))
.unwrap_or(false)
} }
/// Frames between LTR marks (`PUNKTFUNK_LTR_INTERVAL_FRAMES`, shared with AMF); default ~1/4 s /// Frames between LTR marks (`PUNKTFUNK_LTR_INTERVAL_FRAMES`, shared with AMF); default ~1/4 s
@@ -171,13 +176,22 @@ fn ltr_test_force_at() -> Option<i64> {
/// Mirrors [`super::amf`]'s `PUNKTFUNK_INTRA_REFRESH` opt-in: request the intra-refresh wave /// Mirrors [`super::amf`]'s `PUNKTFUNK_INTRA_REFRESH` opt-in: request the intra-refresh wave
/// instead of LTR (mutually exclusive — the wave sweeps the whole picture, LTR pins references). /// instead of LTR (mutually exclusive — the wave sweeps the whole picture, LTR pins references).
fn intra_refresh_requested() -> bool { fn intra_refresh_requested() -> bool {
// Spelling parity with AMF (see `ltr_disabled` above).
std::env::var("PUNKTFUNK_INTRA_REFRESH") std::env::var("PUNKTFUNK_INTRA_REFRESH")
.is_ok_and(|v| v == "1" || v.eq_ignore_ascii_case("true")) .map(|v| matches!(v.trim(), "1" | "true" | "yes" | "on"))
.unwrap_or(false)
} }
/// The wave period in frames (~0.5 s), the same shape as Linux NVENC / AMF. /// The wave period in frames (~0.5 s), `PUNKTFUNK_IR_PERIOD_FRAMES` overrides — the same knob and
/// default as AMF / Linux NVENC. (This claimed parity while ignoring the env var entirely, so the
/// knob silently did nothing on Intel; the clamp is kept because `mfxU16` bounds the field.)
fn intra_refresh_period(fps: u32) -> u16 { fn intra_refresh_period(fps: u32) -> u16 {
(fps / 2).clamp(8, 240) as u16 std::env::var("PUNKTFUNK_IR_PERIOD_FRAMES")
.ok()
.and_then(|s| s.trim().parse::<u32>().ok())
.filter(|v| *v >= 2)
.unwrap_or(fps / 2)
.clamp(8, 240) as u16
} }
// --------------------------------------------------------------------------------------------- // ---------------------------------------------------------------------------------------------
+27 -4
View File
@@ -223,6 +223,13 @@ impl Encoder for TrackedEncoder {
} }
} }
/// Ceiling applied to the negotiated bitrate before it reaches openh264: software H.264 realistically
/// caps far below the rates a hardware session negotiates, and handing it the full figure just
/// misconfigures its rate control. Module-scope so BOTH software arms share one value — the Linux
/// arm was missing the clamp the Windows arm applied.
#[cfg(any(target_os = "linux", target_os = "windows"))]
const SW_BITRATE_CEIL: u64 = 100_000_000;
/// Open the platform encoder backend. Returns the encoder together with the display label of the /// Open the platform encoder backend. Returns the encoder together with the display label of the
/// branch that ACTUALLY opened (`nvenc`/`vaapi`/`vulkan`/`amf`/`qsv`/`software`) — the label feeds /// branch that ACTUALLY opened (`nvenc`/`vaapi`/`vulkan`/`amf`/`qsv`/`software`) — the label feeds
/// the mgmt API's live-session record, and only the open site knows which internal fallback won /// the mgmt API's live-session record, and only the open site knows which internal fallback won
@@ -407,7 +414,13 @@ fn open_video_backend(
); );
} }
let _ = (cuda, bit_depth); // software path is CPU + 8-bit only let _ = (cuda, bit_depth); // software path is CPU + 8-bit only
sw::OpenH264Encoder::open(format, width, height, fps, bitrate_bps) sw::OpenH264Encoder::open(
format,
width,
height,
fps,
bitrate_bps.min(SW_BITRATE_CEIL),
)
.map(|e| (Box::new(e) as Box<dyn Encoder>, "software")) .map(|e| (Box::new(e) as Box<dyn Encoder>, "software"))
} }
"auto" | "" => { "auto" | "" => {
@@ -610,8 +623,6 @@ fn open_video_backend(
(build a GPU backend: --features nvenc or amf-qsv, or request H264)" (build a GPU backend: --features nvenc or amf-qsv, or request H264)"
); );
let _ = (bit_depth, chroma); // the software H.264 path is 8-bit 4:2:0 only let _ = (bit_depth, chroma); // the software H.264 path is 8-bit 4:2:0 only
// Software H.264 realistically caps far below the negotiated hardware rates.
const SW_BITRATE_CEIL: u64 = 100_000_000;
sw::OpenH264Encoder::open( sw::OpenH264Encoder::open(
format, format,
width, width,
@@ -784,6 +795,12 @@ fn nvidia_present() -> bool {
/// picks its vendor's backend — AMD/Intel → VAAPI on that GPU's render node, NVIDIA → NVENC (still /// picks its vendor's backend — AMD/Intel → VAAPI on that GPU's render node, NVIDIA → NVENC (still
/// requiring the proprietary driver's device nodes; a nouveau NVIDIA GPU can't NVENC) — otherwise /// requiring the proprietary driver's device nodes; a nouveau NVIDIA GPU can't NVENC) — otherwise
/// today's NVIDIA-presence probe, unchanged. /// today's NVIDIA-presence probe, unchanged.
///
/// ⚠ This resolves the **`auto` case only** — it deliberately ignores `encoder_pref`. It is NOT a
/// mirror of [`open_video`]'s dispatch and must not be used to decide which backend a capability
/// probe should ask: use [`linux_zero_copy_is_vaapi`], which layers `encoder_pref` on top of this.
/// (`can_encode_10bit` used this directly and answered for the wrong backend whenever a host
/// forced one.)
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
fn linux_auto_is_vaapi() -> bool { fn linux_auto_is_vaapi() -> bool {
if let Some(g) = pf_gpu::manual_selection() { if let Some(g) = pf_gpu::manual_selection() {
@@ -997,7 +1014,13 @@ pub fn can_encode_10bit(codec: Codec) -> bool {
// only half the Linux gate — the capture side (GNOME 50+ portal monitor in HDR mode) // only half the Linux gate — the capture side (GNOME 50+ portal monitor in HDR mode)
// is resolved separately by the host (`capturer_supports_hdr` / the GameStream RTSP // is resolved separately by the host (`capturer_supports_hdr` / the GameStream RTSP
// honor), since this probe can't know what the compositor will negotiate. // honor), since this probe can't know what the compositor will negotiate.
if linux_auto_is_vaapi() { // Resolve through the SAME helper `can_encode_444` uses (and which mirrors
// `open_video`'s dispatch): `linux_auto_is_vaapi` ignores `encoder_pref`, so on a box
// that forces a backend — e.g. `encoder_pref = "vaapi"` on an NVIDIA host — this probe
// would answer for NVENC while the session actually opens VAAPI, and the negotiated bit
// depth (plus the HDR/SDR colour label derived from it) would describe a backend that
// never runs. That is exactly the dishonesty this probe exists to prevent.
if linux_zero_copy_is_vaapi() {
vaapi::probe_can_encode_10bit(codec) vaapi::probe_can_encode_10bit(codec)
} else { } else {
linux::probe_can_encode_10bit(codec) linux::probe_can_encode_10bit(codec)