Project

General

Profile

Bug #12458

segfault when applying profiled denoise style to single image

Added by Bill Ferguson 3 months ago. Updated 3 months ago.

Status:
Fixed
Priority:
Low
Assignee:
-
Category:
Darkroom
Target version:
Start date:
12/11/2018
Due date:
% Done:

100%

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

Description

Open a high ISO image in darkroom. Adjust the exposure. Adjust the rotation and crop. Apply the style. Doesn't happen every time, but it happens. First discovered on 2.6.0rc1 using imported styles from 2.4.4. Latest run was on a clean 2.6.0rc1. Recreated the style, edited an image, got the same crash.

Tested on 2.4.4 and can't duplicate the problem.

If you select a bunch of images in lighttable and apply the style it works with no problems.

System
I7-6820HK CPU
16GB Ram
Nvidia GeForce GTX 970M
opencl enabled

ubuntu 18.04
nvidia 390.77
ocl-icd-libopencl1 2.2.11-1ubuntu1

Ran it under gdb and got the following:

Thread 1 "darktable" received signal SIGSEGV, Segmentation fault.
gui_update (self=0x555559538000)
at /home/bill/src/darktable/darktable/src/iop/denoiseprofile.c:2259
2259 dt_bauhaus_slider_set(g->radius, p->radius);

Whenn I restart darktable and go to the image that I applied the style to, the history stack shows the style was applied and the image shows the noise reduction. It appears, from gdb, that the crash occurs when the style attempts to update the settings on the multiple module instances that get created. The style uses 2 wavelet and 1 nlmeans module.

NR 3 - High ISO.dtstyle (2.3 KB) Bill Ferguson, 12/11/2018 04:08 AM

darktable_bt_FTKITZ.txt Magnifier (121 KB) Bill Ferguson, 12/11/2018 04:09 AM

Associated revisions

Revision 3d109c55
Added by Bill Ferguson 3 months ago

Added check for gui_data and params to prevent crash when applying denoise(profiled) to single image when module is not visible. Fix #12458

Revision 28cb231d
Added by Pascal Obry 3 months ago

Revert "Added check for gui_data and params to prevent crash when applying denoise(profiled) to single image when module is not visible. Fix #12458"

This reverts commit 3d109c55cf03f92e52ff25217e0f98bba5618660.

Revision e46ad149
Added by Pascal Obry about 1 month ago

Ensure that module without gui_data are properly refreshed.

First we remove the call to gui_update() for module with a NULL
gui_data. This was to fix #12458. So now we add a fake gui_data
to ensure that gui_update() will be called for those modules.

Fixes #12594.

Revision 295005c6
Added by Pascal Obry about 1 month ago

Ensure that module without gui_data are properly refreshed.

First we remove the call to gui_update() for module with a NULL
gui_data. This was to fix #12458. So now we add a fake gui_data
to ensure that gui_update() will be called for those modules.

Fixes #12594.

History

#1 Updated by Pascal Obry 3 months ago

If you can run under GDB, can you post the p->radius value? Or maybe even p is NULL. Looking at the code I don't see how this could happen for the moment. But at least it will put us on track.

#2 Updated by Bill Ferguson 3 months ago

Thread 1 "darktable" received signal SIGSEGV, Segmentation fault.
gui_update (self=0x5555597b8220)
at /home/bill/src/darktable/darktable/src/iop/denoiseprofile.c:2259
2259 dt_bauhaus_slider_set(g->radius, p->radius);
(gdb) print g->radius
Cannot access memory at address 0x10
(gdb) print p->radius
$1 = 1
(gdb) print g
$2 = (dt_iop_denoiseprofile_gui_data_t *) 0x0

Okay, I figured it out. I started with a clean version of 2.6.0rc1. By default, the denoise(profiled) module is not displayed. Therefore when I apply a style that uses denoise(profiled) there is no gui data, because the module is not visible in the gui. Perhaps a simple check to make sure the module is visible before trying to update it? Or check if g is valid? To create this crash, turn off the denoise(profiled) module and apply the style.

#3 Updated by Bill Ferguson 3 months ago

I'm at work right now, so I can't work on this. Give me 12 hours or so and I'll code up a fix, test it, and submit a PR.

#4 Updated by Pascal Obry 3 months ago

Perfect thanks!

#5 Updated by Bill Ferguson 3 months ago

Oops, I lied. Slow day at work so I fixed it. It's PR 1895.

#6 Updated by Bill Ferguson 3 months ago

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

#7 Updated by Roman Lebedev 3 months ago

  • Target version set to 2.6.0

Also available in: Atom PDF