From c95ae0aadea74d3aed0144b08454b9662f34e3ef Mon Sep 17 00:00:00 2001 From: Transaction Science Date: Sat, 18 Jul 2026 02:04:41 -0400 Subject: [PATCH] wai-rs: fix picture-header per-component flag order (grfs_channel_flags were corrupt) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parse_picture_header read four flags before the GRFS channel flags; only three precede them and the fourth trails the group: { multi_threading_r, num_chs, use_cube, rvs_enable, grfs_enable, grfs_channel_flags[num_chs], lsbs_enable } Reading four up-front shifted grfs_channel_flags one bit late, so every channel flag was wrong — while still landing on the correct end_bit (437), because the extra leading bit was cancelled by consuming one fewer channel flag. A cursor-position assertion catches net drift, not compensating errors; the values it produced were silently wrong. Found by deriving the residual-variance-scale (RVS) additive independently: the chroma plane separates cleanly by cwg branch, so its true per-channel assignment is directly derivable and pinned the channel flags to bit 337 (chroma) / 164 (luma) — one earlier than assumed. With the corrected offsets the RVS reproduction is byte-exact on BOTH planes (luma 40960/40960, chroma 24576/24576), which independently confirms the flag counts are 5 (luma) and 10 (chroma), not 4 and 9. Tests updated accordingly and re-validated: end_bit still 437, and the channel-flag counts now match the independently-derived RVS assignment. Co-Authored-By: Claude Opus 4.8 --- wai/wai-rs/src/jpegai_bitstream.rs | 11 +++++++++-- wai/wai-rs/tests/jpegai_bitstream_calibration.rs | 10 ++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/wai/wai-rs/src/jpegai_bitstream.rs b/wai/wai-rs/src/jpegai_bitstream.rs index ed532215b..fe8a1146b 100644 --- a/wai/wai-rs/src/jpegai_bitstream.rs +++ b/wai/wai-rs/src/jpegai_bitstream.rs @@ -206,7 +206,7 @@ pub struct PictureHeader { /// prefix → `CCS_SGMM::decode_header` (`multi_threading_z`, quantizer β F-header, /// `region_partitioning`, quantizer β O-header) → the per-component tool groups /// (`multi_threading_r`, `num_chs`, then the tools in sub-tool order: -/// `use_cube_flags`, `lsbs_enable`, `rvs_enable`, `grfs_enable` + channel flags) → +/// `use_cube_flags`, `rvs_enable`, `grfs_enable` + channel flags, then `lsbs_enable`) → /// `gain_3D_enable` → `synthesis_tile_enable` per component. /// /// Branches not exercised by the 4:4:4 single-tile reference stream (region @@ -464,13 +464,20 @@ pub fn parse_picture_header(data: &[u8], start_bit: usize) -> PictureHeader { let comp = &mut h.components[c]; comp.num_threads_r = if br.read(1) == 1 { 1 << (br.read(2) + 1) } else { 1 }; comp.num_chs = br.read(8) as u32; + // Only THREE flags precede the channel flags; the fourth TRAILS them. The + // tools own different parts of the walk, so `res_var_scale`'s pair + // (rvs_enable, grfs_enable) sits immediately before the channel flags it + // gates, and the remaining tool's flag is emitted after that group. + // Reading four up-front shifts `grfs_channel_flags` one bit late and + // corrupts them (while still landing on the right end_bit, since the extra + // leading bit is cancelled by one fewer channel flag). comp.use_cube_flags = br.read(1) == 1; - comp.lsbs_enable = br.read(1) == 1; comp.rvs_enable = br.read(1) == 1; comp.grfs_enable = br.read(1) == 1; if comp.grfs_enable { comp.grfs_channel_flags = (0..comp.num_chs).map(|_| br.read(1) == 1).collect(); } + comp.lsbs_enable = br.read(1) == 1; } h.gain_3d_enable = br.read(1) == 1; for c in 0..2 { diff --git a/wai/wai-rs/tests/jpegai_bitstream_calibration.rs b/wai/wai-rs/tests/jpegai_bitstream_calibration.rs index 9d71a1de4..7242bb274 100644 --- a/wai/wai-rs/tests/jpegai_bitstream_calibration.rs +++ b/wai/wai-rs/tests/jpegai_bitstream_calibration.rs @@ -114,13 +114,15 @@ fn full_picture_header_matches_reference() { for c in 0..2 { let comp = &h.components[c]; assert!(!comp.use_cube_flags, "c{c} use_cube"); - assert!(comp.lsbs_enable && comp.rvs_enable && comp.grfs_enable, "c{c} lsbs/rvs/grfs"); + assert!(comp.rvs_enable && comp.grfs_enable, "c{c} rvs/grfs precede the channel flags"); + assert!(!comp.lsbs_enable, "c{c} lsbs trails the channel flags"); assert_eq!(comp.grfs_channel_flags.len(), comp.num_chs as usize, "c{c} grfs flag count == num_chs"); assert!(!comp.synthesis_tile_enable, "c{c} tile"); } - // the grfs channel-selection counts match the sigma-additions gain fixture - assert_eq!(h.components[0].grfs_channel_flags.iter().filter(|&&b| b).count(), 4); - assert_eq!(h.components[1].grfs_channel_flags.iter().filter(|&&b| b).count(), 9); + // GRFS channel-selection counts — independently confirmed by deriving the same + // per-channel cwg assignment from the reference RVS output (both planes byte-exact) + assert_eq!(h.components[0].grfs_channel_flags.iter().filter(|&&b| b).count(), 5); + assert_eq!(h.components[1].grfs_channel_flags.iter().filter(|&&b| b).count(), 10); // the whole header consumed exactly to the padding boundary of the 49-byte PIH assert_eq!(h.end_bit, 437, "all PIH fields consumed"); -- 2.54.0 (Apple Git-157)