From d30069c35bb57962a99dfda646de838d9a59b367 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Fri, 6 Nov 2026 20:05:14 -0600 Subject: [PATCH 1/2] fix(j2k-native): keep the declared color space over unspecified extra components A JP2 declaring sRGB over four codestream components with no cdef box failed every Rgb8/Rgba8 decode under the default strict settings with "backend color space cannot be mapped to requested 8-bit pixel format". The component-count fallback in resolve_alpha_and_color_space replaced the declared sRGB with CMYK, which has no 8-bit RGB mapping. Kakadu 7 wrote such files from RGBA TIFFs unless -jp2_alpha was given. Without a channel definition, ISO/IEC 15444-1 I.5.3.6 leaves the components after those of the declared color space unspecified. When a JP2 declares an enumerated or ICC color space, has no cdef or palette, and has more components than that space (plus alpha), keep the color space and record the extras as Image::ignored_components. Decoded output omits them: finish_decoded_image, the native RawBitmap decodes, streamed region rows and the native planes sink keep only the color and alpha components. Device plane reuse is disabled for such images. Lenient decoding still infers alpha for exactly one extra component, and raw codestreams keep their component-count interpretation. Recode keeps every component, since it reuses the source JP2 metadata. The HTJ2K batch direct-plan builder now treats DirectPlanUnsupported as "not direct" and falls back instead of failing the job. --- CHANGELOG.md | 12 + crates/j2k-native/src/color/metadata.rs | 51 +++- crates/j2k-native/src/color/mod.rs | 4 +- crates/j2k-native/src/image.rs | 56 +++- crates/j2k-native/src/image/native/rows.rs | 3 +- crates/j2k-native/src/image/output_api.rs | 26 +- crates/j2k-native/src/j2c/mod.rs | 6 +- crates/j2k-native/src/jp2/container.rs | 14 +- crates/j2k-native/src/lib.rs | 2 +- crates/j2k/src/batch/direct/planning.rs | 5 +- crates/j2k/src/recode/pixel.rs | 5 +- .../j2k/tests/unspecified_extra_components.rs | 259 ++++++++++++++++++ 12 files changed, 406 insertions(+), 37 deletions(-) create mode 100644 crates/j2k/tests/unspecified_extra_components.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c37dda1c7..5557e62ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and stale roadmap entries have been removed from the public documentation set. Staged workspace version: `0.13.2`. +- A JP2 whose codestream has more components than its declared color space + and no `cdef` box keeps that color space under strict decoding instead of + being reinterpreted from its component count. The extra components are + unspecified (ISO/IEC 15444-1 I.5.3.6) and decoded output omits them: + `Image::ignored_components` counts them. An sRGB JP2 over four components, + as Kakadu 7 wrote from RGBA TIFFs without `-jp2_alpha`, decodes to `Rgb8`, + or to `Rgba8` with opaque alpha, instead of failing as CMYK with "backend + color space cannot be mapped to requested 8-bit pixel format". Lenient + decoding still infers alpha for one extra component, raw codestreams keep + their component-count interpretation, and recode re-encodes every + component. HTJ2K batch jobs whose direct color plan is unsupported fall + back to the generic decode instead of failing. - `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 diff --git a/crates/j2k-native/src/color/metadata.rs b/crates/j2k-native/src/color/metadata.rs index b191c7cb2..daa768150 100644 --- a/crates/j2k-native/src/color/metadata.rs +++ b/crates/j2k-native/src/color/metadata.rs @@ -19,12 +19,28 @@ use crate::{ const BITS_PER_ASSOCIATION_WORD: usize = 64; +/// The color interpretation of an image's codestream components. +pub(crate) struct ResolvedColor { + pub(crate) color_space: ColorSpace, + pub(crate) has_alpha: bool, + /// Whether one of the documented lenient metadata recoveries was used. + pub(crate) used_lenient_metadata_recovery: bool, + /// Trailing codestream components beyond the color (and alpha) channels + /// that no channel definition describes. ISO/IEC 15444-1 I.5.3.6 leaves + /// such components to the application; decoded output omits them. + pub(crate) ignored_components: usize, +} + +/// Resolve the color space and alpha channel of `boxes` over `header`'s +/// components. `declared_color` is whether `boxes` carry the file's own +/// color specification rather than one synthesized for a raw codestream. pub(crate) fn resolve_alpha_and_color_space( boxes: &ImageBoxes, header: &Header<'_>, settings: &DecodeSettings, retained_baseline_bytes: usize, -) -> Result<(ColorSpace, bool, bool)> { + declared_color: bool, +) -> Result { let mut num_components = header.component_infos.len(); if settings.resolve_palette_indices { if let Some(palette_box) = &boxes.palette { @@ -57,15 +73,24 @@ pub(crate) fn resolve_alpha_and_color_space( let actual_num_components = header.component_infos.len(); let mut used_lenient_metadata_recovery = false; - if boxes.palette.is_none() - && actual_num_components != usize::from(color_space.num_channels() + u16::from(has_alpha)) - { + let mut ignored_components = 0; + let declared_channels = usize::from(color_space.num_channels()) + usize::from(has_alpha); + if boxes.palette.is_none() && actual_num_components != declared_channels { if !settings.strict && actual_num_components == usize::from(color_space.num_channels()) + 1 && !has_alpha { has_alpha = true; used_lenient_metadata_recovery = true; + } else if declared_color + && boxes.channel_definition.is_none() + && declares_color_space(boxes) + && actual_num_components > declared_channels + { + // Without a channel definition, the components after those of + // the declared color space are unspecified (ISO/IEC 15444-1 + // I.5.3.6), not a reason to reinterpret the color space. + ignored_components = actual_num_components - declared_channels; } else if actual_num_components == 1 || (actual_num_components == 2 && has_alpha) { color_space = ColorSpace::Gray; } else if actual_num_components == 3 { @@ -84,7 +109,23 @@ pub(crate) fn resolve_alpha_and_color_space( } } - Ok((color_space, has_alpha, used_lenient_metadata_recovery)) + Ok(ResolvedColor { + color_space, + has_alpha, + used_lenient_metadata_recovery, + ignored_components, + }) +} + +/// Whether the primary color specification names a color space, as an +/// enumerated value or an ICC profile, rather than leaving it unknown. +fn declares_color_space(boxes: &ImageBoxes) -> bool { + matches!( + boxes + .primary_color_specification() + .map(|specification| &specification.color_space), + Some(jp2::colr::ColorSpace::Enumerated(_) | jp2::colr::ColorSpace::Icc(_)) + ) } fn get_color_space( diff --git a/crates/j2k-native/src/color/mod.rs b/crates/j2k-native/src/color/mod.rs index 058257a11..31410284a 100644 --- a/crates/j2k-native/src/color/mod.rs +++ b/crates/j2k-native/src/color/mod.rs @@ -16,7 +16,9 @@ mod types; #[cfg(test)] pub(crate) use cielab::cielab_to_rgb; -pub(crate) use metadata::{resolve_alpha_and_color_space, validate_and_reorder_channels}; +pub(crate) use metadata::{ + resolve_alpha_and_color_space, validate_and_reorder_channels, ResolvedColor, +}; pub(crate) use output_planes::native_component_plane_dimensions; pub use output_planes::{ ComponentPlane, ComponentPlaneParts, ComponentRowSink, DecodedComponents, diff --git a/crates/j2k-native/src/image.rs b/crates/j2k-native/src/image.rs index fb3a8f45d..f4dee4425 100644 --- a/crates/j2k-native/src/image.rs +++ b/crates/j2k-native/src/image.rs @@ -13,7 +13,7 @@ use crate::{ interleave_and_convert_region, resolve_palette_indices, try_resize_decode_elements, validate_and_reorder_channels, validate_interleaved_output_buffer, validate_roi, Bitmap, ColorSpace, DecodedComponents, DecodedNativeComponents, DecoderContext, DecodingError, - FormatError, HtCodeBlockDecoder, Result, CODESTREAM_MAGIC, JP2_MAGIC, + FormatError, HtCodeBlockDecoder, ResolvedColor, Result, CODESTREAM_MAGIC, JP2_MAGIC, }; mod allocation; @@ -136,6 +136,9 @@ pub struct Image<'a> { pub(crate) has_alpha: bool, /// The color space of the image. pub(crate) color_space: ColorSpace, + /// Trailing codestream components that decoded output omits: extra + /// channels no channel definition describes (see [`ResolvedColor`]). + pub(crate) ignored_components: usize, /// Codec-owned bytes already live when this image was parsed, set by /// `new_with_retained_baseline`. Only the planning entry points charge it: /// `prepare_region_decoder_with_context` and the `build_*_plan*_with_context` @@ -179,25 +182,19 @@ impl<'a> ImageSource<'a> { pub(crate) struct ImageProperties { boxes: ImageBoxes, settings: DecodeSettings, - color_space: ColorSpace, - has_alpha: bool, - used_lenient_metadata_recovery: bool, + color: ResolvedColor, } impl ImageProperties { pub(crate) const fn new( boxes: ImageBoxes, settings: DecodeSettings, - color_space: ColorSpace, - has_alpha: bool, - used_lenient_metadata_recovery: bool, + color: ResolvedColor, ) -> Self { Self { boxes, settings, - color_space, - has_alpha, - used_lenient_metadata_recovery, + color, } } } @@ -254,9 +251,13 @@ impl<'a> Image<'a> { let ImageProperties { boxes, settings, - color_space, - has_alpha, - used_lenient_metadata_recovery, + color: + ResolvedColor { + color_space, + has_alpha, + used_lenient_metadata_recovery, + ignored_components, + }, } = properties; let metadata_bytes = retained_metadata_bytes(&header, &boxes, &color_space)?; allocation::combine_retained_bytes(retained_baseline_bytes, metadata_bytes)?; @@ -269,6 +270,7 @@ impl<'a> Image<'a> { used_lenient_metadata_recovery, has_alpha, color_space, + ignored_components, retained_baseline_bytes, #[cfg(feature = "std")] packet_cache: packet_directory::PacketDirectoryCache::default(), @@ -396,6 +398,31 @@ impl<'a> Image<'a> { &self.color_space } + /// Trailing codestream components that decoded output omits. + /// + /// Without a channel definition box, ISO/IEC 15444-1 I.5.3.6 leaves the + /// components after those of the declared color space unspecified: a + /// JP2 declaring sRGB over four components has three color channels and + /// an extra component, not CMYK. Decoded components, planes and bitmaps + /// hold only the color (and alpha) channels. + #[must_use] + pub fn ignored_components(&self) -> usize { + self.ignored_components + } + + /// Keep every codestream component in decoded output, as a re-encode of + /// the whole codestream needs. + #[doc(hidden)] + pub fn keep_ignored_components(&mut self) { + self.ignored_components = 0; + } + + /// Drop the [`Self::ignored_components`] from the end of `components`. + pub(crate) fn drop_ignored_components(&self, components: &mut Vec) { + let kept = components.len().saturating_sub(self.ignored_components); + components.truncate(kept); + } + /// Return the primary JP2 restricted ICC profile, when present. #[doc(hidden)] #[must_use] @@ -449,7 +476,7 @@ impl<'a> Image<'a> { if self.settings.resolve_palette_indices && self.boxes.palette.is_some() { return false; } - if self.boxes.channel_definition.is_some() { + if self.boxes.channel_definition.is_some() || self.ignored_components != 0 { return false; } !matches!( @@ -1222,6 +1249,7 @@ impl<'a> Image<'a> { retained_baseline_bytes, )?; } + self.drop_ignored_components(decoded_image.decoded_components); let bit_depth = decoded_image .decoded_components diff --git a/crates/j2k-native/src/image/native/rows.rs b/crates/j2k-native/src/image/native/rows.rs index 711cb8be9..fdef67afc 100644 --- a/crates/j2k-native/src/image/native/rows.rs +++ b/crates/j2k-native/src/image/native/rows.rs @@ -156,7 +156,8 @@ impl<'i> NativePlanesSink<'i> { /// yet. fn new(image: &'i Image<'_>, dimensions: (u32, u32)) -> Self { Self { - components: &image.header.component_infos, + components: &image.header.component_infos + [..image.header.component_infos.len() - image.ignored_components], color_space: &image.color_space, dimensions, planes: Vec::new(), diff --git a/crates/j2k-native/src/image/output_api.rs b/crates/j2k-native/src/image/output_api.rs index 22ad2aa0d..eb15db687 100644 --- a/crates/j2k-native/src/image/output_api.rs +++ b/crates/j2k-native/src/image/output_api.rs @@ -162,6 +162,7 @@ impl<'a> Image<'a> { let parallel = decoder_context.cpu_decode_parallelism() == CpuDecodeParallelism::Auto; let retained_baseline_bytes = self.run_decode(decoder_context, None, None, true, retained_baseline_bytes)?; + self.drop_ignored_components(&mut decoder_context.tile_decode_context.channel_data); let components = &decoder_context.tile_decode_context.channel_data; let component_owner_capacity = components.capacity(); @@ -251,6 +252,7 @@ impl<'a> Image<'a> { let parallel = decoder_context.cpu_decode_parallelism() == CpuDecodeParallelism::Auto; let retained_image_bytes = self.run_decode(decoder_context, Some(roi), None, true, retained_image_bytes)?; + self.drop_ignored_components(&mut decoder_context.tile_decode_context.channel_data); let components = &decoder_context.tile_decode_context.channel_data; let component_owner_capacity = components.capacity(); @@ -495,13 +497,17 @@ impl<'a> Image<'a> { ) -> Result<()> { validate_roi((self.width(), self.height()), roi)?; self.validate_component_plane_precision()?; + let mut kept_sink = KeptComponentsSink { + sink: &mut *sink, + ignored_components: self.ignored_components, + }; let charged_image_bytes = self.run_decode_rows( decoder_context, Some(roi), None, true, self.retained_header_metadata_bytes()?, - Some(&mut *sink), + Some(&mut kept_sink), )?; if decoder_context.tile_decode_context.stream_rows { return Ok(()); @@ -677,3 +683,21 @@ impl<'a> Image<'a> { ) } } + +/// Hands a sink the streamed rows of every component but the image's +/// [`Image::ignored_components`]. +struct KeptComponentsSink<'s> { + sink: &'s mut dyn ComponentRowSink, + ignored_components: usize, +} + +impl ComponentRowSink for KeptComponentsSink<'_> { + fn write_rows(&mut self, x: u32, y: u32, planes: &[ComponentPlane<'_>]) { + let kept = planes.len().saturating_sub(self.ignored_components); + self.sink.write_rows(x, y, &planes[..kept]); + } + + fn rgb8_region(&mut self, width: u32, height: u32) -> Option<&mut [u8]> { + self.sink.rgb8_region(width, height) + } +} diff --git a/crates/j2k-native/src/j2c/mod.rs b/crates/j2k-native/src/j2c/mod.rs index ea07094be..7618fb3d5 100644 --- a/crates/j2k-native/src/j2c/mod.rs +++ b/crates/j2k-native/src/j2c/mod.rs @@ -132,13 +132,15 @@ pub(crate) fn parse_with_retained_baseline<'a>( retained_baseline_bytes, )?; - let (color_space, has_alpha, _) = resolve_alpha_and_color_space( + let mut color = resolve_alpha_and_color_space( &boxes, &parsed_codestream.header, &strict_settings, retained_baseline_bytes, + false, )?; - let properties = ImageProperties::new(boxes, *settings, color_space, has_alpha, false); + color.used_lenient_metadata_recovery = false; + let properties = ImageProperties::new(boxes, *settings, color); if retained_baseline_bytes == 0 { Image::from_parsed_parts( ImageSource::new(stream, parsed_codestream.data), diff --git a/crates/j2k-native/src/jp2/container.rs b/crates/j2k-native/src/jp2/container.rs index 5949b84eb..195073442 100644 --- a/crates/j2k-native/src/jp2/container.rs +++ b/crates/j2k-native/src/jp2/container.rs @@ -181,21 +181,15 @@ pub(crate) fn parse_with_retained_baseline( image_boxes.component_mapping = Some(ComponentMappingBox { entries: mappings }); } - let (color_space, has_alpha, recovered_alpha_metadata) = resolve_alpha_and_color_space( + let mut color = resolve_alpha_and_color_space( &image_boxes, &parsed_codestream.header, &settings, retained_baseline_bytes, + true, )?; - let used_lenient_metadata_recovery = - container.used_lenient_metadata_recovery || recovered_alpha_metadata; - let properties = ImageProperties::new( - image_boxes, - settings, - color_space, - has_alpha, - used_lenient_metadata_recovery, - ); + color.used_lenient_metadata_recovery |= container.used_lenient_metadata_recovery; + let properties = ImageProperties::new(image_boxes, settings, color); if retained_baseline_bytes == 0 { Image::from_parsed_parts( ImageSource::new(data, parsed_codestream.data), diff --git a/crates/j2k-native/src/lib.rs b/crates/j2k-native/src/lib.rs index 5d00c464a..e539dd5a4 100644 --- a/crates/j2k-native/src/lib.rs +++ b/crates/j2k-native/src/lib.rs @@ -169,7 +169,7 @@ pub use color::pack_rgb8; pub(crate) use color::{ convert_color_space, interleave_and_convert_after_mct, interleave_and_convert_region, resolve_alpha_and_color_space, resolve_palette_indices, validate_and_reorder_channels, - validate_interleaved_output_buffer, + validate_interleaved_output_buffer, ResolvedColor, }; pub use color::{ Bitmap, ColorSpace, ComponentPlane, DecodedComponents, DecodedNativeComponents, diff --git a/crates/j2k/src/batch/direct/planning.rs b/crates/j2k/src/batch/direct/planning.rs index 4937856dc..93fdf8d6b 100644 --- a/crates/j2k/src/batch/direct/planning.rs +++ b/crates/j2k/src/batch/direct/planning.rs @@ -86,6 +86,9 @@ fn direct_color_plan_uses_only_htj2k(plan: &J2kDirectColorPlan) -> bool { fn is_unsupported_direct_color_plan_error(error: NativeDecodeError) -> bool { matches!( error, - NativeDecodeError::Decoding(NativeDecodingError::UnsupportedFeature(_)) + NativeDecodeError::Decoding( + NativeDecodingError::UnsupportedFeature(_) + | NativeDecodingError::DirectPlanUnsupported(_) + ) ) } diff --git a/crates/j2k/src/recode/pixel.rs b/crates/j2k/src/recode/pixel.rs index 7f732823e..607a4f797 100644 --- a/crates/j2k/src/recode/pixel.rs +++ b/crates/j2k/src/recode/pixel.rs @@ -35,8 +35,11 @@ pub(super) fn recode( }); let encoded = { let components = { - let source = Image::new(bytes, &DecodeSettings::default()) + let mut source = Image::new(bytes, &DecodeSettings::default()) .map_err(|err| map_native_decode_error(err, "source JPEG 2000 parse failed"))?; + // The re-encode keeps the source's JP2 metadata, so it carries + // every codestream component, unspecified extras included. + source.keep_ignored_components(); source.decode_native_components().map_err(|err| { map_native_decode_error(err, "source JPEG 2000 component pixel fallback failed") })? diff --git a/crates/j2k/tests/unspecified_extra_components.rs b/crates/j2k/tests/unspecified_extra_components.rs new file mode 100644 index 000000000..c4dacd333 --- /dev/null +++ b/crates/j2k/tests/unspecified_extra_components.rs @@ -0,0 +1,259 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 + +//! JP2 files whose codestream has more components than the declared color +//! space and no channel definition box. ISO/IEC 15444-1 I.5.3.6 leaves the +//! extra components unspecified, so the declared color space stands and the +//! extras are left out of decoded output. Kakadu wrote such files from RGBA +//! TIFFs: an `sRGB` `colr` box over four components and no `cdef` box. + +use j2k::{DecodeSettings, J2kDecoder, J2kScratchPool, PixelFormat, Rect}; +use j2k_native::{encode, ColorSpace, EncodeOptions, Image}; + +const WIDTH: u32 = 24; +const HEIGHT: u32 = 20; +const SRGB: u32 = 16; +const GREYSCALE: u32 = 17; + +/// Interleaved 8-bit samples for `components` components. +fn pixels(components: u16) -> Vec { + (0..WIDTH * HEIGHT * u32::from(components)) + .map(|i| u8::try_from((i * 37 + i / 7) % 256).expect("byte")) + .collect() +} + +fn codestream(components: u16) -> Vec { + encode( + &pixels(components), + WIDTH, + HEIGHT, + components, + 8, + false, + &EncodeOptions { + reversible: true, + num_decomposition_levels: 2, + ..EncodeOptions::default() + }, + ) + .expect("encode fixture") +} + +fn jp2_box(kind: [u8; 4], payload: &[u8]) -> Vec { + let len = u32::try_from(payload.len() + 8).expect("box length"); + let mut out = len.to_be_bytes().to_vec(); + out.extend_from_slice(&kind); + out.extend_from_slice(payload); + out +} + +/// A JP2 with an enumerated `colr` box and no `cdef` box, whatever the +/// codestream's component count. +fn jp2_without_cdef(components: u16, enumerated: u32) -> Vec { + let mut ihdr = Vec::new(); + ihdr.extend_from_slice(&HEIGHT.to_be_bytes()); + ihdr.extend_from_slice(&WIDTH.to_be_bytes()); + ihdr.extend_from_slice(&components.to_be_bytes()); + ihdr.extend_from_slice(&[7, 7, 0, 0]); + let mut colr = vec![1, 0, 0]; + colr.extend_from_slice(&enumerated.to_be_bytes()); + let mut header = jp2_box(*b"ihdr", &ihdr); + header.extend(jp2_box(*b"colr", &colr)); + + let mut out = jp2_box(*b"jP ", &[0x0D, 0x0A, 0x87, 0x0A]); + out.extend(jp2_box(*b"ftyp", b"jp2 \0\0\0\0jp2 ")); + out.extend(jp2_box(*b"jp2h", &header)); + out.extend(jp2_box(*b"jp2c", &codestream(components))); + out +} + +/// The first `kept` of every `components` interleaved samples. +fn leading_channels(samples: &[u8], components: usize, kept: usize) -> Vec { + samples + .chunks_exact(components) + .flat_map(|pixel| pixel[..kept].iter().copied()) + .collect() +} + +fn with_opaque_alpha(rgb: &[u8]) -> Vec { + rgb.as_chunks::<3>() + .0 + .iter() + .flat_map(|pixel| [pixel[0], pixel[1], pixel[2], u8::MAX]) + .collect() +} + +fn region_into( + decoder: &mut J2kDecoder<'_>, + 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]; + decoder.decode_region_into(&mut J2kScratchPool::new(), &mut out, stride, fmt, roi)?; + Ok(out) +} + +fn whole_into(decoder: &mut J2kDecoder<'_>, fmt: PixelFormat) -> Vec { + let stride = WIDTH as usize * fmt.bytes_per_pixel(); + let mut out = vec![0_u8; stride * HEIGHT as usize]; + decoder + .decode_into(&mut out, stride, fmt) + .expect("whole-image decode"); + out +} + +const FULL: Rect = Rect { + x: 0, + y: 0, + w: WIDTH, + h: HEIGHT, +}; + +#[test] +fn srgb_with_an_extra_component_decodes_as_rgb() { + let bytes = jp2_without_cdef(4, SRGB); + let rgb = leading_channels(&pixels(4), 4, 3); + let mut decoder = J2kDecoder::new(&bytes).expect("decoder"); + + assert_eq!(whole_into(&mut decoder, PixelFormat::Rgb8), rgb); + assert_eq!( + region_into(&mut decoder, PixelFormat::Rgb8, FULL).expect("Rgb8 region"), + rgb + ); + // The extra component is not alpha: RGBA output is opaque. + assert_eq!( + whole_into(&mut decoder, PixelFormat::Rgba8), + with_opaque_alpha(&rgb) + ); + assert_eq!( + region_into(&mut decoder, PixelFormat::Rgba8, FULL).expect("Rgba8 region"), + with_opaque_alpha(&rgb) + ); + + let roi = Rect { + x: 5, + y: 3, + w: 11, + h: 9, + }; + let expected: Vec = (roi.y..roi.y + roi.h) + .flat_map(|y| { + let start = (y * WIDTH + roi.x) as usize * 3; + rgb[start..start + roi.w as usize * 3].to_vec() + }) + .collect(); + assert_eq!( + region_into(&mut decoder, PixelFormat::Rgb8, roi).expect("Rgb8 sub-region"), + expected + ); +} + +#[test] +fn srgb_with_an_extra_component_decodes_scaled_and_sixteen_bit_regions() { + let bytes = jp2_without_cdef(4, SRGB); + let rgb = leading_channels(&pixels(4), 4, 3); + let mut decoder = J2kDecoder::new(&bytes).expect("decoder"); + + for levels in 1..=2_u8 { + let (w, h) = (WIDTH.div_ceil(1 << levels), HEIGHT.div_ceil(1 << levels)); + let stride = w as usize * 3; + let mut out = vec![0_u8; stride * h as usize]; + let outcome = decoder + .decode_region_scaled_pow2_into( + &mut J2kScratchPool::new(), + &mut out, + stride, + PixelFormat::Rgb8, + FULL, + levels, + ) + .expect("scaled Rgb8 region"); + assert_eq!((outcome.decoded.w, outcome.decoded.h), (w, h)); + } + + let stride = WIDTH as usize * PixelFormat::Rgb16.bytes_per_pixel(); + let mut out = vec![0_u8; stride * HEIGHT as usize]; + decoder + .decode_into(&mut out, stride, PixelFormat::Rgb16) + .expect("Rgb16 decode"); + let samples: Vec = out + .as_chunks::<2>() + .0 + .iter() + .map(|sample| u16::from_ne_bytes(*sample)) + .collect(); + let widened: Vec = rgb.iter().map(|&sample| u16::from(sample) * 257).collect(); + assert_eq!(samples, widened); +} + +#[test] +fn greyscale_with_an_extra_component_decodes_as_gray() { + let bytes = jp2_without_cdef(2, GREYSCALE); + let gray = leading_channels(&pixels(2), 2, 1); + let mut decoder = J2kDecoder::new(&bytes).expect("decoder"); + + assert_eq!(whole_into(&mut decoder, PixelFormat::Gray8), gray); + assert_eq!( + region_into(&mut decoder, PixelFormat::Gray8, FULL).expect("Gray8 region"), + gray + ); +} + +#[test] +fn native_output_omits_the_extra_components() { + for (components, enumerated, kept) in [(4, SRGB, 3), (5, SRGB, 3), (2, GREYSCALE, 1)] { + let bytes = jp2_without_cdef(components, enumerated); + let image = Image::new(&bytes, &j2k_native::DecodeSettings::default()).expect("image"); + assert_eq!( + image.ignored_components(), + usize::from(components) - kept, + "{components} components" + ); + assert!(!image.has_alpha()); + let expected_space = if kept == 1 { + ColorSpace::Gray + } else { + ColorSpace::RGB + }; + assert_eq!( + core::mem::discriminant(image.color_space()), + core::mem::discriminant(&expected_space) + ); + assert_eq!( + image + .decode_native_components() + .expect("planes") + .planes() + .len(), + kept + ); + assert_eq!( + usize::from(image.decode_native().expect("raw bitmap").num_components), + kept + ); + // Streamed native planes cover up to four components. + let streamed = image.decode_native_region_components_rows_with_context( + (0, 0, WIDTH, HEIGHT), + &mut j2k_native::DecoderContext::default(), + ); + if components <= 4 { + assert_eq!(streamed.expect("streamed planes").planes().len(), kept); + } + } +} + +#[test] +fn lenient_decoding_still_infers_alpha_for_one_extra_component() { + let bytes = jp2_without_cdef(4, SRGB); + let mut decoder = + J2kDecoder::new_with_settings(&bytes, DecodeSettings::lenient()).expect("decoder"); + assert_eq!(whole_into(&mut decoder, PixelFormat::Rgba8), pixels(4)); +} + +#[test] +fn raw_codestreams_keep_their_component_count_interpretation() { + let bytes = codestream(4); + let image = Image::new(&bytes, &j2k_native::DecodeSettings::default()).expect("image"); + assert_eq!(image.ignored_components(), 0); + assert!(matches!(image.color_space(), ColorSpace::CMYK)); +} From 530d7c0e26ee387480029ce07d856ebea85fb093 Mon Sep 17 00:00:00 2001 From: GF Date: Sun, 8 Nov 2026 17:06:29 -0500 Subject: [PATCH 2/2] fix(j2k): complete extra-component row decoding and recode validation Choose bounded u8/u16 row layouts from resolved color and alpha metadata. Compare all codestream components during recode validation and cover row output and preservation with regression tests. Update the API inventories and review records for the original JP2 color-space change. --- CHANGELOG.md | 4 +- crates/j2k/src/recode/validation.rs | 8 +- crates/j2k/src/view/rows.rs | 34 ++++--- .../j2k/tests/unspecified_extra_components.rs | 89 ++++++++++++++++++- ...able-api-1.0.implementation-public-api.txt | 1 + xtask/api/stable-api-1.0.public-api.txt | 1 + .../public-api/public-api-review-0.13.2.yml | 10 +-- .../reviewed-public-api-diff-0.13.2.md | 5 +- 8 files changed, 127 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5557e62ae..34fd62cea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,9 @@ Staged workspace version: `0.13.2`. decoding still infers alpha for one extra component, raw codestreams keep their component-count interpretation, and recode re-encodes every component. HTJ2K batch jobs whose direct color plan is unsupported fall - back to the generic decode instead of failing. + back to the generic decode instead of failing. Bounded 8-bit and 16-bit + rows use the resolved color channels and alpha state, and recode validation + compares every component even when display output omits unspecified ones. - `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 diff --git a/crates/j2k/src/recode/validation.rs b/crates/j2k/src/recode/validation.rs index 25f16afd0..0267d4724 100644 --- a/crates/j2k/src/recode/validation.rs +++ b/crates/j2k/src/recode/validation.rs @@ -16,16 +16,20 @@ pub(super) fn roundtrip( context: &'static str, ) -> Result<(), J2kError> { let settings = DecodeSettings::default(); - let source_image = Image::new_with_retained_baseline(source, &settings, encoded.capacity()) + let mut source_image = Image::new_with_retained_baseline(source, &settings, encoded.capacity()) .map_err(|err| map_native_decode_error(err, "source JPEG 2000 validation parse failed"))?; let source_metadata_bytes = source_image.retained_allocation_bytes().map_err(|err| { map_native_decode_error(err, "source JPEG 2000 validation metadata failed") })?; let encoded_parse_baseline = encoded.capacity().saturating_add(source_metadata_bytes); - let encoded_image = + let mut encoded_image = Image::new_with_retained_baseline(encoded, &settings, encoded_parse_baseline) .map_err(|err| map_native_decode_error(err, "HTJ2K validation parse failed"))?; + // Recode preserves every component, including channels omitted from + // display output. Compare those samples for both raw and wrapped output. + source_image.keep_ignored_components(); + encoded_image.keep_ignored_components(); let equal = source_image .decoded_samples_equal_with_retained_bytes(&encoded_image, encoded) .map_err(|err| map_native_decode_error(err, "JPEG 2000 paired validation decode failed"))?; diff --git a/crates/j2k/src/view/rows.rs b/crates/j2k/src/view/rows.rs index cbd965936..91309c395 100644 --- a/crates/j2k/src/view/rows.rs +++ b/crates/j2k/src/view/rows.rs @@ -104,7 +104,11 @@ impl J2kDecoder<'_> { sink: &mut R, options: J2kRowDecodeOptions, ) -> Result> { - let fmt = row_format_u8(self.info()).map_err(DecodeRowsError::Decode)?; + self.ensure_image().map_err(DecodeRowsError::Decode)?; + let fmt = self + .cached_image() + .and_then(row_format_u8) + .map_err(DecodeRowsError::Decode)?; let row_bytes = row_bytes_for(self.info(), fmt).map_err(DecodeRowsError::Decode)?; let width = self.info.dimensions.0; let height = self.info.dimensions.1; @@ -115,7 +119,6 @@ impl J2kDecoder<'_> { ); let (stripe_rows, max_stripe_len) = bounded_row_stripe_layout(row_bytes, height, options) .map_err(DecodeRowsError::Decode)?; - self.ensure_image().map_err(DecodeRowsError::Decode)?; let mut pool = J2kScratchPool::new(); if self.rows_stream_whole() { let stripe = pool @@ -210,7 +213,11 @@ impl J2kDecoder<'_> { sink: &mut R, options: J2kRowDecodeOptions, ) -> Result> { - let fmt = row_format_u16(self.info()).map_err(DecodeRowsError::Decode)?; + self.ensure_image().map_err(DecodeRowsError::Decode)?; + let fmt = self + .cached_image() + .and_then(row_format_u16) + .map_err(DecodeRowsError::Decode)?; let row_bytes = row_bytes_for(self.info(), fmt).map_err(DecodeRowsError::Decode)?; let samples_per_row = row_samples_for(self.info(), fmt).map_err(DecodeRowsError::Decode)?; let width = self.info.dimensions.0; @@ -245,7 +252,6 @@ impl J2kDecoder<'_> { if self.info.bit_depth > 24 { return self.decode_rows_u16_exact_reparsed(sink, fmt, layout); } - self.ensure_image().map_err(DecodeRowsError::Decode)?; if self.rows_stream_whole() { let mut pool = J2kScratchPool::new(); let (packed, row) = pool @@ -380,11 +386,11 @@ fn widen_row_u16(packed: &[u8], row: &mut [u16]) { } } -fn row_format_u8(info: &j2k_core::Info) -> Result { - match info.components { - 1 => Ok(PixelFormat::Gray8), - 3 => Ok(PixelFormat::Rgb8), - 4 => Ok(PixelFormat::Rgba8), +fn row_format_u8(image: &j2k_native::Image<'_>) -> Result { + match (image.color_space().num_channels(), image.has_alpha()) { + (1, false) => Ok(PixelFormat::Gray8), + (3, false) => Ok(PixelFormat::Rgb8), + (3, true) => Ok(PixelFormat::Rgba8), _ => Err(j2k_core::Unsupported { what: "row decode only supports Gray/RGB/RGBA images", } @@ -392,11 +398,11 @@ fn row_format_u8(info: &j2k_core::Info) -> Result { } } -fn row_format_u16(info: &j2k_core::Info) -> Result { - match info.components { - 1 => Ok(PixelFormat::Gray16), - 3 => Ok(PixelFormat::Rgb16), - 4 => Ok(PixelFormat::Rgba16), +fn row_format_u16(image: &j2k_native::Image<'_>) -> Result { + match (image.color_space().num_channels(), image.has_alpha()) { + (1, false) => Ok(PixelFormat::Gray16), + (3, false) => Ok(PixelFormat::Rgb16), + (3, true) => Ok(PixelFormat::Rgba16), _ => Err(j2k_core::Unsupported { what: "row decode only supports Gray/RGB/RGBA images", } diff --git a/crates/j2k/tests/unspecified_extra_components.rs b/crates/j2k/tests/unspecified_extra_components.rs index c4dacd333..36815005b 100644 --- a/crates/j2k/tests/unspecified_extra_components.rs +++ b/crates/j2k/tests/unspecified_extra_components.rs @@ -6,7 +6,10 @@ //! extras are left out of decoded output. Kakadu wrote such files from RGBA //! TIFFs: an `sRGB` `colr` box over four components and no `cdef` box. -use j2k::{DecodeSettings, J2kDecoder, J2kScratchPool, PixelFormat, Rect}; +use j2k::{ + recode_j2k_to_htj2k_lossless, DecodeSettings, J2kDecoder, J2kRowDecodeOptions, J2kScratchPool, + J2kToHtj2kMode, J2kToHtj2kOptions, PixelFormat, Rect, RowSink, +}; use j2k_native::{encode, ColorSpace, EncodeOptions, Image}; const WIDTH: u32 = 24; @@ -257,3 +260,87 @@ fn raw_codestreams_keep_their_component_count_interpretation() { assert_eq!(image.ignored_components(), 0); assert!(matches!(image.color_space(), ColorSpace::CMYK)); } + +struct Rows { + samples: Vec, + row_len: usize, + next_y: u32, +} + +impl Rows { + fn new(channels: usize) -> Self { + Self { + samples: Vec::new(), + row_len: WIDTH as usize * channels, + next_y: 0, + } + } +} + +impl RowSink for Rows { + type Error = core::convert::Infallible; + + fn write_row(&mut self, y: u32, row: &[T]) -> Result<(), Self::Error> { + assert_eq!(y, self.next_y); + assert_eq!(row.len(), self.row_len); + self.samples.extend_from_slice(row); + self.next_y += 1; + Ok(()) + } +} + +#[test] +fn bounded_u8_rows_omit_unspecified_components() { + for (components, enumerated, kept) in [(2, GREYSCALE, 1), (5, SRGB, 3), (4, SRGB, 3)] { + let bytes = jp2_without_cdef(components, enumerated); + let expected = leading_channels(&pixels(components), usize::from(components), kept); + for stripe_rows in [1, 7, HEIGHT] { + let mut decoder = J2kDecoder::new(&bytes).expect("decoder"); + let mut rows = Rows::::new(kept); + decoder + .decode_rows_u8_bounded(&mut rows, J2kRowDecodeOptions::new(stripe_rows)) + .expect("bounded u8 rows"); + assert_eq!(rows.next_y, HEIGHT); + assert_eq!(rows.samples, expected, "{components} components"); + assert_eq!(decoder.info().components, components); + } + } +} + +#[test] +fn bounded_u16_rows_omit_unspecified_components() { + for (components, enumerated, kept) in [(2, GREYSCALE, 1), (5, SRGB, 3), (4, SRGB, 3)] { + let bytes = jp2_without_cdef(components, enumerated); + let expected: Vec = + leading_channels(&pixels(components), usize::from(components), kept) + .into_iter() + .map(|sample| u16::from(sample) * 257) + .collect(); + for stripe_rows in [1, 7, HEIGHT] { + let mut decoder = J2kDecoder::new(&bytes).expect("decoder"); + let mut rows = Rows::::new(kept); + decoder + .decode_rows_u16_bounded(&mut rows, J2kRowDecodeOptions::new(stripe_rows)) + .expect("bounded u16 rows"); + assert_eq!(rows.next_y, HEIGHT); + assert_eq!(rows.samples, expected, "{components} components"); + assert_eq!(decoder.info().components, components); + } + } +} + +#[test] +fn recode_preserves_unspecified_component_samples() { + let bytes = jp2_without_cdef(4, SRGB); + let recoded = recode_j2k_to_htj2k_lossless(&bytes, J2kToHtj2kOptions::default()) + .expect("recode all four components"); + assert_eq!(recoded.report.mode, J2kToHtj2kMode::PixelPreserving); + assert_eq!(recoded.report.components, 4); + // Decode the raw output codestream so display-channel filtering cannot + // hide the fourth component from this comparison. + let image = Image::new(&recoded.bytes, &j2k_native::DecodeSettings::default()) + .expect("recoded codestream"); + let bitmap = image.decode_native().expect("all recoded samples"); + assert_eq!(bitmap.num_components, 4); + assert_eq!(bitmap.data, pixels(4)); +} diff --git a/xtask/api/stable-api-1.0.implementation-public-api.txt b/xtask/api/stable-api-1.0.implementation-public-api.txt index c32ff36b2..f720c06c2 100644 --- a/xtask/api/stable-api-1.0.implementation-public-api.txt +++ b/xtask/api/stable-api-1.0.implementation-public-api.txt @@ -2511,6 +2511,7 @@ pub fn j2k_native::Image<'a>::decode_region_rows_for_integer_output_with_context pub fn j2k_native::Image<'a>::decode_reversible_53_coefficients(&self) -> j2k_native::Result pub fn j2k_native::Image<'a>::decode_reversible_53_coefficients_with_context(&self, &mut j2k_native::DecoderContext<'a>) -> j2k_native::Result pub fn j2k_native::Image<'a>::full_decode_planes_floor(&self, usize) -> j2k_native::Result> +pub fn j2k_native::Image<'a>::keep_ignored_components(&mut self) pub fn j2k_native::Image<'a>::new_with_reduction(&'a [u8], &j2k_native::DecodeSettings, u8) -> j2k_native::Result pub fn j2k_native::Image<'a>::new_with_retained_baseline(&'a [u8], &j2k_native::DecodeSettings, usize) -> j2k_native::Result pub fn j2k_native::Image<'a>::output_tile_rects(&self) -> j2k_native::Result> diff --git a/xtask/api/stable-api-1.0.public-api.txt b/xtask/api/stable-api-1.0.public-api.txt index 9f3b45b94..ca72d40cc 100644 --- a/xtask/api/stable-api-1.0.public-api.txt +++ b/xtask/api/stable-api-1.0.public-api.txt @@ -3966,6 +3966,7 @@ pub fn j2k_native::Image<'a>::decode_wavelets_with_context(&self, &mut j2k_nativ pub fn j2k_native::Image<'a>::decode_with_context(&self, &mut j2k_native::DecoderContext<'a>) -> j2k_native::Result pub fn j2k_native::Image<'a>::has_alpha(&self) -> bool pub fn j2k_native::Image<'a>::height(&self) -> u32 +pub fn j2k_native::Image<'a>::ignored_components(&self) -> usize pub fn j2k_native::Image<'a>::new(&'a [u8], &j2k_native::DecodeSettings) -> j2k_native::Result pub fn j2k_native::Image<'a>::original_bit_depth(&self) -> u8 pub fn j2k_native::Image<'a>::width(&self) -> u32 diff --git a/xtask/release-evidence/public-api/public-api-review-0.13.2.yml b/xtask/release-evidence/public-api/public-api-review-0.13.2.yml index f256ed652..c5d76a796 100644 --- a/xtask/release-evidence/public-api/public-api-review-0.13.2.yml +++ b/xtask/release-evidence/public-api/public-api-review-0.13.2.yml @@ -83,11 +83,11 @@ reviews: hidden_rationale: "Compared the complete hidden API inventory against v0.13.1; it is unchanged." j2k-native: removed_fingerprint: "none" - added_fingerprint: "fnv1a64:22b943651a07318e" - hidden_count: 595 - hidden_fingerprint: "fnv1a64:0d8774f02739bc1e" - rationale: "Compared the complete ordinary API inventory against v0.13.1. Adds DecodeErrorClass::ResourceLimit { requested, cap } to an already non-exhaustive enum; no existing item changes and existing variant discriminants are preserved. AllocationTooLarge and ImageTooLarge now classify as ResourceLimit instead of the generic Backend class. This is a compatible refinement of the extensible error taxonomy: invalid-input and allocator-failure classes stay unchanged. Callers that want to handle memory limits separately can match the new variant; existing wildcard matches remain valid." - hidden_rationale: "Adds referenced_classic_plan_execution_bytes and referenced_htj2k_plan_execution_bytes for sibling CPU batch adapters to check planned live execution memory before allocation. For the facade's streamed routes, j2k-native also adds decode_native_region_components_rows_with_context, full_decode_planes_floor, output_tile_rects, region_decode_streams_rows and streamed_rows_native_precision; cpu_decode_pool_threads reports the pool thread count for facade routing, DecoderContext::set_shared_pool marks CpuBatchDecoder's shared-pool images, and pack_rgb8 shares the existing pixel conversion. ComponentRowSink::rgb8_region is an optional default method, so existing sink implementations remain valid. These are unsupported implementation helpers; existing signatures are unchanged." + added_fingerprint: "fnv1a64:4c2d12d5bbd5aad8" + hidden_count: 596 + hidden_fingerprint: "fnv1a64:d63da58635d2b8d9" + rationale: "Compared the complete ordinary API inventory against v0.13.1. Adds DecodeErrorClass::ResourceLimit { requested, cap } to an already non-exhaustive enum; no existing item changes and existing variant discriminants are preserved. AllocationTooLarge and ImageTooLarge now classify as ResourceLimit instead of the generic Backend class. This is a compatible refinement of the extensible error taxonomy: invalid-input and allocator-failure classes stay unchanged. Callers that want to handle memory limits separately can match the new variant; existing wildcard matches remain valid. Adds Image::ignored_components to report trailing unspecified JP2 components omitted from decoded output. A declared color space without a channel definition box now keeps its color interpretation when extra components are present. Existing signatures are unchanged; lenient alpha inference and raw-codestream interpretation remain unchanged. No caller migration is required." + hidden_rationale: "Adds referenced_classic_plan_execution_bytes and referenced_htj2k_plan_execution_bytes for sibling CPU batch adapters to check planned live execution memory before allocation. For the facade's streamed routes, j2k-native also adds decode_native_region_components_rows_with_context, full_decode_planes_floor, output_tile_rects, region_decode_streams_rows and streamed_rows_native_precision; cpu_decode_pool_threads reports the pool thread count for facade routing, DecoderContext::set_shared_pool marks CpuBatchDecoder's shared-pool images, and pack_rgb8 shares the existing pixel conversion. ComponentRowSink::rgb8_region is an optional default method, so existing sink implementations remain valid. These are unsupported implementation helpers; existing signatures are unchanged. Adds Image::keep_ignored_components for recoding and its round-trip validation to retain and compare all codestream components, including unspecified extras. This is an unsupported implementation helper; no existing hidden signature changes." j2k-jpeg: removed_fingerprint: "none" added_fingerprint: "none" diff --git a/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.2.md b/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.2.md index 4815642d1..3ad2474b9 100644 --- a/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.2.md +++ b/xtask/release-evidence/public-api/reviewed-public-api-diff-0.13.2.md @@ -22,7 +22,7 @@ This report is generated by `cargo xtask semver --write-report`. Normal `cargo x | `j2k-cuda-transcode-engine` | `0.13.1` | `0.13.2` | `minor` | 0 | 0 | `none` | `none` | 84 | `fnv1a64:b2fcaa9b0171aba7` | | `j2k-metal-support` | `0.13.1` | `0.13.2` | `minor` | 0 | 0 | `none` | `none` | 11 | `fnv1a64:02139606f1202938` | | `j2k-mpsgraph-support` | `0.13.1` | `0.13.2` | `minor` | 0 | 0 | `none` | `none` | 0 | `none` | -| `j2k-native` | `0.13.1` | `0.13.2` | `minor` | 3 | 0 | `none` | `fnv1a64:22b943651a07318e` | 595 | `fnv1a64:0d8774f02739bc1e` | +| `j2k-native` | `0.13.1` | `0.13.2` | `minor` | 4 | 0 | `none` | `fnv1a64:4c2d12d5bbd5aad8` | 596 | `fnv1a64:d63da58635d2b8d9` | | `j2k-jpeg` | `0.13.1` | `0.13.2` | `minor` | 0 | 0 | `none` | `none` | 701 | `fnv1a64:bd0cf738a0f96592` | | `j2k-tilecodec` | `0.13.1` | `0.13.2` | `minor` | 0 | 0 | `none` | `none` | 32 | `fnv1a64:eb677842559d9b0e` | | `j2k` | `0.13.1` | `0.13.2` | `minor` | 6 | 0 | `none` | `fnv1a64:78b9622111db3437` | 109 | `fnv1a64:0427dd567531c8dd` | @@ -174,7 +174,7 @@ None. ### `j2k-native` -Baseline items: 422. Candidate items: 425. Computed release type: `minor`. Rustdoc-hidden candidate items: 595. Full hidden-inventory fingerprint: `fnv1a64:0d8774f02739bc1e`. +Baseline items: 422. Candidate items: 426. Computed release type: `minor`. Rustdoc-hidden candidate items: 596. Full hidden-inventory fingerprint: `fnv1a64:d63da58635d2b8d9`. #### Removed or changed baseline API items @@ -183,6 +183,7 @@ None. #### Added candidate API items ```text +pub fn j2k_native::Image<'a>::ignored_components(&self) -> usize pub j2k_native::DecodeErrorClass::ResourceLimit pub j2k_native::DecodeErrorClass::ResourceLimit::cap: core::option::Option pub j2k_native::DecodeErrorClass::ResourceLimit::requested: core::option::Option