diff --git a/CHANGELOG.md b/CHANGELOG.md index 9153adcd..cbf71747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,13 @@ and stale roadmap entries have been removed from the public documentation set. 18000 x 12000 RGB image decodes 3 to 11% faster at 12 threads and 9 to 18% faster at 4, with about 3 MiB less peak heap. Output is unchanged, and a failure reruns whole-window Tier-1 serially for the same error. +- Decodes into a caller buffer whose stride is wider than a row and which + ends right after the last row's pixels, the length `strided_output_len` + reports and buffer validation accepts, now write that last row. Full-image + and scaled 8-bit decodes at such a stride and every 16-bit decode at one, + regions included, left it unwritten and still returned `Ok`. 8-bit region + decodes, and buffers with any bytes after the last row, were not affected. + The other rows are unchanged. ## [0.13.1] - 2026-10-29 diff --git a/crates/j2k/src/decode/output/u16.rs b/crates/j2k/src/decode/output/u16.rs index 9fd5ff7d..f76306fe 100644 --- a/crates/j2k/src/decode/output/u16.rs +++ b/crates/j2k/src/decode/output/u16.rs @@ -6,7 +6,7 @@ use crate::backend::{ColorSpace, DecodedComponents as NativeDecodedComponents, R use crate::J2kError; use j2k_core::{PixelFormat, Unsupported}; -use super::u8::{component_sample_count, validate_component_planes}; +use super::u8::{component_sample_count, output_rows, validate_component_planes}; pub(in crate::decode) fn write_components_u16_output( components: &NativeDecodedComponents<'_>, @@ -281,12 +281,8 @@ fn write_u16_channel_rows(job: U16ChannelRows<'_, '_>) { let dst_row_bytes = width * dst_channels * 2; let alpha = opaque_alpha_u16(bytes_per_sample, bit_depth); - for (src_row, dst_row) in src - .chunks_exact(src_row_bytes) - .zip(out.chunks_exact_mut(stride)) - .take(height) - { - let dst_row = &mut dst_row[..dst_row_bytes]; + let dst_rows = output_rows(out, stride, dst_row_bytes, height); + for (src_row, dst_row) in src.chunks_exact(src_row_bytes).zip(dst_rows) { for x in 0..width { let src_pixel = &src_row[x * source_channels * bytes_per_sample..]; let dst_pixel = &mut dst_row[x * dst_channels * 2..(x + 1) * dst_channels * 2]; @@ -347,12 +343,8 @@ fn convert_or_copy_u16( let (width, height) = dims; let dst_row_bytes = width * channels * 2; let src_row_bytes = width * channels * usize::from(bytes_per_sample); - for (src_row, dst_row) in src - .chunks_exact(src_row_bytes) - .zip(out.chunks_exact_mut(stride)) - .take(height) - { - let dst_row = &mut dst_row[..dst_row_bytes]; + let dst_rows = output_rows(out, stride, dst_row_bytes, height); + for (src_row, dst_row) in src.chunks_exact(src_row_bytes).zip(dst_rows) { if bytes_per_sample == 2 { dst_row.copy_from_slice(src_row); continue; diff --git a/crates/j2k/src/decode/output/u8.rs b/crates/j2k/src/decode/output/u8.rs index 060291b1..0797d3aa 100644 --- a/crates/j2k/src/decode/output/u8.rs +++ b/crates/j2k/src/decode/output/u8.rs @@ -173,6 +173,21 @@ pub(super) fn validate_component_planes( Ok(()) } +/// The first `row_bytes` of each of the first `height` rows of `out` at +/// `stride`. A validated buffer may end right after the last row's pixels +/// (see `j2k_core::strided_output_len`), so the last row may be shorter +/// than `stride`. +pub(super) fn output_rows( + out: &mut [u8], + stride: usize, + row_bytes: usize, + height: usize, +) -> impl Iterator { + out.chunks_mut(stride) + .take(height) + .map(move |row| &mut row[..row_bytes]) +} + fn write_component_rows_u8( plane: &j2k_native::ComponentPlane<'_>, out: &mut [u8], @@ -303,26 +318,22 @@ pub(in crate::decode) fn write_u8_output( fn copy_rows_exact(src: &[u8], out: &mut [u8], stride: usize, row_bytes: usize, height: usize) { for (src_row, dst_row) in src .chunks_exact(row_bytes) - .zip(out.chunks_exact_mut(stride)) - .take(height) + .zip(output_rows(out, stride, row_bytes, height)) { - dst_row[..row_bytes].copy_from_slice(src_row); + dst_row.copy_from_slice(src_row); } } fn add_opaque_alpha_u8(src: &[u8], out: &mut [u8], stride: usize, width: usize, height: usize) { let src_row_bytes = width * 3; let dst_row_bytes = width * 4; - for (src_row, dst_row) in src - .chunks_exact(src_row_bytes) - .zip(out.chunks_exact_mut(stride)) - .take(height) - { + let dst_rows = output_rows(out, stride, dst_row_bytes, height); + for (src_row, dst_row) in src.chunks_exact(src_row_bytes).zip(dst_rows) { for (rgb, rgba) in src_row .as_chunks::<3>() .0 .iter() - .zip(dst_row[..dst_row_bytes].as_chunks_mut::<4>().0) + .zip(dst_row.as_chunks_mut::<4>().0) { rgba[..3].copy_from_slice(rgb); rgba[3] = u8::MAX; @@ -355,16 +366,13 @@ pub(in crate::decode) fn expand_rgb8_to_rgba8(out: &mut [u8]) { fn drop_alpha_u8(src: &[u8], out: &mut [u8], stride: usize, width: usize, height: usize) { let src_row_bytes = width * 4; let dst_row_bytes = width * 3; - for (src_row, dst_row) in src - .chunks_exact(src_row_bytes) - .zip(out.chunks_exact_mut(stride)) - .take(height) - { + let dst_rows = output_rows(out, stride, dst_row_bytes, height); + for (src_row, dst_row) in src.chunks_exact(src_row_bytes).zip(dst_rows) { for (rgba, rgb) in src_row .as_chunks::<4>() .0 .iter() - .zip(dst_row[..dst_row_bytes].as_chunks_mut::<3>().0) + .zip(dst_row.as_chunks_mut::<3>().0) { rgb.copy_from_slice(&rgba[..3]); } diff --git a/crates/j2k/tests/decode.rs b/crates/j2k/tests/decode.rs index c8d315a7..eeaac395 100644 --- a/crates/j2k/tests/decode.rs +++ b/crates/j2k/tests/decode.rs @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use j2k::{ - encode_j2k_lossless, encode_j2k_lossless_components, DecodeSettings as FacadeDecodeSettings, - EncodeBackendPreference, J2kBlockCodingMode, J2kCodec, J2kComponentPlane, J2kContext, - J2kDecodeWarning, J2kDecoder, J2kError, J2kLosslessComponentPlane, J2kLosslessComponentSamples, - J2kLosslessEncodeOptions, J2kLosslessSamples, J2kRowDecodeOptions, J2kView, - ReversibleTransform, + encode_j2k_lossless, encode_j2k_lossless_components, CpuDecodeParallelism, + DecodeSettings as FacadeDecodeSettings, EncodeBackendPreference, J2kBlockCodingMode, J2kCodec, + J2kComponentPlane, J2kContext, J2kDecodeWarning, J2kDecoder, J2kError, + J2kLosslessComponentPlane, J2kLosslessComponentSamples, J2kLosslessEncodeOptions, + J2kLosslessSamples, J2kRowDecodeOptions, J2kView, ReversibleTransform, }; use j2k_core::{ BufferError, CodecContext, Downscale, ImageDecodeRows, PixelFormat, Rect, RowSink, @@ -1003,6 +1003,132 @@ fn decode_rejects_too_small_stride() { )); } +/// The decode entry points [`decode_to_strided_buffer`] covers. +#[derive(Debug, Clone, Copy)] +enum StridedRoute { + Full, + Region, + Scaled, + RegionScaled, +} + +/// Decode `bytes` on `route` to `fmt` at a stride three bytes past the +/// output rows, into a buffer of exactly [`j2k_core::strided_output_len`] +/// bytes when `tight`, which has no padding after its last row, and of one +/// padded row per output row otherwise. Every byte starts as `0xa5`. +fn decode_to_strided_buffer( + bytes: &[u8], + parallelism: CpuDecodeParallelism, + route: StridedRoute, + fmt: PixelFormat, + tight: bool, +) -> Vec { + let mut decoder = J2kDecoder::new(bytes).expect("decoder"); + decoder.set_cpu_decode_parallelism(parallelism); + let dims = decoder.info().dimensions; + let roi = Rect { + x: 3, + y: 5, + w: dims.0 - 7, + h: dims.1 - 6, + }; + let scale = Downscale::Half; + let output = match route { + StridedRoute::Full => Rect::full(dims), + StridedRoute::Region => roi, + StridedRoute::Scaled => Rect::full((dims.0.div_ceil(2), dims.1.div_ceil(2))), + StridedRoute::RegionScaled => roi.scaled_covering(scale), + }; + let stride = output.w as usize * fmt.bytes_per_pixel() + 3; + let len = if tight { + j2k_core::strided_output_len((output.w, output.h), stride, fmt).expect("output length") + } else { + stride * output.h as usize + }; + let mut out = vec![0xa5; len]; + let pool = &mut j2k::J2kScratchPool::new(); + let outcome = match route { + StridedRoute::Full => decoder.decode_into(&mut out, stride, fmt), + StridedRoute::Region => decoder.decode_region_into(pool, &mut out, stride, fmt, roi), + StridedRoute::Scaled => decoder.decode_scaled_into(pool, &mut out, stride, fmt, scale), + StridedRoute::RegionScaled => { + decoder.decode_region_scaled_into(pool, &mut out, stride, fmt, roi, scale) + } + } + .unwrap_or_else(|error| panic!("{route:?} {fmt:?} decode: {error}")); + assert_eq!(outcome.decoded, output); + out +} + +#[test] +fn decode_writes_the_last_row_of_a_buffer_without_trailing_padding() { + let (width, height) = (45_u32, 31_u32); + let encode_image = |components: u16, bit_depth: u8, reversible: bool| { + let bytes_per_sample = u32::from(bit_depth).div_ceil(8); + let mut pixels: Vec = (0..width * height * u32::from(components) * bytes_per_sample) + .map(|i| (i * 37 + i / 11).to_le_bytes()[0]) + .collect(); + if bit_depth > 8 { + // Keep each little-endian sample within its bit depth. + for sample in pixels.as_chunks_mut::<2>().0 { + sample[1] &= (1 << (bit_depth - 8)) - 1; + } + } + let options = EncodeOptions { + reversible, + num_decomposition_levels: 3, + ..EncodeOptions::default() + }; + encode( + &pixels, width, height, components, bit_depth, false, &options, + ) + .expect("encode") + }; + let gray8 = encode_image(1, 8, true); + let gray12 = encode_image(1, 12, false); + let rgb8 = encode_image(3, 8, false); + let rgb12 = encode_image(3, 12, true); + let rgba8_jp2 = wrap_jp2_rgba_codestream(&encode_image(4, 8, true), width, height, 8); + let gray_formats = [PixelFormat::Gray8, PixelFormat::Gray16]; + let color_formats = [ + PixelFormat::Rgb8, + PixelFormat::Rgba8, + PixelFormat::Rgb16, + PixelFormat::Rgba16, + ]; + let cases: [(&str, &[u8], &[PixelFormat]); 5] = [ + ("gray8", &gray8, &gray_formats), + ("gray12", &gray12, &gray_formats), + ("rgb8", &rgb8, &color_formats), + ("rgb12", &rgb12, &color_formats), + ("rgba8", &rgba8_jp2, &color_formats), + ]; + let mut failures = Vec::new(); + for (name, bytes, formats) in cases { + for &fmt in formats { + for route in [ + StridedRoute::Full, + StridedRoute::Region, + StridedRoute::Scaled, + StridedRoute::RegionScaled, + ] { + for parallelism in [CpuDecodeParallelism::Auto, CpuDecodeParallelism::Serial] { + let padded = decode_to_strided_buffer(bytes, parallelism, route, fmt, false); + let tight = decode_to_strided_buffer(bytes, parallelism, route, fmt, true); + if tight != padded[..tight.len()] { + failures.push(format!("{name} {fmt:?} {route:?} {parallelism:?}")); + } + } + } + } + } + assert!( + failures.is_empty(), + "a buffer that ends after the last row's pixels must hold the padded decode's \ + bytes: {failures:#?}" + ); +} + #[test] fn decode_scaled_into_matches_backend_target_resolution_decode() { let pixels: Vec = (0_u8..48).collect();