-
-
Notifications
You must be signed in to change notification settings - Fork 646
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 nclx->icc colour management to heifload #3912
Comments
Hi @mertalev, You two images above ^^ look identical on firefox on linux, but different in firefox on macos, interestingly. libvips uses ICC profiles for colour management, but your image has no ICC profile. I would guess it is using nclx for encoding HDR, and that is (as far as I know) only supported by heif. I tried in imagemagick6 and 7, and they both gave the same result as libvips. We could add nclx support, but that would be a huge amount of work. Maybe you could save your image with an ICC profile? It might at least be less work to support. |
... you're right, we could also add an |
It does apparently use nclx:
A flag to enable |
Oh, but would this still let us use a Display P3 profile in the output image? Or would it have to be sRGB? |
It'd become a regular SDR image, yes. All the extra HDR bits would be removed. I think you'd need to switch to an ICC workflow and encode with 10 or 12 bits to get HDR with a P3 profile. Your test image has no profile attached. Though I'm a bit unclear on the details -- I've never actually tried this. I ought to get an HDR display. |
For context, this is for a photo backup / library app where users import images that get compressed, resized and stripped to serve more quickly. On our end, the goal is to make the most of what we're given when converting these images. Looking into this a bit more, NCLX seems to be popular for this format - all the online examples for HDR HEIC/AVIF images I could find use it. I wonder if there's a straightforward way to convert NCLX to ICC? I could find this that uses liblcms2, but it's beyond my knowledge haha |
Oh, that's a good find! It looks like they have this code: https://github.com/mnaydenov/FreeImage-Sidecar/blob/master/src/PluginHEIF.cpp#L98-L169 Which is pretty simple and should be easy to add to libvips. Testing would be a PITA though. I have a mac mini, which has quite good HDR support, but of course it's plugged into a non-HDR display. Perhaps I can persuade libvips.org to buy me a HDR monitor ... |
If it helps, I'm happy to test a branch with the changes! |
Let's make this into an enhancement issue and make a test PR. |
I've started coding a bit and wanted to share my findings. Setting the correct ICC profile (Display P3; SMPTE ST 2084 PQ) using this in vips_image_set_blob(out, VIPS_META_ICC_NAME,
(VipsCallbackFn) vips_area_free_cb, data, length); Thus generating an ICC profile from the NCLX data seems to be the way to go. colorist does exactly that. Simply using
I can look into implementing this more cleanly than what I have right now. I've included a zip file with some relevant results and ICC profiles. EDIT: I've emailed the author of colorist and his response was very helpful to understanding more about the choices colorist made. The curves definitely need to be in the repo. He created them himself from the spec. |
Oh very nice @Starbix, I'll try to have a look this weekend! |
Embedding the corresponding ICC works, but if exporting AVIF/HEIC or any other image format capable of NCLX tags we should instead opt for setting the same NCLX tags again. This leads to smaller file sizes and better support for certain viewers. I've observed that Apple Preview handles AVIFs with the generated PQ ICC well, whereas mpv doesn't, since it doesn't detect HDR without the NCLX tags. As far as I understand here's also no good way to signal PG/HLG in ICC profiles and the way colorist handles it is a bit hacky. While I don't have a great overview over the image pipeline of libvips, I imagine that's a bit harder than the proposed embedding of the equivalent ICC profile. |
We can probably do both. Maybe:
I'm a bit surprised that ICC profiles don't have useful HDR support, but I don't know much about it. Maybe we should ask an ICC profile expert for an opinion? Marti Maria (lcms author) is usually very helpful. |
Hello -- I'm the author of colorist and libavif and I'd like to clear up some confusion here, as I'm clearly responsible for the error. 😄
This is bad/wrong; please don't do this. Let me explain. When I first wrote colorist, any decision I made with regards to PQ and HLG in an ICC profile was simply to try to fit a square peg in a round hole, not to follow any standard, but specifically to abuse it. I needed a means to signal PQ and maxLum in a file that I could then read back, so I chose a specific binary payload (alluded to above) for a curve, and to abuse the informational-only The three ICC profiles in colorist's The only real standards-honoring, supported, and reasonable way to signal PQ in a modern file right now is by setting the CICP I use CICP instead of nclx when referring to h.273 values stored in an nclx box (colour_primaries, transfer_characteristics, matrix_coefficients) not only because that is where those enums are declared/defined, but also because nclx isn't the only way to signal these as of recent updates(!). The next PNG standard is adding a TL;DR - Do not attempt to signal PQ or HLG by generating or embedding any curves; they are pretty much nonsense hallucinations I made while R&D'ing proper HDR support. Instead lean heavily into CICP signaling, and over time, implement all possible reads/writes for where to find these values (nclx, PNG cICP, ICC cicpType). Video formats have been signaling color profiles with these 3 enums for a while now, and they're right. This is the way! |
Thanks for chiming in and clearing up some stuff! As far as I can see JPG does not support signalling cICP values right? Thus we'd need to tonemap the original HDR image to SDR for JPG. (ignoring gain maps/Ultra HDR) |
The only way you'll see CICP show up in a JPEG will be via a I haven't personally used libvips yet -- does it support generic color profile conversion already, or is this a whole new avenue to implement? |
Yep that makes total sense. However Google's jpegli somehow can squeeze 10+bits out of JPEGs. The end-device obviously also needs to support that, so it's not really practical. |
Heh yeah, the original libjpeg source simply has a define for bits per channel and you can make it values other than 8 and it'll roundtrip data just fine, but it is a compile-time choice that the whole world sets to 8. Any library out there that implements its own JPEG can be quite flexible here, but as you mentioned, you're probably asking for trouble compatibility-wise. |
I've added a really loud warning to colorist just now which points back at this discussion in case anyone else runs into this. |
Modern libjpegs let you select 12 bit encoding at runtime, and I expect you could encode HDR somehow in the ICC profile. But the number of applications that would be able to read it as intended must be near zero haha. |
Yes, it uses lcms2 for colour management, so all V4.4 profiles should work. @mm2 could I ping you on this? What's the state of HDR support in lcms2? |
Hi @jcupitt nice to contact you again. |
Thank you @mm2. We're hoping to make HDR profiles automatically from the nclx tags on formats like AVIF. It sounds like it should just work if we use algebraic LUT specification plus a matrix. |
As long as you use matrix-shaper with parametric tone curves (those that are specified by a formula) the profile will be HDR-ready, You could use cmsCreateRGBProfile() to create such kind of profiles |
Bug report
HDR AVIF input images look very desaturated in the output image. This happens for both
srgb
andrgb16
pipeline colorspaces and bothsrgb
andp3
target ICC profiles. It also occurs when outputting to any of JPEG, WebP, AVIF or PNG (these are the outputs formats I tested).To Reproduce
Steps to reproduce the behavior:
Expected behavior
When outputting to JPEG or WebP, the colors should be somewhat representative of the input image (minus the HDR, of course). When outputting to 10-bit AVIF, it should have roughly the same colors including HDR.
Actual behavior
The output image is unnaturally desaturated.
Screenshots
Apple Preview:
libvips:
Environment
(please complete the following information)
Additional context
libheif has a
convert_hdr_to_8bit
setting that's referenced here, but doesn't seem to be used anywhere.The text was updated successfully, but these errors were encountered: