Project

General

Profile

Bug #11272

Module `demosaic' outputs NaNs! [preview]

Added by Roman Lebedev almost 3 years ago. Updated almost 3 years ago.

Status:
Fixed
Priority:
Medium
Assignee:
-
Category:
General
Target version:
Start date:
10/29/2016
Due date:
% Done:

100%

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

Description

For X-Trans image.
No other module before it complains, so it must be the demosaic itself.
Happens for both MARKESTEIJN and MARKESTEIJN3, but not for VNG or monochrome.

highlights5.RAF (32.2 MB) highlights5.RAF Roman Lebedev, 10/30/2016 03:53 PM

Related issues

Related to darktable - Bug #11258: Darktable renders (exports) black image for Fuji RAWsFixed10/26/2016

Related to darktable - Bug #11167: Downscaling without demosaicing: artefactsIn Progress09/20/2016

Associated revisions

Revision 37b3ba58 (diff)
Added by Dan Torop almost 3 years ago

demosaic: don't produce NaN in Markesteijn

The TRANSLATE macro which mirrors the image when looking beyond its edge
had an off-by-one in its calculation: the first column beyond the right
edge of the image duplicated the rightmost column of the image, rather
than the second-to-rightmost (and similarly for the first row beyond the
bottom of the image). This caused interpolation code, which looks in a
3x3 area of mosaiced data, to fail, in the case of some red/blue pixels
at the image edge, to find any pixels from which to interpolate. This
produced a division by zero for these pixels, which resulted in an
output of NaN at the edge of Markesteijn demosaiced data.

This bug should only affect the CPU variant of Markesteijn. The OpenCL
version does not mirror the edges, but instead uses VNG
interpolation. Note also that this bug is not in the original dcraw
code.

This fixes #11272. This fixes #11258.

History

#1 Updated by Roman Lebedev almost 3 years ago

UBsan sanitizer says:

/home/lebedevri/darktable/src/iop/demosaic.c:775:67: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:778:30: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:873:32: runtime error: index -1 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:778:30: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:778:30: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:874:34: runtime error: index -1 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:778:30: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:875:34: runtime error: index -1 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:776:59: runtime error: left shift of negative value -1
/home/lebedevri/darktable/src/iop/demosaic.c:873:32: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:873:32: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:874:34: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:874:34: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:875:34: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:875:34: runtime error: index 122 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:874:34: runtime error: index -1 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:873:32: runtime error: index -1 out of bounds for type 'float [122]'
/home/lebedevri/darktable/src/iop/demosaic.c:875:34: runtime error: index -1 out of bounds for type 'float [122]'

#2 Updated by Roman Lebedev almost 3 years ago

That's odd, it seems there is no NaN's in the input of demosaic.

#3 Updated by Roman Lebedev almost 3 years ago

@Dan given that it seems to be specific to X-Trans demosaicing algos, adding you.

#4 Updated by Roman Lebedev almost 3 years ago

  • Related to Bug #11258: Darktable renders (exports) black image for Fuji RAWs added

#5 Updated by Dan Torop almost 3 years ago

Will look into this (and finally getting to the downscaling for x-trans as well). @LebedevRI, are you seeing this for all X-Trans images or particular ones?

#6 Updated by Roman Lebedev almost 3 years ago

Can't tell, but this image does have it. (from jo)

#7 Updated by Roman Lebedev almost 3 years ago

  • Target version set to 2.2.0

#8 Updated by Dan Torop almost 3 years ago

Working on this, apologies for the delay. UBsan output is helpful, but I don't think either of these are causing NaN's, this is just overly clever dcraw code which I ported over literally. I'll fix the left shift of negative value, which is legit undefined behavior. The index out of bounds is dcraw being wily and using one index (say x-axis) of a multidimensional array to move by another index (say y-axis) as well. It may be best to just leave this in if changing to remove the warning produce slower code.

More soon, still looking for what makes the NaN.

BTW, is there an easy way to compile with UBsan sanitizer? I see the "--asan" flag, but it looks like would still need to modify build.sh to invoke -fsanitize=undefined, and then I see a slew of compile-time errors starting with:

/home/dtorop/src/darktable/src/develop/imageop_math.c: In function ‘dt_iop_clip_and_zoom_mosaic_half_size_plain’:
/home/dtorop/src/darktable/src/develop/imageop_math.c:331:28: error: ‘*.Lubsan_data9’ not specified in enclosing parallel
num = ((maxj - py) / 2 + 1 - dy) * (samples + 1);

#9 Updated by Roman Lebedev almost 3 years ago

Dan Torop wrote:

Working on this, apologies for the delay. UBsan output is helpful, but I don't think either of these are causing NaN's, this is just overly clever dcraw code which I ported over literally. I'll fix the left shift of negative value, which is legit undefined behavior. The index out of bounds is dcraw being wily and using one index (say x-axis) of a multidimensional array to move by another index (say y-axis) as well. It may be best to just leave this in if changing to remove the warning produce slower code.

I will just not touch that, since i do not understand that code.

More soon, still looking for what makes the NaN.

Cool, thanks for that :)

BTW, is there an easy way to compile with UBsan sanitizer?

Nope

I see the "--asan" flag, but it looks like would still need to modify build.sh to invoke -fsanitize=undefined, and then I see a slew of compile-time errors starting with:

/home/dtorop/src/darktable/src/develop/imageop_math.c: In function ‘dt_iop_clip_and_zoom_mosaic_half_size_plain’:
/home/dtorop/src/darktable/src/develop/imageop_math.c:331:28: error: ‘*.Lubsan_data9’ not specified in enclosing parallel
num = ((maxj - py) / 2 + 1 - dy) * (samples + 1);

Each time i have to mass-replace " default(none)" with " ". Else it does not build.
And i do not think we should either just drop " default(none)" in master, or mark it with #ifndef UBSN

#10 Updated by Dan Torop almost 3 years ago

Good to know about asan.

Interim result: This may be related to the "confetti"-artifacts in preview (#11167). It seems demosaic is getting bad (negative) data. Markesteijn demosaic in particular doesn't handle this well and produces NaN's. So am looking at two issues: make Markesteijn demosaic handle negative input, and making X-Trans downscaling not produce negative output. The latter is more important. As it's past time to fix the downscaling, not a bad thing if both these problems are related.

More soon...

#11 Updated by Roman Lebedev almost 3 years ago

  • Related to Bug #11167: Downscaling without demosaicing: artefacts added

#12 Updated by Dan Torop almost 3 years ago

Made PR 1347 which should fix the NaNs and also #11258. It looks like #11167 is something else, though, still working on that.

#13 Updated by Dan Torop almost 3 years ago

  • % Done changed from 0 to 100
  • Status changed from New to Fixed

Also available in: Atom PDF

Go to top