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

Extended WebP image decoding #1685

Merged
merged 19 commits into from
Mar 24, 2022
Merged

Conversation

tristanphease
Copy link
Contributor

Allows for decoding of extended WebP headers(VP8X) as described here, including lossy alpha images and animated WebP files.

Integrates with the current AnimationDecoder trait for animation. One thing that I think would make sense on that trait is a way to get the loop count out of it. Also when testing, I was trying the frame offsets in the Frame::from_parts() which fit what I was doing quite well, but it never seemed to work properly when encoded to a GIF, so it just manually draws the background with the offset.

Ignores ICCP colour profiles, XMP and EXIF metadata. If #1448 happens, maybe we could integrate that in.

Feedback would be appreciated.

@fintelia

This comment was marked as resolved.

Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

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

So nice to see this PR 🥺

src/codecs/webp/decoder.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/vp8.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/extended.rs Outdated Show resolved Hide resolved
src/codecs/webp/lossless.rs Show resolved Hide resolved
Tristan Phease added 2 commits March 10, 2022 19:22
Also updated lib.rs and fixed some number conversion issues.
@tristanphease
Copy link
Contributor Author

Also, with the tests I created an animated image to test but I couldn't figure out how the reference tests check against the frames of animated images if at all. At the moment, it just checks against the first frame of the animation but ideally it would check more.

@tristanphease
Copy link
Contributor Author

@HeroicKatora Sorry, but do you have any further feedback?
I believe all your recommended changes have been resolved.

Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@HeroicKatora
Copy link
Member

All files had been marked 'viewed' for me, somehow must have missed pressing Approve before 😄

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.

4 participants