From 7307e9678223a7eca9e94bc6bfbce04e304be3ba Mon Sep 17 00:00:00 2001 From: Alan Race Date: Tue, 2 Aug 2022 10:07:05 +0200 Subject: [PATCH 1/4] Removed `unwrap` and set colour transform with Jpeg compressed tiffs --- src/decoder/image.rs | 54 ++++++++++++++++++++++++++++--------------- src/decoder/stream.rs | 52 ----------------------------------------- src/error.rs | 27 ++++++++++++++++++++++ 3 files changed, 62 insertions(+), 71 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index c5c884f..afe6334 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -1,8 +1,5 @@ use super::ifd::{Directory, Value}; -use super::stream::{ - add_app14segment, ByteOrder, DeflateReader, JpegReader, JpegTagApp14Transform, LZWReader, - PackBitsReader, -}; +use super::stream::{ByteOrder, DeflateReader, JpegReader, LZWReader, PackBitsReader}; use super::tag_reader::TagReader; use super::{fp_predict_f32, fp_predict_f64, DecodingBuffer, Limits}; use super::{stream::SmartReader, ChunkType}; @@ -121,7 +118,7 @@ impl Image { let jpeg_tables = if compression_method == CompressionMethod::ModernJPEG && ifd.contains_key(&Tag::JPEGTables) { - let mut vec = tag_reader + let vec = tag_reader .find_tag(Tag::JPEGTables)? .unwrap() .into_u8_vec()?; @@ -131,11 +128,6 @@ impl Image { )); } - // TODO: Can we avoid this somehow? - if photometric_interpretation == PhotometricInterpretation::RGB { - add_app14segment(&mut vec, JpegTagApp14Transform::App14TransformUnknown) - } - Some(Arc::new(vec)) } else { None @@ -332,6 +324,7 @@ impl Image { fn create_reader<'r, R: 'r + Read>( reader: R, + photometric_interpretation: PhotometricInterpretation, compression_method: CompressionMethod, compressed_length: u64, jpeg_tables: Option>>, @@ -354,14 +347,32 @@ impl Image { let jpeg_reader = JpegReader::new(reader, compressed_length, jpeg_tables)?; let mut decoder = jpeg::Decoder::new(jpeg_reader); - let data = match decoder.decode() { - Ok(data) => data, - Err(e) => { - return Err(TiffError::FormatError(TiffFormatError::Format( - e.to_string(), - ))) + + match photometric_interpretation { + PhotometricInterpretation::RGB => { + decoder.set_color_transform(jpeg::ColorTransform::RGB) } - }; + PhotometricInterpretation::WhiteIsZero => { + decoder.set_color_transform(jpeg::ColorTransform::None) + } + PhotometricInterpretation::BlackIsZero => { + decoder.set_color_transform(jpeg::ColorTransform::None) + } + PhotometricInterpretation::RGBPalette => todo!(), + PhotometricInterpretation::TransparencyMask => { + decoder.set_color_transform(jpeg::ColorTransform::None) + } + PhotometricInterpretation::CMYK => { + decoder.set_color_transform(jpeg::ColorTransform::CMYK) + } + PhotometricInterpretation::YCbCr => { + decoder.set_color_transform(jpeg::ColorTransform::YCbCr) + } + PhotometricInterpretation::CIELab => todo!(), + _ => todo!(), + } + + let data = decoder.decode()?; Box::new(Cursor::new(data)) } @@ -509,8 +520,13 @@ impl Image { let padding_right = chunk_dims.0 - data_dims.0; let jpeg_tables = self.jpeg_tables.clone(); - let mut reader = - Self::create_reader(reader, compression_method, *compressed_bytes, jpeg_tables)?; + let mut reader = Self::create_reader( + reader, + photometric_interpretation, + compression_method, + *compressed_bytes, + jpeg_tables, + )?; if output_width == data_dims.0 as usize && padding_right == 0 { let total_samples = data_dims.0 as usize * data_dims.1 as usize * samples; diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index 160ea38..76f3b28 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -250,58 +250,6 @@ impl JpegReader { } } -#[repr(u8)] -pub(crate) enum JpegTagApp14Transform { - // App14TransformYCCK = 2, - // App14TransformYCbCr = 1, - App14TransformUnknown = 0, -} - -pub(crate) fn add_app14segment(jpeg_tables: &mut Vec, transform: JpegTagApp14Transform) { - // Add JPEG Tag APP14 Adobe segment to jpeg_tables. - // This segment stores image encoding information for DCT filters. - // When `transform` value is 0 which is defined as Unknown, jpeg-decoder interpret the - // color-space of image as RGB(3 channels) or CMYK(4). - let mut app14_offset = None; - let mut dht_offset = None; - for (offset, window) in jpeg_tables.windows(2).enumerate() { - if window == [0xff, 0xee] { - app14_offset = Some(offset); - break; - } - if window == [0xff, 0xc4] { - dht_offset = Some(offset); - } - } - match (app14_offset, dht_offset) { - (Some(i), _) => { - jpeg_tables[i + 16] = transform as u8; - } - (None, Some(sos_offset)) => { - let app14segment: [u8; 16] = [ - 0xff, - 0xee, - 0x00, - 0x0e, - 0x41, - 0x64, - 0x6f, - 0x62, - 0x65, - 0x00, - 0x64, - 0x00, - 0x00, - 0x00, - 0x00, - transform as u8, - ]; - jpeg_tables.splice(sos_offset..sos_offset, app14segment.iter().copied()); - } - (None, None) => {} - } -} - impl Read for JpegReader { // #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result { diff --git a/src/error.rs b/src/error.rs index 3a4aeae..0f979bb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,6 +4,8 @@ use std::io; use std::str; use std::string; +use jpeg::UnsupportedFeature; + use crate::decoder::{ifd::Value, ChunkType}; use crate::tags::{ CompressionMethod, PhotometricInterpretation, PlanarConfiguration, SampleFormat, Tag, @@ -146,6 +148,10 @@ pub enum TiffUnsupportedError { UnsupportedBitsPerChannel(u8), UnsupportedPlanarConfig(Option), UnsupportedDataType, + UnsupportedJpegFeature(UnsupportedFeature), + #[doc(hidden)] + /// Do not match against this variant. It may get removed. + __NonExhaustive, } impl fmt::Display for TiffUnsupportedError { @@ -191,6 +197,10 @@ impl fmt::Display for TiffUnsupportedError { write!(fmt, "Unsupported planar configuration “{:?}”.", config) } UnsupportedDataType => write!(fmt, "Unsupported data type."), + UnsupportedJpegFeature(ref unsupported_feature) => { + write!(fmt, "Unsupported JPEG feature {:?}", unsupported_feature) + } + __NonExhaustive => unreachable!(), } } } @@ -310,5 +320,22 @@ impl From for TiffError { } } +impl From for TiffError { + fn from(error: jpeg::Error) -> Self { + match error { + jpeg::Error::Format(format) => { + TiffError::FormatError(TiffFormatError::Format(format!("JPEG format: {}", format))) + } + jpeg::Error::Unsupported(unsupported) => TiffError::UnsupportedError( + TiffUnsupportedError::UnsupportedJpegFeature(unsupported), + ), + jpeg::Error::Io(io_error) => TiffError::IoError(io_error), + jpeg::Error::Internal(internal) => { + panic!("According to jpeg create, this is not used, so this shouldn't be reached..") + } + } + } +} + /// Result of an image decoding/encoding process pub type TiffResult = Result; From 4f1c88e35f580be1b193d466c2abe0680ea054b9 Mon Sep 17 00:00:00 2001 From: Alan Race Date: Tue, 22 Nov 2022 14:38:27 +0100 Subject: [PATCH 2/4] Added unsupported error for interpretations not supported by JPEG --- src/decoder/image.rs | 10 +++++++--- src/error.rs | 12 ++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index afe6334..6d82d7f 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -358,7 +358,6 @@ impl Image { PhotometricInterpretation::BlackIsZero => { decoder.set_color_transform(jpeg::ColorTransform::None) } - PhotometricInterpretation::RGBPalette => todo!(), PhotometricInterpretation::TransparencyMask => { decoder.set_color_transform(jpeg::ColorTransform::None) } @@ -368,8 +367,13 @@ impl Image { PhotometricInterpretation::YCbCr => { decoder.set_color_transform(jpeg::ColorTransform::YCbCr) } - PhotometricInterpretation::CIELab => todo!(), - _ => todo!(), + photometric_interpretation => { + return Err(TiffError::UnsupportedError( + TiffUnsupportedError::UnsupportedInterpretation( + photometric_interpretation, + ), + )); + } } let data = decoder.decode()?; diff --git a/src/error.rs b/src/error.rs index 0f979bb..d191295 100644 --- a/src/error.rs +++ b/src/error.rs @@ -148,10 +148,8 @@ pub enum TiffUnsupportedError { UnsupportedBitsPerChannel(u8), UnsupportedPlanarConfig(Option), UnsupportedDataType, + UnsupportedInterpretation(PhotometricInterpretation), UnsupportedJpegFeature(UnsupportedFeature), - #[doc(hidden)] - /// Do not match against this variant. It may get removed. - __NonExhaustive, } impl fmt::Display for TiffUnsupportedError { @@ -197,10 +195,16 @@ impl fmt::Display for TiffUnsupportedError { write!(fmt, "Unsupported planar configuration “{:?}”.", config) } UnsupportedDataType => write!(fmt, "Unsupported data type."), + UnsupportedInterpretation(interpretation) => { + write!( + fmt, + "Unsupported photometric interpretation \"{:?}\".", + interpretation + ) + } UnsupportedJpegFeature(ref unsupported_feature) => { write!(fmt, "Unsupported JPEG feature {:?}", unsupported_feature) } - __NonExhaustive => unreachable!(), } } } From b1cd459330b134bd8f13b73648f7b0fa3ebc3270 Mon Sep 17 00:00:00 2001 From: Alan Race Date: Tue, 22 Nov 2022 15:44:10 +0100 Subject: [PATCH 3/4] Updated error handling --- src/error.rs | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/src/error.rs b/src/error.rs index d191295..e302209 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ use std::error::Error; use std::fmt; +use std::fmt::Display; use std::io; use std::str; use std::string; @@ -71,6 +72,7 @@ pub enum TiffFormatError { RequiredTagEmpty(Tag), StripTileTagConflict, CycleInOffsets, + JpegDecoder(JpegDecoderError), } impl fmt::Display for TiffFormatError { @@ -121,6 +123,7 @@ impl fmt::Display for TiffFormatError { RequiredTagEmpty(ref val) => write!(fmt, "Required tag {:?} was empty.", val), StripTileTagConflict => write!(fmt, "File should contain either (StripByteCounts and StripOffsets) or (TileByteCounts and TileOffsets), other combination was found."), CycleInOffsets => write!(fmt, "File contained a cycle in the list of IFDs"), + JpegDecoder(error) => write!(fmt, "{}", error), } } } @@ -324,20 +327,32 @@ impl From for TiffError { } } +#[derive(Debug, Clone, PartialEq)] +struct JpegDecoderError { + inner: jpeg::Error, +} + +impl From for JpegDecoderError { + fn from(error: jpeg::Error) -> Self { + Self { inner: error } + } +} + +impl Display for JpegDecoderError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.inner.fmt(f) + } +} + +impl From for TiffError { + fn from(error: JpegDecoderError) -> Self { + TiffError::FormatError(TiffFormatError::JpegDecoder(error)) + } +} + impl From for TiffError { fn from(error: jpeg::Error) -> Self { - match error { - jpeg::Error::Format(format) => { - TiffError::FormatError(TiffFormatError::Format(format!("JPEG format: {}", format))) - } - jpeg::Error::Unsupported(unsupported) => TiffError::UnsupportedError( - TiffUnsupportedError::UnsupportedJpegFeature(unsupported), - ), - jpeg::Error::Io(io_error) => TiffError::IoError(io_error), - jpeg::Error::Internal(internal) => { - panic!("According to jpeg create, this is not used, so this shouldn't be reached..") - } - } + JpegDecoderError::from(error).into() } } From a733ff13d95ad9cf33669f0560b35e722570637f Mon Sep 17 00:00:00 2001 From: Alan Race Date: Tue, 22 Nov 2022 15:53:42 +0100 Subject: [PATCH 4/4] Changed to using private constructor rather than From<> JpegDecoderError --- src/error.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/error.rs b/src/error.rs index e302209..a401c6f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,6 +4,7 @@ use std::fmt::Display; use std::io; use std::str; use std::string; +use std::sync::Arc; use jpeg::UnsupportedFeature; @@ -123,7 +124,7 @@ impl fmt::Display for TiffFormatError { RequiredTagEmpty(ref val) => write!(fmt, "Required tag {:?} was empty.", val), StripTileTagConflict => write!(fmt, "File should contain either (StripByteCounts and StripOffsets) or (TileByteCounts and TileOffsets), other combination was found."), CycleInOffsets => write!(fmt, "File contained a cycle in the list of IFDs"), - JpegDecoder(error) => write!(fmt, "{}", error), + JpegDecoder(ref error) => write!(fmt, "{}", error), } } } @@ -327,14 +328,22 @@ impl From for TiffError { } } -#[derive(Debug, Clone, PartialEq)] -struct JpegDecoderError { - inner: jpeg::Error, +#[derive(Debug, Clone)] +pub struct JpegDecoderError { + inner: Arc, } -impl From for JpegDecoderError { - fn from(error: jpeg::Error) -> Self { - Self { inner: error } +impl JpegDecoderError { + fn new(error: jpeg::Error) -> Self { + Self { + inner: Arc::new(error), + } + } +} + +impl PartialEq for JpegDecoderError { + fn eq(&self, other: &Self) -> bool { + Arc::ptr_eq(&self.inner, &other.inner) } } @@ -352,7 +361,7 @@ impl From for TiffError { impl From for TiffError { fn from(error: jpeg::Error) -> Self { - JpegDecoderError::from(error).into() + JpegDecoderError::new(error).into() } }