1709814 - Support color profiles for JPEG XL

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, enhancement)

Description

[Kagami Rosylight [:saschanaz] (they/them)]

A JXL file can include an ICC profile in it. Without the proper support such images will look in unexpected way.

Comment 1

[Henri Sivonen (:hsivonen)]

Is this the reason why https://res.cloudinary.com/demo/sample.jxl looks very different from https://res.cloudinary.com/demo/image/fetch/f_png/https://res.cloudinary.com/demo/sample.jxl , or should I file a separate bug about that issue?

Comment 2

[Kagami Rosylight [:saschanaz] (they/them)]

I suspect yes, but haven't actually investigated yet. Let's see whether fixing this also fixes that issue.

Comment 3

[Samuel]

Attachment: Bug 1709814 - Add support for color profiles for JPEG XL. r=tnikkel

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 5

[zhou]

Some API of libjxl have changed. FWIW, here is the patch for the patch:

diff --git a/image/decoders/nsJXLDecoder.cpp b/image/decoders/nsJXLDecoder.cpp
index 78bb1c81e63e..a984d06df7ab 100644
--- a/image/decoders/nsJXLDecoder.cpp
+++ b/image/decoders/nsJXLDecoder.cpp
@@ -189,9 +189,9 @@ LexerTransition<nsJXLDecoder::State> nsJXLDecoder::ReadJXLData(
       case JXL_DEC_COLOR_ENCODING: {
         size_t size = 0;
         JXL_TRY(JxlDecoderGetICCProfileSize(
-            mDecoder.get(), &mFormat, JXL_COLOR_PROFILE_TARGET_DATA, &size))
+            mDecoder.get(), JXL_COLOR_PROFILE_TARGET_DATA, &size))
         std::vector<uint8_t> icc_profile(size);
-        JXL_TRY(JxlDecoderGetColorAsICCProfile(mDecoder.get(), &mFormat,
+        JXL_TRY(JxlDecoderGetColorAsICCProfile(mDecoder.get(),
                                                JXL_COLOR_PROFILE_TARGET_DATA,
                                                icc_profile.data(), size))
 

Comment 6

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 9

[Martin Bruse]

Attachment: Bug 1709814 - Add CMS support to JXL decoder. r=tnikkel,saschanaz

Comment 10

[Taylor McCall]

How is the progress on this issue? It is getting annoying that some JXL images are darker then they should be.

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

People have been recently discussing what would be the best step. Firefox is receiving 8bit value for each color channel, which is raised as a potential issue. But I don't have much details about that, either Martin or Timothy can provide better details.

We just landed the rust migration and then animation support, so I expect this will be one of the next work item to prioritize.

Comment 12

[Martin Bruse]

This is suitable to land modulo a few more comments and fixes in my opinion.

I know there is a plan to enable using floats instead of chars to call qcms, and I'm not clear on whether we want to wait for that, or land and then patch. Timothy can answer that part I think.

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Did we agree that we want to call qcms separately like in the patch though?

Comment 14

[Martin Bruse]

(In reply to Kagami Rosylight [:saschanaz] (they/them) from comment #13)

Did we agree that we want to call qcms separately like in the patch though?

AFAIU Timothy heard that it was ~simple to change QCMS to handle floats instead of chars (since it used floats internally anyway), and he and Luca agreed that then we should keep this design. I don't remember hearing about whether we should land this before or after that change was done though.

Comment 15

[Timothy Nikkel (:tnikkel)]

Apologie for the delay, I was sick last week. I think we can work toward landing the patch as is. Changing to use a float based qcms interface should not be too hard.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5dccc269fa45
https://hg.mozilla.org/integration/autoland/rev/47f9afe2ccaf
Add CMS support to JXL decoder. r=tnikkel,saschanaz

Comment 17

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/47f9afe2ccaf