diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index dc8828fd..376c3d12 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -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 // can't reconstruct full chroma, so clear the flag so `caps().chroma_444` is truthful. 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 { // Steady state: the copy helpers need the shared context current on this thread. cuda::make_current().context("cuCtxSetCurrent (encode thread)")?; diff --git a/crates/pf-encode/src/enc/windows/nvenc.rs b/crates/pf-encode/src/enc/windows/nvenc.rs index d3edc23b..6c2cd7dc 100644 --- a/crates/pf-encode/src/enc/windows/nvenc.rs +++ b/crates/pf-encode/src/enc/windows/nvenc.rs @@ -1135,7 +1135,19 @@ impl Encoder for NvencD3d11Encoder { self.chroma_444 = false; } 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; } // The session's opening frame — NVENC emits it as an IDR regardless of pic flags, so the diff --git a/crates/pf-encode/src/enc/windows/qsv.rs b/crates/pf-encode/src/enc/windows/qsv.rs index af52f1f2..d30c3d3a 100644 --- a/crates/pf-encode/src/enc/windows/qsv.rs +++ b/crates/pf-encode/src/enc/windows/qsv.rs @@ -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_AMF_LTR`); loss recovery then always falls back to IDR. 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 @@ -171,13 +176,22 @@ fn ltr_test_force_at() -> Option { /// 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). fn intra_refresh_requested() -> bool { + // Spelling parity with AMF (see `ltr_disabled` above). 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 { - (fps / 2).clamp(8, 240) as u16 + std::env::var("PUNKTFUNK_IR_PERIOD_FRAMES") + .ok() + .and_then(|s| s.trim().parse::().ok()) + .filter(|v| *v >= 2) + .unwrap_or(fps / 2) + .clamp(8, 240) as u16 } // --------------------------------------------------------------------------------------------- diff --git a/crates/pf-encode/src/lib.rs b/crates/pf-encode/src/lib.rs index 6fde1365..292eb9db 100644 --- a/crates/pf-encode/src/lib.rs +++ b/crates/pf-encode/src/lib.rs @@ -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 /// 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 @@ -407,8 +414,14 @@ fn open_video_backend( ); } let _ = (cuda, bit_depth); // software path is CPU + 8-bit only - sw::OpenH264Encoder::open(format, width, height, fps, bitrate_bps) - .map(|e| (Box::new(e) as Box, "software")) + sw::OpenH264Encoder::open( + format, + width, + height, + fps, + bitrate_bps.min(SW_BITRATE_CEIL), + ) + .map(|e| (Box::new(e) as Box, "software")) } "auto" | "" => { // A CUDA frame can ONLY be consumed by NVENC. Otherwise the shared auto decision @@ -610,8 +623,6 @@ fn open_video_backend( (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 - // Software H.264 realistically caps far below the negotiated hardware rates. - const SW_BITRATE_CEIL: u64 = 100_000_000; sw::OpenH264Encoder::open( format, 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 /// requiring the proprietary driver's device nodes; a nouveau NVIDIA GPU can't NVENC) — otherwise /// 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")] fn linux_auto_is_vaapi() -> bool { 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) // is resolved separately by the host (`capturer_supports_hdr` / the GameStream RTSP // 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) } else { linux::probe_can_encode_10bit(codec)