Project

General

Profile

Bug #12490

nlmeans random issue

Added by Edgardo Hoszowski about 1 month ago. Updated 10 days ago.

Status:
New
Priority:
Low
Assignee:
-
Category:
-
Target version:
-
Start date:
12/21/2018
Due date:
% Done:

0%

Affected Version:
git master branch
System:
Ubuntu
bitness:
64-bit
hardware architecture:
amd64/x86

Description

Randomly image turns blue and/or with black squares. Seems to be with nlmeans and opencl, but since is random I can't be really sure.

I have traced it back to:
https://github.com/darktable-org/darktable/commit/33b8242055fb9dc6a47bc9419c16daee6ae560be#diff-bc66c51d278d815cd71476afc0def62c

but again, is random, so...

Screenshot.jpg (228 KB) Edgardo Hoszowski, 12/21/2018 04:26 PM

EH_20160410_DSC0160_DT.jpg (499 KB) Edgardo Hoszowski, 12/21/2018 04:28 PM

IMG_1652_306.CR2.xmp (7.48 KB) rawfiner -, 01/03/2019 07:00 PM

xmp_nlmeans.jpg (132 KB) rawfiner -, 01/03/2019 07:03 PM

History

#1 Updated by Ulrich Pegelow 25 days ago

It would be great if you could supply an xmp file that has revealed the issue. Please also tell what GPU and driver you are using.

#2 Updated by Edgardo Hoszowski 22 days ago

History should be with the .jpg
I use nvidia drivers:
[opencl_init] device 0: GeForce GT 740M
GLOBAL_MEM_SIZE: 2002MB
MAX_WORK_GROUP_SIZE: 1024
MAX_WORK_ITEM_DIMENSIONS: 3
MAX_WORK_ITEM_SIZES: [ 1024 1024 64 ]
DRIVER_VERSION: 384.130
DEVICE_VERSION: OpenCL 1.2 CUDA

I've found two ways to reproduce it, both still random, meaning that does not happen every single time:
1) discard history
load sidecar (jpg in this case)
go to darkroom

2) with history in place go to darkroom
zoom in
go to lighttable
go back to darkroom

I've done some more digging and disabling this two if() seems to do the trick:
https://github.com/darktable-org/darktable/commit/2e34d1de2c5e1722b0560ac6532baba6ffefab33#diff-92b45fbb293bbcbfaaf5e387e3673617R75
Maybe U4 needs to be set even if the conditions are not met.

#3 Updated by rawfiner - 21 days ago

I will try to reproduce once I manage to get some time.
These ifs seemed to be necessary to prevent oversmoothing of boundaries (see https://github.com/darktable-org/darktable/pull/1834 for some examples images)

#4 Updated by Edgardo Hoszowski 21 days ago

I see, probably U4 has garbage and is still accessed on the positions that not meet the if(), that may explain the issue being random. I'm not familiar with the code so not sure if initializing buckets on nlmeans.c process_cl() will be enough.

#5 Updated by rawfiner - 18 days ago

I tried to reproduce today but I did not manage to reproduce the bug :-/
As for the hypothesis of garbage inside u4, that may be possible but I am not familiar enough with the code either to know if initializing buckets would be enough...
Have you experienced the same issue with denoiseprofile in nlmeans mode? The codes are very similar, so I guess denoiseprofile is probably also affected by the bug

#6 Updated by Edgardo Hoszowski 18 days ago

I can't duplicate it with denoise profiled, with nlmeans usually I try +/-10 times and I get the issue.

#7 Updated by rawfiner - 18 days ago

I finally manage to somewhat reproduce: I did not manage to get the full image blue, but I got the thumbnail completely blue once. I managed to get the black squares on thumbnail images too, and even completely black thumbnails.

#8 Updated by rawfiner - 18 days ago

When trying to reproduce, I came up with this setting, which always gives me either completely black square image, or some very uniform images (see attached image). Could you try it too (I am not perfectly confident in my GPU drivers)?
Also I do not know if such behavior is linked to this bug (the behavior is identical even before the commit you indicated in the description), or if it is in fact due to another bug...
Note that with some other values of rotation in crop and rotate, I do not get this behavior.

#9 Updated by Edgardo Hoszowski 18 days ago

The crop & rotate make it worst, I do get an uniform image very much like yours, but disabling it I still get the reported issue. This may very well be another bug that's interacting with the nlmeans, I had some issues with the crop & rotate, but nothing consistent enough to report it (or fix it).

#10 Updated by rawfiner - 16 days ago

After some tests, it appears that issue https://redmine.darktable.org/issues/12400 is still visible.
Thus, the PR https://github.com/darktable-org/darktable/pull/1834 did not correctly solve the issue it was supposed to solve. If your issue disapears by reverting the commits of the PR, then we should probably do that

#11 Updated by Edgardo Hoszowski 15 days ago

I have removed the two if(), with just that I don't get the issue anymore, I have no idea if there's any side effects by just removing that instead of the entire PR.

The crop & rotate issue persists even reverting the if(), so its a different issue, with some quick tests it seems to be related to the fact that the orientation is disabled.

#12 Updated by rawfiner - 15 days ago

Ok thanks Edgardo.
I have maybe found a fix for #12400 that, indeed, does not need those ifs.
Do you want me to do one PR to fix the 2 issues, once I am confident about the fix for #12400, or do you prefer to do a PR for this issue, and I will do later the PR for issue #12400?

#13 Updated by Edgardo Hoszowski 15 days ago

What you think is best is OK with me.

#14 Updated by rawfiner - 15 days ago

Ok, I will do one PR for this

#15 Updated by rawfiner - 12 days ago

@Edgardo, could you try replacing the nlmeans_dist code by the code bellow and tell me if the issue is still present please?

kernel void
nlmeans_dist(read_only image2d_t in, global float *U4, const int width, const int height,
             const int2 q, const float nL2, const float nC2)
{
  const int x = get_global_id(0);
  const int y = get_global_id(1);
  const int gidx = mad24(y, width, x);
  const float4 norm2 = (float4)(nL2, nC2, nC2, 1.0f);

  if(x >= width || y >= height) return;

  float dist = 0.0f;
  if(x+q.x < width && y+q.y < height
    && x+q.x >= 0 && y+q.y >= 0)
  {
    float4 p1 = read_imagef(in, sampleri, (int2)(x, y));
    float4 p2 = read_imagef(in, sampleri, (int2)(x, y) + q);
    float4 tmp = (p1 - p2)*(p1 - p2)*norm2;
    dist = tmp.x + tmp.y + tmp.z;
  }

  U4[gidx] = dist;
}

#16 Updated by Edgardo Hoszowski 11 days ago

Just did a quick test and the issue seems to be gone. Without this I can duplicate it with a couple of steps.

#17 Updated by rawfiner - 10 days ago

Cool, thanks for testing! :-)
I will do the PR this week end.

Also available in: Atom PDF