-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add JPEG XL Open/Read support via libjxl #7848
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
May you please commit the images as LFS? |
Do you mean those |
They already setup lfs and they only use for other larger stuff. Let it stay as you did. |
Removed jxl feature
for more information, see https://pre-commit.ci
Mac OS builds were failing because clang complained about goto labels being declared before variables in scope. |
src/PIL/JxlImagePlugin.py
Outdated
elif n_frames > 0: | ||
self.n_frames = n_frames | ||
self._tps_dur_secs = tps_num / tps_denom | ||
# TODO: handle libjxl timecods |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# TODO: handle libjxl timecods | |
# TODO: handle libjxl time codes |
?
src/PIL/__init__.py
Outdated
@@ -47,6 +47,7 @@ | |||
"IptcImagePlugin", | |||
"JpegImagePlugin", | |||
"Jpeg2KImagePlugin", | |||
"JxlImagePlugin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming things: like other plugins, shall we use the name of the format rather than the extension?
"JxlImagePlugin", | |
"JpegXlImagePlugin", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. Not sure why I decided to go with "jxl" from the beginning.
Would you want me to change all the references to jxl (including tests, function/class names, feature.jxl) or only plugin and .c
extension filename?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think let's just use jxl for the filename extension (so the Tests/images/jxl/
) dir is fine too) and of course the MIME type, and change the rest.
Tests/test_jxl_leaks.py
Outdated
# TODO: lower the limit, I'm not sure what is correct limit | ||
# since I have libjxl debug system-wide | ||
mem_limit = 16 * 1024 # kb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we find out the correct limit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it on release version of libjxl and I've set it to 6 megabytes which seem to work well, that's also similar to memory usage of djxl
for decoding hopper.jxl
.
I think libjxl memory usage could depend also on number of decoding threads used. By default we use all available cores.
src/PIL/JxlImagePlugin.py
Outdated
# jpeg xl does some weird shenanigans when storing exif | ||
# it omits first 6 bytes of tiff header but adds 4 byte offset instead | ||
if len(exif) <= 4: | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test case for this and _getexit
?
src/PIL/JxlImagePlugin.py
Outdated
|
||
def seek(self, frame): | ||
if not self._seek_check(frame): | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And a test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already test_file_jxl_animated.py
test which also does seeking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return
isn't covered: https://app.codecov.io/gh/python-pillow/Pillow/pull/7848/blob/src/PIL/JxlImagePlugin.py
I've merged in And this is failing to build for me with:
Full log: log2.txt |
Thanks for reporting. The |
src/PIL/JxlImagePlugin.py
Outdated
if is_jxl and not SUPPORTED: | ||
return "image file could not be identified because JXL support not installed" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exception instead of return string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we raise SyntaxError
like here in Jpeg2K plugin?
I based my work on WebP plugin which just returns string.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
It would be excellent to see some progress in this area. I am currently working on introducing JPEG XL support to https://immich.app for previews and thumbnails (as Python is used for ML). However, an additional plugin pillow-jpegxl-plugin complicates the deployment process. |
Could you still add support for 16 bit single channel files? Such images are commonly used in fluorescent microscopy, and storing them in jxl instead of tiff or png would lead to substantial storage savings. Pillow is used by various relevant software including imageio and CellProfiler. In
and in
should be changed into:
|
Any relation-to or overlap-with #1888 here? 🤔 |
@develar Sounds awesome, thank you for working on JPEG XL support in Immich. I presume that merging this PR is now up to Pillow core developers since all checks have passed. However by now we would probably need to resolve some merge conflicts.
@bgorissen Yes, of course it's possible. Thank you for detailed description of changes. Could you provide some image(s) to test this mode? |
@olokelo I 2nd @bgorissen request around 16 bit single channel support. I work with imaging systems that produce 16 bit single channel images, like thermal and depth, and the JXL format provides some significant cost savings storage-wise compared to our 16 bit single channel PNG format we are currently using. I would love to see the JXL ecosystem mature and this plugin would go a long way towards our adoption of the format |
@olokelo I've attached a 16-bits grayscale jxl file (zipped because Github does not support jxl). You could crop it to create a smaller test file. The file was converted from a public tiff image from the dataset cpg0011 (Laber et al., 2023), available from the Cell Painting Gallery on the Registry of Open Data on AWS (https://registry.opendata.aws/cellpainting-gallery/), and released under CC0 1.0 Universal (which puts it in the public domain). subcutaneous__D14__45adb24f-23b7-43f8-9612-0a7ecaa22ada__BR00101077__r02c03f01p01-ch1sk1fk1fl1.zip |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This is very good work @olokelo! Is there any plan to add support for writing to JXL from Pillow? |
@hugovk: It certainly would be nice to have this as feature in Pillow 11, wouldn't it? |
The iPhone 16 Pro models will support the JPEG-XL file format, according to code found in iOS 18. I really hope this will be the next standard in photography, I would really like to see support for this in Pillow 11 by default. |
Helps #4247
This PR enables opening and reading JPEG XL images and animations.
Supported image modes are: RGB, RGBA, RGBa, L, LA, La.
A relatively recent libjxl version is needed to compile Pillow with libjxl support.
The main changes are the addition of
_jxl.c
andJxlImagePlugin.py
.I'm also the author of jxlpy so this PR was influenced by the work of contributors there. This PR is also largely based on
WebPImagePlugin.py
which had similar implementation.Why?
JPEG XL has recently seen increased adoption especially in Apple ecosystem. A lot of users are requesting Pillow support for JPEG XL as their products use Pillow and need to be able to handle
jxl
files.I'm open to suggestions and comments. I understand such change would need a lot of testing and probably changes. After all Pillow would need to become somewhat dependent on libjxl. Creating documentation will not be a big problem however I decided to wait for feedback from Pillow core developers.