Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.77k stars 314 forks source link

Using "as_shot" WB in early stages instead of current WB results in artifacts when WB difference is large #2043

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2059

Using "as_shot" WB before demosaic results in artifacts when "as shot" dosen't match
real scene WB.

The problem is most evident when UniWB (all WB multipliers =1 ..or around) was used
for a shot.

http://rawtherapee.com/forum/viewtopic.php?f=1&t=3776&p=27263#p27263
http://rawtherapee.com/forum/viewtopic.php?f=1&t=4410#p34227

UniWB Files from the second link corrected to autoWB with exiftool

http://filebin.net/ha15wlpjmh

Reported by iliasgiarimis on 2013-11-17 14:04:20

Beep6581 commented 9 years ago
Isn't UniWB used to get as most detail as possible in the HL? Technically, is this setting
set in camera, and is there a real gain in HL detail to do so regarding the raw data?

Do you have any solution to provide that we could code (you don't code IIRC?). Would
it be sufficient to move the WB operation after demozaicing?

Reported by natureh.510 on 2013-11-17 22:53:12

Beep6581 commented 9 years ago
No, not for detail .. UniWB is used to get the closest to "RAW histogram" in Camera.
In fact it's useful because this way the histogram shows 1:1 the raw channels before
WB multiplication so the user based on "raw histogram" can expose with safety to the
right (ETTR) i.e use the greater exposure possible. And greater exposure gives better
SNR.

The solution I'm thinking of is, instead of using the "as shotWB" tag that is now used,
use the "as metered WB" or "autoWB" exif tag  (I think all raw formats include this).
Or else perform in RT the autoWB calculation and use this result.

In the following link Emil describes exactly this (I think) with the difference that
the in camera Custom WB should not be used because it is dangerous .. either due to
UniWB either due to user error custom WB could be set far from the real scene's WB
..  

http://rawtherapee.com/forum/viewtopic.php?f=1&t=3776&p=27263#p27250

This only for the input needed at the first stage before demosaic. The WB setting used
in RT's default WB could stay as is i.e. "camera"("as shot").

Reported by iliasgiarimis on 2013-11-18 02:14:36

Beep6581 commented 9 years ago
#1 "Would it be sufficient to move the WB operation after demozaicing?"

As one can see reading from the start the linked thread, a correct WB is necessary
for a correct demozaicing (Gulliermo and Emil say so ..). Moving it after gives artifacts.

http://rawtherapee.com/forum/viewtopic.php?f=1&t=3776&p=27263#p27234

Reported by iliasgiarimis on 2013-11-18 02:29:41

Beep6581 commented 9 years ago
I think it's a quite uncommon design to require white balancing before demosaicing,
but there are some advantages to that so we probably don't want to change that.

I think the solution is to derive a default D50 wb from the dcraw D65 color matrix,
and when the "as shot wb" is detected to be too far off the default D50 wb is used
during the initial steps which require sane wb, and then we switch back to the as shot
wb. That is you won't notice that the demosaicer is using a different WB when needed.

Reported by torger@ludd.ltu.se on 2013-11-18 09:43:21

Beep6581 commented 9 years ago
"as metered WB" or "autoWB" exif tags is not included in all formats, not even Canon
CR2 afaik, when you use manual wb it doesn't record metered, so we need to derive our
own "sane" balance.

Possibly we should make a special case just for uniwb, ie if it looks far off *and*
multipliers are close to 1,1,1, then we use the default D50 wb, otherwise not. I guess
it could be the case that uniwb is less far off D50 than tungsten is, and we don't
want to convert manually set tungsten to D50. Or we relate to an auto white balance
instead of a default D50, it could be more robust, but also more CPU intensive.

Reported by torger@ludd.ltu.se on 2013-11-18 09:52:43

Beep6581 commented 9 years ago
Uh, wait a minute, maybe I'm making it more complicated than it is:

What happens if you change the white balance after loaded, ie you change your green
color to appropriate white balance? Do the artifacts disappear? Or is the "as shot"
white balance used still in demosaicing when you have manually set a different white
balance in the program?

I think it should be like this: any step in the pipeline that needs to know the white
balance should use the user-set white balance. And if that is the case, this should
be a non-issue. Who cares if there are artifacts when the image is first loaded before
white balance is corrected?

Reported by torger@ludd.ltu.se on 2013-11-18 09:57:14

Beep6581 commented 9 years ago
#5, Torger at least for Canon 400D/7D the corrected samples that I provided in #0 where
a manual WB was used (UniWB is a  manual setting) and I found in maker (Canon) data
the tags 
0x0000 (WB_RGGBLevelsAsShot - which is currently used by RT), 
0x0005 (WB_RGGBLevelsAuto) and 
0x000a (WB_RGGBLevelsMeasured)
Then changed the AsShot values with the Auto values and the problem solved. I agree
that this "Auto" or "Measured" info lies in different tags depending on Maker or even
Model so it is too much work to cover all cases.

So RT can either use a standard "sane balance" as you say or 
- use a calculated in RT "Auto" for better matching with the scene's WB (this code
should be readily available in Dcraw .. as when we use the -a switch).. will the delay
be that much for this calculation ?.

Maybe we should ask for less abnormal than UniWB manual settings to investigate the
behavior .. a shot under tungsten illumination with the WB manually set at Daylight/Shade..
then decide for the better. 

Reported by iliasgiarimis on 2013-11-18 10:43:41

Beep6581 commented 9 years ago
"What happens if you change the white balance after loaded, ie you change your green
color to appropriate white balance? Do the artifacts disappear?"
I tried on the UniWB raw from Elle Stone's article, and encountered the same green
fringing as she had after setting correct white balance inside RT.

Reported by entertheyoni on 2013-11-18 12:44:40

Beep6581 commented 9 years ago
Can someone provide direct link to the file(s) to test with? I don't like having to
read through forum threads to see what's wrong.

Reported by torger@ludd.ltu.se on 2013-11-18 13:31:30

Beep6581 commented 9 years ago
#6 "I think it should be like this: any step in the pipeline that needs to know the
white balance should use the user-set white balance. And if that is the case, this
should be a non-issue. Who cares if there are artifacts when the image is first loaded
before white balance is corrected?"

Do you mean recursively redo the early steps based on the desired output ?. This may
work or not, I don't know, and I think that we should communicate with Emil (or any
knowledgeable on demosaic person) to explain the role of WB for demosaicing and the
best solution quality wise  .. then decide how to adapt RT on this.   

Reported by iliasgiarimis on 2013-11-18 13:32:20

Beep6581 commented 9 years ago
Direct links ..

http://www.rawtherapee.com/shared/test_images/LunchingRoom.CR2

http://ninedegreesbelow.com/photography/raw-processor-review/test-files/rawtherapee-test-files.tar.bz2

or http://ninedegreesbelow.com/photography/raw-processor-review/test-files/rawtherapee-test-files.zip

Reported by iliasgiarimis on 2013-11-18 13:36:03

Beep6581 commented 9 years ago
thanks for helping out a lazy developer. Now I have both files and have confirmed the
problem.

#10: As said in #8 my idea did not work, and of course it would be a pain if we would
have to redo demosaicing for each white balance change. So yes we need some reference
wb to do demosaicing with.

Reported by torger@ludd.ltu.se on 2013-11-18 14:35:22

Beep6581 commented 9 years ago
Hmm... still, probably the right way to do it is to redo demosaicing if the currently
selected white balance deviates too much from the white balance used at demosaicing.

Tested through a number of demosaicing algorithms, and most/all seem to do a better
job if the white balance is not too far off. So this is not just an amaze issue. And
I think it seems quite natural that a demosaicer, which to some extent does guessing,
is much better of making good guesses if white balancing is fairly close to the desired
output.

The task to implement involves almost no algorithmic coding, it's just finding one's
way through RT engine pipeline and fire off the proper triggers ("if new wb is too
far off old wb, then redo demosaicing"). Ie quite tough job for a relative newcomer
to RT internals like me. So I'd recommend that someone else (did I hear someone say
Hombre? ;-) ) look into this issue.

Reported by torger@ludd.ltu.se on 2013-11-18 14:54:32

Beep6581 commented 9 years ago
#10: what emil could help us with is when this function should return true:

bool redoDemosaicingDueToChangedWhiteBalance(double old_R_multiplier,
                                             double old_G_multpilier,
                                             double old_B_multpilier,
                                             double new_R_multpilier,
                                             double new_G_multpilier,
                                             double new_B_multpilier)
{
    if (<too large difference between old and new>)
        return true; // we need to redo demosaicing
    else
        return false; // not large enough difference, we can keep current demosaicing
}

on the other hand we could probably find out with experiments.

Reported by torger@ludd.ltu.se on 2013-11-18 15:04:24

Beep6581 commented 9 years ago
Looked at the code. Changed my mind of what to do. I think we can rerun demosaicing
for every white balance change, as it's only necessary when we simultaneously look
at 100%. Changing white balance when zoomed out will work without re-demosaicing. By
doing like this we don't need to find out when the deviance is too large, we just do
with the right white balance every time (ie we don't need to implement the redoDemosaicingDueToChangedWhiteBalance
function). Possibly we could optimize by having such a function anyway, but it will
probably not be necessary.

We just need to change the current flow of always using "as shot white balance" in
the early stages and then remultiply to appropriate wb to instead use the current wb
from start. I don't see any obvious obstacle to do so.

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

Beep6581 commented 9 years ago
What is "current WB"? The selected one in the WB tool? It can be Automatic, i.e. based
on the analysis of the image, which would create an infinite update loop or a wrong
calculation, or it could be "As shot", with wrong multipliers. Just ignore me if i've
said silly things, i'm not a "color scientist" at all! :]

I'm not fond of the "redemozaice on WB change with preview at 100% only" solution either...

Reported by natureh.510 on 2013-11-18 18:09:54

Beep6581 commented 9 years ago
I've played with the code for a couple of hours. I think now I'm quite suitable to solve
this issue, but I'm not likely to complete it before 4.1 I think, as I need to reduce
my workload quite a bit.

The demosaicers expect to know what the color of the final picture to be, ie they expect
to know the current white balance, that is the one in the WB tool. The current pipeline
has however a fixed WB for demosaicing, it cannot be changed, and it's set to the AsShot
white balance. As it's often close to what we want in the end it's rarely a problem,
but with uniwb shooting technniques it is. It also breaks the typical layman knowledge
many have about raw files: that the WB in the raw file does not matter. For RT it does.

I consider this to be something that should be fixed eventually, a raw converter pipeline
should not be dependent on he raw file white balance setting. Not sure if it's urgent
though, don't know how popular the uniwb technique is. Many talk about it, but I don't
know how many that actually use it.

AutoWB looks at raw values, I there will be no infinity loop. Or I think it won't be,
haven't actually tested that but it can be fixed if there is a problem. AutoWB does
not need a WB itself.

We need to provide current wb to the ri->preprocess() step (before demosaic). This
is no problem. However preprocess() is rarely run. Instead re-scaling with current
WB takes place in ri->getImage(), this is also okay. However if white balance strays
too far away from what we had in ri->preprocess(), ie we get too large multipliers
in ri->getImage() and we look at 100% view we will se artifacts. We then need to demosaic
again if we want to have a good preview.

We could ignore the artifacts in the preview if we want, as preprocess() with the correct
wb will be used when generating output.

This could be made as a first step, as such a patch is (probably) quite easy to do.

If we want a correct preview we need to regenerate, and then to avoid redemosaicing
all the time when at 100% and moving the wb slider we would need some limit of how
far off we can get before redemosaicing is necessary.

Reported by torger@ludd.ltu.se on 2013-11-18 18:25:50

Beep6581 commented 9 years ago
I could possibly fix the first step of the issue, ie correct output but still artifacts
in preview.

Reported by torger@ludd.ltu.se on 2013-11-18 18:26:45

Beep6581 commented 9 years ago
Here's a preliminary patch. Actually it's only the files rawimagesource.cc improccoordinator.cc
and simpleprocess.cc and headers that are touched, but I have a gazillion of other
patches which I've not yet had opportunity to commit, and didn't have time to separate
out it tonight. There's overlap with the DCP patch mainly.

It seems to work. Current wb is used both in preprocess() and get_image(), but of coures
preprocess() is only run sometimes, so when you change WB using the WB tool only get_image()
sees the change and adapts accordingly. When you generate output, or close and open
the file again, a new preprocess() call is made with the latest white balance.

Issue that remains to be solved is how/when to update the preview, if caring at all.

Hopefully I get a cleaner patch later this week when I've committed the other stuff.

Reported by torger@ludd.ltu.se on 2013-11-18 19:38:54


Beep6581 commented 9 years ago
Note that we don't really know if it's almost as good to simply set the fixed white
balance to D50 rather than "as shot" (I haven't tested). That would be a simpler fix.
However I think the "right" way to do it is like this to have the target white balance
from start.

Reported by torger@ludd.ltu.se on 2013-11-18 20:52:35

Beep6581 commented 9 years ago
other work committed. Here's a clean patch.

Reported by torger@ludd.ltu.se on 2013-11-19 08:01:40


Beep6581 commented 9 years ago
Test case with patch:

http://www.rawtherapee.com/shared/test_images/LunchingRoom.CR2

1. Open green (uniwb) lunch room, set white balance on clock.
2. Zoom to 200% see artifacts (green pixels along the clock edge).
3. Close the image and open it again, zoom in and see that artifacts is no longer 
   there.

You need to close and open as preprocess() is only called at loading (I think), and
now the last set white balance is provided to it rather than always "AsShot".

Reported by torger@ludd.ltu.se on 2013-11-19 08:10:26

Beep6581 commented 9 years ago
Here's a patch that auto-remakes the preprocess() step if white balance change is "too
large". Ie this patch represents a "complete" solution.

I've not really made a test to find what "too large" really is, just set it to when
any of the multipliers is a factor 2 or more off, which probably is more conservative
than it needs to be. Adjust the ImProcCoordinator::isLargeWhiteBalanceChange() method
to change criteria.

There's a quite big risk that this patch disturbs things like highlight reconstruction,
or auto levels or something like that, haven't really tested it thoroughly concerning
those aspects.

Reported by torger@ludd.ltu.se on 2013-11-19 08:59:14


Beep6581 commented 9 years ago
refinement: only do it when at high detail mode, as it only matters there.

Reported by torger@ludd.ltu.se on 2013-11-19 09:14:40


Beep6581 commented 9 years ago
Torger, I think it matters even on low detail preview, because not only the artifacts
are affected by preprocessWB but the color rendering also (sometimes by much) and even
picture brightness, highlight detail etc. ... I wasn't expecting this !!

Looks like we opened a room of worms here .. much testing needed

I am still not sure if it's better to match preprocessWB with outputWB or with the
scenes content i.e. preprocessAutoWB so that we balance the relative strengths of R,
G, B in order to help demosaicing.

Reported by iliasgiarimis on 2013-11-19 12:00:51

Beep6581 commented 9 years ago
ok, I haven't really looked at picture brightness and those effects, but I would not
be surprised if there are issues there. It *should* be quite easy to fix though, because
the only thing we really do is that we change what white balance preprocess() sees,
just like you did by hacking the CR2 file. However there may be some more variables
that need updating.

Reported by torger@ludd.ltu.se on 2013-11-19 12:22:20

Beep6581 commented 9 years ago
Concerning which white balance to use I think we need to ask Emil for advice. Added
his address at Cc, hopes he reads this:

Emil can you answer this: which white balance is best to feed the demosaicers with,
same as the user-selected white balance (ie the white balance we get in the output),
or some other white balance such as auto-white balance which balance the relative strengths
of RGB?

We're working on a fix to handle extreme changes from "AsShot" white balance better,
which happens for the users using the "UniWB" shooting technique.

Reported by torger@ludd.ltu.se on 2013-11-19 12:26:41

Beep6581 commented 9 years ago
#26 I don't think we should have any problems with highlight rendering, that's already
taken care of (could be some bug somewhere though, haven't tested it thoroughly).

However, auto levels is affected, and we need to relate to that in some way. It should
not be much different from how it works without this patch though, if you change white
balance you can get more or less space in the color space depending on how white your
highlights become (the whiter highlight, the more you can push levels), so I guess
autolevels should be recalculated for each white balance change, even small ones.

Or you just see autolevels as a slider-setting helper which you push once and then
you need to manually push it again if you want help after you have altered things that
affects it. I don't know how autolevels is supposed to work.

Reported by torger@ludd.ltu.se on 2013-11-19 13:40:07

Beep6581 commented 9 years ago
#29 hmmm... okay my patch has seriously broken something with autolevels. In theory
it should work exactly as before the patch, but it's gone haywire. Looking into it.

Reported by torger@ludd.ltu.se on 2013-11-19 13:50:21

Beep6581 commented 9 years ago
found one scale factor issue, but still at least one remains. I *think* with a bug-free
patch the only difference should be that demosaicing artifacts disappear. I don't think
there's an issue with highlights or levels, as scale factors are propagated so the
old calculations become exactly the same, it's just the demosaicers that see the new
color.

In theory at least. We'll see how it goes. Hopefully I can complete a patch within
a few hours. Have some other stuff too to take care of though :-).

So I'd stay low with testing the current patch, the scale factor errors makes it a
bit confusing to use (image changing brightness seemingly at random occasions).

Reported by torger@ludd.ltu.se on 2013-11-19 14:22:27

Beep6581 commented 9 years ago
Ilias, you were right, can't skip fast demosaic, need to redo that to avoid brightness
shifts. With that in place I think this attached patch works all the way.

However, still need to know if we are better off with a fixed "autoWB" or if we should
follow current WB like this.

Reported by torger@ludd.ltu.se on 2013-11-19 15:58:07


Beep6581 commented 9 years ago
And here's a patch for the simple alternative, when we instead of using "AsShot" white
balance in preprocess we use an AutoWB (using dcraw's AutoWB code for this)

When playing around it seems to me that this patch does the job just as good as the
more advanced approach, or maybe even better. And you don't need to re-demosaic, the
patch is simple and much less likely to introduce new problems than the more advanced
approach.

Reported by torger@ludd.ltu.se on 2013-11-19 16:26:20


Beep6581 commented 9 years ago
last patch for today.

Here's a variant of the simple approach were we use a fixed 5000K white balance instead
of autoWB (the patch contains disabled code for autowb too, so it can be further simplified).

After thinking some more I find it likely that this approach is the best. The CA Autocorrect
function expects chromatic abberation to have the expected colors, and for that it's
probably better with a fixed white balance.

Hopefully 5000K works well for demosaicers too, even for shots made in extreme light.

In rawimagesource.cc you have if (0) for autowb and if (1) for 5000K, simply by swapping
one can enable the other way to work if you like to test.

It could be the case that for auto CA correct 5000K is better and for demosaicers autoWB
is better, but I find it unlikely that it will matter.

I think I prefer the 5000K version, as it's a fixed predictable behavior.

Reported by torger@ludd.ltu.se on 2013-11-19 16:54:48


Beep6581 commented 9 years ago
Ok now I've through experiments tried to find out which of the approaches of A) using
current user set wb, B) using auto WB C) using fixed 5000K and D) unchanged, ie as
shot WB that is best.

My conclusion is that Auto WB is the best approach. Not only demosaicing becomes better,
but also CA autocorrect. I thought that CA autocorrect would possible gain from 5000K
fixed temperature, but in my test pictures Auto WB was still better.

So now I've attached a "final" patch which uses the auto WB approach. This is a simple
patch and not very intrusive, ie likely to not cause any problems.

I'd like to commit soon.

Reported by torger@ludd.ltu.se on 2013-11-20 07:36:11


Beep6581 commented 9 years ago

Reported by torger@ludd.ltu.se on 2013-11-20 07:45:30

Beep6581 commented 9 years ago
I also found in my experiments (changing the WB multipliers at exif maker data) that
AutoWB works better for demosaicing and autoCa.

Andreas, which method do you use for autoWB ?. The one currently used by RT (temp/tint)
or by weighting-balancing the raw channels and take the same raw R/B multipliers as
if the AutoWB setting was used in camera ?. 

I ask because sometimes RT's autoWB fails, like for underwater shots with the camera
set at autoWB (so the "as shot" tag equals AUtoWB tag). Then the R multilier is extra
high, at the area of 5-10 and even the expanded improved WB cannon handle it. 
My thought is to use some limits on the permitted WB multiplier's range. As an early
approach I found that keeping (clipping) R/B multipliers at 1.1 (low limit) and 4.0-4.5
(high limit) makes RT's wb work better.

http://filebin.net/jpxk7phc39 (underwater shot, download expires in 6 days ..)
http://rawtherapee.com/forum/viewtopic.php?f=2&t=4607
https://code.google.com/p/rawtherapee/issues/detail?id=1894 

Reported by iliasgiarimis on 2013-11-20 10:21:55

Beep6581 commented 9 years ago
I use the dcraw method (already in the code), which is very basic, samples the whole
image (skips clipped values) and makes an average. I thought it would be better to
use a simple and predictable method rather than "something smart" for this application.
Yes the resulting multipliers are used straight off, not going via RT temp/tint, so
no worries. For a verbose message I convert the multipliers to a temp/tint though,
but that's only for a message in the log, it's not used in any way.

The autowb failure you are referring to was with an older version of RT, the current
works okay as the range has been extended, temperature gets 41000 tint 0.3, about the
same as dcraw's autoWB if you convert to temp/tint.

I've tried with some other images too with extreme temperatures, although this is the
most extreme of them.

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

Beep6581 commented 9 years ago
I thought about using RT's own AutoWB, but as I expect that to possibly evolve to some
more intelligent approach I chose to use dcraw method as simply the average multipliers
is probably what's best for this purpose.

Reported by torger@ludd.ltu.se on 2013-11-20 11:25:40

Beep6581 commented 9 years ago
Fine, this autoWB approach is exactly what I had in mind too at #7.

About the underwater sample, I had a forgotten manipulation of R/B weights in channel
mixer (this was my solution before Jacques implemented the expanded range and the r/b
equalizer, in fact I was doing in channel mixer what r/b equalizer does now in WB)
so the rendering was off, no problem with the prototype indeed although the r multiplier
is 5.23.

Ι have somewhere 2 more extreme underwater shots to try .. There r multiplier is really
extreme (8-9) and RT autoWB fails. Wait some mimutes .. 

Reported by iliasgiarimis on 2013-11-20 12:18:41

Beep6581 commented 9 years ago
#40 yep... you had a good approach from start, I just had to try all other alternatives
to get to the same conclusion :-)

Yes good to dig out that example to test with, because this patch should handle those
cases too.

Reported by torger@ludd.ltu.se on 2013-11-20 12:47:14

Beep6581 commented 9 years ago
Two Extreme WB underwater shots, plus hacked WB to sane levels ..

http://filebin.net/c4k8l6p59k

Not that it's absolutely necessary at the moment but as soon as a preprocess-AutoWB
is executed for every shot why not make things easier to handle by setting sane limits
for the multipliers ..

Another (more preferable) option would be when temp/tint approaches the limits RT WB
could auto-use the r/b equalization .. 

Reported by iliasgiarimis on 2013-11-20 13:36:14

Beep6581 commented 9 years ago
Now that I'm thinking of it 

- such limits should exist in HumanVisualSystem as I find myself not fully adapting
at extreme color temps

- Nikon uses WB preconditioning harcoded in it's raws, you can see around 1/10 gaps
in R/B channels in raw histograms. This could be for the same reason as what we find
now. And maybe we have to count this in if we will set WB multipliers's limits, (set
lower limits for Nikon) ..    

Reported by iliasgiarimis on 2013-11-20 13:48:34

Beep6581 commented 9 years ago
Thanks for the test files. These work with this patch, but is too extreme for RT's own
AutoWB. As this patch only use employ raw multipliers it can handle it, so we're good.

The AutoWB/spot needs some work to handle this type of file well, but that's a different
issue, and something for Jaques I guess. I *think* the problem is that RT's white balance
model works in sRGB colorspace (there's conversion to sRGB multipliers before feeding
into colortemp, and one of those multipliers gets negative for these temperatures due
to the limited colorspace). We should probably file a separate issue for handling more
extreme white balances and discuss those issues there.

Feels like this patch is about ready to commit though...

Reported by torger@ludd.ltu.se on 2013-11-20 14:46:41

Beep6581 commented 9 years ago
(yes the eye does not fully adapt to extreme white balances, I'm very aware of this
as I often shoot at winter here up north where color temperatures outdoor get pretty
extreme. This means that one have to set white balance manually on these files to something
that "looks good". But that's a whole different story.)

Reported by torger@ludd.ltu.se on 2013-11-20 14:48:55

Beep6581 commented 9 years ago
I'll wait for a confirmation from one of the other devs or testers before committing.

Re-attaching the patch here so they don't need to scroll-through lots of messages and
potentially pick the wrong patch :-)

Reported by torger@ludd.ltu.se on 2013-11-20 14:51:19


Beep6581 commented 9 years ago
I'm testing. As a first result, with your patch Auto CA correction for this image http://www.i-weyrich.de/rt/D700_20131003_2556.NEF
works much better than without patch. With one of my test pictures, after patch is
noticeable brighter and shows more clipping in highlights than before patch. I can
send you an example per PM, if you like.

Reported by heckflosse@i-weyrich.de on 2013-11-20 15:25:35

Beep6581 commented 9 years ago
I should say that yes there can be a brightness change.

"Autolevels" work on the preprocess white balance, ie "AsShot" previously, and now
on auto white balance. Pure white highlights, which auto white balance typically provides,
allows autolevels to push the image more. If you then choose a white balance for your
picture which is a bit off the auto WB you may experience some more clipping.

To be "correct" autolevels should work on the current white balance always, ie if you
change white balance autolevels would need to be replayed and get another result. One
of my patches was like that. However, I think it becomes confusing to the user if autolevels
change around values when you change white balance so having it to work with a fixed
balance is better.

I though the changes would be relatively mild so one could let it pass, but if there
are large changes one could change the patch so autolevels always work with "AsShot"
as before.

Reported by torger@ludd.ltu.se on 2013-11-20 15:52:36

Beep6581 commented 9 years ago
Maybe I'm testing wrong. Tested with the picture from this link http://ninedegreesbelow.com/photography/raw-processor-review/test-files/rawtherapee-test-files.zip
Look at the apples in upper left corner with before and after patch.

Ingo

Reported by heckflosse@i-weyrich.de on 2013-11-20 16:07:24

Beep6581 commented 9 years ago
You can test to change to "Auto WB" with your file that got brighter, I'm assuming that
clipping is reduced to almost nothing in that case? It's that white balance which autolevels
now is run with, before it was "Camera". Note that in extreme cases "Auto WB" may fail
(it gets negative multipliers), while preprocess white balance do not as it operates
in camera space and thus never get negative multipliers.

Reported by torger@ludd.ltu.se on 2013-11-20 16:07:26

Beep6581 commented 9 years ago
You're right. With Auto-WB the problem is gone.

Reported by heckflosse@i-weyrich.de on 2013-11-20 16:16:19