From 9e6f9771d3e52810ac669587ca5b53ce16fa0601 Mon Sep 17 00:00:00 2001 From: Alan Race Date: Thu, 28 Apr 2022 08:54:32 +0200 Subject: [PATCH 1/4] Updated create_reader to only require &mut SmartReader and not &mut Self --- src/decoder/mod.rs | 49 ++++++++++++++++++++++++++---------------- tests/decode_images.rs | 4 +--- 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 3bcf1b5..e721eb1 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1031,7 +1031,17 @@ impl Decoder { self.goto_offset_u64(offset)?; let byte_order = self.reader.byte_order; - let reader = Self::create_reader(self, length)?; + let photometric_interpretation = self.image().photometric_interpretation; + let compression_method = self.image().compression_method; + let jpeg_tables = self.image().jpeg_tables.clone(); + + let reader = Self::create_reader( + &mut self.reader, + photometric_interpretation, + compression_method, + length, + jpeg_tables, + )?; // Read into output buffer. { @@ -1089,8 +1099,16 @@ impl Decoder { let line_samples = output_width * self.image().bits_per_sample.len(); let photometric_interpretation = self.image().photometric_interpretation; + let compression_method = self.image().compression_method; + let jpeg_tables = self.image().jpeg_tables.clone(); - let mut reader = Self::create_reader(self, compressed_length)?; + let mut reader = Self::create_reader( + &mut self.reader, + photometric_interpretation, + compression_method, + compressed_length, + jpeg_tables, + )?; for row in 0..(tile_length - padding_down) { let buf = match &mut buffer { @@ -1129,35 +1147,30 @@ impl Decoder { } fn create_reader<'r>( - decoder: &'r mut Self, + reader: &'r mut SmartReader, + photometric_interpretation: PhotometricInterpretation, + compression_method: CompressionMethod, compressed_length: u64, + jpeg_tables: Option>, ) -> TiffResult> { - let compression_method = decoder.image().compression_method; - Ok(match compression_method { - CompressionMethod::None => Box::new(&mut decoder.reader), - CompressionMethod::LZW => Box::new(LZWReader::new( - &mut decoder.reader, - usize::try_from(compressed_length)?, - )), - CompressionMethod::PackBits => { - Box::new(PackBitsReader::new(&mut decoder.reader, compressed_length)) + CompressionMethod::None => Box::new(reader), + CompressionMethod::LZW => { + Box::new(LZWReader::new(reader, usize::try_from(compressed_length)?)) } + CompressionMethod::PackBits => Box::new(PackBitsReader::new(reader, compressed_length)), CompressionMethod::Deflate | CompressionMethod::OldDeflate => { - Box::new(DeflateReader::new(&mut decoder.reader)) + Box::new(DeflateReader::new(reader)) } CompressionMethod::ModernJPEG => { - if decoder.image().jpeg_tables.is_some() && compressed_length < 2 { + if jpeg_tables.is_some() && compressed_length < 2 { return Err(TiffError::FormatError( TiffFormatError::InvalidTagValueType(Tag::JPEGTables), )); } - let jpeg_tables = decoder.image().jpeg_tables.clone(); - let photometric_interpretation = decoder.image().photometric_interpretation; - let jpeg_reader = JpegReader::new( - &mut decoder.reader, + reader, compressed_length, jpeg_tables, &photometric_interpretation, diff --git a/tests/decode_images.rs b/tests/decode_images.rs index 6e01fb9..d2a4419 100644 --- a/tests/decode_images.rs +++ b/tests/decode_images.rs @@ -390,10 +390,8 @@ fn oom() { let err = decoder.read_image().unwrap_err(); match err { - tiff::TiffError::LimitsExceeded => {} tiff::TiffError::UnsupportedError(tiff::TiffUnsupportedError::InterpretationWithBits( - _, - _, + .., )) => {} unexpected => panic!("Unexpected error {}", unexpected), } From 623a3d957d1ae1e16eaab16312f116dd06771e03 Mon Sep 17 00:00:00 2001 From: Alan Race Date: Sat, 30 Apr 2022 14:33:12 +0200 Subject: [PATCH 2/4] Avoid cloning jpeg_tables and instead pass Arc> --- src/decoder/image.rs | 14 ++++++-- src/decoder/mod.rs | 78 +++++-------------------------------------- src/decoder/stream.rs | 71 ++++++++++++++++++++++++++++++--------- 3 files changed, 75 insertions(+), 88 deletions(-) diff --git a/src/decoder/image.rs b/src/decoder/image.rs index a4a9189..a811b52 100644 --- a/src/decoder/image.rs +++ b/src/decoder/image.rs @@ -1,4 +1,5 @@ use super::ifd::{Directory, Value}; +use super::stream::{add_app14segment, JpegTagApp14Transform}; use super::tag_reader::TagReader; use super::Limits; use super::{stream::SmartReader, ChunkType}; @@ -6,6 +7,7 @@ use crate::tags::{CompressionMethod, PhotometricInterpretation, Predictor, Sampl use crate::{TiffError, TiffFormatError, TiffResult, TiffUnsupportedError}; use std::convert::{TryFrom, TryInto}; use std::io::{Read, Seek}; +use std::sync::Arc; #[derive(Debug)] pub(crate) struct StripDecodeState { @@ -87,7 +89,7 @@ pub(crate) struct Image { pub photometric_interpretation: PhotometricInterpretation, pub compression_method: CompressionMethod, pub predictor: Predictor, - pub jpeg_tables: Option>, + pub jpeg_tables: Option>>, pub chunk_type: ChunkType, pub strip_decoder: Option, pub tile_attributes: Option, @@ -130,7 +132,7 @@ impl Image { let jpeg_tables = if compression_method == CompressionMethod::ModernJPEG && ifd.contains_key(&Tag::JPEGTables) { - let vec = tag_reader + let mut vec = tag_reader .find_tag(Tag::JPEGTables)? .unwrap() .into_u8_vec()?; @@ -139,7 +141,13 @@ impl Image { TiffFormatError::InvalidTagValueType(Tag::JPEGTables), )); } - Some(vec) + + // TODO: Can we avoid this somehow? + if photometric_interpretation == PhotometricInterpretation::RGB { + add_app14segment(&mut vec, JpegTagApp14Transform::App14TransformUnknown) + } + + Some(Arc::new(vec)) } else { None }; diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index e721eb1..a200f37 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1,6 +1,7 @@ use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; use std::io::{self, Cursor, Read, Seek}; +use std::sync::Arc; use std::{cmp, ops::Range}; use crate::{ @@ -1151,7 +1152,7 @@ impl Decoder { photometric_interpretation: PhotometricInterpretation, compression_method: CompressionMethod, compressed_length: u64, - jpeg_tables: Option>, + jpeg_tables: Option>>, ) -> TiffResult> { Ok(match compression_method { CompressionMethod::None => Box::new(reader), @@ -1228,70 +1229,9 @@ impl Decoder { Ok(u32::try_from(self.image().chunk_offsets.len())?) } + #[deprecated = "Use read_image instead"] pub fn read_jpeg(&mut self) -> TiffResult { - self.check_chunk_type(ChunkType::Strip)?; - - if self.image().chunk_offsets.len() == 0 { - return Err(TiffError::FormatError(TiffFormatError::RequiredTagEmpty( - Tag::StripOffsets, - ))); - } - - if self.image().chunk_offsets[0] as usize > self.limits.intermediate_buffer_size - || self - .image() - .chunk_bytes - .iter() - .any(|&x| x as usize > self.limits.intermediate_buffer_size) - { - return Err(TiffError::LimitsExceeded); - } - - let mut res_img = Vec::with_capacity(self.image().chunk_offsets[0] as usize); - - for idx in 0..self.image.chunk_offsets.len() { - let offset = self.image.chunk_offsets[idx]; - let length = self.image.chunk_bytes[idx]; - - self.goto_offset_u64(offset)?; - - if self.image().jpeg_tables.is_some() && length < 2 { - return Err(TiffError::FormatError( - TiffFormatError::InvalidTagValueType(Tag::JPEGTables), - )); - } - - let jpeg_tables = self.image().jpeg_tables.clone(); - let photometric_interpretation = self.image().photometric_interpretation; - - let jpeg_reader = JpegReader::new( - &mut self.reader, - length, - jpeg_tables, - &photometric_interpretation, - )?; - let mut decoder = jpeg::Decoder::new(jpeg_reader); - - match decoder.decode() { - Ok(mut val) => res_img.append(&mut val), - Err(e) => { - return match e { - jpeg::Error::Io(io_err) => Err(TiffError::IoError(io_err)), - jpeg::Error::Format(fmt_err) => { - Err(TiffError::FormatError(TiffFormatError::Format(fmt_err))) - } - jpeg::Error::Unsupported(_) => Err(TiffError::UnsupportedError( - TiffUnsupportedError::UnknownInterpretation, - )), - jpeg::Error::Internal(_) => Err(TiffError::UnsupportedError( - TiffUnsupportedError::UnknownInterpretation, - )), - } - } - } - } - - Ok(DecodingResult::U8(res_img)) + self.read_image() } pub fn read_strip_to_buffer(&mut self, mut buffer: DecodingBuffer) -> TiffResult<()> { @@ -1546,11 +1486,9 @@ impl Decoder { /// Decodes the entire image and return it as a Vector pub fn read_image(&mut self) -> TiffResult { - let result = match self.image().chunk_type { - ChunkType::Strip => self.read_stripped_image()?, - ChunkType::Tile => self.read_tiled_image()?, - }; - - Ok(result) + match self.image().chunk_type { + ChunkType::Strip => self.read_stripped_image(), + ChunkType::Tile => self.read_tiled_image(), + } } } diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index 0743daa..e5992fe 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -1,7 +1,8 @@ //! All IO functionality needed for TIFF decoding use std::convert::TryFrom; -use std::io::{self, BufRead, BufReader, Read, Seek, Take}; +use std::io::{self, BufRead, BufReader, Read, Seek, SeekFrom, Take}; +use std::sync::Arc; use crate::tags::PhotometricInterpretation; @@ -189,7 +190,13 @@ impl Read for LZWReader { /// pub(crate) struct JpegReader { + jpeg_tables: Option>>, + + photometric_interpretation: Option, + buffer: io::Cursor>, + + offset: usize, } impl JpegReader { @@ -208,7 +215,7 @@ impl JpegReader { pub fn new( reader: &mut SmartReader, length: u64, - jpeg_tables: Option>, + jpeg_tables: Option>>, photometric_interpretation: &PhotometricInterpretation, ) -> io::Result where @@ -220,11 +227,11 @@ impl JpegReader { reader.read_exact(&mut segment[..])?; match jpeg_tables { - Some(mut jpeg_data) => { + Some(jpeg_tables) => { assert!( - jpeg_data.len() >= 2, + jpeg_tables.len() >= 2, "jpeg_tables, if given, must be at least 2 bytes long. Got {:?}", - jpeg_data + jpeg_tables ); assert!( @@ -233,33 +240,43 @@ impl JpegReader { length ); - if photometric_interpretation == &PhotometricInterpretation::RGB { - add_app14segment(&mut jpeg_data, JpegTagApp14Transform::App14TransformUnknown) - } + // if photometric_interpretation == &PhotometricInterpretation::RGB { + // add_app14segment(&mut jpeg_data, JpegTagApp14Transform::App14TransformUnknown) + // } - let truncated_length = jpeg_data.len() - 2; - jpeg_data.truncate(truncated_length); - jpeg_data.extend_from_slice(&segment[2..]); + // let truncated_length = jpeg_data.len() - 2; + // jpeg_data.truncate(truncated_length); + // jpeg_data.extend_from_slice(&segment[2..]); + + let mut buffer = io::Cursor::new(segment); + // Skip the first two bytes + buffer.seek(SeekFrom::Start(2))?; Ok(JpegReader { - buffer: io::Cursor::new(jpeg_data), + buffer, + jpeg_tables: Some(jpeg_tables), + photometric_interpretation: Some(*photometric_interpretation), + offset: 0, }) } None => Ok(JpegReader { buffer: io::Cursor::new(segment), + jpeg_tables: None, + photometric_interpretation: None, + offset: 0, }), } } } #[repr(u8)] -enum JpegTagApp14Transform { +pub(crate) enum JpegTagApp14Transform { // App14TransformYCCK = 2, // App14TransformYCbCr = 1, App14TransformUnknown = 0, } -fn add_app14segment(jpeg_tables: &mut Vec, transform: JpegTagApp14Transform) { +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 @@ -307,7 +324,31 @@ fn add_app14segment(jpeg_tables: &mut Vec, transform: JpegTagApp14Transform) impl Read for JpegReader { // #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.buffer.read(buf) + let mut start = 0; + + if let Some(jpeg_tables) = &self.jpeg_tables { + if jpeg_tables.len() - 2 > self.offset { + // Read (rest of) jpeg_tables to buf (without the last two bytes) + let size_remaining = jpeg_tables.len() - self.offset - 2; + let to_copy = size_remaining.min(buf.len()); + + buf[start..start + to_copy] + .copy_from_slice(&jpeg_tables[self.offset..self.offset + to_copy]); + + self.offset += to_copy; + + if to_copy == buf.len() { + return Ok(to_copy); + } + + start += to_copy; + } + } + + let read = self.buffer.read(&mut buf[start..])?; + self.offset += read; + + Ok(read + start) } } From ce00bb5a284815b8a045d047808bb4e3d69def13 Mon Sep 17 00:00:00 2001 From: Alan Race Date: Sat, 30 Apr 2022 14:35:33 +0200 Subject: [PATCH 3/4] Removed old code --- src/decoder/stream.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index e5992fe..b125bff 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -240,16 +240,8 @@ impl JpegReader { length ); - // if photometric_interpretation == &PhotometricInterpretation::RGB { - // add_app14segment(&mut jpeg_data, JpegTagApp14Transform::App14TransformUnknown) - // } - - // let truncated_length = jpeg_data.len() - 2; - // jpeg_data.truncate(truncated_length); - // jpeg_data.extend_from_slice(&segment[2..]); - let mut buffer = io::Cursor::new(segment); - // Skip the first two bytes + // Skip the first two bytes (marker bytes) buffer.seek(SeekFrom::Start(2))?; Ok(JpegReader { From a133056287ee8d24178bbf727c25b66dc6f082fc Mon Sep 17 00:00:00 2001 From: Alan Race Date: Sat, 30 Apr 2022 14:55:52 +0200 Subject: [PATCH 4/4] Removed passing of PhotometricInterpretation to `create_reader` as this is now handled for JPEG globally --- src/decoder/mod.rs | 19 +++---------------- src/decoder/stream.rs | 5 ----- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index a200f37..48ee92e 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1032,17 +1032,11 @@ impl Decoder { self.goto_offset_u64(offset)?; let byte_order = self.reader.byte_order; - let photometric_interpretation = self.image().photometric_interpretation; let compression_method = self.image().compression_method; let jpeg_tables = self.image().jpeg_tables.clone(); - let reader = Self::create_reader( - &mut self.reader, - photometric_interpretation, - compression_method, - length, - jpeg_tables, - )?; + let reader = + Self::create_reader(&mut self.reader, compression_method, length, jpeg_tables)?; // Read into output buffer. { @@ -1105,7 +1099,6 @@ impl Decoder { let mut reader = Self::create_reader( &mut self.reader, - photometric_interpretation, compression_method, compressed_length, jpeg_tables, @@ -1149,7 +1142,6 @@ impl Decoder { fn create_reader<'r>( reader: &'r mut SmartReader, - photometric_interpretation: PhotometricInterpretation, compression_method: CompressionMethod, compressed_length: u64, jpeg_tables: Option>>, @@ -1170,12 +1162,7 @@ impl Decoder { )); } - let jpeg_reader = JpegReader::new( - reader, - compressed_length, - jpeg_tables, - &photometric_interpretation, - )?; + let jpeg_reader = JpegReader::new(reader, compressed_length, jpeg_tables)?; let mut decoder = jpeg::Decoder::new(jpeg_reader); let data = decoder.decode().unwrap(); diff --git a/src/decoder/stream.rs b/src/decoder/stream.rs index b125bff..5e8ffdd 100644 --- a/src/decoder/stream.rs +++ b/src/decoder/stream.rs @@ -192,8 +192,6 @@ impl Read for LZWReader { pub(crate) struct JpegReader { jpeg_tables: Option>>, - photometric_interpretation: Option, - buffer: io::Cursor>, offset: usize, @@ -216,7 +214,6 @@ impl JpegReader { reader: &mut SmartReader, length: u64, jpeg_tables: Option>>, - photometric_interpretation: &PhotometricInterpretation, ) -> io::Result where R: Read + Seek, @@ -247,14 +244,12 @@ impl JpegReader { Ok(JpegReader { buffer, jpeg_tables: Some(jpeg_tables), - photometric_interpretation: Some(*photometric_interpretation), offset: 0, }) } None => Ok(JpegReader { buffer: io::Cursor::new(segment), jpeg_tables: None, - photometric_interpretation: None, offset: 0, }), }