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

Handling of HDR color spaces #6505

Open
spillerrec opened this issue Jan 18, 2025 · 6 comments
Open

Handling of HDR color spaces #6505

spillerrec opened this issue Jan 18, 2025 · 6 comments
Labels
Feature A new feature to add to ComfyUI.

Comments

@spillerrec
Copy link

Feature Idea

I have created a model which can produce images in Rec. 2100 PQ to natively create HDR images. However for this to work well, two things are missing:

  1. A way for the model to indicate the images it is outputting is not sRGB
  2. A way to include that color space information in the saved files, preferably automatically

I think it would make a lot of sense to include some meta-data in the models. My primary motivation for creating this issue is to know how I could tag my models such that it could be potentially supported in the future, even if there is no interest to support HDR for now.

I see that there are several difficulties with supporting HDR:

  • PNG has support for HDR through a new iCIP chunk, but pillow requires a hack to actually write it: https://gist.github.com/zhuowei/96b6e184bcf2de64433fbb86e15f7762
  • JPG can be supported through Googles UltraHDR extension, but I'm currently not aware of a Python library for writing these
  • AVIF/HEIC/JPEG XL supports two different way of storing HDR (10-bit vs. gain maps)
  • Currently only Chromium based browsers support HDR AFAIK.

And then there is the question on what part of the model should decide what the color space is. Both the UNET, Lora, and VAE can affect the final image. My approach ended up by fine-tuning a VAE to remap the SDR latents to Rec. 2100 PQ and then using a Lora to push it towards creating images outside the normal SDR range. (Only acting on the UNET made the results very finicky whenever the prompt affected colors and/or brightness.)

But with all that said, I personally think HDR support would be very exciting. Here are some examples:
https://civitai.com/images/52054640 (PNG 8-bit)
https://civitai.com/images/52208460 (PNG 8-bit)
https://civitai.com/images/51872956 (UltraHDR tweaked in Adobe Camera Raw)

Existing Solutions

No response

Other

No response

@spillerrec spillerrec added the Feature A new feature to add to ComfyUI. label Jan 18, 2025
@catboxanon
Copy link
Contributor

I think it would make a lot of sense to include some meta-data in the models. My primary motivation for creating this issue is to know how I could tag my models such that it could be potentially supported in the future

ModelSpec is the technically correct way to do this. There is nothing stopping you from adding the metadata elsewhere in the checkpoint though.

@spillerrec
Copy link
Author

ModelSpec is the technically correct way to do this. There is nothing stopping you from adding the metadata elsewhere in the checkpoint though.

Thank you, I have created an issue there: Stability-AI/ModelSpec#4

@comfyanonymous
Copy link
Owner

The easiest would most likely be to add a "HDR" key to the VAE so that ComfyUI could detect it as an HDR VAE and handle it correctly, either in the state dict or the safetensors metadata.

Have you released this model and VAE anywhere?

I do want to natively support different color spaces.

@spillerrec
Copy link
Author

@comfyanonymous I was thinking CICP would be a great and easy option, it is four values you can just copy as-is into PNG/AVIF/HEIF/JPEG XL and then you have support for all the most common color spaces and transfer curves including HDR.

I plan on uploading it to Civitai, but I wanted to at least bring up the question on how to define the meta-data before making a proper release. I can share a link here if it can aid development.

@comfyanonymous
Copy link
Owner

CICP sounds good. It's your choice if you put it in a tensor in the state dict or in the safetensors metadata.

@spillerrec
Copy link
Author

Since there have been no discussion on the ModelSpec issue, I have gone ahead and released the model with the CICP metadata:
https://civitai.com/models/1171618/sdxl-rec-2100-pq-vae-for-hdr?modelVersionId=1318211
https://civitai.com/models/1171634
The VAE works for any SDXL based model, while the LORA is currently only for IllustriousXL. I'm unaware of any existing tools to edit the metadata of safetensors and had to make my own, so let me know if I did anything incorrectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature A new feature to add to ComfyUI.
Projects
None yet
Development

No branches or pull requests

3 participants