Beep6581 / RawTherapee

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

Optimization for dark frames and hot/dead pixel detection/interpolation #2469

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2486

When you use a dark frame, RT stores the coordinates of the bad pixels of the dark frame
in memory until you close RT. Actually it uses a std::list for this. Let's have a look
at the memory footprint of this list:

per coordinate it has to store 4 byte for x and 4 byte for y
but (because std:list is a doubly linked list) it needs also (on 64 bit OS) 16 byte
per coordinate for linking the list elements.

Let's calculate an example. Assume you have a dark frame with 100000 bad pixels.

Then the std::list needs about 100000 * (8+16) = 2400000 byte to store the bad pixel
coordinates.

I changed this to std::vector, which, if correctly used, only needs about 800000 byte
to store the same information.

I also made some changes to dfInfo::updateBadPixelList which result in a speedup from
about 100 ms to about 20 ms for that function (measured with a dark frame I got from
DrSlony '0214.pef', which has about 400000 bad pixels).
Additionally I changed the int calculations in that function to floating point calculations,
which should give more accurate results. Of course by changing to float the resulting
list of bad pixels now is different from before.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-02 12:22:04


Beep6581 commented 9 years ago
Compiled well and takes 18-37ms to populate the list using my PEFs.
The result looks correct.

Reported by entertheyoni on 2014-09-03 06:50:21

Beep6581 commented 9 years ago
Next one:

1.) reduced memory footprint of bad pixel list by another 50% compared to last patch,
so for the example from #0, it now needs about 400000 byte

2.) rawpedia mentioned that RT can use .badpixel files using serial number of camera
(e.g. 'Pentax K10D 4644134.badpixels' instead of 'Pentax K10D.badpixels') but that
didn't work in past. Now it works.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-03 10:13:02


Beep6581 commented 9 years ago
Next one with following changes:

1.) Improved interpolation of bad pixels for bayer sensors.
    Old version used pixels at offsets 

       (-2,-2) (-2,0) (-2,2)
       (0,-2)          (0,2)
       (2,-2)   (2,0)  (2,2)

    for all channels.

    New version uses offsets from above for red and blue channel, but

                    (-2,0)
            (-1,-1)        (-1,1)
    (0,-2)                         (0,2)
            (1,-1)          (1,1)
                     (2,0)

    for the green channel.
    It's also a bit faster than the old version, though mostly you won't notice the
speedup.
    For the example files mentioned in #0 processing time for this step went down from
about 40 ms to about 10 ms.

2.) I added a very simple bad pixel detection for the cases where dcraw sets the flag
'zeroisbad'.
    This affects some Panasonic and Leica cameras (maybe some more, don't know).
    It marks all pixels which have a value of zero as bad pixels.
    Solves Issue 2265.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-03 18:44:06


Beep6581 commented 9 years ago
Issue 2265 has been merged into this issue.

Reported by heckflosse@i-weyrich.de on 2014-09-03 18:51:19

Beep6581 commented 9 years ago
Issue 2453 has been merged into this issue.

Reported by heckflosse@i-weyrich.de on 2014-09-03 18:52:15

Beep6581 commented 9 years ago

Reported by heckflosse@i-weyrich.de on 2014-09-03 18:53:28

Beep6581 commented 9 years ago
Here's an example of the improved bad pixel interpolation. Have a look at upper right
and lower right corner of the window. Left is before, right is after patch. Test image
was hot_pixels_long_exposure_381s.pef.

http://www.i-weyrich.de/rt/badpixelinterpolation_00.png

Reported by heckflosse@i-weyrich.de on 2014-09-03 19:30:43

Beep6581 commented 9 years ago

Reported by heckflosse@i-weyrich.de on 2014-09-03 19:50:48

Beep6581 commented 9 years ago
Ingo, OK with panasonic zero valued pixels .. they disappeared after I applied the patch.
Have you improved the interpolation relative to Dcraw's ?.
I mean to interpolate from all the same channel neighboring pixels instead of only
the previous in the scanned line. It's a bit of trouble as you 'll have to remove outliers
(neighbor bad pixels) from calculations ..  

With the other hot/dead pixels I thing that the algo still fails to detect some of
the problematic pixels. Many times some remain untouched.

Which is the difference-threshold used for detection ?. Can we change it according
to an "optically impossible" value that we can calculate (depending on the F# and the
diffraction it generates ..).
I see a "HotDeadPixelThresh=40" in the pp3s .. what exactly is the meaning of this
?. Will the detection change if we change (by hand) this value in our pp3?.

A long exposure with D800 and it's black frame .. http://filebin.net/xlij7z5pwq

Reported by iliasgiarimis on 2014-09-04 08:59:37

Beep6581 commented 9 years ago
Ilias, the interpolation of bad pixels in RT was already improved relative to Dcraw's.
Even removing neighbor bad pixels from calculations was done before patch. I only changed
the interpolation of the green channel bad pixels to use closer pixels of the green
channel for calculation.

Actually the patch (except for panasonic zeroisbad) didn't change anything in detection
of hot/dead pixels. That will come in one of the next patches.

The HotDeadPixelThresh is used in RawImageSource::findHotDeadPixel (rawimagesource.cc
line 543 ff. and lines 1139,1140)
Yes, if you change HotDeadPixelThresh=40 in the pp3, that should change the detection.

In next patch, I'll split the actually combined Hot/Dead Pixel detection in two parts
(hot pixel detection and dead pixel detection), which then can be activated separately
in GUI. Often only hot pixel detection is needed. By splitting them we can avoid a
lot of artifacts actually produced by the dead pixel detection part.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-04 09:48:13

Beep6581 commented 9 years ago
Ilias, your dark frame example clearly shows a problem in actual dark frame bad pixel
detection (patched and unpatched). It doesn't detect pairs of bad pixels (e.g. at (x,y)
= (2722,1375) and (2722,1377) in your dark frame. I'll have a look at it, when I make
changes to the hot/dead pixel detection.

Reported by heckflosse@i-weyrich.de on 2014-09-04 10:20:02

Beep6581 commented 9 years ago
Ok, have a look at line 176 of dfmanager.cc

    const float threshold=10.f/8.f;

That means, that to detect a pixel as hot pixel in dark frame its value has to be greater
than 10 times the average of the surrounding 8 pixels of the same channel.

Let's make an example. Assume we have a pair of hot pixels both having value 10000.
The remaining 7 pixels are assumed to be zero. Using the threshold from above we will
never be able to detect the both pixels as hot pixels because

10000 > (10000/8)*10

will never be true.

We have to lower it to a value less than 8.f/8.f to allow detection of hot pixel pairs
if we stay with the actual method.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-04 11:14:53

Beep6581 commented 9 years ago
Yes, it's the way Nikons behave with long exposures the reason to upload this sample
and needs special attention.

Nikon applies secretly a hot pixel suppression algo for exposures longer than 1/4 -
1/2 sec depending on the model. But as it has to be fast the algo is not of good quality
and while it removes many hot pixels it fails on hot pairs. May be these hot pairs
have undergone a haircut to the lower value of the pair .. and this makes RTs job more
difficult.
This in camera HPS destroys and some point lights and this is the reason that Nikons
are not widely used by astrophotographers .. 

http://actionphotosbymarianne.com/Tech/
http://www.dpreview.com/forums/post/34309201

We can see in the third sample which is the result of in camera BF subtraction that
it's almost perfect. I believe Nikon does not apply the HPS/HPP when it is set to use
BF subtraction so has an easier job :) 

Reported by iliasgiarimis on 2014-09-04 12:01:38

Beep6581 commented 9 years ago
Ilias, can you think of any reason for comparing only with pixels of same channel when
detecting hot pixels in a dark frame shot? In theory we could compare with the direct
neighbors (even if they have a different channel), because for dark frame shots the
color of the filter in front of the pixel shouldn't matter at all...

Reported by heckflosse@i-weyrich.de on 2014-09-04 13:11:25

Beep6581 commented 9 years ago
I find no reason to check per channel for all brands except Nikon. Only Nikon applies
"WB preconditionig" i.e. multiplies by around 1.10 - 1.20 the R and B channels.

I also find no reason to check the neighborhood of each pixel .. just decide (or better
calculate according to stdev and assuming a gaussian distribution) the acceptable deviation
from black level to characterize a pixel as hot and mark it. Up to a point some bright
pixels are just noisy and as there is no consistency from frame to frame we cannot
mark them as hot .. :).  

Reported by iliasgiarimis on 2014-09-04 14:35:16

Beep6581 commented 9 years ago
Hmm .. is the hot pixel calculation before black level subtraction ?  

BTW ..I find the 10X relationship hot/normal a bit low for the cases (like Nikon -
Pentax etc) where BL is near zero .. 

Reported by iliasgiarimis on 2014-09-04 14:40:39

Beep6581 commented 9 years ago
Yes. The hot pixel detection in dark frame shots is done before black level subtraction.

Reported by heckflosse@i-weyrich.de on 2014-09-04 14:49:15

Beep6581 commented 9 years ago
I've a question about the two green channels which can be used in RT at several places
in RAW tab. Is this a relict from the times, where we didn't have the choice to use
camconst.json instead or is it still necessary to have this things like green equilibration
etc. (I don't speak about backwards compatibility, that's another case)

Reported by heckflosse@i-weyrich.de on 2014-09-06 18:28:06

Beep6581 commented 9 years ago
Ingo, I am not sure that I understand your question ..

At old times RT had not the ability to process separately the raw channels nor the
two green channels (same as Dcraw) .. then the per channel were features added. Now
the per (four) channels BL is possible and I think it is the correct way to do this
... as manufacturers give all four BL values in their exif data ;).  

It is not only with camconst.json ... BL calculations (for Canon) and, BL exif data
parsing use 4 values ..

I am not sure if we can substitute the green equilibration with different scaling of
G1 vs G2. I think that we cannot because the G1/G2 white clipping does not differ as
much as the average G1/G2..  
 .. you can try with
http://rawtherapee.com/forum/viewtopic.php?f=3&t=5601  
http://www.imaging-resource.com/PRODS/645D/645DhSLI0100_NR_LOW.DNG.HTM
where the G1 vs G2 differ by around 2% ..

If you need more info ask again .. there is plenty of free time this WE :)

Reported by iliasgiarimis on 2014-09-06 19:16:06

Beep6581 commented 9 years ago
Ilias, thanks a lot, I'll ask again :-)

Reported by heckflosse@i-weyrich.de on 2014-09-06 19:21:56

Beep6581 commented 9 years ago
Ilias, i asked because I changed Hot/Dead/Bad pixel interpolation method of green channel.
In past it used only offsets of two, which means that it stayed in same G-Channel.
Not it also uses offsets of one, which means that it uses the other G-Channels too.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-06 20:04:12

Beep6581 commented 9 years ago
I saw the new interpolation in #3 :)

I think the slight difference in the case of G-channels imbalance (I don't remember
any case with more than 5% difference) cannot affect the hot pixel detection. But I
recall that there are some (old) models with four totally different channels, green-emerald
instead of G1-G2 ..http://en.wikipedia.org/wiki/RGBE_filter

Also for dark pixels the different BLs could play a role ..

For the detection of hot/dead pixels in dark frames, isn't it better to calculate the
BL from all the frame at once at the start instead of calculating the neibourhood of
each pixel ?. You can also calculate the stdev so that you will have data about the
noise and find an adaptive threshold to declare hot/dead outliers .. 

Reported by iliasgiarimis on 2014-09-06 21:02:06

Beep6581 commented 9 years ago
Ilias, Hot/dead pixel interpolation is (and was) after scalecolors(). So BL and so on
are already applied when interpolating hot/dead pixels. I now moved it also after green
equilibration.
Hot/dead pixel detection still is before green equilibration. I'll have a look at the
detection pass later.

For the RGBE filters: Are they supported by RT? I can't find any raw file from e.g.
Sony DSC F828.

About hot/dead pixel detection in dark frames: I'm testing a method based on stdev
actually.

Reported by heckflosse@i-weyrich.de on 2014-09-07 10:02:35

Beep6581 commented 9 years ago
Next one. Now you can enable hot/dead pixel filter in preprocessing separately. When
you open an image with an old pp3 where hot/dead pixel filter was enabled, both will
be enabled. I also moved Interpolation of bad pixels behind green equilibration.

http://www.rawtherapee.com/shared/test_images/hot_pixels_long_exposure_381s.pef is
a good example to see how much detail is destroyed by Dead pixel correction (look at
the left window).

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-07 16:13:57


Beep6581 commented 9 years ago
Same as last one, but changed deutsch and francais language file

Reported by heckflosse@i-weyrich.de on 2014-09-07 22:37:31


Beep6581 commented 9 years ago
Confirmed, this patch kills hot pixels like a pro. I don't have any dead pixel raws
to test, AFAIK.

Reported by entertheyoni on 2014-09-08 22:54:59

Beep6581 commented 9 years ago
Removed the Stop watches.
Improved usage of .badpixel file
Before patch you had to add 4 for each coordinate in .badpixel file. With patch you
can specify this offset in first line of .badpixel file and use the coordinates you
see in RT without adding 4 to each one manually. If there is no single value in first
line an offset of 0 is assumed for backwards compatibility to old .badpixel files.

Example

before patch

4054 4079
4044 3010
after patch

4
4050 4075
4040 3006

Ingo

Reported by heckflosse@i-weyrich.de on 2014-09-09 10:11:50

Beep6581 commented 9 years ago
Fixed a copy paste error in last patch

Reported by heckflosse@i-weyrich.de on 2014-09-09 10:41:48


Beep6581 commented 9 years ago
Confirmed, works fine using the new offset line.

Reported by entertheyoni on 2014-09-09 18:25:17

Beep6581 commented 9 years ago
issue2486_05.patch committed to revision 73c163de35d1. I let the Issue open to continue
in about 10 days

Reported by heckflosse@i-weyrich.de on 2014-09-10 12:57:42

Beep6581 commented 9 years ago
Hi Ingo, any chance you'll make the filter working with x-trans sensors?

Reported by sguyader on 2014-10-06 16:59:01

Beep6581 commented 9 years ago
yes :-)

Reported by heckflosse@i-weyrich.de on 2014-10-06 18:00:13

Beep6581 commented 9 years ago
Hi Sébastien, can you again post a link to a x-trans file with dead pixels?

Reported by heckflosse@i-weyrich.de on 2014-10-06 18:06:15

Beep6581 commented 9 years ago
Ingo, I uploaded darkframes on my gdrive, when you were working on that part. Here's
the link: https://drive.google.com/folderview?id=0B_AvPFlUj8t5eWotclNVVG1mMWM&usp=drive_web

You should be able to spot what looks like a hot/stuck pixel at coordinates (2660,1458).
This pixel always shows up, even on moderately fast shutter speeds. If you need some
other frames, I can upload more.

Thanks!

Reported by sguyader on 2014-10-06 19:08:28

Beep6581 commented 9 years ago
The same pixel is visible on this flatfield frame (https://drive.google.com/file/d/0B_AvPFlUj8t5cHUtUnBuTHg0UjA/view?usp=sharing)
taken at 1/2900 shutter speed.

"Unfortunately" it looks like it's the only defective pixel I have for now (no dead
pixel found).

Reported by sguyader on 2014-10-06 19:13:43

Beep6581 commented 9 years ago
Sébastien, thank you very much. I'll start with the bad pix interpolation for x-trans.
Then you at least can mask it out using a .badpixel file

Ingo

Reported by heckflosse@i-weyrich.de on 2014-10-06 19:43:59

Beep6581 commented 9 years ago
We need to update http://rawpedia.rawtherapee.com/Dark_Frame#Bad_Pixels

Reported by entertheyoni on 2014-10-20 16:10:15

Beep6581 commented 9 years ago
I sent DrSlony the relevant infos for documentation in rawpedia. Closing this Issue
now. I opened Issue 2543 for x-trans hot/dead pixels.

Reported by heckflosse@i-weyrich.de on 2014-10-21 22:44:00

Beep6581 commented 9 years ago
Forgot to mention. I'll open a new Issue for better hot/bad pixel detection in general
(especially bad pixels)

Reported by heckflosse@i-weyrich.de on 2014-10-21 22:45:02