Improved 8bit thumbnail cache quality for RAWs

[Beep6581]

Originally reported on Google Code with ID 357

Especially on darker or high ISO RAWs you'll mention severe posterizations on the cached
8bit thumbs (Custom/JPEG).

The problem was that the algorithm for stretching the 16bit image to 8bit was too simple,
just looking for the maximum. So a single white noisy pixel could destroy the image
quality.
The new algorithm builds a histogram and allows small amount of pixels clip a bit,
which greatly increases thumb cache quality on some RAWs.

It's compatible with old thumb cache images, but you must clear the thumb cache to
see the effect. Already gammaed JPGs are not touched, the 16bit thumb generator is
untouched, too.

Samples attached.

Reported by oduis@hotmail.com on 2010-11-27 14:44:57

---

• Attachment: JPGAfterPatch.png
  
• Attachment: JPGBeforePatch.png
  
• Attachment: 16BitReference.png
  
• Attachment: ThumbQuality.patch

[Beep6581]

Very good.  I noticed this posterization on a number of darker images.

I am a bit confused though -- why does the data need scaling?  I would have thought
one could simply generate a properly exposure compensated image and then simply truncate
the bit depth.

Reported by ejm.60657 on 2010-11-27 15:14:33

[Beep6581]

Actually the cache stores the very raw data, so for RAWs it has no gamma applied, no
exposure etc. Espc. for darker images the histogram of the input is a big peak way
to the left. If you'd just dither this to 8 bits and then JPG it, lots of information
is lost (the posterization).

To work around this, the old code searched for the maximum value in 16 bit space, then
scaled the 16bit up, then used a gamma 2.2 lookup to create an 8 bit bit a  normal
looking histogram. It stores the scaling factor and reverts the gamma and scaling on
the way back, querying the cache.

The problem was the maximum search for the scaling. I now calc the histogram and define
a small amount of channel pixels to be clippable (most time noise anyway). So the stretching
is more efficient.

You can repro the problem if you set the thumb format to jpg, clear the cache and navigate
to an image with posterization effects. If you look at the thumb cache file (normal
JPG) you'll probably find its way too dark, so the 8bit space available was not filled
properly.

Reported by oduis@hotmail.com on 2010-11-27 16:13:31

[Beep6581]

Also -- will this affect autoexposure calculations, or is that done prior to this scaling?

Reported by ejm.60657 on 2010-11-27 16:14:27

[Beep6581]

It should be transparent to the upward layers. The cache using codes just push a thumb
to the cache or retrieve it. The way it's stored internally in the cache file is hidden
from the using code.

Reported by oduis@hotmail.com on 2010-11-27 16:19:25

[Beep6581]

Do I understand correctly that the 8-bit thumbs data is stored in linear gamma?  That
is likely the source of the problem, one should never do gamma on 8-bit data.  The
thumbs should have a standard gamma applied before truncation to 8 bits, that will
likely cure the posterization issue without having to resort to data multipliers etc.

Reported by ejm.60657 on 2010-11-27 16:27:28

[Beep6581]

> Do I understand correctly that the 8-bit thumbs data is stored in linear gamma? 
No, vica versa. Even the old code applies gamma before dithering to 8bit of course.

Reported by oduis@hotmail.com on 2010-11-27 16:37:22

[Beep6581]

So what did you mean by saying "the cache stores the very raw data, so for RAWs it has
no gamma applied, no exposure etc."?

Reported by ejm.60657 on 2010-11-27 16:55:03

[Beep6581]

The incoming master data seems to have nothing applied to it. That was probably the
reason Gabor implemented the Mini-Exposure correction and Mini-Gamma correction to
have a "normal" histogram thumb image that could be saved in 8bit.

Reported by oduis@hotmail.com on 2010-11-27 17:09:23

[Beep6581]

I would have thought the thing to do is to calculate standard auto-exposure, apply it,
apply gamma=2.2 or sRGB gamma curve, dither to 8-bit and save the thumb.  Then the
stored thumb shouldn't need anything done to it unless one starts moving sliders, and
won't posterize unless sliders are moved a lot.

Where in the code are these "Mini" corrections (file and line numbers)?

Reported by ejm.60657 on 2010-11-27 17:33:43

[Beep6581]

> need anything done to it unless one starts moving sliders, and won't posterize 
> unless sliders are moved a lot.
I guess thats the thinking error: the cached thumb is in a stage BEFORE all the sliders
and exposures, filters etc. are applied. And that's a good thing: It would create a
lot of disk traffic if you would have to update the several megabytes thumb every time
you hit a slider on the image.

So there is no need to have standard processing or something- the mini-gamma and mini-exposure
is just and only an intermediate step to allow better 8 bit dithering. Gamma and Exposure
are only used since it's simple and obvious. They could have used
any other transformation if it would fit better.
Maybe see this as "they mathematically ZIP the thumb before storing it to disk". They
could have used any other compression/transformation. It has nothing to do with the
normal exposure/gamma sliders.

Reported by oduis@hotmail.com on 2010-11-27 17:50:32

[Beep6581]

Where in the code is this?  rtengine/rtthumbnail.cc ?

I didn't have in mind updating the cached thumb according to sliders, rather to use
the built-in autoexposure and gamma transforms to store the data in a form less susceptible
to posterization issues.  Since gamma and autoexposure are determined from data stored
in cache, undoing them to get the linear data back for further manipulations would
seem straightforward unless I'm missing something.  It seems to me a question of where
one is going to spend one's 8 bits -- if one doesn't do gamma prior to storage, one
spends far too many levels in highlights and not enough in shadows and that is what
leads to posterization.

Reported by ejm.60657 on 2010-11-27 18:12:20

[Beep6581]

The code positions is in the patch, in rtthumbnail. Please be aware that the code needs
to be quick and easy to inverse without needing to many parameter to describe the transformation.
It's a thumbnail, no the final picture.

Reported by oduis@hotmail.com on 2010-11-27 18:18:45

[Beep6581]

I would advise *always* applying gamma of about 2 or so.  In the code this is done by
LUT (gammatab and igammatab); I don't understand the logic of sometimes applying it
and sometimes not, since not applying it leads to posterization.

Reported by ejm.60657 on 2010-11-27 18:29:03

[Beep6581]

OK, I had a chance to look at the code and I agree that what you are proposing makes
sense (if I am understanding the patch).  I would only suggest that the clipping percentage
use params.toneCurve.clip rather than 0.03, and replace

hist16[thumbImg->g[row][col]]++;

by 

hist16[thumbImg->g[row][col]]+=2;

Then the histogram calculation agrees with autoExposure calculation that sets the brightness
of thumbs, thus minimizing the amount of tonemapping (and resulting roundoff error)
being done on 8-bit data.  Better yet, instead of duplicating the same code all over
the place (a real problem with the current code), how about running

ipf.getAutoExp (aeHistogram, aeHistCompression, logDefGain, params.toneCurve.clip,
expcomp, blackpt);

and then using exp(expcomp*log(2)) as your scaling parameter?

Reported by ejm.60657 on 2010-11-27 20:58:22

[Beep6581]

I am afraid we are still not on the same page yet.

> I would only suggest that the clipping percentage use params.toneCurve.clip rather
than 0.03
>
> ipf.getAutoExp (aeHistogram, aeHistCompression, logDefGain, 
params.toneCurve.clip, expcomp, blackpt);
> and then using exp(expcomp*log(2)) as your scaling parameter?
As pointed out the thumb cache images are by design independent of the picture editing
parameters. So there are no “params” at that position, no exposure compensation, clipping
level etc.
Actually most thumbs cache image are even built when there are no picture params selected
at all yet, when RT first scans a directory with new pictures.
And even if the params were there, you would try to not use them, since you don’t want
to rewrite the thumb cache if you edited the parameters.

> hist16[thumbImg->g[row][col]]+=2;
> Then the histogram calculation agrees with autoExposure calculation that sets the
brightness of thumbs, thus
> minimizing the amount of tonemapping (and resulting roundoff error) being done on
8-bit data
The bayer-pattern-has-two-green-pixels correction? Added it, but could not find a visible
difference.

Reported by oduis@hotmail.com on 2010-11-28 08:37:50

[Beep6581]

OK, I admit being confused; isn't the thumb generated using 

IImage8* Thumbnail::processImage

in rtthumbnail.cc?  That basically is a copy of the processing for raw images if the
source of the thumb is a raw, and would use the parameters in the "default" profile
to generate the thumb. Or is there somewhere else that there is a raw data processing
pipeline for thumbs? 

But if not, just to avoid code duplication can one not call

ipf.getAutoExp (<a histogram array>, <your choice of compression>, 0, 
<clipping choice>, expcomp, blackpt);

The function calculates a histogram, and uses the input choice of cliping to return
the (log) scale factor "expcomp" which is the base 2 log of the multiplier one wants
to use as scaleForSave in Thumbnail::writeImage in rtthumbnail.cc

Just a suggestion, it's OK if you don't like it.

Reported by ejm.60657 on 2010-11-28 13:38:55

[Beep6581]

Forgot to mention -- the doubling of green counts is for the same reason as Bayer has
doubled greens, the eye is more sensitive in greens and it's a decent descrete approximation
to luminance to use R+2G+B.  I agree it won't make much difference on most images,
but it's nice to have all such evaluations agree across the code.

Reported by ejm.60657 on 2010-11-28 14:10:02

[Beep6581]

> isn't the thumb generated using  IImage8* Thumbnail::processImage
Still the same problem: The thumbnail generation is independent of the cached thumb.
The cached thumb is unprocessed. The output of processImage is not the input of the
function we're talking about.

> But if not, just to avoid code duplication can one not call ipf.getAutoExp 
As I see it the double code is exactly three lines long that would be reduced to 1.
We're not really gaining much, but might cause side effects because getAutoExp is much
more complicated.
Apart from that it's unfortunately completely undocumented.

Since I got the impression we are not really getting this further, how about this:
I'll check in the fix as it is (including the bayer correction to have it standard).
I think we agree it fixes the problem, it creates not crashes or leaks. And the main
objective is to fix the problem.

Reported by oduis@hotmail.com on 2010-11-28 17:18:24

[Beep6581]

>Since I got the impression we are not really getting this further, how about this:
I'll check in the fix as it is (including the bayer correction to have it standard).
I think we agree it fixes the problem, it creates not crashes or leaks. And the main
objective is to fix the problem.

That's fine, and yes I agree it fixes the problem.  By all means go ahead and check
it in, and thanks for engaging in the discussion.  I've still not understood in what
sense the cached thumb is "unprocessed", but so be it.

Reported by ejm.60657 on 2010-11-28 17:37:22

[Beep6581]

Ok. Thanks at last, I value Your intention and time taken to test and check if something
can be reused, which is generally a good thing.

Reported by oduis@hotmail.com on 2010-11-28 18:14:07

• Status changed: Fixed