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.

Sign up for GitHub

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

Jump to bottom

Use Capsule for WEBP saving #8386

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Use Capsule for WEBP saving #8386

wants to merge 8 commits into from

Conversation

homm
Copy link
Member

@homm homm commented Sep 16, 2024 •
edited
Loading

Adoption of #8341 ideas

Improvements

  • Doesn't create extra image copy (.tobytes("raw", rawmode))
  • Uses the same more appropriate API .has_transparency_data for save and save_all.
  • Supports saving RGBX mode without conversion
  • Adds ImagingSectionEnter/Leave for WebPAnimEncoder

@homm
Use im.has_transparency_data for webp._save_all
0a8e6db 
Also:
remove _VALID_WEBP_MODES and _VALID_WEBP_LEGACY_MODES consts
RGBX is not faster RGB since demands more bandwidth
Do not convert to str paths in tests
@homm
call _webp.WebPEncode with ptr
8bb3134
@homm
ImagingSectionEnter for WebPAnimEncoder
0962b46
@homm
import_frame for anim_encoder_add
4d271c8
@homm homm added the WebP label Sep 16, 2024
radarhere
radarhere reviewed Sep 16, 2024
View reviewed changes
Tests/test_file_webp.py Outdated Show resolved Hide resolved
@Yay295
Copy link
Contributor

Yay295 commented Sep 16, 2024

You're essentially using _VALID_WEBP_MODES in two locations, but you removed that constant. I assume you inlined them when _VALID_WEBP_MODES and _VALID_WEBP_LEGACY_MODES were each only used in one place, but it seems like _VALID_WEBP_MODES could be kept. Also it could just be a Set instead of a Dict with True values.

@homm
Copy link
Member Author

homm commented Sep 17, 2024

@Yay295 I've removed this constants primary since it's mistyping. Now I'm added separate _convert_frame routine for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radarhere radarhere radarhere left review comments

Assignees
No one assigned
Labels
WebP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@homm @Yay295 @radarhere