Remove AnimationDecoder trait

Author: 197g

src/codecs/gif.rs

@@ -4,6 +4,7 @@
 //!
 //! # Related Links
 //! * <http://www.w3.org/TR/PNG/> - The PNG Specification
+use core::num::NonZeroU32;
 use std::borrow::Cow;
 use std::io::{BufRead, Seek, Write};
 
@@ -16,8 +17,11 @@ use crate::error::{
     ParameterErrorKind, UnsupportedError, UnsupportedErrorKind,
 };
 use crate::io::decoder::DecodedMetadataHint;
-use crate::io::{DecodedImageAttributes, DecoderAttributes, SequenceControl};
+use crate::io::{
+    DecodedAnimationAttributes, DecodedImageAttributes, DecoderAttributes, SequenceControl,
+};
 use crate::math::Rect;
+use crate::metadata::LoopCount;
 use crate::utils::vec_try_with_capacity;
 use crate::{
     DynamicImage, GenericImage, GenericImageView, ImageDecoder, ImageEncoder, ImageFormat,
@@ -256,6 +260,11 @@ impl<R: BufRead + Seek> ImageDecoder for PngDecoder<R> {
         }
     }
 
+    /// Only for [`ApngDecoder`].
+    fn animation_attributes(&mut self) -> Option<DecodedAnimationAttributes> {
+        None
+    }
+
     fn icc_profile(&mut self) -> ImageResult<Option<Vec<u8>>> {
         let reader = self.ensure_reader_and_header()?;
         Ok(reader.info().icc_profile.as_ref().map(|x| x.to_vec()))
@@ -361,11 +370,10 @@ impl<R: BufRead + Seek> ImageDecoder for PngDecoder<R> {
     }
 }
 
-/// An [`AnimationDecoder`] adapter of [`PngDecoder`].
+/// An animated adapter of [`PngDecoder`].
 ///
 /// See [`PngDecoder::apng`] for more information.
 ///
-/// [`AnimationDecoder`]: ../trait.AnimationDecoder.html
 /// [`PngDecoder`]: struct.PngDecoder.html
 /// [`PngDecoder::apng`]: struct.PngDecoder.html#method.apng
 pub struct ApngDecoder<R: BufRead + Seek> {
@@ -602,20 +610,6 @@ impl<R: BufRead + Seek> ApngDecoder<R> {
     }
 }
 
-/*
-impl<'a, R: BufRead + Seek + 'a> AnimationDecoder<'a> for ApngDecoder<R> {
-    fn loop_count(&self) -> LoopCount {
-        match self.inner.reader.info().animation_control() {
-            None => LoopCount::Infinite,
-            Some(actl) if actl.num_plays == 0 => LoopCount::Infinite,
-            Some(actl) => LoopCount::Finite(
-                NonZeroU32::new(actl.num_plays).expect("num_plays should be non-zero"),
-            ),
-        }
-    }
-}
-*/
-
 impl<R: BufRead + Seek> ImageDecoder for ApngDecoder<R> {
     fn attributes(&self) -> DecoderAttributes {
         DecoderAttributes {
@@ -624,6 +618,24 @@ impl<R: BufRead + Seek> ImageDecoder for ApngDecoder<R> {
         }
     }
 
+    fn animation_attributes(&mut self) -> Option<DecodedAnimationAttributes> {
+        let count = if let Ok(reader) = self.inner.ensure_reader_and_header() {
+            reader.info().animation_control()
+        } else {
+            return None;
+        };
+
+        let loop_count = match count {
+            None => LoopCount::Infinite,
+            Some(actl) if actl.num_plays == 0 => LoopCount::Infinite,
+            Some(actl) => LoopCount::Finite(
+                NonZeroU32::new(actl.num_plays).expect("num_plays should be non-zero"),
+            ),
+        };
+
+        Some(DecodedAnimationAttributes { loop_count })
+    }
+
     fn peek_layout(&mut self) -> ImageResult<ImageLayout> {
         self.inner.peek_layout()
     }

src/codecs/png.rs

@@ -1,29 +1,29 @@
-use std::io::{BufRead, Read, Seek};
+use std::io::{BufRead, Seek};
 use std::num::NonZeroU32;
 
 use image_webp::LoopCount;
 
-use crate::buffer::ConvertBuffer;
-use crate::error::{DecodingError, ImageError, ImageResult};
-use crate::io::decoder::DecodedMetadataHint;
-use crate::io::{DecodedImageAttributes, DecoderAttributes};
-use crate::{
-    AnimationDecoder, ColorType, Delay, Frame, Frames, ImageDecoder, ImageFormat, RgbImage, Rgba,
-    RgbaImage,
+use crate::error::{DecodingError, ImageError, ImageResult, ParameterError, ParameterErrorKind};
+use crate::io::{
+    DecodedAnimationAttributes, DecodedImageAttributes, DecodedMetadataHint, DecoderAttributes,
+    SequenceControl,
 };
+use crate::{ColorType, Delay, ImageDecoder, ImageFormat, Rgba};
 
 /// WebP Image format decoder.
 ///
 /// Supports both lossless and lossy WebP images.
 pub struct WebPDecoder<R> {
     inner: image_webp::WebPDecoder<R>,
+    current: u32,
 }
 
 impl<R: BufRead + Seek> WebPDecoder<R> {
     /// Create a new `WebPDecoder` from the Reader `r`.
     pub fn new(r: R) -> ImageResult<Self> {
         Ok(Self {
             inner: image_webp::WebPDecoder::new(r).map_err(ImageError::from_webp_decode)?,
+            current: 0,
         })
     }
 
@@ -52,6 +52,17 @@ impl<R: BufRead + Seek> ImageDecoder for WebPDecoder<R> {
         }
     }
 
+    fn animation_attributes(&mut self) -> Option<DecodedAnimationAttributes> {
+        let loop_count = match self.inner.loop_count() {
+            LoopCount::Forever => crate::metadata::LoopCount::Infinite,
+            LoopCount::Times(n) => crate::metadata::LoopCount::Finite(
+                NonZeroU32::new(n.get().into()).expect("LoopCount::Times should be non-zero"),
+            ),
+        };
+
+        Some(DecodedAnimationAttributes { loop_count })
+    }
+
     fn peek_layout(&mut self) -> ImageResult<crate::ImageLayout> {
         let (width, height) = self.inner.dimensions();
 
@@ -67,14 +78,35 @@ impl<R: BufRead + Seek> ImageDecoder for WebPDecoder<R> {
     }
 
     fn read_image(&mut self, buf: &mut [u8]) -> ImageResult<DecodedImageAttributes> {
+        let is_animated = self.inner.is_animated();
+
+        if is_animated && self.current == self.inner.num_frames() {
+            return Err(ImageError::Parameter(ParameterError::from_kind(
+                ParameterErrorKind::NoMoreData,
+            )));
+        }
+
         let layout = self.peek_layout()?;
         assert_eq!(u64::try_from(buf.len()), Ok(layout.total_bytes()));
 
-        self.inner
-            .read_image(buf)
-            .map_err(ImageError::from_webp_decode)?;
+        // `read_frame` panics if the image is not animated.
+        let delay = if is_animated {
+            let delay = self
+                .inner
+                .read_frame(buf)
+                .map_err(ImageError::from_webp_decode)?;
+            Some(Delay::from_numer_denom_ms(delay, 1))
+        } else {
+            self.inner
+                .read_image(buf)
+                .map_err(ImageError::from_webp_decode)?;
+            None
+        };
+
+        self.current += 1;
 
         Ok(DecodedImageAttributes {
+            delay,
             ..DecodedImageAttributes::default()
         })
     }
@@ -99,69 +131,23 @@ impl<R: BufRead + Seek> ImageDecoder for WebPDecoder<R> {
             .xmp_metadata()
             .map_err(ImageError::from_webp_decode)
     }
-}
 
-impl<'a, R: 'a + BufRead + Seek> AnimationDecoder<'a> for WebPDecoder<R> {
-    fn loop_count(&self) -> crate::metadata::LoopCount {
-        match self.inner.loop_count() {
-            LoopCount::Forever => crate::metadata::LoopCount::Infinite,
-            LoopCount::Times(n) => crate::metadata::LoopCount::Finite(
-                NonZeroU32::new(n.get().into()).expect("LoopCount::Times should be non-zero"),
-            ),
+    fn more_images(&self) -> SequenceControl {
+        if self.current == self.inner.num_frames() {
+            SequenceControl::None
+        } else {
+            SequenceControl::MaybeMore
         }
     }
-
-    fn into_frames(self) -> Frames<'a> {
-        struct FramesInner<R: Read + Seek> {
-            decoder: WebPDecoder<R>,
-            current: u32,
-        }
-        impl<R: BufRead + Seek> Iterator for FramesInner<R> {
-            type Item = ImageResult<Frame>;
-
-            fn next(&mut self) -> Option<Self::Item> {
-                if self.current == self.decoder.inner.num_frames() {
-                    return None;
-                }
-                self.current += 1;
-                let (width, height) = self.decoder.inner.dimensions();
-
-                let (img, delay) = if self.decoder.inner.has_alpha() {
-                    let mut img = RgbaImage::new(width, height);
-                    match self.decoder.inner.read_frame(&mut img) {
-                        Ok(delay) => (img, delay),
-                        Err(image_webp::DecodingError::NoMoreFrames) => return None,
-                        Err(e) => return Some(Err(ImageError::from_webp_decode(e))),
-                    }
-                } else {
-                    let mut img = RgbImage::new(width, height);
-                    match self.decoder.inner.read_frame(&mut img) {
-                        Ok(delay) => (img.convert(), delay),
-                        Err(image_webp::DecodingError::NoMoreFrames) => return None,
-                        Err(e) => return Some(Err(ImageError::from_webp_decode(e))),
-                    }
-                };
-
-                Some(Ok(Frame::from_parts(
-                    img,
-                    0,
-                    0,
-                    Delay::from_numer_denom_ms(delay, 1),
-                )))
-            }
-        }
-
-        Frames::new(Box::new(FramesInner {
-            decoder: self,
-            current: 0,
-        }))
-    }
 }
 
 impl ImageError {
     fn from_webp_decode(e: image_webp::DecodingError) -> Self {
         match e {
             image_webp::DecodingError::IoError(e) => ImageError::IoError(e),
+            image_webp::DecodingError::NoMoreFrames => {
+                ImageError::Parameter(ParameterError::from_kind(ParameterErrorKind::NoMoreData))
+            }
             _ => ImageError::Decoding(DecodingError::new(ImageFormat::WebP.into(), e)),
         }
     }

src/codecs/webp/decoder.rs

@@ -21,7 +21,8 @@ pub type Limits = limits::Limits;
 pub type LimitSupport = limits::LimitSupport;
 
 pub use decoder::{
-    DecodedImageAttributes, DecodedMetadataHint, DecoderAttributes, SequenceControl,
+    DecodedAnimationAttributes, DecodedImageAttributes, DecodedMetadataHint, DecoderAttributes,
+    SequenceControl,
 };
 
 /// Adds `read_exact_vec`

src/io.rs

@@ -1,4 +1,3 @@
-use crate::animation::Frames;
 use crate::color::ExtendedColorType;
 use crate::error::ImageResult;
 use crate::metadata::{LoopCount, Orientation};
@@ -50,6 +49,15 @@ pub trait ImageDecoder {
         DecoderAttributes::default()
     }
 
+    /// Retrieve animation attributes.
+    ///
+    /// You should check [`DecoderAttributes::is_animated`] before calling this method. It will
+    /// only be available on animated images. Additionally, most file formats store the metadata in
+    /// the header which might not be read until after calling [`ImageDecoder::peek_layout`].
+    fn animation_attributes(&mut self) -> Option<DecodedAnimationAttributes> {
+        None
+    }
+
     /// Consume the header of the image, determining the image's layout.
     ///
     /// This must be called before a call to [`Self::read_image`] to ensure that the initial
@@ -158,6 +166,22 @@ pub struct DecoderAttributes {
     pub iptc: DecodedMetadataHint,
 }
 
+/// Additional attributes of animated image sequences.
+#[derive(Clone, Debug)]
+#[non_exhaustive]
+pub struct DecodedAnimationAttributes {
+    /// Loop count of the animated image.
+    pub loop_count: LoopCount,
+}
+
+impl Default for DecodedAnimationAttributes {
+    fn default() -> Self {
+        Self {
+            loop_count: LoopCount::Infinite,
+        }
+    }
+}
+
 /// Additional attributes of an image available after decoding.
 ///
 /// The [`Default`] is implemented and returns a value suitable for very basic images from formats
@@ -262,14 +286,6 @@ impl<T: ?Sized + ImageDecoder> ImageDecoder for Box<T> {
     }
 }
 
-/// `AnimationDecoder` trait
-pub trait AnimationDecoder<'a> {
-    /// Consume the decoder producing a series of frames.
-    fn into_frames(self) -> Frames<'a>;
-    /// Loop count of the animated image.
-    fn loop_count(&self) -> LoopCount;
-}
-
 #[cfg(test)]
 mod tests {
     use super::{DecodedImageAttributes, ImageDecoder, ImageResult};

src/io/decoder.rs

@@ -162,7 +162,7 @@ pub use crate::images::dynimage::{
 pub use crate::io::free_functions::{guess_format, load, save_buffer, save_buffer_with_format};
 
 pub use crate::io::{
-    decoder::{AnimationDecoder, ImageDecoder},
+    decoder::ImageDecoder,
     encoder::ImageEncoder,
     format::ImageFormat,
     image_reader_type::{ImageReader, ImageReaderOptions},

src/lib.rs

@@ -172,7 +172,7 @@ enum ExifEndian {
 }
 
 /// The number of times animated image should loop over.
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 pub enum LoopCount {
     /// Loop the image Infinitely
     Infinite,

src/metadata.rs

@@ -3,7 +3,7 @@ use std::io::{BufReader, Cursor};
 use std::path::PathBuf;
 
 #[cfg(feature = "webp")]
-use image::{codecs::webp::WebPDecoder, AnimationDecoder};
+use image::{codecs::webp::WebPDecoder, ImageReader};
 
 const BASE_PATH: [&str; 2] = [".", "tests"];
 const IMAGE_DIR: &str = "images";
@@ -60,10 +60,11 @@ fn check_webp_frames_regressions() {
             .unwrap()
             .num_frames() as usize;
         let decoder = WebPDecoder::new(cursor).unwrap();
+        let reader = ImageReader::from_decoder(Box::new(decoder));
         // The `take` guards against a potentially infinitely running iterator.
         // Since we take `frame_count + 1`, we can assume that the last iteration already returns `None`.
         // We then check that each frame has been decoded successfully.
-        let decoded_frames_count = decoder
+        let decoded_frames_count = reader
             .into_frames()
             .take(frame_count + 1)
             .enumerate()