From 7d91a73491326e0c7c4f7d14fcb8467aae2b4be3 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Thu, 5 Nov 2026 12:05:41 -0600 Subject: [PATCH] fix(j2k): write sRGB-equivalent ICC samples to RGB8/RGBA8 region output A JP2 tagged with an IEC 61966-2-1 sRGB ICC profile failed region and pow2-scaled region decodes to Rgb8/Rgba8 with "backend color space cannot be mapped to requested 8-bit pixel format", leaving decode_srgb8, a whole-image decode, as the only route. Treat a three-channel ICC profile as RGB for 8-bit RGB output when it is a restricted matrix/TRC RGB profile whose linear matrix to sRGB is within 2.5e-4 (off-diagonal row sums) and 1e-3 (diagonal) of identity, and whose 8-bit transform to sRGB moves no channel or neutral ramp sample by more than one code value. The matrix is checked directly because the ramps alone miss negative crosstalk, which clips on single-channel ramps and cancels on neutrals but shifts mixed colours by tens of code values. This applies to decode_region_into, decode_region_scaled_pow2_into, the prepared region path, the whole-image decode_into fallback, and the streamed stripes of decode_rows_u8_bounded. Other ICC profiles and 16-bit outputs remain unsupported. Fixes #321 --- CHANGELOG.md | 9 + crates/j2k/src/decode/output.rs | 6 +- crates/j2k/src/decode/output/u8.rs | 26 ++- crates/j2k/src/decode/srgb8.rs | 86 ++++++++ crates/j2k/src/decode/stripes.rs | 4 +- crates/j2k/tests/srgb_icc_region.rs | 308 ++++++++++++++++++++++++++++ 6 files changed, 432 insertions(+), 7 deletions(-) create mode 100644 crates/j2k/tests/srgb_icc_region.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6284bfdf7..c37dda1c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and stale roadmap entries have been removed from the public documentation set. Staged workspace version: `0.13.2`. +- `decode_region_into`, `decode_region_scaled_pow2_into`, `decode_into`, + and `decode_rows_u8_bounded` accept a JP2 whose ICC profile is equivalent + to IEC 61966-2-1 sRGB when the output is `Rgb8` or `Rgba8`, writing its + samples as RGB without a colour transform. A profile qualifies when it is + a restricted RGB matrix/TRC profile whose linear matrix to sRGB is within + 2.5e-4 (off-diagonal row sums) and 1e-3 (diagonal) of identity, and whose + transform to sRGB moves no 8-bit channel or neutral ramp sample by more + than one code value. Other ICC profiles remain unsupported for these + outputs (#321). - SOP marker segments a decode parses require `Lsop` 4 and an `Nsop` equal to the tile-wide packet ordinal modulo 65536. The ordinal continues across tile-parts and counts packets skipped without parsing; an invalid SOP in a diff --git a/crates/j2k/src/decode/output.rs b/crates/j2k/src/decode/output.rs index 00bdf0455..f6cbf4dee 100644 --- a/crates/j2k/src/decode/output.rs +++ b/crates/j2k/src/decode/output.rs @@ -21,8 +21,8 @@ pub(super) use u16::{ write_components_u16_output, write_plane_rows_u16, write_u16_output, }; pub(super) use u8::{ - can_decode_u8_directly, expand_rgb8_to_rgba8, write_components_u8_output, write_plane_rows_u8, - write_u8_output, + can_decode_u8_directly, expand_rgb8_to_rgba8, resolve_u8_color_space, + write_components_u8_output, write_plane_rows_u8, write_u8_output, }; /// Shared destination and error plumbing for the 8- and 16-bit component @@ -48,7 +48,7 @@ impl<'o> ComponentRowOutput<'o> { fmt: PixelFormat, ) -> Self { Self { - color_space, + color_space: resolve_u8_color_space(color_space, fmt), has_alpha, out, stride, diff --git a/crates/j2k/src/decode/output/u8.rs b/crates/j2k/src/decode/output/u8.rs index 6364d1b99..b600f6a79 100644 --- a/crates/j2k/src/decode/output/u8.rs +++ b/crates/j2k/src/decode/output/u8.rs @@ -3,10 +3,32 @@ //! Eight-bit direct-layout and component-plane conversion. use crate::backend::{ColorSpace, DecodedComponents as NativeDecodedComponents}; +use crate::decode::srgb8::is_srgb_equivalent_icc_profile; use crate::J2kError; use core::ops::Range; use j2k_core::{PixelFormat, Unsupported}; +static SRGB_ICC_AS_RGB: ColorSpace = ColorSpace::RGB; + +/// The colour space whose samples `fmt` receives. A three-channel ICC +/// profile equivalent to IEC 61966-2-1 sRGB is written to RGB output as +/// RGB; every other colour space is returned unchanged. +pub(in crate::decode) fn resolve_u8_color_space( + color_space: &ColorSpace, + fmt: PixelFormat, +) -> &ColorSpace { + match (color_space, fmt) { + ( + ColorSpace::Icc { + profile, + num_channels: 3, + }, + PixelFormat::Rgb8 | PixelFormat::Rgba8, + ) if is_srgb_equivalent_icc_profile(profile) => &SRGB_ICC_AS_RGB, + _ => color_space, + } +} + pub(in crate::decode) fn can_decode_u8_directly( color_space: &ColorSpace, has_alpha: bool, @@ -30,7 +52,7 @@ pub(in crate::decode) fn write_components_u8_output( fmt: PixelFormat, ) -> Result<(), J2kError> { write_planes_u8( - components.color_space(), + resolve_u8_color_space(components.color_space(), fmt), components.has_alpha(), components.planes(), components.dimensions(), @@ -277,7 +299,7 @@ pub(in crate::decode) fn write_u8_output( ) -> Result<(), J2kError> { let width = dims.0 as usize; let height = dims.1 as usize; - match (color_space, has_alpha, fmt) { + match (resolve_u8_color_space(color_space, fmt), has_alpha, fmt) { (ColorSpace::RGB, false, PixelFormat::Rgb8) => { copy_rows_exact(decoded, out, stride, width * 3, height); Ok(()) diff --git a/crates/j2k/src/decode/srgb8.rs b/crates/j2k/src/decode/srgb8.rs index 7616636a4..75f4d2a57 100644 --- a/crates/j2k/src/decode/srgb8.rs +++ b/crates/j2k/src/decode/srgb8.rs @@ -167,6 +167,92 @@ fn convert_icc_data( } } +/// Whether `profile` describes IEC 61966-2-1 sRGB closely enough for its +/// samples to be written as sRGB without a colour transform. +/// +/// The profile must be a restricted matrix/TRC RGB profile whose linear +/// RGB-to-sRGB matrix is within [`SRGB_MATRIX_OFF_DIAGONAL_TOLERANCE`] and +/// [`SRGB_MATRIX_DIAGONAL_TOLERANCE`] of identity, and whose 8-bit transform +/// to sRGB, as [`decode_image_srgb8`] would apply it, moves no sample of the +/// per-channel and neutral ramps by more than one code value. The ramps +/// alone cannot bound mixed colours: a negative off-diagonal term clips to +/// zero on a single-channel ramp and cancels on neutrals, yet shifts a +/// mixed colour such as (30, 255, 0) by tens of code values, so the matrix +/// is checked directly. Any parse or transform failure reports the profile +/// as not sRGB. +pub(in crate::decode) fn is_srgb_equivalent_icc_profile(profile: &[u8]) -> bool { + let Some(max_profile_size) = profile.len().checked_add(1) else { + return false; + }; + let Ok(source) = ColorProfile::new_from_slice_with_options( + profile, + ParsingOptions { + max_profile_size, + max_allowed_clut_size: 0, + max_allowed_trc_size: 65_536, + }, + ) else { + return false; + }; + if source.color_space != DataColorSpace::Rgb || reject_non_restricted_profile(&source).is_err() + { + return false; + } + let srgb = ColorProfile::new_srgb(); + if !is_near_identity(&source.transform_matrix(&srgb).v) { + return false; + } + let Ok(transform) = + source.create_transform_8bit(Layout::Rgb, &srgb, Layout::Rgb, TransformOptions::default()) + else { + return false; + }; + + // Red, green, blue, and neutral ramps over every 8-bit code value. + let mut probe = Vec::new(); + if probe.try_reserve_exact(256 * 4 * 3).is_err() { + return false; + } + for value in 0..=u8::MAX { + probe.extend_from_slice(&[value, 0, 0, 0, value, 0, 0, 0, value, value, value, value]); + } + let mut converted = Vec::new(); + if converted.try_reserve_exact(probe.len()).is_err() { + return false; + } + converted.resize(probe.len(), 0_u8); + transform.transform(&probe, &mut converted).is_ok() + && probe + .iter() + .zip(&converted) + .all(|(&expected, &actual)| expected.abs_diff(actual) <= 1) +} + +/// Largest sum of one row's off-diagonal entries in the linear +/// source-to-sRGB matrix. Leakage into a channel that should be zero is +/// steepest on the sRGB toe, where one 8-bit code value is +/// `1 / (12.92 * 255)` (about 3.0e-4) of linear light; this keeps the +/// leakage under one code value. +const SRGB_MATRIX_OFF_DIAGONAL_TOLERANCE: f64 = 2.5e-4; + +/// Largest distance of a diagonal entry from one. Its error in code values +/// peaks at full intensity, where the sRGB curve changes about 112 code +/// values per unit of linear light, so this stays near 0.1 code value. +const SRGB_MATRIX_DIAGONAL_TOLERANCE: f64 = 1e-3; + +fn is_near_identity(matrix: &[[f64; 3]; 3]) -> bool { + matrix.iter().enumerate().all(|(row, entries)| { + let off_diagonal: f64 = entries + .iter() + .enumerate() + .filter(|&(column, _)| column != row) + .map(|(_, entry)| entry.abs()) + .sum(); + (entries[row] - 1.0).abs() <= SRGB_MATRIX_DIAGONAL_TOLERANCE + && off_diagonal <= SRGB_MATRIX_OFF_DIAGONAL_TOLERANCE + }) +} + fn reject_non_restricted_profile(profile: &ColorProfile) -> Result<(), J2kError> { if profile.lut_a_to_b_perceptual.is_some() || profile.lut_a_to_b_colorimetric.is_some() diff --git a/crates/j2k/src/decode/stripes.rs b/crates/j2k/src/decode/stripes.rs index da2298ab3..883cc4623 100644 --- a/crates/j2k/src/decode/stripes.rs +++ b/crates/j2k/src/decode/stripes.rs @@ -15,7 +15,7 @@ use crate::backend::{ColorSpace, Image}; use crate::J2kError; use j2k_core::{DecodeRowsError, PixelFormat}; -use super::output::{write_plane_rows_u16, write_plane_rows_u8}; +use super::output::{resolve_u8_color_space, write_plane_rows_u16, write_plane_rows_u8}; #[cfg(test)] std::thread_local! { @@ -124,7 +124,7 @@ pub(crate) fn decode_image_rows_in_stripes<'a, E: core::error::Error + 'static>( #[cfg(test)] let skip_decode = injected == Some(InjectedFailure::BeforeRows); let mut sink = StripeSink { - color_space: image.color_space(), + color_space: resolve_u8_color_space(image.color_space(), fmt), has_alpha: image.has_alpha(), fmt, pack, diff --git a/crates/j2k/tests/srgb_icc_region.rs b/crates/j2k/tests/srgb_icc_region.rs new file mode 100644 index 000000000..79ec793a9 --- /dev/null +++ b/crates/j2k/tests/srgb_icc_region.rs @@ -0,0 +1,308 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! Integer RGB output for JP2 files tagged with an IEC 61966-2-1 sRGB ICC +//! profile (issue #321). + +use j2k::{ + wrap_j2k_codestream, J2kDecoder, J2kError, J2kFileColorSpec, J2kFileWrapOptions, + J2kRowDecodeOptions, J2kScratchPool, PixelFormat, Rect, RowSink, +}; +use j2k_core::{Colorspace, Unsupported}; +use j2k_native::{encode, EncodeOptions}; +use moxcms::{ColorProfile, ToneReprCurve, Xyzd}; + +const WIDTH: u32 = 16; +const HEIGHT: u32 = 16; + +fn rgb_codestream() -> Vec { + let pixels: Vec = (0..WIDTH * HEIGHT * 3) + .map(|i| u8::try_from((i * 37 + i / 7) % 256).expect("byte")) + .collect(); + encode( + &pixels, + WIDTH, + HEIGHT, + 3, + 8, + false, + &EncodeOptions { + reversible: true, + ..EncodeOptions::default() + }, + ) + .expect("encode RGB fixture") +} + +fn jp2(color: J2kFileColorSpec<'_>) -> Vec { + wrap_j2k_codestream( + &rgb_codestream(), + J2kFileWrapOptions::jp2().with_color(color), + ) + .expect("wrap JP2 fixture") +} + +fn enumerated_srgb_jp2() -> Vec { + jp2(J2kFileColorSpec::Enumerated(Colorspace::SRgb)) +} + +fn icc_jp2(profile: &ColorProfile) -> Vec { + let profile = profile.encode().expect("encode ICC profile"); + jp2(J2kFileColorSpec::IccProfile(&profile)) +} + +/// sRGB with sampled `curv` tables, as in the widely distributed +/// "sRGB IEC61966-2.1" profile, rather than moxcms's parametric curves. +#[expect( + clippy::cast_possible_truncation, + clippy::cast_sign_loss, + reason = "linear sRGB values lie in [0, 1], so scaled samples fit u16" +)] +fn sampled_trc_srgb_profile() -> ColorProfile { + let table: Vec = (0..1024_u32) + .map(|i| { + let encoded = f64::from(i) / 1023.0; + let linear = if encoded <= 0.040_45 { + encoded / 12.92 + } else { + ((encoded + 0.055) / 1.055).powf(2.4) + }; + (linear * 65535.0).round() as u16 + }) + .collect(); + let mut profile = ColorProfile::new_srgb(); + profile.red_trc = Some(ToneReprCurve::Lut(table.clone())); + profile.green_trc = Some(ToneReprCurve::Lut(table.clone())); + profile.blue_trc = Some(ToneReprCurve::Lut(table)); + profile.cicp = None; + profile +} + +fn region(bytes: &[u8], fmt: PixelFormat, roi: Rect) -> Result, j2k::J2kError> { + let stride = roi.w as usize * fmt.bytes_per_pixel(); + let mut out = vec![0_u8; stride * roi.h as usize]; + J2kDecoder::new(bytes)?.decode_region_into( + &mut J2kScratchPool::new(), + &mut out, + stride, + fmt, + roi, + )?; + Ok(out) +} + +fn scaled_region(bytes: &[u8], fmt: PixelFormat, roi: Rect, levels: u8) -> Vec { + let scaled_w = (roi.x + roi.w).div_ceil(1 << levels) - roi.x / (1 << levels); + let scaled_h = (roi.y + roi.h).div_ceil(1 << levels) - roi.y / (1 << levels); + let stride = scaled_w as usize * fmt.bytes_per_pixel(); + let mut out = vec![0_u8; stride * scaled_h as usize]; + J2kDecoder::new(bytes) + .expect("decoder") + .decode_region_scaled_pow2_into( + &mut J2kScratchPool::new(), + &mut out, + stride, + fmt, + roi, + levels, + ) + .expect("scaled region decode"); + out +} + +fn whole(bytes: &[u8], fmt: PixelFormat) -> Vec { + let stride = WIDTH as usize * fmt.bytes_per_pixel(); + let mut out = vec![0_u8; stride * HEIGHT as usize]; + J2kDecoder::new(bytes) + .expect("decoder") + .decode_into(&mut out, stride, fmt) + .expect("whole-image decode"); + out +} + +const ROI: Rect = Rect { + x: 3, + y: 5, + w: 9, + h: 7, +}; + +#[test] +fn srgb_icc_region_rgb8_and_rgba8_match_enumerated_srgb() { + let reference = enumerated_srgb_jp2(); + for profile in [ColorProfile::new_srgb(), sampled_trc_srgb_profile()] { + let tagged = icc_jp2(&profile); + for fmt in [PixelFormat::Rgb8, PixelFormat::Rgba8] { + assert_eq!( + region(&tagged, fmt, ROI).expect("sRGB ICC region decode"), + region(&reference, fmt, ROI).expect("enumerated sRGB region decode"), + "{fmt:?}" + ); + } + } +} + +#[test] +fn srgb_icc_scaled_region_matches_enumerated_srgb() { + let reference = enumerated_srgb_jp2(); + let tagged = icc_jp2(&ColorProfile::new_srgb()); + for fmt in [PixelFormat::Rgb8, PixelFormat::Rgba8] { + for levels in [1, 2] { + assert_eq!( + scaled_region(&tagged, fmt, ROI, levels), + scaled_region(&reference, fmt, ROI, levels), + "{fmt:?} at {levels} levels" + ); + } + } +} + +#[test] +fn srgb_icc_whole_image_rgb8_matches_enumerated_srgb_and_decode_srgb8() { + let reference = enumerated_srgb_jp2(); + let tagged = icc_jp2(&ColorProfile::new_srgb()); + let rgb = whole(&tagged, PixelFormat::Rgb8); + assert_eq!(rgb, whole(&reference, PixelFormat::Rgb8)); + assert_eq!( + whole(&tagged, PixelFormat::Rgba8), + whole(&reference, PixelFormat::Rgba8) + ); + let normalized = J2kDecoder::new(&tagged) + .expect("decoder") + .decode_srgb8() + .expect("decode_srgb8"); + assert!(rgb + .iter() + .zip(normalized.data()) + .all(|(&raw, &converted)| raw.abs_diff(converted) <= 1)); +} + +#[derive(Default)] +struct CollectRows(Vec); + +impl RowSink for CollectRows { + type Error = J2kError; + + fn write_row(&mut self, _y: u32, row: &[u8]) -> Result<(), Self::Error> { + self.0.extend_from_slice(row); + Ok(()) + } +} + +fn bounded_rows(bytes: &[u8], rows_per_stripe: u32) -> Vec { + let mut sink = CollectRows::default(); + J2kDecoder::new(bytes) + .expect("decoder") + .decode_rows_u8_bounded(&mut sink, J2kRowDecodeOptions::new(rows_per_stripe)) + .expect("bounded row decode"); + sink.0 +} + +#[test] +fn srgb_icc_bounded_rows_match_enumerated_srgb() { + let reference = enumerated_srgb_jp2(); + let tagged = icc_jp2(&ColorProfile::new_srgb()); + for rows_per_stripe in [1, 5, HEIGHT] { + assert_eq!( + bounded_rows(&tagged, rows_per_stripe), + bounded_rows(&reference, rows_per_stripe), + "{rows_per_stripe} rows per stripe" + ); + } +} + +#[test] +fn non_srgb_icc_region_rgb8_remains_unsupported() { + let unsupported = j2k::J2kError::from(Unsupported { + what: "backend color space cannot be mapped to requested 8-bit pixel format", + }); + for profile in [ + ColorProfile::new_display_p3(), + ColorProfile::new_adobe_rgb(), + ColorProfile::new_pro_photo_rgb(), + ] { + let tagged = icc_jp2(&profile); + assert_eq!( + region(&tagged, PixelFormat::Rgb8, ROI).expect_err("non-sRGB ICC"), + unsupported + ); + } +} + +/// Near-sRGB profile whose linear matrix to sRGB is +/// `[[1.01, -0.01, 0], [0, 1, 0], [0, 0, 1]]`. Its red, green, blue, and +/// neutral 8-bit ramps all convert to sRGB within one code value, because +/// the negative term clips to zero on a green ramp and cancels on neutrals, +/// yet it moves the red of (30, 255, 0) to 10. +fn hidden_crosstalk_profile() -> ColorProfile { + let srgb = ColorProfile::new_srgb(); + let (red, green) = (srgb.red_colorant, srgb.green_colorant); + let mut profile = ColorProfile::new_srgb(); + profile.red_colorant = Xyzd { + x: red.x * 1.01, + y: red.y * 1.01, + z: red.z * 1.01, + }; + profile.green_colorant = Xyzd { + x: green.x - 0.01 * red.x, + y: green.y - 0.01 * red.y, + z: green.z - 0.01 * red.z, + }; + profile.cicp = None; + profile +} + +#[test] +fn near_srgb_icc_with_mixed_colour_crosstalk_remains_unsupported() { + let profile = hidden_crosstalk_profile(); + let pixels = [30_u8, 255, 0]; + let codestream = encode( + &pixels, + 1, + 1, + 3, + 8, + false, + &EncodeOptions { + reversible: true, + ..EncodeOptions::default() + }, + ) + .expect("encode mixed-colour fixture"); + let encoded = profile.encode().expect("encode ICC profile"); + let tagged = wrap_j2k_codestream( + &codestream, + J2kFileWrapOptions::jp2().with_color(J2kFileColorSpec::IccProfile(&encoded)), + ) + .expect("wrap mixed-colour fixture"); + + // The real transform shifts this mixed colour well beyond one code value, + // so writing the samples unchanged would be wrong. + let normalized = J2kDecoder::new(&tagged) + .expect("decoder") + .decode_srgb8() + .expect("decode_srgb8"); + assert!(normalized.data()[0].abs_diff(pixels[0]) > 10); + + let mut out = [0_u8; 3]; + let error = J2kDecoder::new(&tagged) + .expect("decoder") + .decode_region_into( + &mut J2kScratchPool::new(), + &mut out, + 3, + PixelFormat::Rgb8, + Rect { + x: 0, + y: 0, + w: 1, + h: 1, + }, + ) + .expect_err("crosstalk profile must not bypass the ICC transform"); + assert_eq!( + error, + j2k::J2kError::from(Unsupported { + what: "backend color space cannot be mapped to requested 8-bit pixel format", + }) + ); +}