lut3d: separate input and output profile #12265
Conversation
TurboGit
added
feature: enhancement
src/common/colorspaces.c
Outdated
| { | ||
| cmsFloat64Number srgb_parameters[5] = { 1/0.45, 1.0 / 1.099, 0.099 / 1.099, 1.0 / 4.5, 0.081 }; | ||
| cmsToneCurve *transferFunction = cmsBuildParametricToneCurve(NULL, 4, srgb_parameters); | ||
|
|
||
| cmsHPROFILE profile = _create_lcms_profile("Gamma Rec709 RGB", "Gamma Rec709 RGB", | ||
| cmsHPROFILE profile = _create_lcms_profile("Rec709 RGB", "Rec709 RGB", |
There was a problem hiding this comment.
Please leave the previous name untouched.
There was a problem hiding this comment.
Care to elaborate please? Is there some dependency on this originally brain-dead string? Or you changed your mind since https://discuss.pixls.us/t/please-ditch-the-g-word/18853
There was a problem hiding this comment.
Rec 709 (https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.709-6-201506-I!!PDF-E.pdf) defines only the RGB primaries chromaticities and is a scene-referred space (no EOTF). Used as-is, it defines the linear Rec709 color space. It defines an OETF which matters only if you are going to break RGB into Y'CbCr signal for TV broadcasting.
Rec 709 primaries are used with the sRGB EOTF to define the sRGB color space.
Rec 709 primaries are used with the ITU-R BT.1886 EOTF (power function of exponent 2.4, see https://www.itu.int/dms_pubrec/itu-r/rec/bt/R-REC-BT.1886-0-201103-I!!PDF-E.pdf) to define what we call the "gamma Rec 709".
The proper name for Gamma Rec 709 should probably be ITU-R BT.1886, and I'm pretty sure nobody will understand what it is, so "gamma" is the lesser evil here. In any case, you can't just remove the "gamma" because then you imply "linear Rec 709".
There was a problem hiding this comment.
Rec709 clearly specifies a transfer curve in item 1.2 (used verbatim in dt code to construct this profile), slightly different to the sRGB one. No further qualification is necessary.
There was a problem hiding this comment.
Item 1.2 covers the Opto-electronic conversion (OETF). There is no EOTF specified because Rec709 is not a display-referred space (as sRGB is) and the OETF specified in BT.1886 is actually not the inverse of the BT.709 OETF but results in darkening to account for dim surround.
In all this mess, it's simply misleading and confusing to have a list composed of "linear Rec 709", "linear Rec 2020", "PQ/HLG Rec 2020", and "Rec 709" alone. The rest of the list mentions explicitely the transfer function used.
There was a problem hiding this comment.
I also found this nugget also offering two conversion options.
There was a problem hiding this comment.
I'll therefore drop this part of the change so this PR concerns the lut3d changes only. How Rec709 is handled becomes a separate issue then.
There was a problem hiding this comment.
I wonder if having actually two profiles would be required here? e.g. "Rec709 (encoding)" for import/export using the OETF and its inverse, and "Rec709 (display)" that is using the BT.1886 EOTF assumption for soft-proofing?
This would be depend on the use case and on its context, which is why I'm always asking for a definite use case before opening an IDE.
There was a problem hiding this comment.
In the context of "we might have an actual bug here": we only have the one "gamma rec709" ICC profile defined that we use for both input decoding to scene linear (from e.g. AVIFs/HEIFs etc.) and output encoding for display/export. From the name you (or some other user) think is should be using BT1886 2.4 gamma as EOTF, while in fact it is implemented w/ the inverse of the OETF.
There was a problem hiding this comment.
Also, I just realized the same is true for PQ_* and HLG_ * color spaces as well: there is no accounting for the scene to display OOTF anywhere AFAICT (the equivalents of the eotf1886(oetf709(L)) compensation in this case). We only go to/from scene-referred and in/out encoding through OETF and its inverse, so why should (non-linear) Rec709 be any different? Again, in this context, the "gamma" label is highly misleading and should be removed or replaced with something else.
|
This is a mess. I don't understand what problem you are trying to solve. I will therefore assume that you try to account for the case where the LUT contains the color space transform, instead of the usual matrix transform typically used in ICC profiles. In that case, what you do here is wrong because you convert twice to output color space : once in the LUT, once again in your code. What should be done is to simply change the tag of the color space, not apply the transform. |
aurelienpierre
added
the
controversial
That's certainly not the idea. Today we already have two transforms: working->foo, then apply lut, then back foo->working This is just assuming you can have foo and bar color spaces in the middle, the same two transforms are kept, one always outputs in the working one from the module. So if there is any "mess", it seems it was there to begin with. |
Which is correct. The rest of the pipeline expects the output of the LUT module to be in working RGB space. Change that and you mess up the whole pipeline. Modules output either Lab or working RGB. |
|
Agreed. I have not changed this behaviour. |
|
So what did you do here then, and more importantly, what problem does it solve ? |
|
I simply extended the module from "working->foo->lut->foo->working" to "working->foo->lut->bar->working". This is to support off the shelf or custom LUTs (not in ICC format) that contain some creative color space space conversion/gamut remapping. Yes, you could argue this is beyond the intent/scope of dt and this module, but could potentially open up dt for some new creative uses IMHO. However, the fixed list of color spaces currently offered is holding this back even further. |
|
Just for my understanding: What exactly is the benefit over a canonical look lut that only performs the look operation in lut application space without the transformation, given that the lut module's output will be backtransformed into the pipeline working space anyway? |
|
That's the idea, yes, with the exception
...if bar is smaller than foo (the LUT input color space) In that case, someone might have baked some secret, hand-crafted, non-parametric gamut compression into such a LUT. Ideally this is already and better addressed by the output ICC profile w/ a LUT, but sometimes all you have is a .cube file... (It is almost guaranteed that both foo and bar are smaller than the internal pipeline working space, that's the whole point of it.) |
|
Btw, happy to leave this hanging/open until someone shows a compelling demo. |
Actually, very few ICC profiles provide the A-to-B and B-to-A LUTs supposed to handle the perceptual colorimetric intent, so the ICC workflows "gamut maps" by simply clipping RGB to [0;1]. The only LUTs I know that will connect 2 spaces are the ACES IDT (from camera RGB to ACES P1), and we are not using these for obvious reasons. Even if we were to, that would go into input color profile module. |
There are non-ACES related examples for e.g. HDR to SDR down-conversion, camera log to rec709, etc.: https://xtremestuff.net/store/hdr-to-sdr-lut-pack/ Some are probably not applicable for dt as they output the "narrow/legal" video signal range for e.g. rec709, but you get the gist... You could go from dt's scene-referred linear working space to an output space through some wider gamut proxy space using a 3rd party LUT that has some look baked in (to address precisely the lack of such ICC profiles), not just limit the module to "apply LUT in space foo". There is of course an unnecessary conversion back to working and then again from working to output in |
|
This pull request has been marked as stale due to inactivity for the last 60 days. It will be automatically closed in 300 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
|
This pull reguest was closed because it has been inactive for 300 days since being marked as stale. |
Addresses the first part of #12261
No idea how to handle all the introspection code and the switching to external ICC file list though.
Also, removed any remaining mention of the "gamma Rec709" misnomer.