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

Add ability to export JXL images #7097

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
Open

Conversation

xiota
Copy link
Contributor

@xiota xiota commented Jun 11, 2024

Closes #6273. Related #6367, #6773.

Main problem is color management. When saving with output profile other than sRGB (eg, P3):

  • Thumbnails of the resulting images look wrong, but image is correct after import.
  • Images look wrong when opened in other viewers or editors, like GIMP.

Potential improvements to add or leave for another PR...

  • Force import/export profile to sRGB. Is this possible? How? Need help.
  • Add Exif. Need metadata as a binary blob to add as box. Don't know how. Need help.
    • Exiv2 doesn't support writing BMFF metadata.
  • Update CLI for JXL conversion. Currently seems to assume -j* options are for jpeg.

Following seem okay:

  • Building with and without libjxl 0.10.2, and with WITH_JXL=no.
  • Building with libjxl 0.7.x (via dispatch-only Ubuntu 24.04 workflow)
  • Exporting with different quality settings.
  • Batch processing.

rtengine/imageio.cc Fixed Show fixed Hide fixed
rtengine/imageio.cc Fixed Show fixed Hide fixed
rtengine/imageio.cc Fixed Show fixed Hide fixed
rtengine/dcp.cc Outdated Show resolved Hide resolved
rtengine/imageio.cc Outdated Show resolved Hide resolved
@Lawrence37 Lawrence37 added scope: file format Camera or image file formats type: feature Something new to make labels Jun 15, 2024
@Lawrence37 Lawrence37 added this to the v5.11 milestone Jun 17, 2024
@xiota
Copy link
Contributor Author

xiota commented Jul 5, 2024

Force push to rebase on dev, including #7104, #7096

@Lawrence37
Copy link
Collaborator

Do you think the output profile issue is a problem with RawTherapee or GIMP? If it's a RawTherapee issue, then I think it should be addressed in this pull request unless the jxl specification says images without color profile information should be assumed to be in sRGB.

@kmilos
Copy link
Contributor

kmilos commented Jul 10, 2024

JPEG XL is very weird when it comes to color management (it takes on tasks it shouldn't IMHO) - if you do lossy compression, data is stored internally in float and "XYB" color space. The decoder can then decode it straight to any color space on the other end (sRGB usually), unless the "original" target profile is requested by the decoder (which had to be stored at encoding time of course, either as ICC or JPEG XL color space enum), and that is out of the control of the encoding app (which is the weird part, at least for me). For lossless compression, one can/should keep the "original" color profile throughout of course.

@xiota
Copy link
Contributor Author

xiota commented Jul 10, 2024

The best solution at this time, for cross-application and cross-version libjxl, would probably be to force sRGB when exporting jxl, but I do not know how. If images are intended only for use with RawTherapee with a single version of libjxl, the current state of this PR is very close to working as expected.

When exporting and importing across different versions of libjxl (0.7.x, 0.10.x), images may not display correctly. A workaround for both RawTherapee and GIMP is to assign the correct profile after opening.

Cross-version problems may be related to 0.10.x converting ICC profiles to internal representation when saving, and not delivering an ICC profile when images use internal representation (color space enum). So an image viewer, for instance, will display the image incorrectly because libjxl doesn't give it an ICC profile.

@kmilos
Copy link
Contributor

kmilos commented Jul 10, 2024

Why would you ever want to force sRGB?! It's a narrow gamut.

On export, you just need to supply libjxl w/ the profile of the data you're feeding it (ICC or enum).

So an image viewer, for instance, will display the image incorrectly because libjxl doesn't give it an ICC profile.

No, it is because it didn't ask for one from lbjxl upon decoding. That's why it's weird, it's not implicit.

@xiota
Copy link
Contributor Author

xiota commented Jul 10, 2024

Why would you ever want to force sRGB?! It's a narrow gamut.

For cross app compatibility. Nothing I currently have installed works with non-sRGB jxl, except djxl. jxl seems to use color completely differently from every other image format.

On export, you just need to supply libjxl w/ the profile of the data you're feeding it (ICC or enum).

I'm feeding it ICC. The images are coming out with enum set.

No, it is because it didn't ask for one from lbjxl upon decoding. That's why it's weird, it's not implicit.

What do I need to change to ask for one? Right now, I'm expecting the decoder to tell me whether one is available.

} else if (status == JXL_DEC_COLOR_ENCODING) {
// check for ICC profile
deleteLoadedProfileData();
embProfile = nullptr;
std::size_t icc_size = 0;
if (JXL_DEC_SUCCESS !=
#if JPEGXL_NUMERIC_VERSION < JPEGXL_COMPUTE_NUMERIC_VERSION(0, 9, 0)
JxlDecoderGetICCProfileSize(dec.get(), &format, _PROFILE_, &icc_size)
#else
JxlDecoderGetICCProfileSize(dec.get(), _PROFILE_, &icc_size)
#endif
) {
std::cerr << "Warning: JxlDecoderGetICCProfileSize failed" << std::endl;
}
if (icc_size > 0) {
icc_profile.resize(icc_size);
if (JXL_DEC_SUCCESS !=
#if JPEGXL_NUMERIC_VERSION < JPEGXL_COMPUTE_NUMERIC_VERSION(0, 9, 0)
JxlDecoderGetColorAsICCProfile(
dec.get(), &format, _PROFILE_,
icc_profile.data(), icc_profile.size())
#else
JxlDecoderGetColorAsICCProfile(
dec.get(), _PROFILE_,
icc_profile.data(), icc_profile.size())
#endif
) {
std::cerr << "Warning: JxlDecoderGetColorAsICCProfile failed" << std::endl;
} else {
embProfile = cmsOpenProfileFromMem(icc_profile.data(),
icc_profile.size());
}
} else {
std::cerr << "Warning: Empty ICC data." << std::endl;
}

@kmilos
Copy link
Contributor

kmilos commented Jul 10, 2024

Yes, that's the way. But in recent libjxl versions (can't remember when this behaviour was added exactly), that doesn't only mean an ICC profile was actually embedded at encoding - it just means the decoder can generate and supply one on the fly, which it should do in most cases, as most internally stored enum combos are known:

https://github.com/libjxl/libjxl/blob/4a3b22d2600f92d8706fb72d85d52bfee2acbd54/lib/include/jxl/decode.h#L768-L791

@xiota
Copy link
Contributor Author

xiota commented Jul 10, 2024

Using libjxl 0.10.x, sRGB and P3 JXLs exported with this PR seem to use JXL enums, no embedded ICC. They are displayed correctly in RawTherapee with this PR (made adjustment to import), darktable, and GIMP 2.99/3.x. The P3 JXL did not display correctly in GIMP 2.10. Probably problem with the plugin or limitations of GIMP 2.10 libraries. Workaround is to assign the correct profile after import.

Compiles with libjxl 0.7.x. Can export and import images. Issues mainly with non-sRGB profiles. Not doing further testing because fixing 0.7.x seems to break 0.10.x, and vice versa.

Fixing thumbnail display could be left for another PR, but may be hopeless, especially with old libjxl.

To add metadata, Exif blobs are needed. If someone tells me how to get them, I can add to this PR. Otherwise, could leave for another PR. Exiv2 does not support writing to JXL.

Updating CLI options, I could look at next, or leave for another PR.

@0fbcb238c0
Copy link

Compiles with libjxl 0.7.x. Can export and import images. Issues mainly with non-sRGB profiles. Not doing further testing because fixing 0.7.x seems to break 0.10.x, and vice versa.

0.11.x is expected in the near future according to libjxl/libjxl#3700 (comment).
Maybe that will work better?

@xiota
Copy link
Contributor Author

xiota commented Jul 11, 2024

0.11.x is expected in the near future... Maybe that will work better?

I tested RawTherapee built with libjxl 0.10.3, but run with libjxl-git 0.10.3.r227.g5ddb303a. No difference in previously observed behavior. Images open correctly with this PR, darktable, and GIMP 2.99/3.x. I did not rebuild the applications because of how time consuming it would be, but don't think it matters.

If libjxl 11.x is released before this PR is ready for merge, should workflows be updated as part of this PR or in a separate PR?

Wasn't paying close attention earlier, but darktable thumbnails are correct. So it may be worth looking at for inspiration.

Somewhat related, had idea to use libjpegli to expand bit depth of imported jpegs. #7125

@Lawrence37 Lawrence37 modified the milestones: v5.11, v6.0 Jul 28, 2024
@Lawrence37
Copy link
Collaborator

For adding EXIF, maybe ExifParser::encode() can help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: file format Camera or image file formats type: feature Something new to make
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JPEG XL (*.jxl) file format support
5 participants