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

Initial fuzzing harness for animated images #32

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

Shnatsel
Copy link
Contributor

@Shnatsel Shnatsel commented Dec 27, 2023

Animated images with the "loop forever" flag currently cause an infinite loop in the decoder.

The AFL seed images didn't seem to contain any animated ones, so I made these seed images myself from the sample image linked from the official Google documentation: webp_animated_fuzz_seeds.tar.gz

Resolves (?) #31

@fintelia
Copy link
Contributor

At the moment, next_frame intentionally starts over back at the beginning of the animation until it has concluded loop_count many passes. With the "loop forever" flag it'll return each frame an infinite number of times.

For displaying the image, that seems like desirable behavior. However, if you're trying to collect all the frames of the animation (say to convert them to a different format) then it is far less helpful. Perhaps the right approach would be to expose extra methods to query the loop count and number of frames?

@Shnatsel
Copy link
Contributor Author

I think there needs to be a lower-level interface that exposes all the frames and separately the data on how many times they should be looped, and optionally a higher-level one that does the right thing for displaying the image (that's how the current interface works).

I wonder, does the current interface decode all the frames once and store them in memory, or does it decode them over and over?

@fintelia
Copy link
Contributor

The current interface decodes each frame as it is requested and also does compositing at the same time. Which can involve blending pixels with the values from previous frames. And the raw frame data can also be limited to only the "cropped" section that changed from the previous frame so we account for that as well.

Other than not automatically looping the animation, are you imagining there would be any other differences with the lower-level interface?

@Shnatsel
Copy link
Contributor Author

Well, an image editing tool like GIMP would require access to the raw data of the layers - but I don't think anyone else would care. I think just not automatically looping is a good start. We can add the even lower-level interface useful for image editors once someone who wants to use it actually shows up. I feel it's easy to design something useless in absence of actual users who can tell us what they need.

@fintelia
Copy link
Contributor

I'll go ahead and merge this, and then API improvements can come as a followup.

@fintelia fintelia merged commit 67a0f4e into image-rs:main Dec 28, 2023
9 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