Rawdenoise: change force by frequency and channel #1752

rawfiner · 2018-10-16T07:17:19Z

This pull request adds a GUI to rawdenoise similar to the one of the equalizer.

It does NOT change the underlying algorithm, but allows the user to finely control the denoising:

the user can change the force band by band. For instance, this is useful if user wants to preserve fine-grain noise
the user can change the force band by band for each channel. As color channels are usually not noisy in the same way (coarseness of the noise and amount of noise), this allows the user to adjust the denoising accordingly (this may produce an image with some pixels of wrong color, but it is easily fixed with a denoiseprofile instance with wavelet mode and blendmode color).

I think this gives much more power to the user, without adding complexity in the algorithm.
(almost all the code change in this pull request are for the GUI.)

But bug remaining: when changing from an image to another, the GUI is not showing

The frequencies of the wavelets cannot be changed.

…orce band by band

When adding new instances, the GUI was incorrect: we did not have either the nonraw label or the curve, but we add both. Now, when adding new instance, the GUI is correct

The curves do not influence the algorithm yet

rawfiner · 2018-10-16T07:28:04Z

Here is an example of what we can do with it. In both images, there is a denoise profile in wavelet mode and blendmode color. Left is with these changes, by playing with the curve, and right is without. As we can see, the noise is much more fine grain on the left, whereas on the right is is coarse.
The changes made allow to preserve fine grain details while reducing coarse grain noise if the user needs it.

aurelienpierre · 2018-10-16T08:20:57Z

I have run it for 2 days, no issue.

TurboGit · 2018-10-16T12:50:58Z

Same remark here (see #1753), without tweaking the graphic, do we get the same result than before?

rawfiner · 2018-10-16T13:10:22Z

Same remark here (see #1753), without tweaking the graphic, do we get the same result than before?

Yes, we get the same result.
The way it works is the same as in #1753.

This part of the code handles the "all" curve:
for(int i = 0; i < DT_IOP_RAWDENOISE_BANDS; i++)
{
// scale the value from [0,1] to [0,16],
// and makes the "0.5" neutral value become 1
float threshold_exp_4 = data->force[rawdenoise_all][DT_IOP_RAWDENOISE_BANDS - i - 1];
threshold_exp_4 *= threshold_exp_4;
threshold_exp_4 *= threshold_exp_4;
noise_all[i] = noise_all[i] * threshold_exp_4 * 16.0;
}

The point gathered from the curve is squared twice. Thus 0.5 (the neutral value of the curve) is mapped to 1/16. We multiply this by 16, which maps our 0.5 to 1.
Then, this number is used to multiply the noise threshold of the scale, and thus, if the "all" curve is flat the noise threshold does not change.

The code bellow does the same thing for each "channel" (we do not really have channel before demosaic, but each pixel has one of the 3 channels. Thus we use the green curve on "green" pixels, red curve on "red" pixels etc..)

float threshold_exp_4;
switch(c)
{
case 0:
threshold_exp_4 = data->force[rawdenoise_R][DT_IOP_RAWDENOISE_BANDS - i - 1];
break;
case 3:
threshold_exp_4 = data->force[rawdenoise_B][DT_IOP_RAWDENOISE_BANDS - i - 1];
break;
default:
threshold_exp_4 = data->force[rawdenoise_G][DT_IOP_RAWDENOISE_BANDS - i - 1];
break;
}
threshold_exp_4 *= threshold_exp_4;
threshold_exp_4 *= threshold_exp_4;
noise[i] = noise_all[i] * threshold_exp_4 * 16.0;
}

The switch case is slightly different for xtrans code, as in bayer code c==1 an d c==2 both corresponds to green, whereas in xtrans code, c directly corresponds to the pixel channel (0 for R, 1 for G, and 2 for B)

rawfiner · 2018-10-16T13:12:01Z

Sorry for the code formatting in the quotes, I am not sure yet how to refer to code from https://github.com/darktable-org/darktable/pull/1752/files

TurboGit · 2018-10-16T16:34:39Z

I've tested it, this is really nice. But please if you read this test it before it is merged. I'd like to have this in 2.6, so more testing would be nice.

aurelienpierre · 2018-10-16T16:37:10Z

I'm playing with it as well. Works great so far.

rawfiner · 2018-10-16T19:29:29Z

I have just found a bug for bayer: the colors are not always with same index. This means that with current implementation, sometimes the blue slider modify the green, sometimes the blue, etc.
I will fix this as soon as possible.

c does not correspond to a color, but to the number of an "origin" pixel in the top left 2x2 square. We have to convert it to a color.

rawfiner · 2018-10-16T19:49:53Z

The bug should be fixed now. I misinterpreted a variable, thinking it was representing a color, whereas it represented "somewhat" a coordinate in the top left 2x2 square of pixels.

jpg54 · 2018-10-17T07:22:43Z

I have been using the new RawFiner feature in the Rawdenoise module for some time. I like that he introduced in darktable the experiments he did on noise processing with dtStyles and shared. I would like to see them in the next version 2.6.

TurboGit · 2018-10-17T17:07:47Z

src/iop/rawdenoise.c

+#define DT_IOP_RAWDENOISE_RES 64
+#define DT_IOP_RAWDENOISE_BANDS 5
+
+typedef enum dt_iop_rawdenoise_channel_t {


The { should be under typedef.
All entries should be capitalized and with DT_ as prefix:

DT_RAWDENOISE_ALL = 0,
...

TurboGit · 2018-10-17T17:08:38Z

src/iop/rawdenoise.c

+{
+  if(old_version == 1 && new_version == 2)
+  {
+    dt_iop_rawdenoise_params_t *o = (dt_iop_rawdenoise_params_t *)old_params;


See comment about legacy_param in #1753

This one is lot simpler to solve as this new version is only the second one!

I added a comment to explain how it works.
I used this as I saw this was the way it was done in denoiseprofile and nlmeans for example.

But if you prefer, I can define a new struct so that the changes are more explicit.

No, fine I read as if you were copying the would struct with memcpy() and that you were reading past the old struct. I don't this that now! Good to me, you are copying the field one by one.

src/iop/rawdenoise.c

TurboGit · 2018-10-17T17:19:14Z

src/iop/rawdenoise.c

+    for(int k = 0; k < DT_IOP_RAWDENOISE_BANDS; k++)
+      (void)dt_draw_curve_add_point(d->curve[ch], default_params->x[ch][k], default_params->y[ch][k]);
+  }
+  self->commit_params(self, self->default_params, pipe, piece); // TODO necessary?


Yes it is necessary! That's the only way to passe the params between pixelpipe and module params.

TurboGit · 2018-10-17T17:21:10Z

src/iop/rawdenoise.c

+  g_signal_connect(G_OBJECT(c->channel_tabs), "switch_page", G_CALLBACK(rawdenoise_tab_switch), self);
+
+  c->channel = dt_conf_get_int("plugins/darkroom/rawdenoise/gui_channel");
+  int ch = (int)c->channel;


can probably be made const?

…sions 1 and 2

rawfiner · 2018-10-17T19:06:20Z

Thank you for the code review ! :-)

TurboGit · 2018-10-17T19:04:43Z

src/iop/rawdenoise.c

+{
+  if(old_version == 1 && new_version == 2)
+  {
+    dt_iop_rawdenoise_params_t *o = (dt_iop_rawdenoise_params_t *)old_params;


No, fine I read as if you were copying the would struct with memcpy() and that you were reading past the old struct. I don't this that now! Good to me, you are copying the field one by one.

TurboGit · 2018-10-17T19:43:01Z

All good now too. Thanks.

rawfiner added 15 commits October 16, 2018 08:49

First step to have a GUI like equalizer for rawdenoise

9eae299

Rawdenoise: GUI initialized correctly for jpg and raws

f94a6f7

But bug remaining: when changing from an image to another, the GUI is not showing

rawdenoise GUI: correctly switch between jpg GUI and raw GUI

a3deca3

Rawdenoise: little cleanup of GUI code

63f7942

Rawdenoise GUI:make it impossible to move x triangles

66c67b9

The frequencies of the wavelets cannot be changed.

Rawwdenoise GUI: Add labels

fae504f

Rawdenoise: adapt denoising band by band

b480477

RawdenoiseGUI: lower mouse width as user will probably need to tune f…

e039d5c

…orce band by band

Rawdenoise GUI: GUI is now displaying correctly in new instances

f791fa6

When adding new instances, the GUI was incorrect: we did not have either the nonraw label or the curve, but we add both. Now, when adding new instance, the GUI is correct

Rawdenoise GUI:add curves for R, G, and B

4927217

The curves do not influence the algorithm yet

Rawdenoise GUI: color the curves by their channel color

d98b132

Rawdenoise: change force by channel

299740a

Rawdenoise GUI: remove x cursors as they are useless for algorithm

071f0f8

Rawdenoise: clang-format

f6b1712

Rawdenoise: add legacy_params

b646e42

TurboGit added the feature: enhancement current features to improve label Oct 16, 2018

rawfiner mentioned this pull request Oct 16, 2018

Denoise profile wavelets: change force by frequency and channel #1753

Merged

TurboGit self-assigned this Oct 16, 2018

TurboGit added this to the 2.6 milestone Oct 16, 2018

TurboGit self-requested a review October 16, 2018 16:34

Rawdenoise: fix the way color are determined for threshold

2c35284

c does not correspond to a color, but to the number of an "origin" pixel in the top left 2x2 square. We have to convert it to a color.

TurboGit requested changes Oct 17, 2018

View reviewed changes

rawfiner added 4 commits October 17, 2018 20:22

Rawdenoise: code formatting and remove TODO

1551264

Rawdenoise: add some "const" qualifiers

5061161

Rawdenoise: add code in reload default to do a full init

8fbe62a

Rawdenoise: add a comment to explain struct compatibility between ver…

9f7bb28

…sions 1 and 2

TurboGit approved these changes Oct 17, 2018

View reviewed changes

TurboGit merged commit b90f23f into darktable-org:master Oct 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rawdenoise: change force by frequency and channel #1752

Rawdenoise: change force by frequency and channel #1752

rawfiner commented Oct 16, 2018

rawfiner commented Oct 16, 2018

aurelienpierre commented Oct 16, 2018

TurboGit commented Oct 16, 2018

rawfiner commented Oct 16, 2018 •

edited

rawfiner commented Oct 16, 2018

TurboGit commented Oct 16, 2018

aurelienpierre commented Oct 16, 2018

rawfiner commented Oct 16, 2018

rawfiner commented Oct 16, 2018

jpg54 commented Oct 17, 2018

TurboGit Oct 17, 2018

TurboGit Oct 17, 2018

TurboGit Oct 17, 2018

rawfiner Oct 17, 2018

TurboGit Oct 17, 2018

TurboGit Oct 17, 2018

TurboGit Oct 17, 2018

rawfiner commented Oct 17, 2018

TurboGit Oct 17, 2018

TurboGit commented Oct 17, 2018

Rawdenoise: change force by frequency and channel #1752

Rawdenoise: change force by frequency and channel #1752

Conversation

rawfiner commented Oct 16, 2018

rawfiner commented Oct 16, 2018

aurelienpierre commented Oct 16, 2018

TurboGit commented Oct 16, 2018

rawfiner commented Oct 16, 2018 • edited

rawfiner commented Oct 16, 2018

TurboGit commented Oct 16, 2018

aurelienpierre commented Oct 16, 2018

rawfiner commented Oct 16, 2018

rawfiner commented Oct 16, 2018

jpg54 commented Oct 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rawfiner commented Oct 17, 2018

Choose a reason for hiding this comment

TurboGit commented Oct 17, 2018

rawfiner commented Oct 16, 2018 •

edited