Beep6581 / RawTherapee

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

Raw CA correction created an artifact #4116

Closed falket closed 7 years ago

falket commented 7 years ago

Version: 5.2-158-gc465655 Branch: dev Commit: c465655 Commit date: 2017-09-13 Compiler: cc 5.4.0 Processor: x86_64 System: Linux Bit depth: 64 bits Gtkmm: V3.18.0 Lensfun: V0.3.2.0 Build type: release Build flags: -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -O3 -DNDEBUG Link flags: -march=native OpenMP support: ON MMAP support: ON

Loaded IMG_0080.DNG (from a Canon Powershot SX150 with CHDK), neutral profile.

Choosed chromatic aberration autocorrection and got a strange line (purple on the left, yellow on the right, outside the shown preview) at the bottom.

Files here:

https://we.tl/fch6gYXxMR

Could it be due to the high number of dead pixels? (is ca autocorrection done before or after?)

rawca

Thank you in advance.

Beep6581 commented 7 years ago

Confirmed, but it's not related to AutoCA, it happens with manual raw CA correction as well.

heckflosse commented 7 years ago

The blueish region is also visible without raw ca-correction. Take a look at the following screenshot where I disabled raw ca-correction and set demosaic to none

Ingo

Edit: I edited the my post

heckflosse commented 7 years ago

Edit: scratch my post. I should not write after driving several hours...

heckflosse commented 7 years ago

Ok, next try: The artifacts clearly not depend on the dead pixels. How can I know that? I tested this patch on your file. This perfectly corrects all bad pixel in the chdk file. Even then the raw ca-correction, which is processed after the dead-pixel correction, fails in that region.

diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
index a1f15787..92cb09aa 100644
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -1752,7 +1752,7 @@ void RawImageSource::preprocess  (const RAWParams &raw, const LensProfParams &le

     int totBP = 0; // Hold count of bad pixels to correct

-    if(ri->zeroIsBad()) { // mark all pixels with value zero as bad, has to be called before FF and DF. dcraw sets this flag only for some cameras (mainly Panasonic and Leica)
+    if(true || ri->zeroIsBad()) { // mark all pixels with value zero as bad, has to be called before FF and DF. dcraw sets this flag only for some cameras (mainly Panasonic and Leica)
         bitmapBads = new PixelsMap(W, H);
 #ifdef _OPENMP
         #pragma omp parallel for reduction(+:totBP) schedule(dynamic,16)

This means:

1 ) as my patch can not be applied as is, but perfectly corrects the bad pixels on chdk files (I already tested the zeroIsbad thing a while ago with that kind of files), we should think about how to enable it for chdk files. Currently that method is only used for panasonic and leica files. It's also faster than the Hot/dead pixel correction you can enable in ui.

2 ) There is an issue with raw ca correction in some special cases, which I will inspect.

falket commented 7 years ago

Thank you for investigation and for the patch.

To correct bad pixel i used Hot/dead and worked. Anyway CHDK has also a way to create an older version of DNGs (iirc 1.1) but this only after creating a badpixel.bin (within the camera) file that contains the positions of wrong pixels. For the camera, this badpixel mapping as far as i remember makes saving files a bit slower. Link: http://chdk.wikia.com/wiki/Badpixel_removal

In this case, i bought the camera that day and i installed CHDK, using default settings (after i moved to DNG 1.1, i think that days to hot-dead pixel filter wasn't as good as today).

Older chdk (but also newer, even if turned off by default) saved only a "really raw" file with crw extension, without metadata and info at all. If you need some files of that kind let me know.

So i think auto enabling for chdk isn't that linear, maybe a checkable option with a tooltip is better.

StefanGyuzelev commented 7 years ago

RawTherapee-5.3 Version: Branch: Commit: Commit date: Compiler: cc 7.2.0 Processor: AMD\ FX(tm)-8300\ Eight-Core\ Processor System: Linux Bit depth: 64 bits Gtkmm: V3.22.1 Lensfun: V0.3.2.0 Build type: Release Build flags: -O3 -fPIC -march=bdver1 -pipe -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -O3 -DNDEBUG Link flags: -march=native OpenMP support: ON MMAP support: ON In 1 image from 28 is visible defect on zoom 1:1 at the bottom of the photo. Without Chromatic Aberration Auto-correction there is no visible defect. With Chromatic Aberration Auto-correction and Demosaicing None there is no visible defect. Link to raw file: https://drive.google.com/file/d/0Bwd1yqYI35MoMHdUSFZyaG1fbWc/view?usp=sharing

heckflosse commented 7 years ago

@StefanGyuzelev Can you please post a screenshot which shows where in you image the artifacts are? I can't find them.

StefanGyuzelev commented 7 years ago

At the bottom of the photo blue-purple artifacts. https://drive.google.com/file/d/0Bwd1yqYI35MoX1FEdkNJTWI2dEE/view?usp=sharing

gaaned92 commented 7 years ago

The artefact appears also when using demosaicing none. It is less strong when using manual correction

iliasg commented 7 years ago

@heckflosse See row 4813 The problem is visible mostly (only ??) on blue pixels and is also there with manual corrections (more visible if we put the slider at -4.0)

BTW inspecting this I have crashes with RT when I pan (having the display at 600-800% and demosaic_none ) !!

heckflosse commented 7 years ago

I'm already searching the culprit. It's clearly a bug in my latest optimization. But don't expect too much before the weekend, because I'm still on holidays.

Floessie commented 7 years ago

It's (as expected) commit 779f4dd.

Floessie commented 7 years ago

It's also present with __SSE2__ undefined.

heckflosse commented 7 years ago

@Floessie Thanks for analyzing. Do you think I should revert the commit until I found the culprit? It seems to occur only at the upper border of the bottom tile row btw...

Floessie commented 7 years ago

@heckflosse

It seems to occur only at the upper border of the bottom tile row btw...

Yes, that's significant. I reviewed the whole diff, but as both storage and computation changed, I couldn't see something obvious. I bet on storage, probably an off-by-one taking in some previous data.

Do you think I should revert the commit until I found the culprit?

If @Beep6581 want's to release a 5.3-r1 to fix it, that would be opportune. But I believe, you'll find the problem faster than Morgan can release a -r1. 👍

Floessie commented 7 years ago

I knew it. 😁 👍

aferrero2707 commented 7 years ago

In case this could help, I have checked the railway image with PhotoFlow, which in its current version uses almost the same code as RT for CA corrections.

The main differences with respect to RT are that:

  1. I do not use SSE2 for copying the corrected pixels back to the main buffer (but it was checked that in RT also the non-SSE code gives the same artifact)
  2. I am completely skipping the filling of image borders, because I have my own way of dealing with this. In the PhF case the algorithm processes the inner portion of a larger image, and the input RAW data is already extended at the edges...

Here is a screenshot of one of the regions with artefacts in RT, processed with PhF:

screen shot 2017-10-10 at 10 36 41

Could it be that the issue is in the border filling?

heckflosse commented 7 years ago

Could it be that the issue is in the border filling?

It was. See 2f32afa

aferrero2707 commented 7 years ago

Ooopsss... I arrived too late!!!

Beep6581 commented 7 years ago

We can of course do a 5.3.1 if this is serious enough, but is it?

It seems to occur only at the upper border of the bottom tile row

heckflosse commented 7 years ago

I think it's serious :(