RawTherapee
RawTherapee copied to clipboard
Beep6581
Add ability to import JXL images
This allows RawTherapee to import JXL images. Addresses [the import portion] of #6273.
@xiota Thanks for this!
Addresses half of #6273.
Which half? And can you elaborate a bit about what is still missing?
@xiota Thanks for your contribution! :+1: There seem to be some (error) conditions under which ImageIO::loadJxl() leaks allocated memory in buffer. Could you double-check?
@Floessie Thanks for letting me know. I added a couple lines to free the buffer.
@xiota Just out of curiosity, do you have a particular reason to force-push the last few commits instead of committing regularly with descriptions? We can squash-merge things in the end so the master commit log won't be polluted.
@Thanatomanic Was just to keep the commit log clean. Didn't know you would squash merge.
This was working yesterday, but it isn't working after I recompiled today without making any changes.
It's working again. I was trying to make compiling with libjxl optional, but something was being cached, so it looked like it was working when it wasn't. Someone else will have to make the changes needed to make cmake detect and load libjxl as needed. (Right now it's required.)
One of the disabled extensions is png. Why is that?
There was a time, where some png files led to crashes of RT when starting RT on a folder which such a file. I think we can enable png per default now. Iirc the crashes should be fixed...
I remember now. It was issue #3794 right? There was also a question about it in #4915.
Should I reenable PNG in this PR? Also, any advice/help for conditional compilation?
I think we can enable png per default now.
Yes, but not in this PR imo (keep it cleanly focused on JXL).
Someone else will have to make the changes needed to make cmake detect and load libjxl as needed. (Right now it's required.)
@xiota Well, this is an issue indeed. I found out that on Windows MSYS2 does not provide the libjxl package yet. So anyone on Windows should first manually compile libjxl or we should start working with submodules or something. Not very convenient.
My suggestion is to create a build option WITH_JXL here https://github.com/Beep6581/RawTherapee/blob/0e122a485a2339bf2331a7c54a2f5cb2ee2f2dd3/CMakeLists.txt#L197-L225
and set it to OFF by default.
Then you should be able to conditionally check for the package, like here
https://github.com/Beep6581/RawTherapee/blob/0e122a485a2339bf2331a7c54a2f5cb2ee2f2dd3/CMakeLists.txt#L496-L498
But I'm not at all familiar with the intricacies of CMake, so perhaps this doesn't work at all...
Thanks for this effort! I would love to have JPEG XL import support in RawTherapee, what's needed to move this PR forward?
Builds fine in Manjaro against libjxl-0.7.0 and libjxl_threads-0.7.0, and opens a sample .jxl file fine, though without any metadata.
though without any metadata
Is it to be expected that once we support evix2 this automagically works for jxl too?
Is it to be expected that once we support evix2 this automagically works for jxl too?
If exiv2 is built w/ BMFF support, then yes.
Converting this to a draft because the GitHub Actions workflows still need to be updated to use the JXL library. Making JXL optional may also be necessary for the moment.
@xiota Would you be able to further work on this? If so, is there anything you need to move this forward?
Issues I think holding this back:
- [x] Conditional compilation to build libjxl support only when the library is present.
- [x] Windows library availability?
- [ ] I'd need a refresher on how rawtherapee works
- [ ] I'd need a refresher on how libjxl works
- [x] update and rebase to current rawtherapee
- [x] update for current libjxl
- [ ] I don't understand how libjxl color management works
Windows library availability?
No problem w/ this one: https://packages.msys2.org/base/mingw-w64-libjxl
I don't understand how libjxl color management works
You are not alone on that one 😉
I force pushed to rebase to the current dev branch. It's updated to the current libjxl API (only one function changed). I also figured out how to make libjxl optional, so if it's not installed, compilation should continue with jpeg xl disabled.
@Lawrence37 Would you mind running this through workflows to check that the cmake files are compatible?
I updated them to add a tristate option WITH_JXL. Default is AUTO, which enables JXL import when libjxl is available. YES force enables the feature, showing error when libjxl is not available. NO force disables the feature, even when libjxl is found.
I looked at the workflow files, and it seems for Ubuntu and MSYS, libjxl can be added to the depends lists. However, Ubuntu has libjxl-dev 0.7.0, while MSYS has libjxl 0.10.2. When I rebased, I updated and test built with 0.10.2.
The workflows will probably not support jxl without installing libjxl, but I'll run them anyway.
@Lawrence37 Thanks for running the workflows earlier. They all passed (deps not installed and build completed with jxl export disabled).
Would you mind running the workflows again? I added libjxl to the dep lists. Hoping they'll build with jxl export enabled.
Looks like... macOS and Windows workflows succeeded. I suppose people using those OSes can grab the artifacts to test?
The Linux workflows failed because they're running on Ubuntu 20.04 and 22.04, but libjxl was added in 23.04. Should be available in 24.04 in a few weeks.
How would RT devs like to proceed?
It seems like there is a deb package for Ubuntu 20.04 (https://github.com/libjxl/libjxl/releases/tag/v0.10.2). We can use that or compile libjxl in the AppImage workflow. Note that the runner must be the oldest LTS Ubuntu available (currently 20.04) to maximize compatibility of the AppImage.
Is figuring out the deps in the workflows important for this PR or could it be deferred?
Conditional compilation seems to be working, so the deps could be left out of the Linux workflows until someone who knows how can update them.
The AppImage workflow generates the official AppImage for releases, so it is important that libjxl is included. Otherwise, there will be no jxl support for AppImage users.
I found the button to run workflows and got the AppImage workflow to complete successfully. The resulting appimage was able to thumbnail and load a jxl file.
It uses debs from the libjxl github repo, but libhwy is from a PPA. The highway repo doesn't publish debs, and the 22.04 deb won't install on 20.04. The debian highway package has the same problem.
I'm unable to test the codeql workflow. Assuming no mistakes, it should also work. It uses libjxl from github, and libhwy from the official Ubuntu 22.04 repo.
Nice! Getting a package from a PPA is less than ideal. I see highway can be installed using vcpkg install highway. vcpkg comes preinstalled in the Ubuntu 20.04 environment. I have never used vcpkg before, but it might be worth a shot.