WebP Extended animation format needs to be rendered into frames… #1925
Conversation
…e it can be copied. Previous code was copying raw webp frames which can be smaller than the canvas. Fixes image-rs#1856.
|
The CI seems to detect one failure with |
| @@ -321,16 +321,35 @@ impl<'a, R: 'a + Read> ImageDecoder<'a> for WebPDecoder<R> { | |||
| } | |||
|
|
|||
| fn read_image(self, buf: &mut [u8]) -> ImageResult<()> { | |||
| assert_eq!(u64::try_from(buf.len()), Ok(self.total_bytes())); | |||
There was a problem hiding this comment.
Let's not get rid of this assert. The spec for read_image guarantees a panic if this condition doesn't hold.
Theoretically the values of {vp8_frame, lossless_frame, extended}.get_buf_size() should match, but there could be bugs
There was a problem hiding this comment.
Why doesn't the spec make use of ImageResult instead? Much cleaner.
Also the bug I'm trying to fix happily passed this check and then panicked at copy_from_slice() in the fill_buf() function. So didn't do much good there.
|
I still think that the whole concept of the WebP animation decoder, decoding all frames before being able to show them, is fundamentally broken for image viewers. See #1924 and https://gitlab.gnome.org/GNOME/Incubator/loupe/-/issues/160 Not sure if it's in the scope of this MR to fix it, but a bunch of things happening in this MR would have to be changed again when adding frame-by-frame decoding. |
| @@ -147,6 +147,54 @@ impl ExtendedImage { | |||
| Frames::new(Box::new(frame_iter)) | |||
| } | |||
|
|
|||
| pub(crate) fn as_frames<'a>(&'a self) -> Frames<'a> { | |||
There was a problem hiding this comment.
Thanks for adding the as_frames API. That's very useful. However, does the code have to be duplicated with into_frames, or could into_frames cust call as_frames?
|
I don't have the time to do the perfect fix, happy to drop this PR as long as the bug gets fixed. @sophie-h It does not decode all frames, I was just trying to fix the bug while changing as little as possible on the existing code. |
|
Superseded by #1959 |
before it can be copied.
Previous code was copying raw webp frames which can be smaller than the canvas.
Fixes #1856.
I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to choose either at their option.