Skip to content

Commit

Permalink
New API: Reader.next_frame_control (for advancing to the next frame).
Browse files Browse the repository at this point in the history
  • Loading branch information
anforowicz authored and kornelski committed Oct 9, 2024
1 parent 1ec7613 commit ffed4de
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 75 deletions.
33 changes: 33 additions & 0 deletions src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::common::{
BitDepth, BytesPerPixel, ColorType, Info, ParameterErrorKind, Transformations,
};
use crate::filter::{unfilter, FilterType};
use crate::FrameControl;

pub use interlace_info::InterlaceInfo;
use interlace_info::InterlaceInfoIter;
Expand Down Expand Up @@ -396,6 +397,38 @@ struct SubframeInfo {
}

impl<R: Read> Reader<R> {
/// Advances to the start of the next animation frame and
/// returns a reference to the `FrameControl` info that describes it.
/// Skips and discards the image data of the previous frame if necessary.
///
/// Returns a [`ParameterError`] when there are no more animation frames.
/// To avoid this the caller can check if [`Info::animation_control`] exists
/// and consult [`AnimationControl::num_frames`].
pub fn next_frame_info(&mut self) -> Result<&FrameControl, DecodingError> {
let remaining_frames = if self.subframe.consumed_and_flushed {
self.remaining_frames
} else {
// One remaining frame will be consumed by the `finish_decoding` call below.
self.remaining_frames - 1
};
if remaining_frames == 0 {
return Err(DecodingError::Parameter(
ParameterErrorKind::PolledAfterEndOfImage.into(),
));
}

if !self.subframe.consumed_and_flushed {
self.subframe.current_interlace_info = None;
self.finish_decoding()?;
}
self.read_until_image_data()?;

// The PNG standard (and `StreamingDecoder `) guarantes that there is an `fcTL` chunk
// before the start of image data in a sequence of `fdAT` chunks. Therefore `unwrap`
// below is guaranteed to not panic.
Ok(self.info().frame_control.as_ref().unwrap())
}

/// Reads all meta data until the next frame data starts.
/// Requires IHDR before the IDAT and fcTL before fdAT.
fn read_until_image_data(&mut self) -> Result<(), DecodingError> {
Expand Down
231 changes: 156 additions & 75 deletions src/decoder/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2300,6 +2300,15 @@ mod tests {
Decoder::new(png).read_info().unwrap()
}

fn get_fctl_sequence_number(reader: &Reader<impl Read>) -> u32 {
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number
}

/// Tests that [`Reader.next_frame`] will report a `PolledAfterEndOfImage` error when called
/// after already decoding a single frame in a non-animated PNG.
#[test]
Expand All @@ -2313,7 +2322,27 @@ mod tests {
let err = reader
.next_frame(&mut buf)
.expect_err("Main test - expecting error");
assert!(matches!(err, DecodingError::Parameter(_)));
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// Tests that [`Reader.next_frame_info`] will report a `PolledAfterEndOfImage` error when
/// called when decoding a PNG that only contains a single frame.
#[test]
fn test_next_frame_info_polling_after_end_non_animated() {
let mut reader = create_reader_of_ihdr_idat();

let err = reader
.next_frame_info()
.expect_err("Main test - expecting error");
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// Tests that [`Reader.next_frame`] will report a `PolledAfterEndOfImage` error when called
Expand All @@ -2324,47 +2353,27 @@ mod tests {
let mut reader = create_reader_of_ihdr_actl_fctl_idat_fctl_fdat();
let mut buf = vec![0; reader.output_buffer_size()];

assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
0
);
assert_eq!(get_fctl_sequence_number(&reader), 0);
reader
.next_frame(&mut buf)
.expect("Expecting no error for IDAT frame");

// `next_frame` doesn't advance to the next `fcTL`.
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
0
);
assert_eq!(get_fctl_sequence_number(&reader), 0);

reader
.next_frame(&mut buf)
.expect("Expecting no error for fdAT frame");
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
1
);
assert_eq!(get_fctl_sequence_number(&reader), 1);

let err = reader
.next_frame(&mut buf)
.expect_err("Main test - expecting error");
assert!(matches!(err, DecodingError::Parameter(_)));
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// Tests that [`Reader.next_frame`] will report a `PolledAfterEndOfImage` error when called
Expand All @@ -2386,33 +2395,21 @@ mod tests {
reader
.next_frame(&mut buf)
.expect("Expecting no error for 1st fdAT frame");
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
0
);
assert_eq!(get_fctl_sequence_number(&reader), 0);

reader
.next_frame(&mut buf)
.expect("Expecting no error for 2nd fdAT frame");
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
2
);
assert_eq!(get_fctl_sequence_number(&reader), 2);

let err = reader
.next_frame(&mut buf)
.expect_err("Main test - expecting error");
assert!(matches!(err, DecodingError::Parameter(_)));
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// Tests that after decoding a whole frame via [`Reader.next_row`] the call to
Expand All @@ -2422,41 +2419,125 @@ mod tests {
let mut reader = create_reader_of_ihdr_actl_fctl_idat_fctl_fdat();
let mut buf = vec![0; reader.output_buffer_size()];

assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
0
);
assert_eq!(get_fctl_sequence_number(&reader), 0);
while let Some(_) = reader.next_row().unwrap() {}
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
0
);
assert_eq!(get_fctl_sequence_number(&reader), 0);

buf.fill(0x0f);
reader
.next_frame(&mut buf)
.expect("Expecting no error from next_frame call");

// Verify if we have read the next `fcTL` chunk + repopulated `buf`:
assert_eq!(
reader
.info()
.frame_control
.as_ref()
.unwrap()
.sequence_number,
1
);
assert_eq!(get_fctl_sequence_number(&reader), 1);
assert!(buf.iter().any(|byte| *byte != 0x0f));
}

/// Tests that after decoding a whole frame via [`Reader.next_row`] it is possible
/// to use [`Reader.next_row`] to decode the next frame (by using the `next_frame_info` API to
/// advance to the next frame when `next_row` returns `None`).
#[test]
fn test_row_by_row_of_two_frames() {
let mut reader = create_reader_of_ihdr_actl_fctl_idat_fctl_fdat();

let mut rows_of_frame1 = 0;
assert_eq!(get_fctl_sequence_number(&reader), 0);
while let Some(_) = reader.next_row().unwrap() {
rows_of_frame1 += 1;
}
assert_eq!(rows_of_frame1, 16);
assert_eq!(get_fctl_sequence_number(&reader), 0);

let mut rows_of_frame2 = 0;
assert_eq!(reader.next_frame_info().unwrap().sequence_number, 1);
assert_eq!(get_fctl_sequence_number(&reader), 1);
while let Some(_) = reader.next_row().unwrap() {
rows_of_frame2 += 1;
}
assert_eq!(rows_of_frame2, 16);
assert_eq!(get_fctl_sequence_number(&reader), 1);

let err = reader
.next_frame_info()
.expect_err("No more frames - expecting error");
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// This test is similar to `test_next_frame_polling_after_end_idat_part_of_animation`, but it
/// uses `next_frame_info` calls to read to the next `fcTL` earlier - before the next call to
/// `next_frame` (knowing `fcTL` before calling `next_frame` may be helpful to determine the
/// size of the output buffer and/or to prepare the buffer based on the `DisposeOp` of the
/// previous frames).
#[test]
fn test_next_frame_info_after_next_frame() {
let mut reader = create_reader_of_ihdr_actl_fctl_idat_fctl_fdat();
let mut buf = vec![0; reader.output_buffer_size()];

assert_eq!(get_fctl_sequence_number(&reader), 0);
reader
.next_frame(&mut buf)
.expect("Expecting no error for IDAT frame");

// `next_frame` doesn't advance to the next `fcTL`.
assert_eq!(get_fctl_sequence_number(&reader), 0);

// But `next_frame_info` can be used to go to the next `fcTL`.
assert_eq!(reader.next_frame_info().unwrap().sequence_number, 1);
assert_eq!(get_fctl_sequence_number(&reader), 1);

reader
.next_frame(&mut buf)
.expect("Expecting no error for fdAT frame");
assert_eq!(get_fctl_sequence_number(&reader), 1);

let err = reader
.next_frame_info()
.expect_err("Main test - expecting error");
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}

/// This test is similar to `test_next_frame_polling_after_end_idat_not_part_of_animation`, but
/// it uses `next_frame_info` to skip the `IDAT` frame entirely + to move between frames.
#[test]
fn test_next_frame_info_to_skip_first_frame() {
let mut reader = create_reader_of_ihdr_actl_idat_fctl_fdat_fctl_fdat();
let mut buf = vec![0; reader.output_buffer_size()];

// First (IDAT) frame doesn't have frame control info, which means
// that it is not part of the animation.
assert!(reader.info().frame_control.is_none());

// `next_frame_info` can be used to skip the IDAT frame (without first having to separately
// discard the image data - e.g. by also calling `next_frame` first).
assert_eq!(reader.next_frame_info().unwrap().sequence_number, 0);
assert_eq!(get_fctl_sequence_number(&reader), 0);
reader
.next_frame(&mut buf)
.expect("Expecting no error for 1st fdAT frame");
assert_eq!(get_fctl_sequence_number(&reader), 0);

// Get the `fcTL` for the 2nd frame.
assert_eq!(reader.next_frame_info().unwrap().sequence_number, 2);
reader
.next_frame(&mut buf)
.expect("Expecting no error for 2nd fdAT frame");
assert_eq!(get_fctl_sequence_number(&reader), 2);

let err = reader
.next_frame_info()
.expect_err("Main test - expecting error");
assert!(
matches!(&err, DecodingError::Parameter(_)),
"Unexpected kind of error: {:?}",
&err,
);
}
}

0 comments on commit ffed4de

Please sign in to comment.