ICC input profiles with LUTs not handled properly.

[Beep6581]

Originally reported on Google Code with ID 2123

ICC profiles with a color matrix only are straightforward, apply with no worries. We
don't even use LCMS for this.

ICC profiles with LUTs is a bit of black magic though. The main problem is that in
order for a LUT to produce the expected output (ie the proper colors), it must have
the input pre-processed in a way the ICC profile expects. Unfortunately there's no
standard way to do this so different vendors do in different ways, and one has to find
out how they do it. The easiest way is to use the vendor's own raw converter and produce
a TIFF output file with the ICC profile embedded (*if* possible that is), then you
know that in order for the ICC profile to work properly the data should be pre-processed
such that it looks like the data in the TIFF file.

The key issue is that LUT will produce different color depending on what curve the
input has. Rawtherapee assumes a linear curve, and if an ICC profile expects a gamma
that is applied (eg 1/1.8 for Phase One profiles), but the that only compensates gamma,
the effective curve is still linear. Tested through a number of Phase One profiles
I've noted that with this linear curve they produce a greenish cast on skin tones,
but if I instead pre-process such that the curve is close to the expected "standard
curve" in Capture One, the colors look good. Not strange that their bundled ICC profiles
have been designed with the standard curve in mind. After LUT processing one can reverse
the curve and get back to RTs expected linear space.

Currently we support generic ICC profiles (only matrices, which thus require no pre-processing),
Phase One (=Capture One, Leaf excepted) ICC profiles and Nikon NX.

I've started to look deeper into Capture One ICC profiles. As I Leaf owner I'd also
like to get the Leaf ICC profiles to work (I've now managed to reverse-engineer them,
so I can add support for them).

I've got a patch 90% complete which contains general colorconversion cleanups and fix
for Phase One profiles (and added support for the Leaf ICCs) as well as structured
the code so it should be easier than before to add and tune support for more ICC profile
types, but I'm stuck on a highlight problem that needs solving first. I like the fact
that RT can import profiles from other software that you may own, so I like to have
this feature in good shape.

The difference in color rendering is generally not huge, you typically need to layer
two images on top of each other and look at skin-tones to see the improvement, but
it depends on profiles, some are more sensitive than others and produce rather bad
colors with the linear input.

Reported by torger@ludd.ltu.se on 2013-12-13 14:33:59

[Beep6581]

The highlights issue needs to be solved with a dual conversion and mixing of highlights,
got it working but shall look into optimization.

The problem is probably that the highlights get so compressed with the curve that the
LUT get quantization issues. This is no problem when you keep the curve, as the highlights
stay compressed (ie no problem in Capture One etc that has the curve applied), but
when we in RT revert the curve the bad highlight gets visible.

Solution is to mix in highlights from a conversion made with a less aggressive curve.

In this process I tried integrating Argyll's icclib and used that instead of LCMS2
to check if it was a LCMS2 issue. Icclib is floating point throughout, but I got the
exact same result, so it's a profile issue.

Now I'm close to being able to post a patch.

Reported by torger@ludd.ltu.se on 2013-12-13 19:48:31

[Beep6581]

Hi Anders, there is an option in RT to "Blend ICC highlights with matrix". Is this of
any use for what you are trying to do? The purpose was to extend ICC profile coverage
into recoverable highlights range, issue 938.

Reported by michaelezra000 on 2013-12-13 20:46:42

[Beep6581]

Here's a first patch. I'll need to clean it up slightly and perhaps optimize, lots of
slow powf() calls.

This patch does the following:

- Adds support for Leaf ICC profiles
- Improves color rendering of all Capture One ICC profiles
- General cleanup of colorconversion which had got a bit messy
- GamutICC setting removed, it couldn't be controlled via GUI and it's in line with

  the current color space policy to*always* do ICC lut conversion to prophoto to 
  avoid clipping
- Blend ICC highlights removed as we don't have an issue when we convert to prophoto

  space.
- Blend ICC highlights is still in for debugging though: when enabled new color 
  rendering for capture one profiles, when disabled the old.
- To see the improvement look at skin tones of normally exposed faces. Old rendering

  often had a greenish cast.

#2: the problem could be solved by extending matrix blending, but it's better to do
dual conversions, as it's a quite wide range we need to consider. The thing is that
RT is very flexible with curves, so we want good color in the whole range. The default
film curve in capture one (and most other converters) compress the top stop to almost
nothing, and an ICC LUT-based conversion made with such curves will not render good
highlights. Using matrix for the top 1-1.5 stop is a bit too coarse I think, so instead
use the same ICC LUT but with a less compressed curve I think got the best result.

Note that it's impossible to get the 100% exact same rendering with third party profiles
as much of the color sits in the curves. However based on these results I think one
comes pretty close and the improvement by doing the way I've done I think is certainly
worth it.

With Leaf profiles I got very large difference, without pre-film-curve application
the portrait profiles caused clear greenish cast, but with it I get very similar color
as in Capture One, but with the added flexibility in conversion that RT provides :-)

Reported by torger@ludd.ltu.se on 2013-12-13 21:25:05

---

• Attachment: input-icc-patch.txt

[Beep6581]

I haven't tested with Nikon NX profiles since I made this large change to the colorconversion
function, there's a risk I've broken it. On the other hand I'd like to review how the
Nikon NX profiles are dealt with, not sure that the current "reverse engineering" is
the best.

It depends on how big the interest from Nikon users is to support NX profiles though.

Reported by torger@ludd.ltu.se on 2013-12-13 21:28:40

[Beep6581]

Hi Torger,

concerning the 'lots of slow powf() calls':

just replace them by pow_F, if there's no way to avoid them. Gives a nice speedup compared
to powf. We already use it at some places.

#include "sleef.c"
#define pow_F(a,b) (xexpf(b*xlogf(a)))

Additionally, sqrtf(x) is faster than pow_F(x, 0.5f) and even sqrt(sqrt(x)) is faster
than pow_F(x, 0.25f) not to speak from x*x*x instead of pow_F(x,3.0f) ;-)

Ingo

Reported by heckflosse@i-weyrich.de on 2013-12-13 22:09:36

[Beep6581]

Sorry, I should have written 'Hi Anders' 

Reported by heckflosse@i-weyrich.de on 2013-12-13 22:15:50

[Beep6581]

:-)

Thanks for the tips, I'll add those. One thing that probably got better is that three
image loops has been merged into one.

This is not a main path as dcp is the preferred way these days, so I don't think one
need to work too hard to make it fast.

Reported by torger@ludd.ltu.se on 2013-12-13 22:29:36

• Status changed: PatchSubmitted

[Beep6581]

added the basic optimisations.

Reported by torger@ludd.ltu.se on 2013-12-13 22:55:32

---

• Attachment: input-icc-patch-1.txt

[Beep6581]

Here's a "production" patch with the blend cms matrix checkbox removed from gui. If
you want to test turn on/off the new mode you need the previous patch which uses the
checkbox for that

Reported by torger@ludd.ltu.se on 2013-12-14 10:27:23

---

• Attachment: input-icc-patch-2.txt

[Beep6581]

Don't know if I was clear, so I explain this again -- the reason the blend cms matrix
is removed is that regardless of working space the color conversion is now done in
prophoto, which means that there's very little clipping, so we don't have the conversion
issue. The conversion has been made in prophoto for quite some time, as the gamuticc
setting has been always enabled and not possible to change in gui. With this patch
I remove the setting permanently. From the GUI that is, to avoid any pp3 issues it's
still there in the code, but not used.

Reported by torger@ludd.ltu.se on 2013-12-14 16:50:37

[Beep6581]

Note that there's more work required to get a broader support of LUT ICC profiles, but
this patch makes it better than it is today.

Not sure it's worth to make a generic LUT support yet, as preprocessing parameters
can vary infinitely and its not stored in the icc profile itself. DCP is just so much
better. However I still think it's valuable to support third party profiles from other
popular software as that may in many cases be the only source a user can get a high
quality profile. 

Reported by torger@ludd.ltu.se on 2013-12-14 21:10:47

[Beep6581]

Thought about it and this is how I think the RT input ICC profile policy should be (and
I guess already is):

- If a user wants a custom profile DCP is the way. ICC is legacy.
- The only reason we have ICC support still is to make it possible to use profiles
from other popular software that use icc profiles, such as capture one and nikon nx.
- Matrix-only profiles is supported as they don't require preprocessing.
- We don't intend to support various ICC profiles from the past floating around which
may have all sorts of pre- and post-processing requirements not possible to exract
from the profiles itself, ie reverse engineering is required.
- If a user wants support for some specific type of icc profiles from some other third-party
software, file an issue and we'll see what we can do. If it's easy and we have the
time we probably add support.

The current patch is a step in this direction.

RT will load ooooold LUT profiles I found via some ufraw links, but it will generally
look like crap as those have all different ways to process.

Reported by torger@ludd.ltu.se on 2013-12-14 21:31:09

[Beep6581]

"Blend with Matrix" is necessary. The problem with LUTs in ICC is that they are not
deltas like in DCP, but primitive maps. So highlights will get clipped in many profiles
regardless if you select large output spaces.

However there was another try in the code to widen the colorspace before feeding LCMS.
That could be removed, as it's useless for the reasons given above.

Just try with some highlight recovery image e.g. in clouds.
There is also a length discussion with Jacques in the forum somewhere.

Reported by oduis@hotmail.com on 2013-12-15 09:39:27

[Beep6581]

An interesting experiment is to feed ICC profiles with a linear grayscale 0 to max and
then plot the output so you get a transfer function for red, green and blue (for neutral
white that is, as it's LUT it varies a little depending on color).

I've done this with various ICC profiles. Some are neutral in terms of curve, they
have a fixed gamma and that's it. Phase One's ICC profiles are like this, all are exactly
gamma 1.8. Leaf profiles have a slight difference, perhaps a bit closer to gamma 1.9,
but close enough, so I handle those the same way. Those profiles however expect that
a curve has been applied before the ICC profile processes the input.

Looking at a few Nikon NX profiles their curve varies depending on profile and they
apply a contrast curve.

Looking at some old "home made" profiles the curves look more or less like a disaster
with super-high contrast and lots of clipping.

Theoretically one can write code that probes the ICC profile with this type of grayscale
and than makes an inverse of the resulting curves to get the linear output RT would
like. The high contrast curves are so aggressive that they will be hard/impossible
to invert though (too much compression in highlighs/shadows).

So it's probably best to let the ICC profiles that apply a contrast curve do that,
and just find a suitable gamma for them. Nikon NX profiles are such. I've kept the
Nikon code unchanged, but if I'd look deeper into it I'd probably change that code
a little. I don't have Nikon NX software though so I'm not the guy to do that.

Phase One / Capture One profiles with this patch work just as the DCP profiles though,
ie just changing color not the curve. So if you happen to have Lightroom / Adobe DNG
Converter and Capture One installed you'll have lots and lots of third-party profiles
that work great with RawTherapee.

Reported by torger@ludd.ltu.se on 2013-12-15 10:19:45

[Beep6581]

#13: as far as I can see only the very old and very bad profiles have this problem,
I dug up some home-made Kodak DCS profile that clips everything above 0.5 (topmost
stop), but this breaks highlights so much that there's nothing near being repaired
with a matrix blending of the last percent.

Capture One profiles for example don't seem to have this problem.

Do you have a pointer to an example ICC I could test with? LCMS code nowadays run in
floating point and can output values > 1.0 which may have changed things too.

The colorconversion now is made always to prophoto, to avoid clipping of saturated
colors in Lab->Prophoto conversion. We're moving RT towards having Prophoto as default
working space so it's likely the working space is prophoto already. For Phase One profiles
I only to RGB->Lab in LCMS though, the Lab->XYZ->RGB step is done in the code.

Reported by torger@ludd.ltu.se on 2013-12-15 10:39:06

[Beep6581]

If it still needed I think we should always have it enabled and still remove the setting
from the GUI. It's a "ICC fixup step", for which there already are others required
(the curve trick for Phase One profiles introduced in this patch for example). The
user should not need to care about this.

I can add it back in, in fact I'll probably do this for safety.

It goes active 2% from clipping in linear gamma, ie 0.03 stop from clipping. To make
any real difference at all I assume that the intention is that the output should also
have values larger than the clip point (1.0/65535.0, and possible smaller that 0.0).
Before when the working space could be sRGB in this conversion there could be a lot
values >clip and <0, but now when we do Prophoto it's a lot less, but prohpoto is still
smaller than most camera spaces (mainly in the blue channel if I remember correctly)
so there is a tiny amount of clipping that we can restore here, so I think I just do
that.

Reported by torger@ludd.ltu.se on 2013-12-15 11:25:43

[Beep6581]

The feature is only for highlight recovery, and depending on the profile of course LCMS
clips those highlights. Take all RT supplied ICCs for instance.

And it's not a good idea to have it always on, otherwise I would have done it already
when I added that mode.
The matrix is not the one from the profile, but from DCRAW/DNG etc. The feature only
works correctly when matrix matches the profile, which is the case for most normal
daylight profiles. If you have e.g. an ICC created for some artificial light, it will
lead to strange looking highlights, because they don't match.

Reported by oduis@hotmail.com on 2013-12-15 12:54:06

[Beep6581]

Thanks, I've been able to recreate the issue now, I'm looking into it!

Reported by torger@ludd.ltu.se on 2013-12-15 14:40:58

[Beep6581]

Okay, now I see the problem.

To avoid confusion for any readers here "Highlight recovery" in the code is "highlight
reconstruction" in the GUI (highlight recovery in the GUI is something else :-)), and
"highlight reconstruction" is sort of more correct, as some of the data is indeed created.
I've been a bit confused by this myself as a relative newcomer to the code.

What happens is that highlight reconstruction uses leftover highlight range (only one
of r,g,b channels clip at the clip level) and creates new data above the clipping point,
ie larger than 1.0/65535.0. Per definition this gets clipped by the ICC LUT as the
tables are only defined in the 0-clip range. How large extra range that's added on
top depends on the white balance (as that shifts the balance between the camera r,g,b
channels), but maximum seems to be about 1 stop of extra data on top.

The DCP code will clip when the DCP's embedded tone curve is applied, as the curve
is only defined in the 0-clip range. However RT's own DCP's don't have tone curves
and, and the LUT code is written such that it won't clip, however if it makes any sane
conversion can be discussed as it just clamps to the last element (guess it's okay,
but maybe we should just do matrix-only conversion there instead, like for ICC). Adobe's
own reference code does clip, so RT employs a special implementation of DCP. Ie if
we would use Adobe's DNG SDK instead of own custom code to implement DCP we would have
the same clipping problem as we have when using ICC.

In other words, neither ICC LUT or DCP LUT is designed to convert data that is outside
the 0-clip range.

The clip-point in RT is today defined as the minimum of the r,g,b camera channels after
white balance application, ie some of the channel range is left unused. Clipping in
the DNG standard is defined in the same way, meaning that the DCP works on the correct
range as it is today.

How we should convert color for the reconstructed highlight data is up to us.

One way is to do like current, ie matrix conversion only. Problem then is which matrix
to use, the ICC may not contain a matrix at all, or not a sane one. The DCP does contain
a matrix, but with the "new" ForwardMatrix stuff you can't really assume that it will
deliver sane color until the LUT has been applied on top.

The way I'd like to solve it however is to scan for max and compress the range during
the color conversion step. The advantage is that ICC and DCP get to work within their
range and curve application works. The disadvantage is that you get a range shift which
theoretically can cause undesired color shifts as the tables don't work at their targeted
luminance range. I think that up-to-one extra stop won't destroy it though. I'll test
the concept.

Reported by torger@ludd.ltu.se on 2013-12-15 16:08:45

[Beep6581]

You are approaching, and I guess you don't have to test the concept, it will fail in
real life I guess. ;-)

I explicitly designed the RT DCP engine to NOT clip WITH LUT applied, and it works
- except the tone curve - correct in real world. It was one of the most critical features
of DCP and DCP in RT - highlight recovery.
I know the Adobe reference doesn't do it, but DCP as such is designed to do so (and
I would not implement non-clipping code in the reference code if I were Adobe, so I
can understand them).

Before there were variants for reducing the input range and increasing it afterwards
(e.g. Raw White point), but though allowing highlight recovery they produced colors
shifts with LUTs or very only valid for primitive matrix profiles.

So if you tinker with highlight recovery, restrict it to ICC please and test the highlights
with LUTs profiles for color shifts.
Highlights with the DCP engine as I left it are fully recoverable.

Reported by oduis@hotmail.com on 2013-12-15 16:54:42

[Beep6581]

I got inspired by the DCP solution instead and made it the corresponding way for ICC:
if max(r,g,b) > clip in the pixel, then scale all down so max(r,g,b)==clip and do the
conversion there at the brightest point, and restore scaling after conversion. Works
like a charm. So now blend matrix setting no longer needed again :-)

Reported by torger@ludd.ltu.se on 2013-12-15 16:57:30

---

• Attachment: input-icc-patch-3.txt

[Beep6581]

It will probably not work with LUT based profiles. We had the same problem with raw
white point. Look out for color shifts in comparison to a picture developed without
compression.

Reported by oduis@hotmail.com on 2013-12-15 16:59:13

[Beep6581]

Don't worry I haven't touched the DCP code and don't plan to :-). The clipping is not
only in Adobe's code, that clipping should occur is clearly written into the DNG specification
too. It's really only value that should be outside the range though, which often is
not even a parameter if a 2.5D table, and if it is the last value in the table will
be used, ie it will be scaled the same way as the brightest possible would. I think
that's okay.

I've tested #21 on LUT profiles, I don't get any problems. I did not implement it as
in #19, but as described in #21, ie a normal conversion but making the "brighter than
max" pixels converted in the same way as if they were the brightest, ie the same type
of thing that happens in the DCP code.

I'll look into raw whitepoint, probably need to take that into account too as you say.

Reported by torger@ludd.ltu.se on 2013-12-15 17:48:28

[Beep6581]

I've looked at raw whitepoint compensation in the DCP code. It's easy to copy into the
ICC code. What it does is to make sure that raw white point (ie raw clip level adjustment)
will not change which entries you look into the huesatmap, ie the DCP conversion is
made just as there was no change in raw white point. Ie the colors in the picture stay
the same (although the picture itself can be brighter/darker due to white point adjustment).

However, as far as I understand the raw whitepoint feature the reason it exists is
to be able to manually adjust if dcraw has provided a bad whitepoint (which it can
do for some cameras, although it's become better, and now we can also put in constants
via camconst.json with ISO and aperture scaling and all). Then I think that the engine
should work with that user-specified raw whitepoint including the color conversion.

I suppose the reason why raw white point is compensated for is that we assume that
ICC/DCP loaded were designed using RT and thus they expect clip levels provided by
RT. Bad clip levels are however considered as bugs these days and will be corrected,
so I think we have to live with that it might cause a tiny color shift for a DCP which
was built using RT which had a bad clip level.

I'm also guessing that these color shift issues would be much smaller than using a
DCP/ICC at a slightly different white balance than they were designed for, so I don't
think we need to worry.

But please enlighten me if there is some other reason whitepoint should be compensated
for.

Reported by torger@ludd.ltu.se on 2013-12-15 18:04:52

[Beep6581]

Haven't looked at the patch, but if you just make the brightest point scale to max on
input for ICC, you just clip them on input instead on output, don't you?

The raw white point was once used for white point correction (as you say it could be
corrected also, however the slider works with every camera out there), but what I was
referring to is its use to pre-scale the input before an ICC LUT for highlight recovery.
Which is essentially a kind of compression (white point is just a scaling fac). The
highlight recovery worked (because the above 1 values were all scaled back), but it
introduces color shifts.
How much you see or measure them depends on the LUT and pictures used.

But it's logical if you think about it. If you just have a map type LUT (like in ICCs),
there is no way to scale/compress the input range without color shifts at all.
It also explains why Adobe tooks a different LUT approach with DCPs, way smarter.

Reported by oduis@hotmail.com on 2013-12-15 18:18:03

[Beep6581]

There *are* ICC LUT profiles that generate problems, and one will always be able to
find one due to that there are so many crazy designs with extreme contrast curves and
hard clipping built-in to the profile. Simply put, there are profiles out there that
convert any color X stops from clipping into white. Those will lead to that the highlight
is rendered gray-scale, as the colors approach grayscale when clipping is approached.
This is not fully solved with matrix blending either as the desaturation happens early,
much earlier than 2% from max.

Nikon NX has such profiles, ancient profiles associated to UFRaw has even worse problems.
In the Nikon case I'm quite sure what they try to emulate with the profile is film,
they include both the contrast curve *and* highlight desaturation.

But RT's own LUT profiles seem to be well-behaving and all Phase One profiles too,
no problems with those.

So I think the focus should be on supported "well-behaved" ICC profiles, at least when
it comes to highlight reconstruction. If the profile itself includes a contrast curve
and desaturates towards clipping it's not really designed for highlight reconstruction.

Reported by torger@ludd.ltu.se on 2013-12-15 18:27:12

[Beep6581]

I used ICC with RT and raw white point for highlight recovery myself before the DCP
engine, Nokia D700.
All the RT normal profiles have the problem, and as far as I know ICC standard only
knows map based LUTs, so all LUT ICCs will suffer from this problem. How much depends
on the profile and the picture.

Try it out yourself: select a LUT based profile (one of RTs own if you will) and reduce
the raw white point a whole lot to see shifts even visually without the need to measure.
All it does is compress the input space by the given factor.

Reported by oduis@hotmail.com on 2013-12-15 18:32:48

[Beep6581]

#25 No I don't clip. I maybe did not explain it properly, let me show the code for you:

   float maxc = max(r,g,b);
   if (maxc <= 1.0) {
       hl_scale.data[w] = 1.0;
   } else {
       // highlight recovery extend the range past the clip point, 
       // which means we can get values larger than 1.0 here.
       // LUT ICC profiles only work in the 0-1 range so we scale 
       // down to fit and restore after conversion.
       hl_scale.data[w] = 1.0 / maxc;
       r *= hl_scale.data[w];
       g *= hl_scale.data[w];
       b *= hl_scale.data[w];
   }

...and the scaling is reversed after the scanline has been transformed. As described
in #26 problems will occur for profiles that clip/desaturate early, ie bright colors
converts into neutral, but those will suck for highlight recovery anyway so I think
it's acceptable.

As far as I know the raw whitepoint is not used to for highlight recovery any longer
now when RT is floating point. You just set a negative exposure to bring in the reconstructed
highlights. Thus I don't think color conversion should look at the raw whitepoint at
all, as it should only be adjusted if you have a bad whitepoint coming from dcraw.

Reported by torger@ludd.ltu.se on 2013-12-15 18:43:15

[Beep6581]

#27: yes that's true, but that is also how it should work according to me, as raw whitepoint
no longer should be used for highlight recovery, the exposure slider should be used.
The ICC was designed for a specific range, if you change that range you'll make colors
darker/brighter and they get a different conversion.

DCP LUTs with 3D table (ie a value table) has the same property, ie different conversions
are made depending on how bright the color is. The wp scaling code is there to reverse
that. I don't think it should be there, but don't worry I won't touch it in this patch
at least ;-). This is about ICC only.

I know about the shifts, that's why I've adjusted the input tone curve for Phase One
profiles to get the expected output. A non-linear profile must get the input pre-processing
it was designed for. This counts also for DCPs, the great thing with them however is
that it's well-defined in the standard what this pre-processing is.

Reported by torger@ludd.ltu.se on 2013-12-15 18:57:05

[Beep6581]

There you have it, it will produce color shifts.

The code approach is the same as the raw white point slide targeted at bright pixels.
But that moves the out-of-LUT-area point to another color point that will translated
to a different color by the LUT (how much depends on the profile) - a color shift.

DCPs LUTs are totally different. They are delta to a color, not maps as in ICCs. So
they don’t have that problem.

So it's better to keep the existsing blend matrix approach for ICCs, as it recovers
the highlights without color shifts, if the matrix matches.

Reported by oduis@hotmail.com on 2013-12-15 19:03:42

[Beep6581]

#30

The current code does not require the matrix to match, as the whole image is processed
with the same ICC. It also removes a configuration option that few understand. I'm
quite pleased with it, so if it works which I think it does, I intend to keep it.

What color shift are you referring to? If there is a problem I will address it, but
I don't see one know. Not compensating for the raw white point is intentional as I've
described before. To process brighter-than-clipping pixels the same way as the brightest
possible I think is a better approach than revert to a matrix from a different source,
but yes all brighter-than-brightest will get a static conversion, but I wouldn't call
it "color shift".

I've thought about it theoretically and tested with a number of profiles and get good
results. It would be easiest for me if you provide a test-case where it fails so I
can reproduce locally, as it seems I'm not following your theory...

Reported by torger@ludd.ltu.se on 2013-12-15 19:32:22

[Beep6581]

> but yes all brighter-than-brightest will get a static conversion, but I wouldn't 
> call it "color shift".
What else? It is a color shift, as the wrong translation (intended for a different
color) is taken, translating the highlights to a wrong color.

However I get more and more the impression you don't want to understand, so I'll stop
discussing, ICC is legacy anyway.

But please don't mess with the DCP engine the same way.

Reported by oduis@hotmail.com on 2013-12-15 19:47:05

[Beep6581]

I do want to understand. I think you're misunderstanding things and try to figure it
out, and while doing it make sure I'm not misunderstanding. It's important to me that
I make a proper patch that work as intended. I'm quite confident in the current solution
though so if you don't need to contribute more in this discussion if you don't want
to. You helped me pointing out a few things already which I've addressed.

However, here's an attempt to explain:

If we define color with HSV, the H and S is the same but the V is brighter. We make
a conversion like this: "if V > 1.0 then translate the same way as if V had been 1.0,
and then scale the result so V arrives at the correct value". Ie we extend the profile
linearly to cover the extended range built by highlight reconstruction. The ICC profile
may do a RGB->Lab->XYZ->RGB translation rather than RGB->HSV->RGB but the principle
stays the same. The "hl_scale" in my code is "V" if you like.

And guess what, this is exactly what happens in the DCP code. Many DCPs are 2.5D ie
don't have a V table and thus don't make different HS shifts depending on V, and in
this case the DCP is truly brightness-independent. It seems to me that you think that
all DCPs are this way. However a DCP can be 3D, ie different shifts depending on V,
eg a dark blue may be shifted a different way than a bright blue. Many of Adobe's profiles
are this way.

This means that the DCP is only defined in the range 0.0<V<1.0 just as the standard
document says, but the code now extends linearly just as if V had been 1.0, here's
the line where it happens:

        int vIndex0 = max(min((int)vScaled,ti.pc.maxValIndex0),0);

vScaled will be larger than the available table range and is thus clamped to the same
position as for V 1.0. And then the table entry is picked:

        tableBase + vIndex0 * ti.pc.valStep + hIndex0 * ti.pc.hueStep + sIndex0

Ie what happens there is the same as my ICC code does now. So the DCP code is already
messed the same way ;-). The code assumes that the best approximation of the unknown
extended range is to make the same HS shifts as would happen to the brightest known
color with the same H and S. I think that is a good approach, certainly better than
mixing in a matrix from a different source. DCP code does it, and with this patch the
ICC code does it too.

The approach is killed with profiles that apply tone curves, but I think we can survive
without that, and those won't look good with matrix blending either.

Reported by torger@ludd.ltu.se on 2013-12-15 20:56:52

[Beep6581]

Reported by torger@ludd.ltu.se on 2013-12-16 07:29:26

• Blocking: #2032

[Beep6581]

Made icc type identification a bit more detailed, ie matching on "Phase One A/S" instead
of only "Phase One".

Reported by torger@ludd.ltu.se on 2013-12-16 11:52:52

---

• Attachment: input-icc-patch-4.txt

[Beep6581]

You still don't seem to understand, however it seems writing the same all over does
not get through.
Just give me a Skype call, login "oliverduis", and we'll talk through it if you like.
I'll mostly be there tonight.

Reported by oduis@hotmail.com on 2013-12-16 17:46:59

[Beep6581]

#36: let me ask a more narrow question, which maybe makes it simpler to get forward:
how is the approach to do the translation based on the brightest known color of the
same hue and saturation different from the current DCP approach, assuming the DCP has
a 3D LUT (which many do have)?

As described in #33 I claim my ICC solution and your DCP solution to be equivalent,
and as you seem to be satisfied with the approach in the DCP code I hoped that you'd
like my solution.

However, that obviously is not the case so I can only assume that you don't think the
solutions are equivalent as I do, and then I'd like to ask you to describe what makes
them different. Maybe I've missed something, or I've missed something in the explanation
of my patch, because I really think it does the same thing as the DCP code.

The only difference I see is that I don't scale for the raw whitepoint setting. If
that's is the only objection you have, let me know, but I think we can discuss that
separately and it's not hard to add in there. For now let us assume that the raw whitepoint
setting is not used, and highlight reconstruction is recovered by setting a negative
exposure.

Thanks for the skype call offer, but I'd prefer to solve it here in text for future
reference, so I hope you're a little patient with me ;-).

Reported by torger@ludd.ltu.se on 2013-12-16 18:22:39

[Beep6581]

Ok great, let’s tackle them one by one. Maybe my explanations are not good enough, I’m
not a native speaker.

Let’s start with the color shifts, and let’s make it by example this time.

Imaging a LUT ICC profile that shifts to red in normal areas, but shifts to blue just
in the highlights.
Now let’s take a sample input point RGB 1.2/0.9/0.6 (R clipped, the others not). In
normal processing, the input would get clipped to 1.0/0.9/0.6. Then the LUT map would
spit out an output value of, say 1.0/0.9/0.8 (because we said the LUT profile would
shift to blue in the highlights only).

Now comes your algorithm of #28, maxc=1.2, giving a scaling factor of 0.83 for all
channels, resulting in input point 1.0/0.75/0.5. Now feed this the the LUT. But as
we said the normal tones will be mapped to red, it might give something like a 1.0/0.95/0.5.
Et voila, a color shift (that not corrected afterwards by scaling it back).

Hint: I went down the same path once and came up with a more complex solution of one
factor per channel, which solves this problem but leads to the next ;-)

Reported by oduis@hotmail.com on 2013-12-16 19:12:39

[Beep6581]

I'm aware of this, I just cannot get around my head that it would be a problem :-).
When actually running the code I have not yet seen any problem either.

If hlrecovery is not enabled ICC profile sees 1.0/0.9/0.6 and makes a conversion accordingly.
If hlrevocery is enabled we get 1.2/0.9/0.6 in the same pixel, scale it down to 1.0/0.75/0.5
and scale up the result, and yes there is a different conversion, but the input image
also has a different color there than it had before. If the color would have been 0.9999/0.9/0.6
it will be that also after highlight recovery and we'd get the same conversion.

With DCP we have HSV as input, but converted from RGB. For hlrecovery V will get truncated
down to 1.0, hue and saturation is kept the same, in our example color we have rgb
1.0/0.9/0.6 which gets hsv 0.45/0.4/1.0 and according transformation, with hlrecovery
rgb becomes 1.2/0.9/0.6 in the same pixel which is hsv 0.3/0.5/1.2, which gets truncated
to 0.3/0.5/1.0 before translation, ie a different color, and voila there is the same
type of "color shift" as in the ICC code.

Do you agree that the DCP code has the same "problem"?

If you agree with that, the next thing is to figure out if it's a problem or not. If
you do not agree that DCP code has the same problem it would help me if you could explain
why it's different.

Reported by torger@ludd.ltu.se on 2013-12-16 21:33:59

[Beep6581]

> and yes there is a different conversion,
So you agree it has a structural color shift, but you haven’t mentioned it yet in the
profile you tested (which just says something about the profiles you tested, but it's
still a bug). 
Agree on that? Then we are a step further and can tackle the next problem which it
seems you‘re not aware of yet (output space) ;-)

(And no, the DCP does not have these problem at all, but as we said, let's move one
by one)

Reported by oduis@hotmail.com on 2013-12-17 06:49:22

[Beep6581]

#41 the problem I have is that as I see it you have not yet mentioned an issue that
DCP doesn't have, still you maintain that DCP doesn't have the issue. I try in #40
and earlier in #33 to explain why your DCP code works the same as my ICC code, ie that
DCP indeed have "the issue". You have not answered to those descriptions and pointed
out anything wrong with them, but you maintain that DCP doesn't have the issue. Then
it's hard to move forward.

Do you know that DCP does take the V parameter into account? That a dark color can
get a different translation than a bright, just as for ICC profiles? It seems to me
that you may have missed that, and that would explain why you think DCP is free of
the "color shift" you mention. As we look deeper at it we will see that even with 2.5D
DCPs (=value-independent profiles) there will be a shift in translation as hue and
saturation change when we go from partially clipped to partially recovered/reconstructed.

The output space for the ICC processing is ProphotoRGB, ie the same output space as
for DCP processing. It's nowadays decoupled from working space, and we're moving towards
establishing ProphotoRGB as default working space.

I do not really agree with the term "color shift" in this context. Any fully defined
colors are not affected, they are rendered the exact same way as without hlrecovery
enabled. What happens with hlrecovery enabled is that some previously clipped pixels
are replaced by new colors that are no longer clipped. We are faced with the challenge
to render a color which is undefined by both ICC and 3D DCP, ie a V outside the defined
range. These colors are in reconstructed space, ie they may not even have full recorded
color information so it's not really critical to make "accurate" color there since
there is no such thing in this range, what we need to achieve is something that is
reasonable and looks good.

To be clearer, could describe what you mean in this context when you say color shift,
compared to what? What is the reference? Let me guess; in the example you put forward
in #38 it seems to me that the reference is the clipped 1.0/0.9/0.6 which gets a specific
translation (towards bluer in the example), and you would like that the reconstructed
1.2/0.9/0.6 color would get the same shift towards bluer, but since it's scaled down
and gets a shift towards redder instead you say that that is a color shift, and therefore
a bug.

If that is what you mean I would answer the following: 1) Your DCP code makes the same
type of color shifts, 2) it's not fair to call it color shift as we're translating
a different color than we were in the first place, in fact making the shift towards
bluer for the 1.2/0.9/0.6 color would be more of a bug since we than would apply a
translation which is for a different hue and saturation. I think it's more correct
to apply a translation which is for the same hue and saturation, and the most correct
for a "brighter than bright" is picked as the brightest known color with the same hue
and saturation.

I think this will boil down to a matter of taste in the end, you can solve the rendering
of the undefined colors in various ways. None of them are "wrong", it's just different
ways of solving the problem. Another method would be to stretch the DCP/ICC profile
over the new range, you could claim that that is the right way to do it, and for some
profiles there is an advantage in terms of look, ie for those that apply a curve and
desaturate bright colors. Yes you'll render the whole range differently, but you also
feed a different image.

As color of the recovered range is unreliable due to it's in large made up color from
some algorithm I think however that the current approach of limiting the DCP/ICC to
the defined range is better. You'll get a transitional range with partially reconstructed
colors, and then fully reconstructed colors which get a new rendering based on brightest
known colors. There will be no discontinuity in these transitions, and fully defined
colors will not be affected. So I think it's a good approach.

It seems to me that what you do not like is the transitional range of the partially
reconstructed colors, that you somehow would like them to get the same translation
as partially clipped colors, but I don't think that make sense.

Another straight to the point question: do you think there can be some discontinuity
between the fully defined and partially reconstructed areas using this method? As far
as I can see I don't think there is a discontinuity problem (and I have not seen it)
but I have not got to the bottom with that so I'm not 100% sure. If you think that
discontinuity is an issue I could look deeper into that though.

Another way to get forward is if you would describe with your own words how the DCP
code works now, and why it unlike the ICC code does not get a problem. As you've seen
earlier when I describe what the DCP code does I get to the conclusion that it does
exactly the same thing as my ICC code.

Okay, this got long. To spare your time I think the best/quickest way forward I think
is if you could do like I did in #40, take one example and put it through both the
DCP and the ICC code and describe what's different, if you manage to find the difference
between DCP and ICC I don't that would be an eye-opener for me. I have no prestige
in this and have no problem of being proven wrong, I just don't like to let go of a
solution I like as long as I think it's the right thing(tm).

Reported by torger@ludd.ltu.se on 2013-12-17 08:36:28

[Beep6581]

Discussed the issue of discontinuity here at work with some guys and have now got to
the conclusion that there should be no risk of discontinuity either.

The key to unlock now I think is to describe and example which you run through both
my ICC solution and your DCP solution and show that it fails for ICC and succeeds for
DCP. When I do it, as in #40 and #33 I get the same result for both DCP and ICC, and
obviously you do not. If you could describe how you get to different results I think
that will point out what I have supposedly missed.

Reported by torger@ludd.ltu.se on 2013-12-17 09:32:12

[Beep6581]

> I think it's more correct to apply a translation which is for the same hue and saturation,
and the most correct for a "brighter than bright" is picked as the brightest known
color with the 
> same hue and saturation
I disagree. Just taking any maybe way off point of the profile leads to color shifts
(try it out with raw white point to simulate on a LUT). Taking the last profile defined
point that is nearest to the out-of-area point is the most precise way to go. The problem
with your algorithm is also that the further e.g. red would move into clip, the darker
the point that’s taken as substitution will be. So it's even moving more and more off
the point you want to have.
And don't forget that the camera response function is usually non-linear, so you cannot
just take any point of similar hue.

Then output space: you can give LCMS anything you want as output space, the clipping
factor is the LUT within the profile. That is frequently defined for other output spaces.
So LCMS will translate (and clip input AND output) with the LUT, then translate the
(potentially clipped values) further to the desired larger space (just debug LCMS code,
though that's pretty painful ;-).
I had this problem with many ICCs I tested back then, resulting in highlights that
could only be recovered "half" in comparison to a simple matrix (because LUT was clipped
on output).

Then DCP: Handling is way different than LCMS ICC LUT: first the point is translated
with the matrix. Completely clip free, and the matrix does like 90% of the translation
work. Then comes the HSV LUT transformation, which is usually only for fine tuning.
And in contrast to ICCs that LUT does not give a resulting point in target space (that
may be clipped). Instead it gives a delta, a displacement on which to move the point.
So you can just take the nearest displacement known to the profile and apply the displacement
totally clip free to your clip free input point after the matrix. And that is independent
of the dimensions of the HSV LUT, plus - since the clip-free matrix does the heavy
lifting - in contrast to LUT ICCs pretty precise, though out of area of the profile.

That's why highlight recovery works so nice with DCPs. Just try it out in practice,
the highlights you can recovery with DCP will be the same as with the simple DCRAW
matrix, and you won't see color shifts.

But anyway, as already discussed: Since I'm not actively developing RT any more, and
it's just the ICC part which is legacy anyway, I won't hold you up.

Reported by oduis@hotmail.com on 2013-12-17 15:57:33

[Beep6581]

Ok, thanks for the explanation, I now know were to put my focus. I know there are many
problematic ICC profiles, and I think I am less ambitious than you were when it comes
to support the problematic ones, especially now when we can say "use DCP instead, it's
the preferred method".

I've read up on ICC clipping and indeed there could be an issue there. I'll look at
saturated colors and see what happens. If I get good results with RT's own and Capture
One profiles I'll keep it, but if there are problems also with these I'll ditch it.

Reported by torger@ludd.ltu.se on 2013-12-17 16:20:00

[Beep6581]

I've done some more testing with the new and old code, here a screenshot showing 100%
crops of the different alternatives. Explanations:

The image shows a crop of a landscape shot with a saturated sky in an area where there
is clipping, shot with a Canon 5D mk2. Neutral settings, with exposure set to -1 to
bring in reconstructed highlights.

* matrix only, no reconstruction: note desaturation and color shift due to clipping

* matrix only, blend hl reconstruction: note the great improvement in highlight quality

* icc enabled, without matrix blending: pre-patch, matrix blending not enabled, meaning
that the icc clips at 1.0, ie result gets similar to matrix only without highlight
reconstruction.

* icc enabled, with matrix blending: pre-patch, matrix blending enabled. Note the greenish
color casts. The problem is generally not mismatch matrix profile (although there is
a slight mismatch too, using a D700.icc in the example for a 5Dmk2 camera, but you
can see that the mismatch is not too large as the color is similar to matrix only),
but that blending starts only 2% from clipping (ie 0.03 stops), one would need to start
blending from 1-2 stops from clipping to avoid the color shifts caused by the clipped
highlights in the ICC. Or more exactly make sure that the matrix image has fully taken
over before the ICC renders a pixel which has a clipped channel in it.

* icc enabled, new code linear extension into highlights: the result with this patch
enabled

* dcp enabled: for reference how DCP renders which also employ the linear extension
for recovered highlights.

I've tested a number of images, and this is what I see all the time; the icc rendering
post patch becomes very similar to dcp rendering, and the matrix rendering has color
cast issues.

Concerning ICC clipping issues, it's true that ICC v2 LUT profiles often have clipping
issues in the LUT/PCS, but the only effect in practice is that you cannot get as saturated
colors from the camera. It's however extremely hard to achieve such saturated colors
that it becomes a problem in real subjects, as reality is quite low on saturation compared
to what you can achieve inside RawTherapee with the saturation slider, but that happens
after the input stage and you're unaffected by LUT clipping. Any LUT clipping do not
change the performance of this patch, it just means that recovered highlights cannot
have a higher saturation limit than anywhere else in the picture, but value can still
be large.

When we apply an ICC which desaturates highlights and applies a contrast curve, like
some (all?) Nikon NX profiles the whole highlight recovery range will become desaturated,
and the result will not look very good. My intention is to not support highlight reconstruction
with these type of profiles. Without highlight reconstruction they work just fine,
but of course curve application in the profile is not what RT is designed for so such
profiles will not be recommended.

Reported by torger@ludd.ltu.se on 2013-12-17 20:07:28

---

• Attachment: icc-alternatives.png
  

[Beep6581]

My conclusion is that the patch works well enough with a wide enough range of ICC profiles,
and I'm satisfied with the theory behind despite the limitations.

I'm going to commit this.

(If someone has forgot by now the main attraction of the patch is not intended to be
a new hl recovery handling but better rendering of Capture One ICCs and support for
Leaf ICCs, plus some general cleanup of the colorconversion function.)

Reported by torger@ludd.ltu.se on 2013-12-17 20:13:31

[Beep6581]

Reported by torger@ludd.ltu.se on 2013-12-17 20:17:30

• Status changed: FixedPendingConfirmation

[Beep6581]

Judging from the recovery performance I'd say it like Shakespear: Much Ado About Nothing
;-)
However since there are many types of ICCs and different cameras, a sample is no proof.

But as you say, the big news is the better CO/Leaf ICC handling, which is surely great
work.

And we can surely agree that DCP is still the recommended way for maximum precision
highlight recovery, all profiles, full saturation.

Reported by oduis@hotmail.com on 2013-12-17 23:16:32

[Beep6581]

All the little "nothings" make RT a better program in the end:)
Anders, thanks for the fine tuning!

Thanks to you both for the excellent DCP and ICC support in RT:)

Reported by michaelezra000 on 2013-12-18 05:20:14

[Beep6581]

PS: Torger, since you seem to seek work: are you familiar how to design more complex
interpolation? My LCP engine is not as good as Adobes for some dodgy profiles, you
could surely make many friends if you'd do a "V2" adding some better error handling.
If you're interested I could set you on track on what's the problem and where I left.

Reported by oduis@hotmail.com on 2013-12-18 07:21:59