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

Read WebP duration after opening #7311

Merged
merged 7 commits into from Aug 11, 2023
Merged

Read WebP duration after opening #7311

merged 7 commits into from Aug 11, 2023

Conversation

k128
Copy link
Contributor

@k128 k128 commented Jul 31, 2023

Many WebP images don't initially have duration in their info dict. This automatically adds that information during image open.

@k128 k128 changed the title Update WebPImagePlugin.py Automatically load webp duration Jul 31, 2023
@radarhere radarhere changed the title Automatically load webp duration Automatically load WebP duration Jul 31, 2023
@radarhere
Copy link
Member

I've created k128#1 with some suggestions.

@radarhere radarhere changed the title Automatically load WebP duration Read WebP duration after opening Aug 1, 2023
_decoder.get_next() may return None
@radarhere radarhere merged commit 39d866b into python-pillow:main Aug 11, 2023
53 checks passed
@homm
Copy link
Member

homm commented Sep 18, 2023

I'm sorry that I'm writing this so late, I was going to test this by myself, but didn't find a time.

My assumption that _decoder.get_next() call actually decodes the image bitmap. For me this is one of the key concepts in Pillow: you can safely open the image file without decoding the whole image (which is much more expensive operation) and get any info from the file.

radarhere added a commit to radarhere/Pillow that referenced this pull request Sep 18, 2023
This reverts commit 39d866b, reversing
changes made to f39f74f.
@radarhere
Copy link
Member

radarhere commented Sep 18, 2023

As long as feedback is received before a release, I'm happy.

Looking at https://developers.google.com/speed/webp/docs/container-api, I don't see another way to get the timestamp. So your suggestion is simply to revert this? I've created #7406.

I'll ping @k128 to be abundantly clear what is happening.

@homm
Copy link
Member

homm commented Sep 29, 2023

@k128 unfortunately we reverted this due to significant performance degradation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants