fix(encode): NVENC partial-init session leak + three backend-parity gaps
apple / swift (push) Successful in 1m18s
apple / screenshots (push) Successful in 4m33s
android / android (push) Has been cancelled
arch / build-publish (push) Has been cancelled
ci / web (push) Has been cancelled
ci / docs-site (push) Has been cancelled
ci / bench (push) Has been cancelled
ci / rust (push) Has been cancelled
deb / build-publish (push) Has been cancelled
deb / build-publish-host (push) Has been cancelled
decky / build-publish (push) Has been cancelled
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Has been cancelled
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Has been cancelled
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Has been cancelled
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Has been cancelled
docker / deploy-docs (push) Has been cancelled
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Has been cancelled
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Has been cancelled
windows-host / package (push) Has been cancelled
apple / swift (push) Successful in 1m18s
apple / screenshots (push) Successful in 4m33s
android / android (push) Has been cancelled
arch / build-publish (push) Has been cancelled
ci / web (push) Has been cancelled
ci / docs-site (push) Has been cancelled
ci / bench (push) Has been cancelled
ci / rust (push) Has been cancelled
deb / build-publish (push) Has been cancelled
deb / build-publish-host (push) Has been cancelled
decky / build-publish (push) Has been cancelled
docker / build-push (--build-arg FEDORA_VERSION=44, ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora44-rpm) (push) Has been cancelled
docker / build-push (., web/Dockerfile, punktfunk-web) (push) Has been cancelled
docker / build-push (ci, ci/fedora-rpm.Dockerfile, punktfunk-fedora-rpm) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci-noble.Dockerfile, punktfunk-rust-ci-noble) (push) Has been cancelled
docker / build-push (ci, ci/rust-ci.Dockerfile, punktfunk-rust-ci) (push) Has been cancelled
docker / build-push (docs-site, docs-site/Dockerfile, punktfunk-docs) (push) Has been cancelled
docker / deploy-docs (push) Has been cancelled
rpm / build-publish (43, bazzite, punktfunk-fedora-rpm) (push) Has been cancelled
rpm / build-publish (44, fedora-44, punktfunk-fedora44-rpm) (push) Has been cancelled
windows-host / package (push) Has been cancelled
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:
@@ -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<dyn Encoder>, "software"))
|
||||
sw::OpenH264Encoder::open(
|
||||
format,
|
||||
width,
|
||||
height,
|
||||
fps,
|
||||
bitrate_bps.min(SW_BITRATE_CEIL),
|
||||
)
|
||||
.map(|e| (Box::new(e) as Box<dyn Encoder>, "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)
|
||||
|
||||
Reference in New Issue
Block a user