Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webp: Decode animations frame by frame #1924

Merged
merged 1 commit into from
Nov 20, 2023

Conversation

sophie-h
Copy link
Contributor

@sophie-h sophie-h commented May 5, 2023

Previously, animated WebPs could take some time
until they started playing and would consume more memory.

@sophie-h sophie-h changed the title webp: Decoder animations frame by frame webp: Decode animations frame by frame May 5, 2023
@sophie-h sophie-h force-pushed the wip/sophie-h/webp-animation branch 3 times, most recently from d41dd26 to 282a39e Compare November 18, 2023 23:39
@sophie-h sophie-h marked this pull request as ready for review November 18, 2023 23:39
@sophie-h sophie-h force-pushed the wip/sophie-h/webp-animation branch from 282a39e to ad94efe Compare November 18, 2023 23:42
@sophie-h
Copy link
Contributor Author

We can also drop the "skip invalid frames" part. But since the iterator is silently ending with errors I thought we might as well skip invalid frames.

Copy link
Contributor

@fintelia fintelia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code will change a bunch when we switch to image-webp. However, that'll have to wait for a major release (the crate relies on being able to Seek the underlying reader). So in the meantime I think this change does make sense.

Relatedly, if you have thoughts about API changes we should make for animation decoding, please do share them!

src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
@fintelia
Copy link
Contributor

I don't have strong opinions on how invalid frames should be handled, but one thing to be aware of is that frames can be composited onto the image canvas rather than replacing it. This means that portions of the animation may be left unchanged between frames, and thus skipping a single frame can potentially corrupt the displayed contents for all subsequent frames.

Previously, animated WebPs could take some time
until they started playing and would consume more memory.
@sophie-h sophie-h force-pushed the wip/sophie-h/webp-animation branch 2 times, most recently from 6a79659 to a244d84 Compare November 19, 2023 00:37
@sophie-h
Copy link
Contributor Author

I don't have strong opinions on how invalid frames should be handled

I dropped it from this MR. I don't know what the reference implementation does.

@fintelia fintelia merged commit d4748e2 into image-rs:master Nov 20, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants