From c16e07d7464e09a1d8885e24ac0974edcd69b347 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 7 Aug 2026 16:50:17 +0200 Subject: [PATCH] fix(encode/nvenc): AV1 stops shipping half a frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every 4K AV1 frame this host encoded reached the wire truncated to its first tile, and had since AV1 was wired up. Measured on .21 (RTX 5070 Ti, 4K60, split AUTO): each access unit carried a frame header declaring two tile rows and a single Tile Group OBU with tg_start = tg_end = 0, so libdav1d rejected 835 of 836 AUs with "Error parsing frame header". NVIDIA's hardware decoder accepts the truncated stream, which is why native Vulkan Video looked healthy at 60 fps while both conformant software decoders — rav1d in-tree and libdav1d out-of-tree — refused every frame and clients fell to a black screen. The two halves of sub-frame readback are armed by different conditions. build_init_params arms the WRITER (enableSubFrameWrite + reportSliceOffsets) from subframe_on alone; the chunked READER additionally requires slices >= 2, and resolve_slices returns 1 for AV1 unconditionally — before the PUNKTFUNK_NVENC_SLICES override is even read, because AV1 partitions via tiles rather than slices. So an AV1 session asked the driver to publish its output tile by tile and then took only the first tile with one blocking lock_bitstream. resolve_split_subframe — the one arbitration point both direct-SDK backends already call — now disarms sub-frame for AV1 and returns split_mode untouched, so AV1 keeps every engine split encode gives it. Arming the reader instead is not a drop-in alternative: poll_chunk cuts at bitstreamSizeInBytes on the reasoning that "slices are contiguous Annex-B", which AV1's OBUs are not. With sub-frame disarmed and split still AUTO, the same session decodes 654/654 frames clean through libdav1d. The test that pinned this as correct (av1_untouched, "both features are legal together") is replaced by one that pins the disarm, and by one that checks the reader's gate against the writer's — the comparison nothing made. The Linux latch comment claiming the two "can't disagree" is corrected; that claim is what made this invisible. --- crates/pf-encode/src/enc/linux/nvenc_cuda.rs | 17 ++- crates/pf-encode/src/enc/nvenc_core.rs | 122 +++++++++++++++++-- 2 files changed, 128 insertions(+), 11 deletions(-) diff --git a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs index f1a80847..8aec5be5 100644 --- a/crates/pf-encode/src/enc/linux/nvenc_cuda.rs +++ b/crates/pf-encode/src/enc/linux/nvenc_cuda.rs @@ -1668,10 +1668,19 @@ impl NvencCudaEncoder { // Sub-frame chunked poll (§7 LN1 Phase 1; default-on since Phase 3): armed iff this // session was CONFIGURED multi-slice + sub-frame readback (`self.slices` / // `self.subframe_on` were resolved once in `query_caps` and consumed by - // `build_config` / `build_init_params`, so the latch can't disagree with the session - // config) and the retrieve is sync — chunked poll is a depth-1 sync feature; a - // pipelined session's non-blocking poll owns the bitstream from the retrieve thread - // instead (the sub-frame write itself stays armed there; it's harmless). + // `build_config` / `build_init_params`) and the retrieve is sync — chunked poll is a + // depth-1 sync feature; a pipelined session's non-blocking poll owns the bitstream + // from the retrieve thread instead (sub-frame write is not armed there at all — + // `build_init_params` gates it on `!enable_async`). + // + // ⚠ THIS LATCH CAN DISAGREE WITH THE WRITER, and an earlier revision of this comment + // claimed it could not ("so the latch can't disagree with the session config"). It + // can: `build_init_params` arms `enableSubFrameWrite` from `subframe_on` ALONE, + // while this line additionally demands `slices >= 2`. AV1 resolves to 1 slice by + // construction, so it armed the writer with nothing to read the chunks and every + // frame reached the wire truncated to its first tile. `resolve_split_subframe` now + // disarms sub-frame for AV1 so the two agree; that function's docs carry the + // measurement. self.subframe_chunks = self.slices >= 2 && self.subframe_on && self.async_rt.is_none(); if self.subframe_chunks { tracing::info!( diff --git a/crates/pf-encode/src/enc/nvenc_core.rs b/crates/pf-encode/src/enc/nvenc_core.rs index d78420f1..cf986941 100644 --- a/crates/pf-encode/src/enc/nvenc_core.rs +++ b/crates/pf-encode/src/enc/nvenc_core.rs @@ -107,8 +107,35 @@ pub(super) fn subframe_env_forced() -> bool { /// plain AUTO the driver arbitrates — the shipped fleet state (1080p–1440p240 all run /// AUTO+subframe); keying on `!= DISABLE` here would have disarmed the Phase-3 chunked-poll /// feature fleet-wide. -/// - **AV1**: both legal (sub-frame is per-tile; split is constrained only by -/// output-into-vidmem, which we never use) — untouched. +/// - **AV1**: split passes through untouched (it is constrained only by output-into-vidmem, +/// which we never use), but sub-frame readback is **always disarmed** — see below. +/// +/// # Why AV1 must never arm sub-frame readback +/// +/// The two halves of this feature are armed by different conditions, and for AV1 they can only +/// ever disagree: +/// +/// * the WRITER (`enableSubFrameWrite` + `reportSliceOffsets`) is armed by +/// [`build_init_params`] from this `subframe` alone; +/// * the READER ([`Encoder::poll_chunk`]'s `subframe_chunks` latch) additionally requires +/// `slices >= 2`, and [`resolve_slices`] returns 1 for AV1 **unconditionally** — before the +/// `PUNKTFUNK_NVENC_SLICES` override is even read, because AV1 partitions via tiles rather +/// than slices. +/// +/// So an AV1 session armed the driver to publish its output unit by unit and then read it with +/// a single blocking `lock_bitstream`, which returns only the FIRST completed unit. One tile +/// per frame reached the wire. Measured on `.21` (RTX 5070 Ti, 4K60, split AUTO): every frame +/// carried a frame header declaring two tile rows and a single Tile Group OBU with +/// `tg_start = tg_end = 0` — half the picture missing — and libdav1d rejected **835 of 836** +/// access units with "Error parsing frame header". NVIDIA's hardware decoder accepts the +/// truncated stream, which is why native Vulkan Video looked healthy while both conformant +/// software decoders (rav1d in-tree, libdav1d out-of-tree) refused every frame. With sub-frame +/// disarmed and split still AUTO, the same session decoded 654/654 frames clean. +/// +/// This is a plain disarm, NOT a codec restriction: `split_mode` is returned untouched, so AV1 +/// keeps every engine split encode gives it. Arming the reader for AV1 instead is not a +/// drop-in alternative — `poll_chunk` cuts at `bitstreamSizeInBytes` on the reasoning that +/// "slices are contiguous Annex-B", which AV1's OBUs are not. /// /// Returns the `(split_mode, subframe)` to ACTUALLY configure. The caller must store BOTH back /// (the chunked-poll latch and `CeilingKey` key on them) — a silent in-params drop would leave @@ -124,6 +151,27 @@ pub(super) fn resolve_split_subframe( if codec == Codec::H264 { return (M::NV_ENC_SPLIT_DISABLE_MODE as u32, subframe); } + // AV1: disarm sub-frame, keep split. The reader can never arm here (`resolve_slices` gives + // AV1 one slice by construction), so arming the writer only truncates every frame to its + // first tile — see this function's docs for the measurement. + if codec == Codec::Av1 && subframe { + if subframe_forced { + tracing::warn!( + split_mode, + "PUNKTFUNK_NVENC_SUBFRAME=1 cannot be honoured on AV1 — its sub-frame units are \ + TILES and the chunked reader cuts on Annex-B slice boundaries, so arming the \ + writer would ship only the first tile of every frame; sub-frame readback \ + disabled for this session (split encode is unaffected)" + ); + } else { + tracing::debug!( + split_mode, + "NVENC: sub-frame readback disarmed on AV1 (tiles, not slices — nothing consumes \ + the chunks); split encode is unaffected" + ); + } + return (split_mode, false); + } let split_forced = split_mode == M::NV_ENC_SPLIT_TWO_FORCED_MODE as u32 || split_mode == M::NV_ENC_SPLIT_THREE_FORCED_MODE as u32 || split_mode == M::NV_ENC_SPLIT_AUTO_FORCED_MODE as u32; @@ -164,7 +212,7 @@ pub(super) fn resolve_split_subframe( #[cfg(test)] mod split_subframe_tests { - use super::{resolve_split_subframe, Codec}; + use super::{resolve_slices, resolve_split_subframe, Codec}; use nvidia_video_codec_sdk::sys::nvEncodeAPI::NV_ENC_SPLIT_ENCODE_MODE as M; const AUTO: u32 = M::NV_ENC_SPLIT_AUTO_MODE as u32; @@ -241,13 +289,73 @@ mod split_subframe_tests { ); } - /// AV1: both features are legal together (per-tile sub-frame; split constrained only by - /// output-into-vidmem) — the arbitration must not touch it. + /// AV1 KEEPS ITS SPLIT AND LOSES ITS SUB-FRAME, and this test is the one that used to + /// assert the bug. + /// + /// It read `av1_untouched` and pinned `(TWO, true)` on the reasoning that "both features + /// are legal together (sub-frame is per-tile)". Legal for the DRIVER, yes — but the two + /// halves of the feature are armed by different conditions in this crate, and on AV1 they + /// cannot agree: `build_init_params` arms the WRITER from `subframe`, while the READER + /// needs `slices >= 2` and [`resolve_slices`] returns 1 for AV1 before the env override is + /// even read. So the session told the driver to publish tile by tile and then took only the + /// first tile with one blocking lock. Every 4K AV1 frame shipped half a picture; libdav1d + /// rejected 835/836 AUs, and only NVIDIA's lenient hardware decoder hid it. + /// + /// The `true` argument here is `subframe_forced` — even an operator's explicit + /// `PUNKTFUNK_NVENC_SUBFRAME=1` cannot buy a working AV1 sub-frame session, so it is + /// refused (loudly) rather than honoured into a truncated stream. #[test] - fn av1_untouched() { + fn av1_keeps_split_but_never_arms_subframe() { + // Forced split + forced sub-frame: split survives, sub-frame does not. assert_eq!( resolve_split_subframe(Codec::Av1, TWO, true, true), - (TWO, true) + (TWO, false), + "AV1 must keep its split mode and drop sub-frame readback" + ); + // The fleet shape (plain AUTO + default-on sub-frame) — the one that shipped broken. + assert_eq!( + resolve_split_subframe(Codec::Av1, AUTO, true, false), + (AUTO, false) + ); + // Widest split, still untouched: this fix costs AV1 no engines. + assert_eq!( + resolve_split_subframe(Codec::Av1, AUTO_F, true, false), + (AUTO_F, false) + ); + // Already off stays off, and split still passes through. + assert_eq!( + resolve_split_subframe(Codec::Av1, TWO, false, false), + (TWO, false) + ); + } + + /// The two halves of the sub-frame feature, checked against each other on AV1 — the + /// comparison nothing made, which is why the truncation shipped. + /// + /// The reader's gate is `subframe_chunks = slices >= 2 && subframe_on && sync`. For AV1 + /// [`resolve_slices`] returns 1 *by construction* (it early-returns for non-H.26x before + /// reading `PUNKTFUNK_NVENC_SLICES`, which is also what makes this test independent of the + /// environment it runs in), so the reader can never arm — and therefore the writer must + /// never arm either. + /// + /// Deliberately NOT generalised to a loop over all codecs: for H.264/HEVC `slices` is + /// `sliceModeData`, so a single-slice session really does produce ONE output unit and a + /// single blocking lock is complete. The hazard is specific to a codec whose unit count the + /// driver decides (AV1's tiles, via split encode) rather than our slice config. If AV1 ever + /// becomes genuinely multi-slice here, the first assert fires and sends whoever changed it + /// back to the arming rule. + #[test] + fn av1_can_never_arm_the_chunked_reader_so_it_must_not_arm_the_writer() { + assert_eq!( + resolve_slices(Codec::Av1, 4), + 1, + "AV1 is single-slice by construction — `subframe_chunks` (slices >= 2) cannot arm" + ); + let (_, subframe) = resolve_split_subframe(Codec::Av1, AUTO, true, false); + assert!( + !subframe, + "the sub-frame WRITER is armed on a session whose chunked READER cannot arm: every \ + frame would reach the wire truncated to its first tile" ); } }