From 2fb4b8ea9875bd7a20908666787fdfddbca87fe7 Mon Sep 17 00:00:00 2001 From: Lukasz Anforowicz Date: Sat, 5 Oct 2024 17:34:31 +0000 Subject: [PATCH] Add separate `read_decoder.rs` module to enforce encapsulation. This commit helps to keep some aspects of `ReadDecoder` private (e.g. its fields, `decode_next` methods, etc.). This commit also means that `mod.rs` no longer directly depends on `Decoded` nor `StreamingDecoder`. --- src/decoder/mod.rs | 152 ++++-------------------------- src/decoder/read_decoder.rs | 179 ++++++++++++++++++++++++++++++++++++ src/lib.rs | 6 +- 3 files changed, 198 insertions(+), 139 deletions(-) create mode 100644 src/decoder/read_decoder.rs diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs index 30e31e62..fa2c21c7 100644 --- a/src/decoder/mod.rs +++ b/src/decoder/mod.rs @@ -1,17 +1,17 @@ mod interlace_info; -mod stream; +mod read_decoder; +pub(crate) mod stream; pub(crate) mod transform; mod zlib; -pub use self::stream::{DecodeOptions, Decoded, DecodingError, StreamingDecoder}; -use self::stream::{FormatErrorInner, CHUNK_BUFFER_SIZE}; +use self::read_decoder::{ImageDataCompletionStatus, ReadDecoder}; +use self::stream::{DecodeOptions, DecodingError, FormatErrorInner, CHUNK_BUFFER_SIZE}; use self::transform::{create_transform_fn, TransformFn}; -use std::io::{BufRead, BufReader, ErrorKind, Read}; +use std::io::Read; use std::mem; use crate::adam7::{self, Adam7Info}; -use crate::chunk; use crate::common::{ BitDepth, BytesPerPixel, ColorType, Info, ParameterErrorKind, Transformations, }; @@ -128,28 +128,22 @@ impl Decoder { /// Create a new decoder configuration with custom limits. pub fn new_with_limits(r: R, limits: Limits) -> Decoder { - let mut decoder = StreamingDecoder::new(); - decoder.limits = limits; + let mut read_decoder = ReadDecoder::new(r); + read_decoder.set_limits(limits); Decoder { - read_decoder: ReadDecoder { - reader: BufReader::with_capacity(CHUNK_BUFFER_SIZE, r), - decoder, - }, + read_decoder, transform: Transformations::IDENTITY, } } /// Create a new decoder configuration with custom `DecodeOptions`. pub fn new_with_options(r: R, decode_options: DecodeOptions) -> Decoder { - let mut decoder = StreamingDecoder::new_with_options(decode_options); - decoder.limits = Limits::default(); + let mut read_decoder = ReadDecoder::with_options(r, decode_options); + read_decoder.set_limits(Limits::default()); Decoder { - read_decoder: ReadDecoder { - reader: BufReader::with_capacity(CHUNK_BUFFER_SIZE, r), - decoder, - }, + read_decoder, transform: Transformations::IDENTITY, } } @@ -178,7 +172,7 @@ impl Decoder { /// assert!(decoder.read_info().is_ok()); /// ``` pub fn set_limits(&mut self, limits: Limits) { - self.read_decoder.decoder.limits = limits; + self.read_decoder.set_limits(limits); } /// Read the PNG header and return the information contained within. @@ -261,9 +255,7 @@ impl Decoder { /// assert!(decoder.read_info().is_ok()); /// ``` pub fn set_ignore_text_chunk(&mut self, ignore_text_chunk: bool) { - self.read_decoder - .decoder - .set_ignore_text_chunk(ignore_text_chunk); + self.read_decoder.set_ignore_text_chunk(ignore_text_chunk); } /// Set the decoder to ignore iccp chunks while parsing. @@ -277,123 +269,13 @@ impl Decoder { /// assert!(decoder.read_info().is_ok()); /// ``` pub fn set_ignore_iccp_chunk(&mut self, ignore_iccp_chunk: bool) { - self.read_decoder - .decoder - .set_ignore_iccp_chunk(ignore_iccp_chunk); + self.read_decoder.set_ignore_iccp_chunk(ignore_iccp_chunk); } /// Set the decoder to ignore and not verify the Adler-32 checksum /// and CRC code. pub fn ignore_checksums(&mut self, ignore_checksums: bool) { - self.read_decoder - .decoder - .set_ignore_adler32(ignore_checksums); - self.read_decoder.decoder.set_ignore_crc(ignore_checksums); - } -} - -#[derive(Debug, Eq, PartialEq)] -enum ImageDataCompletionStatus { - ExpectingMoreData, - Done, -} - -struct ReadDecoder { - reader: BufReader, - decoder: StreamingDecoder, -} - -impl ReadDecoder { - /// Returns the next decoded chunk. If the chunk is an ImageData chunk, its contents are written - /// into image_data. - fn decode_next(&mut self, image_data: &mut Vec) -> Result { - let (consumed, result) = { - let buf = self.reader.fill_buf()?; - if buf.is_empty() { - return Err(DecodingError::IoError(ErrorKind::UnexpectedEof.into())); - } - self.decoder.update(buf, image_data)? - }; - self.reader.consume(consumed); - Ok(result) - } - - fn decode_next_without_image_data(&mut self) -> Result { - // This is somewhat ugly. The API requires us to pass a buffer to decode_next but we - // know that we will stop before reading any image data from the stream. Thus pass an - // empty buffer and assert that remains empty. - let mut buf = Vec::new(); - let state = self.decode_next(&mut buf)?; - assert!(buf.is_empty()); - Ok(state) - } - - fn decode_next_and_discard_image_data(&mut self) -> Result { - let mut to_be_discarded = Vec::new(); - self.decode_next(&mut to_be_discarded) - } - - pub fn read_header_info(&mut self) -> Result<&Info<'static>, DecodingError> { - while self.info().is_none() { - if let Decoded::ImageEnd = self.decode_next_without_image_data()? { - unreachable!() - } - } - Ok(self.info().unwrap()) - } - - pub fn read_until_image_data(&mut self) -> Result<(), DecodingError> { - loop { - match self.decode_next_without_image_data()? { - Decoded::ChunkBegin(_, chunk::IDAT) | Decoded::ChunkBegin(_, chunk::fdAT) => break, - Decoded::ImageEnd => { - return Err(DecodingError::Format( - FormatErrorInner::MissingImageData.into(), - )) - } - // Ignore all other chunk events. Any other chunk may be between IDAT chunks, fdAT - // chunks and their control chunks. - _ => {} - } - } - Ok(()) - } - - pub fn decode_image_data( - &mut self, - image_data: &mut Vec, - ) -> Result { - match self.decode_next(image_data)? { - Decoded::ImageData => Ok(ImageDataCompletionStatus::ExpectingMoreData), - Decoded::ImageDataFlushed => Ok(ImageDataCompletionStatus::Done), - // Ignore other events that may happen within an `IDAT` / `fdAT` chunks sequence. - Decoded::Nothing - | Decoded::ChunkComplete(_, _) - | Decoded::ChunkBegin(_, _) - | Decoded::PartialChunk(_) => Ok(ImageDataCompletionStatus::ExpectingMoreData), - // Other kinds of events shouldn't happen, unless we have been (incorrectly) called - // when outside of a sequence of `IDAT` / `fdAT` chunks. - unexpected => unreachable!("{:?}", unexpected), - } - } - - /// Consumes and discards the rest of an `IDAT` / `fdAT` chunk sequence. - pub fn finish_decoding(&mut self) -> Result<(), DecodingError> { - loop { - let mut to_be_discarded = vec![]; - if let ImageDataCompletionStatus::Done = self.decode_image_data(&mut to_be_discarded)? { - return Ok(()); - } - } - } - - pub fn read_until_end_of_input(&mut self) -> Result<(), DecodingError> { - while !matches!(self.decode_next_and_discard_image_data()?, Decoded::ImageEnd) {} - Ok(()) - } - - pub fn info(&self) -> Option<&Info<'static>> { - self.decoder.info.as_ref() + self.read_decoder.ignore_checksums(ignore_checksums); } } @@ -486,7 +368,7 @@ impl Reader { // Allocate output buffer. let buflen = self.output_line_size(self.subframe.width); - self.decoder.decoder.limits.reserve_bytes(buflen)?; + self.decoder.reserve_bytes(buflen)?; self.prev_start = self.current_start; @@ -594,7 +476,7 @@ impl Reader { // Discard the remaining data in the current sequence of `IDAT` or `fdAT` chunks. if !self.subframe.consumed_and_flushed { - self.decoder.finish_decoding()?; + self.decoder.finish_decoding_image_data()?; self.mark_subframe_as_consumed_and_flushed(); } diff --git a/src/decoder/read_decoder.rs b/src/decoder/read_decoder.rs new file mode 100644 index 00000000..c3ccbc35 --- /dev/null +++ b/src/decoder/read_decoder.rs @@ -0,0 +1,179 @@ +use super::stream::{ + DecodeOptions, Decoded, DecodingError, FormatErrorInner, StreamingDecoder, CHUNK_BUFFER_SIZE, +}; +use super::Limits; + +use std::io::{BufRead, BufReader, ErrorKind, Read}; + +use crate::chunk; +use crate::common::Info; + +/// Helper for encapsulating reading input from `Read` and feeding it into a `StreamingDecoder` +/// while hiding low-level `Decoded` events and only exposing a few high-level reading operations +/// like: +/// +/// * `read_header_info` - reading until `IHDR` chunk +/// * `read_until_image_data` - reading until `IDAT` / `fdAT` sequence +/// * `decode_image_data` - reading from `IDAT` / `fdAT` sequence into `Vec` +/// * `finish_decoding_image_data()` - discarding remaining data from `IDAT` / `fdAT` sequence +/// * `read_until_end_of_input()` - reading until `IEND` chunk +pub struct ReadDecoder { + reader: BufReader, + decoder: StreamingDecoder, +} + +impl ReadDecoder { + pub fn new(r: R) -> Self { + Self { + reader: BufReader::with_capacity(CHUNK_BUFFER_SIZE, r), + decoder: StreamingDecoder::new(), + } + } + + pub fn with_options(r: R, options: DecodeOptions) -> Self { + let mut decoder = StreamingDecoder::new_with_options(options); + decoder.limits = Limits::default(); + + Self { + reader: BufReader::with_capacity(CHUNK_BUFFER_SIZE, r), + decoder, + } + } + + pub fn set_limits(&mut self, limits: Limits) { + self.decoder.limits = limits; + } + + pub fn reserve_bytes(&mut self, bytes: usize) -> Result<(), DecodingError> { + self.decoder.limits.reserve_bytes(bytes) + } + + pub fn set_ignore_text_chunk(&mut self, ignore_text_chunk: bool) { + self.decoder.set_ignore_text_chunk(ignore_text_chunk); + } + + pub fn set_ignore_iccp_chunk(&mut self, ignore_iccp_chunk: bool) { + self.decoder.set_ignore_iccp_chunk(ignore_iccp_chunk); + } + + pub fn ignore_checksums(&mut self, ignore_checksums: bool) { + self.decoder.set_ignore_adler32(ignore_checksums); + self.decoder.set_ignore_crc(ignore_checksums); + } + + /// Returns the next decoded chunk. If the chunk is an ImageData chunk, its contents are written + /// into image_data. + fn decode_next(&mut self, image_data: &mut Vec) -> Result { + let (consumed, result) = { + let buf = self.reader.fill_buf()?; + if buf.is_empty() { + return Err(DecodingError::IoError(ErrorKind::UnexpectedEof.into())); + } + self.decoder.update(buf, image_data)? + }; + self.reader.consume(consumed); + Ok(result) + } + + fn decode_next_without_image_data(&mut self) -> Result { + // This is somewhat ugly. The API requires us to pass a buffer to decode_next but we + // know that we will stop before reading any image data from the stream. Thus pass an + // empty buffer and assert that remains empty. + let mut buf = Vec::new(); + let state = self.decode_next(&mut buf)?; + assert!(buf.is_empty()); + Ok(state) + } + + fn decode_next_and_discard_image_data(&mut self) -> Result { + let mut to_be_discarded = Vec::new(); + self.decode_next(&mut to_be_discarded) + } + + /// Reads until the end of `IHDR` chunk. + /// + /// Prerequisite: None (idempotent). + pub fn read_header_info(&mut self) -> Result<&Info<'static>, DecodingError> { + while self.info().is_none() { + if let Decoded::ImageEnd = self.decode_next_without_image_data()? { + unreachable!() + } + } + Ok(self.info().unwrap()) + } + + /// Reads until the start of the next `IDAT` or `fdAT` chunk. + /// + /// Prerequisite: **Not** within `IDAT` / `fdAT` chunk sequence. + pub fn read_until_image_data(&mut self) -> Result<(), DecodingError> { + loop { + match self.decode_next_without_image_data()? { + Decoded::ChunkBegin(_, chunk::IDAT) | Decoded::ChunkBegin(_, chunk::fdAT) => break, + Decoded::ImageEnd => { + return Err(DecodingError::Format( + FormatErrorInner::MissingImageData.into(), + )) + } + // Ignore all other chunk events. Any other chunk may be between IDAT chunks, fdAT + // chunks and their control chunks. + _ => {} + } + } + Ok(()) + } + + /// Reads `image_data` and reports whether there may be additional data afterwards (i.e. if it + /// is okay to call `decode_image_data` and/or `finish_decoding_image_data` again).. + /// + /// Prerequisite: Input is currently positioned within `IDAT` / `fdAT` chunk sequence. + pub fn decode_image_data( + &mut self, + image_data: &mut Vec, + ) -> Result { + match self.decode_next(image_data)? { + Decoded::ImageData => Ok(ImageDataCompletionStatus::ExpectingMoreData), + Decoded::ImageDataFlushed => Ok(ImageDataCompletionStatus::Done), + // Ignore other events that may happen within an `IDAT` / `fdAT` chunks sequence. + Decoded::Nothing + | Decoded::ChunkComplete(_, _) + | Decoded::ChunkBegin(_, _) + | Decoded::PartialChunk(_) => Ok(ImageDataCompletionStatus::ExpectingMoreData), + // Other kinds of events shouldn't happen, unless we have been (incorrectly) called + // when outside of a sequence of `IDAT` / `fdAT` chunks. + unexpected => unreachable!("{:?}", unexpected), + } + } + + /// Consumes and discards the rest of an `IDAT` / `fdAT` chunk sequence. + /// + /// Prerequisite: Input is currently positioned within `IDAT` / `fdAT` chunk sequence. + pub fn finish_decoding_image_data(&mut self) -> Result<(), DecodingError> { + loop { + let mut to_be_discarded = vec![]; + if let ImageDataCompletionStatus::Done = self.decode_image_data(&mut to_be_discarded)? { + return Ok(()); + } + } + } + + /// Reads until the `IEND` chunk. + /// + /// Prerequisite: `IEND` chunk hasn't been reached yet. + pub fn read_until_end_of_input(&mut self) -> Result<(), DecodingError> { + while !matches!( + self.decode_next_and_discard_image_data()?, + Decoded::ImageEnd + ) {} + Ok(()) + } + + pub fn info(&self) -> Option<&Info<'static>> { + self.decoder.info.as_ref() + } +} + +#[derive(Debug, Eq, PartialEq)] +pub enum ImageDataCompletionStatus { + ExpectingMoreData, + Done, +} diff --git a/src/lib.rs b/src/lib.rs index 9e8a2113..54a789c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,10 +74,8 @@ mod traits; pub use crate::adam7::expand_pass as expand_interlaced_row; pub use crate::adam7::Adam7Info; pub use crate::common::*; -pub use crate::decoder::{ - DecodeOptions, Decoded, Decoder, DecodingError, InterlaceInfo, InterlacedRow, Limits, - OutputInfo, Reader, StreamingDecoder, -}; +pub use crate::decoder::stream::{DecodeOptions, Decoded, DecodingError, StreamingDecoder}; +pub use crate::decoder::{Decoder, InterlaceInfo, InterlacedRow, Limits, OutputInfo, Reader}; pub use crate::encoder::{Encoder, EncodingError, StreamWriter, Writer}; pub use crate::filter::{AdaptiveFilterType, FilterType};