Closed Beep6581 closed 9 years ago
I think this also applies to ICC. We should really have a dual step profile application,
one that is applied before exposure compensation, this is where you do "scene-referred"
corrections (HueSatMap for DNG profiles), and then a second step after exposure compensation
and fill light / tonemap, here's where output-referred subjective adjustments are applied,
ie LookTable for DNG profiles.
The LookTable should be applied before any tone curve.
When it comes to ICC profiles, they can be designed to be scene-referred or output-referred.
Matrix-only profiles are usually scene-referred (ie designed to match color accurately
with a linear curve), while LUT profiles like those bundled with Capture One are often
output-referred, so it depends if they should be applied before or after exposure.
Some ICC profiles are also designed to be applied after curve rather than before curve,
and really old-school profiles apply a curve through the LUT.
We'll start with making the DNG stuff though, as ICC profiles are not the primary choice
for RT.
Reported by torger@ludd.ltu.se
on 2015-06-25 08:34:15
I'll have a brief look, not sure if there's some natural insertion point for the second
LookTable pass, but I'll see...
Reported by torger@ludd.ltu.se
on 2015-06-25 09:00:16
Reported by torger@ludd.ltu.se
on 2015-06-25 09:00:40
Ok had a brief look, and it may be non-trivial to fix due to that exposure and curves
are merged into the same processing step, "complexCurve".
Someone that has better understanding of the color conversion pipeline should have
a look. Maybe it's just removing exposure compensation from complex curve and make
that a separate step after transform (rotate etc) but before shadows/highlights. It
seems odd to me that shadows/highlights are run before exposure like it is now.
With that separated my plan is just to add an extra parameter to "convertColorspace"
which is the pass, first you do CCPASS_BASE where convertColorspace is now, and then
an additional call with CCPASS_POST_EXPOSURE after exposure and shadows/highlights
but before curves.
I don't think I'm the right guy to break apart the pipeline though. Someone else?
Reported by torger@ludd.ltu.se
on 2015-06-25 10:44:47
The exposure compensation is merged with highlights correction in rgbProc().
It is quite easy to separate exposure compensation from highlight correction inside
complexCurve() in my opinion. Thus, exposure compensation will yield values above MAXVALf,
discussed tables will be applied and highlight curve will bring them back.
The "processing pipe" section in one of the pages you linked to says that corrections
are done withing HSV space. This is much bigger problem than separation of exposure
from highlights correction.
Reported by pinhuer
on 2015-06-28 09:08:59
DNG profiles always make corrections in RGB-HSV linear prophoto, it's in the format,
we can't change that. It's not a real problem though as it's up to the profile designer
to design the result, the space the calculations are done in doesn't really matter
as long as the table is dense enough. My own profile maker DCamProf use CIECAM02 LCh
space for some of its corrections, and then the result is translated into the RGB-HSV
table in the DNG profile. Result becomes identical.
It should be easy to separate out the exposure step in the pipeline though as you say,
but messing around in which order things are made in the rendering pipeline is quite
a sensitive thing "politically", so I was hoping someone that has actually designed
the order of things in it could have a look and have opinions about which is the-right-way(tm)
to do it.
Reported by torger@ludd.ltu.se
on 2015-06-28 13:42:41
Reported by torger@ludd.ltu.se
on 2015-06-28 14:05:12
>DNG profiles always make corrections in RGB-HSV linear prophoto
In the same page about hue twists it says that the mapping is done inside XYZ-HSV (not
ProPhoto-HSV), btw.
Reported by pinhuer
on 2015-06-28 19:53:11
Here's a patch.
LookTable is now applied after exposure but before tonecurve, and also baseline exposure
is applied.
The LT is applied inside rgbProc() in tiled mode.
DCP tonecurve and baselinexposureoffset is applied at the same time.
No GUI changes at this time, that is you cannot turn off looktable or baseline exposure
yet.
Reported by torger@ludd.ltu.se
on 2015-07-01 14:49:02
PatchSubmitted
Hi Anders, this patch does not apply, could you please verify
Reported by michaelezra000
on 2015-07-01 16:16:31
likely a false alarm, patch applied via "apply" action (failed with "import" before)
Reported by michaelezra000
on 2015-07-01 16:19:42
There is a significant improvement in handling highlights near clipping when using negative
exposure compensation!:)
Reported by michaelezra000
on 2015-07-01 16:27:41
Here's one with checkboxes in the colormanagement panel, so you can enable/disable looktable
and baseline exposure as well as tonecurve
Reported by torger@ludd.ltu.se
on 2015-07-01 16:53:27
Note that for many/most profiles that actually has a looktable it does not make sense
in terms of look to disable the looktable, they may not have any huesatmap at all and
look broken with it.
It seems to be quite rare with profiles that are designed according to the "DCP intention",
ie accurate colorimetric with huesatmap and a look on top, but if you have such a profile
you can now enable/disable the look separately
Reported by torger@ludd.ltu.se
on 2015-07-01 17:01:56
About #12 (higlight/clipping)
In this patch I have also removed the premature clipping that was in before, discussed
in issue 2804. I have not decided if I should leave that in or not.
Applying the looktable after exposure like now already removes some negative exposure
strangeness, so it may be those effects you are observing rather than the embedded
2824 patch.
Reported by torger@ludd.ltu.se
on 2015-07-01 17:06:47
We can borrow this profile from lightroom just to have something to test the checkboxes
with, it has a curve, a looktable and a baseline exposure offset (-0.15 stop). As discussed
in #14 it doesn't have a huesatmap and the colors look crazy with the looktable disabled
though.
Reported by torger@ludd.ltu.se
on 2015-07-01 17:08:49
The only commonly used DCP tag we don't look at after this patch is the "DefaultBlackRender",
which tells if the profile wants automatic black level adjustment or not (ie auto adjustment
of the black slider). I can live without that.
Reported by torger@ludd.ltu.se
on 2015-07-01 17:16:43
Here's a little update: I moved looktable application after shadow and highlight compression
which I think is more appropriate.
Reported by torger@ludd.ltu.se
on 2015-07-01 20:58:20
http://torger.dyndns.org/rt-bugs/dcamprof-test.dcp is a raw test profile from DCamProf
which shows the use of a huesatmap to make true colorimetric linear profile, and then
looktable + curve to get a contrast look.
I have changed default to enable the tonecurve in the DNG profile if embedded. Profiles
that have them are generally designed to look good only with the supplied curve applied,
so I think it makes sense to apply it.
The LookTable+ToneCurve application will clip, it does not make sense to make that
clipless (like HueSatMap is), as a LookTable will likely include some desaturation
towards clipping, and the curves applied afterwards will clip anyway, so I think it's
the right thing.
Reported by torger@ludd.ltu.se
on 2015-07-01 21:08:23
Could you post some comparison of RT export vs ACR export?
>huesatmap
>to make true colorimetric linear profile
What do you mean when you say "linear profile"? I always thought that no profile can
be linear if it uses huesatmap even if it is separated from luminance.
Reported by pinhuer
on 2015-07-02 12:05:28
#20
I'm on summer vacation on a remote location, I don't have a full workstation here with
all my software, no Lightroom here :-\. It should be highly similar now. The largest
difference is if the profile has DefaultBlackRender not set to None, because then LR
does automatic black level subtraction.
Correct, I should just say "colorimetric profile", that is the profile is not adapted
to be used with a contrast S-curve, but just to be used with a linear curve and then
produce as accurate color as possible, for reproduction work and similar. A tech explanation
is here: http://www.ludd.ltu.se/~torger/dcamprof.html#tone_curves
In other words a camera needs non-linear corrections (huesatmap) to get as close as
possible to linear colorimetric behavior.
In many (most?) Adobe's own profiles the huesatmap is not made to provide colorimetric
accuracy, but is already there adapted for being used with a curve and then create
a "look". Then they also have a class of profiles which only has a looktable and no
huesatmap which is used to emulate the look of manufacturer ICC profiles.
Turning on/off looktable and get sane results only makes sense if the base huesatmap
is designed for colorimetric accuracy, and the look+curve is designed for providing
a subjective look. The DCamProf profile in #19 is designed like that.
Reported by torger@ludd.ltu.se
on 2015-07-02 14:03:01
A thing to decide: in the current patch there are three checkboxes, 1) use curve 2)
use looktable 3) use baseline exposure.
I have no problem with that, RT is RT and it has many buttons to play with, but if
we want to keep down the number of checkboxes we could have just one checkbox called
"apply embedded DCP look" which will then toggle all three curve+looktable+baseline
in one.
It generally doesn't make sense to apply looktable without a curve, it would be a very
strange profile if the looktable is designed to work both with and without curve, that
is if you have a looktable and a curve embedded it should both be applied. Baseline
exposure is just an exposure offset, usually used for a small static black level subtraction,
I think one can embed that into the look package as well.
What do you think? Three checkboxes like now, or just one, or something else.
Reported by torger@ludd.ltu.se
on 2015-07-02 14:08:58
My initial reaction is to go for the simple option if it is as you describe it - highly
unusual to use them otherwise - and to complicate it only if someone requests it.
Reported by entertheyoni
on 2015-07-02 14:14:30
I find it beneficial to be able to disable the DCP curve to compose one's own. Yet,
for high volume quick processing using the built-in curve speeds things up.
Disabling the look table seems to remove color calibration entirely, I am not sure
why would one ever disable it.
I have no issue with three checkboxes. It was nice to see that when all three checkboxes
are disabled result is the same as "no profile" option.
What is the huesatmap implementation in RT? Adobe's color model often leads to garish
color, it would be great to avoid that:)
Reported by michaelezra000
on 2015-07-02 15:11:00
(I've found a bug introduced in HL handling, will post updated patch later.)
With all checkboxes enabled DNG profiles are applied according to the DNG specification,
with the exception of the automatic black subtraction tag as discussed in #17. With
all checkboxes disabled the first DNG profile pass is still made, where the matrix
and HueSatMap (if any) is applied.
Many DNG profiles has either a huesatmap OR a looktable, only a few have both, like
the DCamProf example in #19. Many of Adobe's own profiles have the extra twist that
they have no embedded curve, but this in LR/ACR means that "Adobe default curve" should
be applied (no word about that in the DNG spec). Most bundled RT DCPs have only a HueSatMap,
no looktable afaik, for those profiles this patch should make no change.
Adobe's often garish colors are due to Adobe's profiles, they are designed with that
look, it's not the fault of the DCP format itself. Then we have the problem discussed
in the link in #20 that if you apply a "film-curve" or "standard" curve to a colorimetric
profile the colors will be garish, as those curve types strongly increase saturation.
Reported by torger@ludd.ltu.se
on 2015-07-02 15:22:36
Updated patch, fixed broken highlights.
Also added a new feature which makes controlled clipping of out of gamut colors. Normally
DCP looktable+curve handles clipping with nice desaturation towards the clip point,
but if you don't have that you can get some pretty bad color shifts for out of gamut
colors. Now this is handled, this sort of fixes issues 2824 (no more out of gamut blue
turning purple), they are interconnected so I thought I could fix both issues in same
patch.
Reported by torger@ludd.ltu.se
on 2015-07-02 15:48:40
To test the highlight fix, you can run without DCP, then open test file from issue 2804,
apply an extremely blue white balance (say 2300K), push exposure and watch the white
shirt turn purple/pink. Apply the patch and watch the shirt stay the same color with
gradual desaturation towards white when increasing exposure.
The look is not perfect, but never will with aggressive out of gamut clipping, much
better than before patch.
Reported by torger@ludd.ltu.se
on 2015-07-02 15:54:21
Demo of HL fix: http://torger.dyndns.org/rt-bugs/hlfix.jpg
Using the strange white balance to provoke an extreme out of gamut blue, but extreme
out of gamut blue can exist with normal white balance too, quite rare though, although
a flower photographer probably sees them now and then...
Reported by torger@ludd.ltu.se
on 2015-07-02 16:03:52
Anders, I made a version of lt-patch-04.txt which applies to tip after latest changes.
Shall I post it or do you already have a new one?
Ingo
Reported by heckflosse@i-weyrich.de
on 2015-07-02 16:43:49
I've been out kayaking so I haven't done anything. Seems to be quite easy to fix. But
if you sit at the computer now, post it, otherwise I'll fix it and post when complete.
Reported by torger@ludd.ltu.se
on 2015-07-02 18:45:57
Ok, here it is
Reported by heckflosse@i-weyrich.de
on 2015-07-02 18:49:54
I'm going to post a new patch which changes highlight clipping again, tested through
and the film-like curve, ie Adobe's, clips out of gamut the best-looking among all
tested curves and clip methods I've gone through. But now it's icecream eating time...
Reported by torger@ludd.ltu.se
on 2015-07-02 19:16:55
Icecream was delayed. Here's an updated patch, now with "film-like" clipping for out
of gamut colors.
I'm going to want this patch (when tested/complete) for 4.3, as it improves usability
of the various DCPs out there a lot.
Reported by torger@ludd.ltu.se
on 2015-07-02 19:39:35
I understand the need in #24 so I went all in, here's a patch where you can toggle huesatmap
as well. Leaving out only one aspect uncontrollable seemed odd. Disabling it all and
you get matrix-only correction which may or may not make sense depending on profile
design. It works for DCamProf profiles.
When changing DCP file the checkboxes will return to their defaults (ie checked). I
think it's okay although I'd rather keep old relevant settings, but combining that
with enabling and disabling checkboxes turned out into chaos. I don't like spending
hours hacking GTK panels with big potential for bugs anyway, so I opted for the simpler
behavior.
Reported by torger@ludd.ltu.se
on 2015-07-02 22:46:53
A valid use case for disabling tone curve is to replace it with an own slightly different
curve. A valid use case for disabling look table + curve is to get colorimetric profile
only for reproduction work (for profiles supporting it), and a valid use case for disabling
huesatmap is to get matrix only correction for exporting files which require strict
linearity say for HDR processing.
So I'm okay with all those checkboxes. Now with two columns they don't take up so much
space either.
Reported by torger@ludd.ltu.se
on 2015-07-02 22:50:43
Did not work out with reset to default checkboxes, it breaks the history. Changed so
the checkboxes holds the latest setting regardless if the current DCP has the setting
or not.
Reported by torger@ludd.ltu.se
on 2015-07-03 16:21:05
I'm starting to get satisfied with my own testing of this patch and I'd like to commit
in the coming days.
Raise voices if anyone is against or wants further adjustments of the behavior.
Reported by torger@ludd.ltu.se
on 2015-07-04 08:52:05
Reported by torger@ludd.ltu.se
on 2015-07-04 08:53:26
Issue 2824 has been merged into this issue.
Reported by entertheyoni
on 2015-07-04 17:26:52
Hi Anders, the patch works great in imaging aspects. There is a bug in GUI signal processing.
Test case:
1. Open raw with an assigned Custom DCP profile.
2. Reset pp3 to Neutral. this switches input profile to "Camera Standard" matrix mode
and the Custom gets set to "None" (OK, as before)
3. Select "Custom" radio button - exception
Reported by michaelezra000
on 2015-07-05 00:19:07
Crash confirmed.
Reported by entertheyoni
on 2015-07-05 16:03:18
Thanks for testing. Fixed that bug, updated the patch to match latest changes, and committed.
Reported by torger@ludd.ltu.se
on 2015-07-06 08:28:44
FixedPendingConfirmation
The only problem remaining is that "reference image for profiling" is terribly broken
where highlights are clipping - with or without "Apply white balance".
Reported by pinhuer
on 2015-07-07 16:45:54
#43 Thanks for testing.
Reference image for profiling is made without profile, so that code is not touched
by this patch. I haven't tested myself but when exporting for profiling there is AFAIK
no clip rendering at all, so I guess you would see magenta/pink flat highlights if
you have clipping. This is no problem though, more of a feature I'd say :-), because
if you have clipping you shouldn't use the shot for profiling (unless the clipping
is outside the test chart, but then the look does not matter).
If we should make highlights look good we need either clip early, ie scale samples
that would be misleading for the profiler, or reconstruct samples, which would also
be false. Exporting for profiling is as close to raw data you get (basically just demosaiced)
and then clipped highlights will look bad as the separate channels don't clip simultaneously.
Reported by torger@ludd.ltu.se
on 2015-07-07 18:14:33
>This is no problem though, more of a feature I'd say :-), because if you have clipping
you shouldn't use the shot for profiling (unless the clipping is outside the test chart,
but then the look does not matter).
I never use "reference image" for profiling. I use it for debugging purposes.
If it is possible to make it "normal" again I would prefer it.
>and then clipped highlights will look bad as the separate channels don't clip simultaneously.
"Apply white balance" is unchecked and it is still bad.
http://i.imgur.com/lvkkAvj.jpg
Reported by pinhuer
on 2015-07-07 18:27:03
Okay, you're right it's this patch that did it. The reason is that I removed the premature
clipping in rawimagesource so highlights could be brought in with negative exposure
settings.
I think the result is as expected, the "cleaned up" result like before was not good
as it clipped raw data, in some cases even when there was no actual clipping in the
file.
I'll have a second look though as I'm not 100% sure what you see is what should be
expected.
Note that the export for profiling "pipeline" is separate from the rest, so it's very
limited in terms of debugging. If you want to debug the pipeline I'd suggest you dump
images directly in the code were you want to see the result. I dump ppm for that, you
can copy paste these functions:
static void
dump_ppm(Imagefloat *im, const char filename[])
{
FILE *stream = fopen(filename, "w");
fprintf(stream,
"P6\n"
"%u %u\n"
"255\n",
im->width, im->height);
for(int i=0;i<im->height;i++) {
for(int j=0;j<im->width;j++) {
uint8_t rgb[3];
rgb[0] = im->r(i,j) / 512;
rgb[1] = im->g(i,j) / 512;
rgb[2] = im->b(i,j) / 512;
fwrite(rgb, 3, 1, stream);
}
}
fclose(stream);
}
static void
dump_ppm_lab(LabImage *im, const char filename[])
{
FILE *stream = fopen(filename, "w");
fprintf(stream,
"P6\n"
"%u %u\n"
"255\n",
im->W, im->H);
for(int i=0;i<im->H;i++) {
for(int j=0;j<im->W;j++) {
uint8_t rgb[3];
rgb[0] = im->L[i][j] / 128;
rgb[1] = im->L[i][j] / 128;
rgb[2] = im->L[i][j] / 128;
fwrite(rgb, 3, 1, stream);
}
}
fclose(stream);
}
Here I divide by 512 (instead of 256) to leave some headroom to check values above
the clip limit. It writes 8 bit ppm but you can do 16 bit too if you want.
Reported by torger@ludd.ltu.se
on 2015-07-07 18:51:41
In profiling export there was wrap-around in clipping, now it's clipped straight off
as it should (committed).
Reported by torger@ludd.ltu.se
on 2015-07-08 08:39:21
Found a bug in parameter handling, committed fix.
Reported by torger@ludd.ltu.se
on 2015-07-13 13:21:40
Anders/DrSlony, please make the text beside the check boxes shorter (I would clip "use"
and "offset" from the text ..)
.. or aligne all vertically in single column ..
As it is now it results to a very wide tool panel or else we have decreased visibility..
:(
Reported by iliasgiarimis
on 2015-07-15 14:42:07
Ok
Reported by entertheyoni
on 2015-07-15 14:56:43
Originally reported on Google Code with ID 2739
Reported by
entertheyoni
on 2015-04-09 22:34:36