Skip to content

Commit

Permalink
Merge branch 'advanced-compression' into high-level-compression-control
Browse files Browse the repository at this point in the history
  • Loading branch information
@Shnatsel
Shnatsel committed Sep 26, 2024
2 parents e7dd3dc + 8b08c9a commit 1e83f78
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 70 deletions.
Binary file added ...orpus/buf_independent/regressions/minimized-from-32e60253faddfdf03b267e0b55c7b91a674d74bd
View file
Binary file not shown.
Binary file added ...orpus/buf_independent/regressions/minimized-from-a1a0f9ae81d63ecf08d1cd0efbc73e40e7909681
View file
Binary file not shown.
18 changes: 6 additions & 12 deletions src/decoder/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ impl<R: Read> Reader<R> {
Some(Decoded::ChunkBegin(_, chunk::IDAT))
| Some(Decoded::ChunkBegin(_, chunk::fdAT)) => break,
Some(Decoded::FrameControl(_)) => {
self.subframe = SubframeInfo::new(self.info());
// The next frame is the one to which this chunk applies.
self.next_frame = SubframeIdx::Some(self.fctl_read);
// TODO: what about overflow here? That would imply there are more fctl chunks
Expand Down Expand Up @@ -474,17 +473,13 @@ impl<R: Read> Reader<R> {
/// Output lines will be written in row-major, packed matrix with width and height of the read
/// frame (or subframe), all samples are in big endian byte order where this matters.
pub fn next_frame(&mut self, buf: &mut [u8]) -> Result<OutputInfo, DecodingError> {
let subframe_idx = match self.decoder.info().unwrap().frame_control() {
None => SubframeIdx::Initial,
Some(_) => SubframeIdx::Some(self.fctl_read - 1),
};

if self.next_frame == SubframeIdx::End {
return Err(DecodingError::Parameter(
ParameterErrorKind::PolledAfterEndOfImage.into(),
));
} else if self.next_frame != subframe_idx {
// Advance until we've read the info / fcTL for this frame.
} else if self.subframe.consumed_and_flushed {
// Advance until the next `fdAT`
// (along the way we should encounter the fcTL for this frame).
self.read_until_image_data()?;
}

Expand Down Expand Up @@ -539,6 +534,7 @@ impl<R: Read> Reader<R> {
// Advance over the rest of data for this (sub-)frame.
if !self.subframe.consumed_and_flushed {
self.decoder.finish_decoding()?;
self.subframe.consumed_and_flushed = true;
}

// Advance our state to expect the next frame.
Expand Down Expand Up @@ -717,10 +713,8 @@ impl<R: Read> Reader<R> {
}

match self.decoder.decode_next(&mut self.data_stream)? {
Some(Decoded::ImageData) => {}
Some(Decoded::ImageDataFlushed) | None /* after IEND chunk */ => {
self.subframe.consumed_and_flushed = true;
}
Some(Decoded::ImageData) => (),
Some(Decoded::ImageDataFlushed) => self.subframe.consumed_and_flushed = true,
_ => (),
}
}
Expand Down
24 changes: 22 additions & 2 deletions src/decoder/stream.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ pub(crate) enum FormatErrorInner {
UnexpectedRestartOfDataChunkSequence {
kind: ChunkType,
},
/// Failure to parse a chunk, because the chunk didn't contain enough bytes.
ChunkTooShort {
kind: ChunkType,
},
}

impl error::Error for DecodingError {
Expand Down Expand Up @@ -376,6 +380,9 @@ impl fmt::Display for FormatError {
UnexpectedRestartOfDataChunkSequence { kind } => {
write!(fmt, "Unexpected restart of {:?} chunk sequence", kind)
}
ChunkTooShort { kind } => {
write!(fmt, "Chunk is too short: {:?}", kind)
}
}
}
}
Expand Down Expand Up @@ -951,9 +958,22 @@ impl StreamingDecoder {
_ => Ok(Decoded::PartialChunk(type_str)),
};

if parse_result.is_err() {
let parse_result = parse_result.map_err(|e| {
self.state = None;
}
match e {
// `parse_chunk` is invoked after gathering **all** bytes of a chunk, so
// `UnexpectedEof` from something like `read_be` is permanent and indicates an
// invalid PNG that should be represented as a `FormatError`, rather than as a
// (potentially recoverable) `IoError` / `UnexpectedEof`.
DecodingError::IoError(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => {
let fmt_err: FormatError =
FormatErrorInner::ChunkTooShort { kind: type_str }.into();
fmt_err.into()
}
e => e,
}
});

parse_result
}

Expand Down
72 changes: 20 additions & 52 deletions src/encoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,6 @@ impl<'a, W: Write> Encoder<'a, W> {
/// The default filter is [`FilterType::Sub`] which provides a basic prediction algorithm for
/// sample values based on the previous. For a potentially better compression ratio, at the
/// cost of more complex processing, try out [`FilterType::Paeth`].
///
/// [`FilterType::Sub`]: enum.FilterType.html#variant.Sub
/// [`FilterType::Paeth`]: enum.FilterType.html#variant.Paeth
pub fn set_filter(&mut self, filter: FilterType) {
self.options.filter = filter;
}
Expand All @@ -338,8 +335,7 @@ impl<'a, W: Write> Encoder<'a, W> {
/// based on heuristics which minimize the file size for compression rather
/// than use a single filter for the entire image. The default method is
/// [`AdaptiveFilterType::NonAdaptive`].
///
/// [`AdaptiveFilterType::NonAdaptive`]: enum.AdaptiveFilterType.html

pub fn set_adaptive_filter(&mut self, adaptive_filter: AdaptiveFilterType) {
self.options.adaptive_filter = adaptive_filter;
}
Expand All @@ -359,9 +355,8 @@ impl<'a, W: Write> Encoder<'a, W> {
/// This method will return an error if the image is not animated.
/// (see [`set_animated`])
///
/// [`write_header`]: struct.Encoder.html#method.write_header
/// [`set_animated`]: struct.Encoder.html#method.set_animated
/// [`Writer::set_frame_delay`]: struct.Writer#method.set_frame_delay
/// [`write_header`]: Self::write_header
/// [`set_animated`]: Self::set_animated
pub fn set_frame_delay(&mut self, numerator: u16, denominator: u16) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.delay_den = denominator;
Expand Down Expand Up @@ -392,11 +387,8 @@ impl<'a, W: Write> Encoder<'a, W> {
/// This method will return an error if the image is not animated.
/// (see [`set_animated`])
///
/// [`BlendOP`]: enum.BlendOp.html
/// [`BlendOP::Source`]: enum.BlendOp.html#variant.Source
/// [`write_header`]: struct.Encoder.html#method.write_header
/// [`set_animated`]: struct.Encoder.html#method.set_animated
/// [`Writer::set_blend_op`]: struct.Writer#method.set_blend_op
/// [`write_header`]: Self::write_header
/// [`set_animated`]: Self::set_animated
pub fn set_blend_op(&mut self, op: BlendOp) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.blend_op = op;
Expand Down Expand Up @@ -424,13 +416,8 @@ impl<'a, W: Write> Encoder<'a, W> {
/// This method will return an error if the image is not animated.
/// (see [`set_animated`])
///
/// [`DisposeOp`]: ../common/enum.BlendOp.html
/// [`DisposeOp::Previous`]: ../common/enum.BlendOp.html#variant.Previous
/// [`DisposeOp::Background`]: ../common/enum.BlendOp.html#variant.Background
/// [`DisposeOp::None`]: ../common/enum.BlendOp.html#variant.None
/// [`write_header`]: struct.Encoder.html#method.write_header
/// [`set_animated`]: struct.Encoder.html#method.set_animated
/// [`Writer::set_dispose_op`]: struct.Writer#method.set_dispose_op
/// [`set_animated`]: Self::set_animated
/// [`write_header`]: Self::write_header
pub fn set_dispose_op(&mut self, op: DisposeOp) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.dispose_op = op;
Expand Down Expand Up @@ -846,9 +833,6 @@ impl<W: Write> Writer<W> {
/// The default filter is [`FilterType::Sub`] which provides a basic prediction algorithm for
/// sample values based on the previous. For a potentially better compression ratio, at the
/// cost of more complex processing, try out [`FilterType::Paeth`].
///
/// [`FilterType::Sub`]: enum.FilterType.html#variant.Sub
/// [`FilterType::Paeth`]: enum.FilterType.html#variant.Paeth
pub fn set_filter(&mut self, filter: FilterType) {
self.options.filter = filter;
}
Expand All @@ -859,8 +843,6 @@ impl<W: Write> Writer<W> {
/// based on heuristics which minimize the file size for compression rather
/// than use a single filter for the entire image. The default method is
/// [`AdaptiveFilterType::NonAdaptive`].
///
/// [`AdaptiveFilterType::NonAdaptive`]: enum.AdaptiveFilterType.html
pub fn set_adaptive_filter(&mut self, adaptive_filter: AdaptiveFilterType) {
self.options.adaptive_filter = adaptive_filter;
}
Expand Down Expand Up @@ -948,7 +930,7 @@ impl<W: Write> Writer<W> {
///
/// This method will return an error if the image is not animated.
///
/// [`reset_frame_position`]: struct.Writer.html#method.reset_frame_position
/// [`reset_frame_position`]: Writer::reset_frame_position
pub fn reset_frame_dimension(&mut self) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.width = self.info.width - fctl.x_offset;
Expand All @@ -965,7 +947,7 @@ impl<W: Write> Writer<W> {
///
/// This method will return an error if the image is not animated.
///
/// [`set_frame_position(0, 0)`]: struct.Writer.html#method.set_frame_position
/// [`set_frame_position(0, 0)`]: Writer::set_frame_position
pub fn reset_frame_position(&mut self) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.x_offset = 0;
Expand All @@ -989,8 +971,6 @@ impl<W: Write> Writer<W> {
/// of each play.*
///
/// This method will return an error if the image is not animated.
///
/// [`BlendOP`]: enum.BlendOp.html
pub fn set_blend_op(&mut self, op: BlendOp) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.blend_op = op;
Expand All @@ -1011,10 +991,6 @@ impl<W: Write> Writer<W> {
/// it will be treated as [`DisposeOp::Background`].*
///
/// This method will return an error if the image is not animated.
///
/// [`DisposeOp`]: ../common/enum.BlendOp.html
/// [`DisposeOp::Previous`]: ../common/enum.BlendOp.html#variant.Previous
/// [`DisposeOp::Background`]: ../common/enum.BlendOp.html#variant.Background
pub fn set_dispose_op(&mut self, op: DisposeOp) -> Result<()> {
if let Some(ref mut fctl) = self.info.frame_control {
fctl.dispose_op = op;
Expand All @@ -1040,16 +1016,18 @@ impl<W: Write> Writer<W> {
///
/// See [`stream_writer`].
///
/// [`stream_writer`]: #fn.stream_writer
/// [`stream_writer`]: Self::stream_writer
pub fn stream_writer_with_size(&mut self, size: usize) -> Result<StreamWriter<W>> {
StreamWriter::new(ChunkOutput::Borrowed(self), size)
}

/// Turn this into a stream writer for image data.
///
/// This allows you to create images that do not fit in memory. The default
/// chunk size is 4K, use `stream_writer_with_size` to set another chunk
/// chunk size is 4K, use [`stream_writer_with_size`] to set another chunk
/// size.
///
/// [`stream_writer_with_size`]: Self::stream_writer_with_size
pub fn into_stream_writer(self) -> Result<StreamWriter<'static, W>> {
self.into_stream_writer_with_size(DEFAULT_BUFFER_LENGTH)
}
Expand All @@ -1058,7 +1036,7 @@ impl<W: Write> Writer<W> {
///
/// See [`into_stream_writer`].
///
/// [`into_stream_writer`]: #fn.into_stream_writer
/// [`into_stream_writer`]: Self::into_stream_writer
pub fn into_stream_writer_with_size(self, size: usize) -> Result<StreamWriter<'static, W>> {
StreamWriter::new(ChunkOutput::Owned(self), size)
}
Expand Down Expand Up @@ -1206,7 +1184,7 @@ impl<'a, W: Write> ChunkWriter<'a, W> {
Ok(())
}

/// Set the `FrameControl` for the following frame
/// Set the [`FrameControl`] for the following frame
///
/// It will ignore the `sequence_number` of the parameter
/// as it is updated internally.
Expand Down Expand Up @@ -1323,9 +1301,10 @@ impl<'a, W: Write> Wrapper<'a, W> {
/// Streaming PNG writer
///
/// This may silently fail in the destructor, so it is a good idea to call
/// [`finish`](#method.finish) or [`flush`] before dropping.
/// [`finish`] or [`flush`] before dropping.
///
/// [`flush`]: https://doc.rust-lang.org/stable/std/io/trait.Write.html#tymethod.flush
/// [`finish`]: Self::finish
/// [`flush`]: Write::flush
pub struct StreamWriter<'a, W: Write> {
/// The option here is needed in order to access the inner `ChunkWriter` in-between
/// each frame, which is needed for writing the fcTL chunks between each frame
Expand Down Expand Up @@ -1393,9 +1372,6 @@ impl<'a, W: Write> StreamWriter<'a, W> {
/// The default filter is [`FilterType::Sub`] which provides a basic prediction algorithm for
/// sample values based on the previous. For a potentially better compression ratio, at the
/// cost of more complex processing, try out [`FilterType::Paeth`].
///
/// [`FilterType::Sub`]: enum.FilterType.html#variant.Sub
/// [`FilterType::Paeth`]: enum.FilterType.html#variant.Paeth
pub fn set_filter(&mut self, filter: FilterType) {
self.filter = filter;
}
Expand All @@ -1406,8 +1382,6 @@ impl<'a, W: Write> StreamWriter<'a, W> {
/// based on heuristics which minimize the file size for compression rather
/// than use a single filter for the entire image. The default method is
/// [`AdaptiveFilterType::NonAdaptive`].
///
/// [`AdaptiveFilterType::NonAdaptive`]: enum.AdaptiveFilterType.html
pub fn set_adaptive_filter(&mut self, adaptive_filter: AdaptiveFilterType) {
self.adaptive_filter = adaptive_filter;
}
Expand Down Expand Up @@ -1491,7 +1465,7 @@ impl<'a, W: Write> StreamWriter<'a, W> {
///
/// This method will return an error if the image is not animated.
///
/// [`reset_frame_position`]: struct.Writer.html#method.reset_frame_position
/// [`reset_frame_position`]: Writer::reset_frame_position
pub fn reset_frame_dimension(&mut self) -> Result<()> {
if let Some(ref mut fctl) = self.fctl {
fctl.width = self.width - fctl.x_offset;
Expand All @@ -1508,7 +1482,7 @@ impl<'a, W: Write> StreamWriter<'a, W> {
///
/// This method will return an error if the image is not animated.
///
/// [`set_frame_position(0, 0)`]: struct.Writer.html#method.set_frame_position
/// [`set_frame_position(0, 0)`]: Writer::set_frame_position
pub fn reset_frame_position(&mut self) -> Result<()> {
if let Some(ref mut fctl) = self.fctl {
fctl.x_offset = 0;
Expand All @@ -1532,8 +1506,6 @@ impl<'a, W: Write> StreamWriter<'a, W> {
/// of each play.*
///
/// This method will return an error if the image is not animated.
///
/// [`BlendOP`]: enum.BlendOp.html
pub fn set_blend_op(&mut self, op: BlendOp) -> Result<()> {
if let Some(ref mut fctl) = self.fctl {
fctl.blend_op = op;
Expand All @@ -1554,10 +1526,6 @@ impl<'a, W: Write> StreamWriter<'a, W> {
/// it will be treated as [`DisposeOp::Background`].*
///
/// This method will return an error if the image is not animated.
///
/// [`DisposeOp`]: ../common/enum.BlendOp.html
/// [`DisposeOp::Previous`]: ../common/enum.BlendOp.html#variant.Previous
/// [`DisposeOp::Background`]: ../common/enum.BlendOp.html#variant.Background
pub fn set_dispose_op(&mut self, op: DisposeOp) -> Result<()> {
if let Some(ref mut fctl) = self.fctl {
fctl.dispose_op = op;
Expand Down
2 changes: 1 addition & 1 deletion src/filter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ impl FilterType {
/// Adaptive filtering performs additional computation in an attempt to maximize
/// the compression of the data. [`NonAdaptive`] filtering is the default.
///
/// [`NonAdaptive`]: enum.AdaptiveFilterType.html#variant.NonAdaptive
/// [`NonAdaptive`]: AdaptiveFilterType::NonAdaptive
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[repr(u8)]
pub enum AdaptiveFilterType {
Expand Down
6 changes: 3 additions & 3 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
//!
//! ## The decoder
//!
//! The most important types for decoding purposes are [`Decoder`](struct.Decoder.html) and
//! [`Reader`](struct.Reader.html). They both wrap a `std::io::Read`.
//! `Decoder` serves as a builder for `Reader`. Calling `Decoder::read_info` reads from the `Read` until the
//! The most important types for decoding purposes are [`Decoder`] and
//! [`Reader`]. They both wrap a [`std::io::Read`].
//! `Decoder` serves as a builder for `Reader`. Calling [`Decoder::read_info`] reads from the `Read` until the
//! image data is reached.
//!
//! ### Using the decoder
Expand Down

0 comments on commit 1e83f78

Please sign in to comment.