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

make kXYB images work, and make color_space=XYB do something better #2506

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonsneyers
Copy link
Member

A JPEG XL image can be in three colorspaces: kRGB, kGray or kXYB. We don't currently produce kXYB images by default (but you can if you encode with -x color_space=XYB_Per). Such images don't decode at the moment; this fixes that.

Also, when loading an input image with cjxl -x color_space=XYB_Per, or when storing an output image with djxl --color_space=XYB_Per, the actual color space that is used is not actually XYB. It is a rescaled variant of XYB.

For PNG and other uint formats, using the rescaled variant makes sense, since the 'real' XYB has weird ranges so it cannot just be converted to uint. For PFM though, using the 'real' XYB makes much more sense than using the scaled one: the whole point of working in XYB is to avoid unnecessary color conversions and to stay in the space that is actually used internally. Also the rescaled XYB is not really documented anywhere so it is a very unexpected behavior to get a PFM output that doesn't correspond at all to what you would expect.

This PR changes the decoder behavior in case XYB output is desired: it now only produces scaled XYB when requesting uint samples and produces unscaled XYB when requesting float samples. It also changes the encoder behavior in case the input is marked as XYB: it now treats that as unscaled XYB, not scaled XYB.

For the case of PNGs saved with --color_space=XYB_Per, nothing changes: djxl will produce the same file as before (scaled XYB), cjxl will interpret it correctly since the ICC profile corresponds to the data. For PFMs saved with --color_space=XYB_Per, the behavior does change: djxl will produce a file that uses unscaled XYB, and when using cjxl on this PFM file with -x color_space=XYB_Per, it will interpret it as unscaled XYB.

This partially/mostly fixes these issues. I left one TODO: the ICC profile that gets generated for kXYB color encodings is still always the scaled variant — this should be changed so it generates a profile corresponding to unscaled XYB whenever that is what will be produced. It's not a huge problem since PFM doesn't support ICC profiles, but it should be addressed so applications that request float data in the original colorspace from the decoder do get a correct ICC profile that describes their data, in case the image is tagged as kXYB.

@jonsneyers jonsneyers marked this pull request as draft May 26, 2023 15:59
@jonsneyers jonsneyers requested a review from sboukortt May 31, 2023 08:17
@jonsneyers jonsneyers marked this pull request as ready for review May 31, 2023 08:17
@jonsneyers
Copy link
Member Author

This is only a partial fix. For a full fix, we should probably change the color description strings and maybe also the color encoding Enum to make a distinction between unscaled and scaled XYB, instead of 'overloading' XYB / kXYB to mean either option depending on context. Let's start with this though, so at least kXYB images can be decoded and produce sensible output with djxl.

@mo271 mo271 added the colorspace relates to hte use of color spaces, ICC profiles, etc label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
colorspace relates to hte use of color spaces, ICC profiles, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants