From 574e3e4e3f96062513e7892f610c4a6441d01aa3 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 18 Jul 2026 11:20:27 +0200 Subject: [PATCH] fix(pyrowave): signal ycbcr_range=LIMITED in the sequence header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pyrowave's encoder fills the BitstreamSequenceHeader with `= {}` and its C API offers no way to set colour/range, so it signals ycbcr_range=0=FULL — but both host CSCs (rgb2yuv.comp on Linux, BgraToYuvPlanes on Windows) always emit BT.709 LIMITED Y'CbCr (black = Y'16). A client that honours the VUI (the Apple wavelet decoder reads bit 30 of word1) then skips the limited→full expansion and shows washed-out, raised blacks — reported on both Linux and Windows hosts. Patch the range bit HONEST (mark_limited_range in the shared pyrowave_wire, called by both encoders after packetize). Clients that hardcode limited (the Vulkan video_pyrowave path) are unaffected, and pyrowave's own decode ignores the flag (raw Y'CbCr reconstruction). No client rebuild needed. Unit-tested + asserted in pyrowave_win_smoke; the smoke decode still round-trips 100/180/60. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/pf-encode/src/enc/linux/pyrowave.rs | 5 +++ crates/pf-encode/src/enc/pyrowave_wire.rs | 33 ++++++++++++++++++++ crates/pf-encode/src/enc/windows/pyrowave.rs | 13 ++++++++ 3 files changed, 51 insertions(+) diff --git a/crates/pf-encode/src/enc/linux/pyrowave.rs b/crates/pf-encode/src/enc/linux/pyrowave.rs index a082bfa5..f315375b 100644 --- a/crates/pf-encode/src/enc/linux/pyrowave.rs +++ b/crates/pf-encode/src/enc/linux/pyrowave.rs @@ -1094,6 +1094,11 @@ impl PyroWaveEncoder { "packetize", )?; packets.truncate(out_n.max(1)); + // Correct pyrowave's zeroed sequence-header VUI: it signals ycbcr_range=FULL, but our CSC + // emits BT.709 LIMITED — patch the bit HONEST so VUI-honoring clients don't wash out blacks. + if let Some(p) = packets.first() { + crate::pyrowave_wire::mark_limited_range(&mut self.bitstream, p.offset); + } // Frame into the wire AU via the shared helper (byte-identical on Linux + Windows): the dense // single packet, or the datagram-aligned windowed AU (§4.4). let pkts: Vec<(usize, usize)> = packets.iter().map(|p| (p.offset, p.size)).collect(); diff --git a/crates/pf-encode/src/enc/pyrowave_wire.rs b/crates/pf-encode/src/enc/pyrowave_wire.rs index d7fbd553..d648e280 100644 --- a/crates/pf-encode/src/enc/pyrowave_wire.rs +++ b/crates/pf-encode/src/enc/pyrowave_wire.rs @@ -26,6 +26,25 @@ pub(crate) fn packet_boundary(wire_chunk: Option, dense_cap: usize) -> us wire_chunk.map(|c| c - WINDOW_PREFIX).unwrap_or(dense_cap) } +/// Patch the frame's `BitstreamSequenceHeader` to signal `ycbcr_range = LIMITED`. pyrowave's C API +/// fills the header with `= {}` (all VUI fields zeroed) and offers NO way to set colour/range, so it +/// signals `ycbcr_range = 0 = YCBCR_RANGE_FULL` — but BOTH host CSCs (`rgb2yuv.comp` on Linux, the +/// D3D11 `BgraToYuvPlanes` on Windows) always emit BT.709 **LIMITED** Y′CbCr (black = Y′16). A client +/// that honours the VUI (the Apple wavelet decoder reads `(word1 >> 30) & 1`) then skips the +/// limited→full expansion and shows washed-out, raised blacks. Patching the bit makes the bitstream +/// HONEST for every client — clients that hardcode limited (the Vulkan `video_pyrowave` path) are +/// unaffected, and pyrowave's own decode ignores the flag (it reconstructs raw Y′CbCr). The other +/// zeroed VUI fields (BT.709 primaries / transform / transfer) are already correct. +/// +/// `seq_offset` is the byte offset of the frame's 8-byte `BitstreamSequenceHeader` in `bitstream` — +/// the SOF packet's offset. `ycbcr_range` is bit 30 of the little-endian second word, i.e. bit 6 of +/// byte `seq_offset + 7` (`0x40`). +pub(crate) fn mark_limited_range(bitstream: &mut [u8], seq_offset: usize) { + if let Some(b) = bitstream.get_mut(seq_offset + 7) { + *b |= 0x40; + } +} + /// Frame pyrowave's `packets` (each an `(offset, size)` into `bitstream`) into the wire AU. /// `wire_chunk = None` copies the single dense packet; `Some(chunk)` produces the windowed /// datagram-aligned AU (a whole number of `chunk`-sized windows). @@ -161,4 +180,18 @@ mod tests { assert_eq!(packet_boundary(Some(1408), 999_999), 1404); assert_eq!(packet_boundary(None, 777), 777); } + + #[test] + fn mark_limited_range_sets_only_the_range_bit() { + let mut bs = vec![0u8; 16]; + mark_limited_range(&mut bs, 0); + // ycbcr_range = bit 30 of the LE second word = bit 6 of byte 7 (0x40); nothing else touched. + assert_eq!(bs[7], 0x40); + assert!(bs[..7].iter().all(|&b| b == 0)); + assert!(bs[8..].iter().all(|&b| b == 0)); + // Idempotent; an out-of-range offset is a silent no-op (never panics). + mark_limited_range(&mut bs, 0); + assert_eq!(bs[7], 0x40); + mark_limited_range(&mut bs, 100); + } } diff --git a/crates/pf-encode/src/enc/windows/pyrowave.rs b/crates/pf-encode/src/enc/windows/pyrowave.rs index 7bd1ff4a..fa9c1b17 100644 --- a/crates/pf-encode/src/enc/windows/pyrowave.rs +++ b/crates/pf-encode/src/enc/windows/pyrowave.rs @@ -478,6 +478,11 @@ impl PyroWaveEncoder { "packetize", )?; packets.truncate(out_n.max(1)); + // Correct pyrowave's zeroed sequence-header VUI: it signals ycbcr_range=FULL, but our CSC + // emits BT.709 LIMITED — patch the bit HONEST so VUI-honoring clients don't wash out blacks. + if let Some(p) = packets.first() { + pyrowave_wire::mark_limited_range(&mut self.bitstream, p.offset); + } let pkts: Vec<(usize, usize)> = packets.iter().map(|p| (p.offset, p.size)).collect(); let au = pyrowave_wire::build_au(&pkts, &self.bitstream, self.wire_chunk); self.pending.push_back(EncodedFrame { @@ -787,6 +792,14 @@ mod tests { let au = enc.poll().expect("poll").expect("one AU per frame"); assert!(au.keyframe, "every pyrowave AU is a keyframe"); assert!(!au.data.is_empty(), "AU is non-empty"); + // The dense AU starts with the 8-byte BitstreamSequenceHeader; the range VUI must read + // LIMITED (bit 30 = byte 7 bit 6 = 0x40) — `mark_limited_range` corrects pyrowave's zeroed + // default so VUI-honoring clients (Apple) don't wash out blacks. + assert_eq!( + au.data[7] & 0x40, + 0x40, + "sequence header must signal ycbcr_range=LIMITED" + ); decode_plane_means(w, h, &au.data) }