Protection against dereferencing NULL that dt_bilateral_init can return

[victoryforce]

When we see that dt_bilateral_init failed, we need to exit processing. In two modules, such a check was not performed and as a result, when memory allocation in dt_bilateral_init failed, the code further called a function that was not NULL-safe and crashed.

The second commit makes dt_bilateral_* functions NULL-safe, which is also desirable, but does not eliminate the need for the first commit. A safer programming style is to check for an error situation and react accordingly (bailout) as early as possible.

It is also worth writing to the log and warning the user in the UI about the error situation. But this will be in the next PR, since this PR makes sense to merge in 5.6.1, where we do not add new strings to the UI.

[ralfbrown]

Simply returning will leave the globaltonemap and monochrome output buffers uninitialized and passing along that garbage data to the rest of the pixelpipe.

I'd have to double-check, but simply skipping the splat in globaltonemap should yield a usable output (same result as setting detail to zero).

For monochrome, call dt_iop_copy_image_roi prior to the error return to make the module a no-op. Or call the copy instead of the splat/blur/slice sequence to get a monochrome conversion without the smoothing.

[victoryforce]

@ralfbrown Thanks for reminding me of the different behavior options in this situation. I didn't really think about the desired behavior when I created the PR. So I took a break to think about how we should behave better in such cases, when we are unable to provide correct processing.

It seems to me that the most important (even critical) thing, after ensuring that we do not crash, is to notify the user about the problem.

As for which processing result is better to leave the user with... For example, in the case of the monochrome module, we can:

• Simply skip the bilateral filter, and perform a monochrome conversion, but without smoothing. This is an option that can be described as still performing the module's function, but without ensuring the quality of the result.

• Skip all the processing performed by the module, making it a no-op in this case. The pipeline run still gives a "decent" result, but far from what the user expected, since the processing module was dropped from the run.

• Do not worry about the result of the pipeline run at all.

The main question is whether it is desirable to provide graceful degradation in such a case.

Do we consider that working beyond the limit, when the resources needed for work are unavailable, is a kinda "normal" situation, or still an abnormal one, and then our only task is to avoid a program crash, so as not to lose the work.

If this is an abnormal situation (I think most people would agree with this), then the user should receive feedback about it and we should maximize the chances that they will notice it. Then the user can take measures to correct the situation: do not run other RAM-intensive programs at the same time as darktable, avoid modules with large memory requirements, and finally, if possible, increase the amount of memory in the computer.

And with this view of things, the worst possible visible result is even better, because it increases the chances that the user will notice that something is wrong and pay attention to the warning...

[victoryforce]

@TurboGit At the moment, option 3 is implemented. As I explained above, it seems to me to be somewhat better for drawing the user's attention to the problem. However, I will not defend this choice and will change it if necessary.

Also, I didn't add a toast (dt_control_log) warning in the UI, although it makes perfect sense, since I'm proposing to merge this PR in 5.6.1. I'm going to add the warning in a separate PR for 5.8.

[victoryforce]

I'm going to add the warning in a separate PR for 5.8.

Well, regarding the restrictions for patch releases... In my opinion, in some cases, when the benefit to the user experience justifies it, we could deviate from the strict rule of not adding new lines to the UI in patch releases. It is better for the user to see an important(!) warning in English than not to see it at all. And, we publish languages ​​that are not 100% translated, in which there may be up to 70 strings untranslated! Given this, it is illogical to strictly avoid +1 line. And another argument in favor in this case: we add a string that the user will not see during normal use of the program, it is only a warning in case of an emergency situation of lack of memory.