Rawtherapee: segfault on RAF files from Fuji S5 pro

[agriggio]

The RAF file can be taken from here:
https://discuss.pixls.us/t/rawtherapee-for-raf-files-of-fuji-s5-pro/3458/8

The pp3 is below. There is a crash due to some NaN float value in dirpyr_equalizer.cc. I suspect a division by zero, but that is as far as I can go regarding that code.

GIT version 06137b02be947fc6ee068069fe6e9f4f1f515170 (head of dev). Other info:

Compiler: gcc-6 6.2.0
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.18.0
Build type: Debug
Build flags: -fdiagnostics-color=always -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -g
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

_DSF3083.RAF.pp3.txt

[agriggio]

Actually, there are many other operations that make RT crash on this file... just playing with various tools (tried CA correction, wavelet, noise reduction) will trigger a segfault very quickly

[heckflosse]

I tried but can not reproduce the crashes using the raf and the attached pp3. Also tried CA correction etc..
No crash here on Win7/64

[agriggio]

I'll try to rebuild from scratch, with a different compiler

[agriggio]

Tried again with the following:

ompiler: gcc-5 5.4.1
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.18.0
Build type: Release
Build flags: -fdiagnostics-color=always -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -O3 -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

The crash is still there. To reproduce:

• delete the pp3 sidecar of the RAF file, and make sure that the default
  processing profile for RAW files is "(Neutral)" (just to be safe)

• open the RAF file with RT

• enable "Contrast by detail levels"

• close RT

• reopen RT in "file browser mode"

• click on the thumbnail of the RAF

I get a segfault immediately. I'm attaching the pp3 again. This time it should
be complete.
_DSF3083.RAF.pp3.txt

[heckflosse]

Still not reproducible on my system :(

[agriggio]

I am on Linux mint 18.1 (KDE version, but not sure if it matters). Maybe you can try on a VM?

[Floessie]

Nor here.

Version: 5.0-r1-gtk3-111-g06137b02
Branch: dev
Commit: 06137b02
Commit date: 2017-03-07
Compiler: cc 6.3.0
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.22.0
Build type: release
Build flags:  -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -O3 -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

[agriggio]

I tried on my laptop at home, and I can't reproduce it either. Must have something to do with specific library version and/or compilation flags (I'm using -march=native on both, but the machines are quite different).
I'll dig deeper as soon as possible.

[Beep6581]

I did not manage to reproduce it.

Version: 5.0-r1-gtk3-113-gdfcab20b
Branch: dev
Commit: dfcab20b
Commit date: 2017-03-08
Compiler: cc 4.9.3
Processor: Intel(R)\ Core(TM)\ m3-6Y30\ CPU\ @\ 0.90GHz
System: Linux
Bit depth: 64 bits
Gtkmm: V3.22.0
Build type: release
Build flags: -std=c++11 -Wno-deprecated-declarations -Wno-unused-result -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -O3 -DNDEBUG
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

[agriggio]

Update. Now I get the crash also at home, when processing the image (from the batch queue). Using the pp3 in attach.
I have a Intel(R) Core(TM)2 Duo CPU P8400 @ 2.26GHz
bad.pp3.txt

[heckflosse]

I processed the image in queue but it did not crash. Can you provide a bt full please?

[agriggio]

this time it did not crash in the queue, but it did crash when opening a preview window in the editor. Here I only have a release build, but still I got the following bt:

Thread 93 "rawtherapee" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff95602700 (LWP 5582)]
0x0000000000a6ae15 in rtengine::ImProcFunctions::MadRgb(float*, int) [clone .part.38] ()

(gdb) bt full
#0  0x0000000000a6ae15 in rtengine::ImProcFunctions::MadRgb(float*, int) [clone .part.38] ()
No symbol table info available.
#1  0x0000000000a7556b in rtengine::ImProcFunctions::ShrinkAll_info(float**, float**, int, int, int, int, float**, float**, float**, int, int, float, float, rtengine::LabImage*, float&, int&, float&, float&, float&, float&, float&, float&, bool, int, int, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, int&, float&, float&, float&, bool, bool) ()
No symbol table info available.
#2  0x0000000000a77f1e in rtengine::ImProcFunctions::RGB_denoise_info(rtengine::Imagefloat*, rtengine::Imagefloat*, bool, LUT<float>&, float, float, float, rtengine::procparams::DirPyrDenoiseParams const&, double, float&, int&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, float&, bool) ()
No symbol table info available.
#3  0x000000000091a8e9 in rtengine::Crop::update(int) [clone ._omp_fn.0] ()
No symbol table info available.
#4  0x00007ffff2f9f43e in ?? () from /usr/lib/x86_64-linux-gnu/libgomp.so.1
No symbol table info available.
---Type <return> to continue, or q <return> to quit---
#5  0x00007ffff2b646ba in start_thread (arg=0x7fff95602700)
    at pthread_create.c:333
        __res = <optimized out>
        pd = 0x7fff95602700
        now = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140735699494656, 
                2447316023277093600, 0, 140736027692767, 140735699495360, 
                1, -2447224584365309216, -2447291227138719008}, 
              mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, 
            data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = <optimized out>
        pagesize_m1 = <optimized out>
        sp = <optimized out>
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#6  0x00007ffff289a82d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
No locals.

I'll try to provide more info tomorrow, from my work machine.

[heckflosse]

@agriggio Though this is already a hint, a bt from debug build would show more :+1:

[agriggio]

A more informative backtrace. Got on:

Compiler: gcc-6 6.2.0
Processor: x86_64
System: Linux
Bit depth: 64 bits
Gtkmm: V3.18.0
Build type: Debug
Build flags: -fdiagnostics-color=always -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -g
Link flags:  -march=native
OpenMP support: ON
MMAP support: ON

bt.txt

EDIT: the CPU is an Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz

[Floessie]

@heckflosse Seems to be pretty much the same as #3410.

[heckflosse]

@agriggio Can you reproduce the crash only with this Fuji file? If yes, then it's most likely a bug in the fuji specific code in rawimagesource.cc
I'm looking...

[agriggio]

@heckflosse so far, I've seen it only on this file, yes.

[agriggio]

BTW, let me know if I can help in any way

[heckflosse]

I will do. Currently I try to understand the fuji-S specific code...

[agriggio]

@heckflosse I just tried converting the RAF to DNG with the Adobe DNG converter (version 9.8.0.692, used through wine). I get the crash also on the DNG.

[heckflosse]

Maybe the DNG runs through the same code.
Can you place a print here to check?

[agriggio]

@heckflosse yes, you are right. I get the print twice before the segfault

[Floessie]

@heckflosse Wouldn't something like this fix the SEGV in that place once and for all?

diff --git a/rtengine/FTblockDN.cc b/rtengine/FTblockDN.cc
index 1ad34f2d..deaf726d 100644
--- a/rtengine/FTblockDN.cc
+++ b/rtengine/FTblockDN.cc
@@ -2149,7 +2149,7 @@ float ImProcFunctions::MadRgb(float * DataList, const int datalen)
     int i;

     for (i = 0; i < datalen; ++i) {
-        histo[min(65535, abs(static_cast<int>(DataList[i])))]++;
+        histo[LIM(65535, 0, abs(static_cast<int>(DataList[i])))]++;
     }

     //find median of histogram

[agriggio]

@Floessie I tried the patch, but I still get a segfault. A different BT now.
bt2.txt

[Floessie]

@agriggio Good catch: Now we see the NaN used as index in another place. That might Ingo help to track down the source of it...

[heckflosse]

@agriggio thanks for checking. These super-ccd files are really strange. For example the raw data has almost square dimensions (3583 x 3584 pixels) while the final output is 3:2 (4281 x 2871 pixels).
I tend to ignore the crashes atm in case they happen only with this super-ccd files.
Here's another file from S5Pro. Can you test whether that crashes also?

[Floessie]

@heckflosse

I tend to ignore the crashes atm in case they happen only with this super-ccd files.

Sure? Not every OOB access is a security issue, but who really knows?

@agriggio Updated patch:

diff --git a/rtengine/FTblockDN.cc b/rtengine/FTblockDN.cc
index 1ad34f2d..deaf726d 100644
--- a/rtengine/FTblockDN.cc
+++ b/rtengine/FTblockDN.cc
@@ -2149,7 +2149,7 @@ float ImProcFunctions::MadRgb(float * DataList, const int datalen)
     int i;

     for (i = 0; i < datalen; ++i) {
-        histo[min(65535, abs(static_cast<int>(DataList[i])))]++;
+        histo[LIM(65535, 0, abs(static_cast<int>(DataList[i])))]++;
     }

     //find median of histogram
diff --git a/rtengine/LUT.h b/rtengine/LUT.h
index d83a431c..72c8d900 100644
--- a/rtengine/LUT.h
+++ b/rtengine/LUT.h
@@ -443,7 +443,7 @@ public:
     template<typename U = T, typename = typename std::enable_if<std::is_same<U, float>::value>::type>
     T operator[](float index) const
     {
-        int idx = (int)index;  // don't use floor! The difference in negative space is no problems here
+        int idx = rtengine::max<int>(0, index);  // ~~don't use floor! The difference in negative space is no problems here~~ It is for NaN!

         if (index < 0.f) {
             if (clip & LUT_CLIP_BELOW) {

[agriggio]

@heckflosse I don't get a segfault with the new RAF. I'll test your patch now

[agriggio]

@Floessie sorry, I noticed now the patch is from you :-)

[heckflosse]

@agriggio Do you also get this wrong left border in the thumb?

[heckflosse]

@Floessie I often thought about fixing this part of code too. But that would be fixing the symptoms, not the source of the evil.

[agriggio]

@Floessie no crash after your new patch :+1:

[agriggio]

@heckflosse yes, I see that too.

[Floessie]

@heckflosse

But that would be fixing the symptoms, not the source of the evil.

Absolutely! But if somebody evil uses RT as his or her attack vector, this won't make up for good PR, especially regarding automated tasks with the new rawtherapee-cli...

[agriggio]

@Floessie I assume it is ok for operator[] to still return NaN if index is NaN, right?

[heckflosse]

@agriggio @Floessie As I wrote above, I also thought about making the checks from above patch. But I did not. If I did, I wouldn't have been able to find the sources of many bugs because they get shadowed by this checks.

[Floessie]

@agriggio

I assume it is ok for operator[] to still return NaN if index is NaN, right?

Yep, so we can catch the next problem.

@heckflosse

But I did not. If I did, I wouldn't have been able to find the sources of many bugs because they get shadowed by this checks.

What about feenableexcept(FE_DIVBYZERO | FE_INVALID | FE_OVERFLOW)? Could be turned on in debug mode.

[agriggio]

@heckflosse could putting an assert(isfinite(x)) (or even a weaker !isnan(x)) be acceptable?

[heckflosse]

I will try to find the source of the bug now...

[heckflosse]

@agriggio Isn't assert only active in debug builds?

[agriggio]

@heckflosse yes, that was the point... to ease debugging

[Floessie]

Isn't a "signaling NaN" mode best? No need for isnan() etc. and help from the FPU.

[heckflosse]

@agriggio I'm absolutely not against an assert!
But I will try to find the source of the bug because LUT access maybe performance critical and heavily used.
At least we should check for impact on performance if we add new checks there.

[heckflosse]

@Floessie wouldn't signaling NaN break the parts of code (especially SSE code) where we allow NaN for intermediate calculations?

[agriggio]

@Floessie trapping NaNs could do, but I understood from @heckflosse that sometimes they are expected.
@heckflosse assert is expanded to nothing in release mode, so no performance penalty

[Floessie]

@heckflosse @agriggio Ah, I forgot about that. Sorry for the noise...

[heckflosse]

@agriggio Yes, I know. For that reason I wrote absolutely NOT against :)

[Floessie]

... but we would still be wide open for security issues.

[heckflosse]

@agriggio Let's analyze your last bt full.

I crashes at the access to a LUT because of a NaN. That NaN came from a calculation of rgb values which all are NaN. This rgb values came from the loop above (line 3411 of improcfun.cc). In that loop the rgb values also have been used to access a LUT but we didn't get a crash there. That means the NaN must be inisde the hltonecurve LUT which is filled in line 546 ff of curves.cc

[heckflosse]

My conclusion from last post was incomplete. Still searching

[heckflosse]

I will valgrind that case now... Will take a while

[agriggio]

@heckflosse I believe converting a NaN to int is undefined behaviour. So, might it be that you get back an integer which is still within the LUT bounds even if the input is a NaN? In that case, the error could come from somewhere else completely...

[agriggio]

Some example code (please take this in the right way, I'm not trying to be the "smartass" :grin:, I just want to provide some more evidence...)

#include <math.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

float make_nan(unsigned infobits)
{
    union {
        float f;
        uint32_t i;
    } val;
    val.i = 0xff800000;
    if (!infobits) {
        infobits = 1;
    }
    val.i |= ((uint32_t)infobits) & ~(val.i);
    return val.f;
}

int main(int argc, const char **argv)
{
    float n1 = make_nan(argc > 1 ? atoi(argv[1]) : 1);
    if (!isnan(n1)) {
        printf("ERROR, not NaN!\n");
        return 1;
    }
    float n2 = make_nan(128000);
    if (!isnan(n2)) {
        printf("ERROR, not NaN!\n");
        return 1;
    }

    int i1 = (int)n1;
    int i2 = (int)n2;

    printf("i1 = %d, i2 = %d\n", i1, i2);
    return 0;
}

On my machine:

$ gcc -O3 -msse2 -mfpmath=sse -fno-strict-aliasing test.c -o test -lm
$ ./test 234
i1 = -2147483648, i2 = 0

[heckflosse]

@agriggio Do you mean when accessing the LUT? It's accessed with a float index. For that reason it crashes insite LUT.h when the index is a NaN. But it still can come from CurveFactory::hlcurve (exp_scale, comp, hlrange, r) or from somewhere else. I guess it's uninitialized memory in processing of the fuji files.
That often has been the case in past when crashed can be reproduced on Linux but not on windows

[agriggio]

@heckflosse I trust your expertise, but if I read LUT.h correctly (around line 462), the indexing on data is done with an int idx obtained from the input float...

[heckflosse]

@agriggio Exactly. And that idx then will be -2147483648 as you already wrote above and it crashes. But if there would be a NaN in the loop above the loop where it currently crashes then it should have crashed already there (with the exception that the NaN was inside hltonecurve LUT or one of the rgb values was > MAXVALF and then CurveFactory::hlcurve(...) would be called.

[heckflosse]

@agriggio ;-)

[agriggio]

@heckflosse if you take a closer look at my code, you could also get 0 for idx (see the value of i2 in the example). If/when that happens, no segfault on the LUT occurs...

BTW, this doesn't mean that your analysis about uninitialized memory being the cluprit is necessarily invalid, of course! You (and @Floessie) are the RT experts here, so take my comments with care :smile:

[heckflosse]

@agriggio @Floessie
First valgrind result:

==16285== Thread 12:
==16285== Conditional jump or move depends on uninitialised value(s)
==16285==    at 0x791BE8: int const& std::min<int>(int const&, int const&) (stl_algobase.h:199)
==16285==    by 0x79489B: int const& rtengine::LIM<int>(int const&, int const&, int const&) (rt_math.h:37)
==16285==    by 0x972BA7: LUT<unsigned int>::operator[](int) const (LUT.h:287)
==16285==    by 0xBE1F16: rtengine::ImProcFunctions::firstAnalysis(rtengine::Imagefloat const*, rtengine::procparams::ProcParams const&, LUT<unsigned int>&) [clone ._omp_fn.8] (improcfun.cc:174)
==16285==    by 0x93679CE: GOMP_parallel (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==16285==    by 0xBC72AD: rtengine::ImProcFunctions::firstAnalysis(rtengine::Imagefloat const*, rtengine::procparams::ProcParams const&, LUT<unsigned int>&) (improcfun.cc:157)
==16285==    by 0xCF80F1: rtengine::ImProcCoordinator::updatePreviewImage(int, rtengine::Crop*) (improccoordinator.cc:367)
==16285==    by 0xCFD66C: rtengine::ImProcCoordinator::process() (improccoordinator.cc:1309)
==16285==    by 0xD00C3C: sigc::bound_mem_functor0<void, rtengine::ImProcCoordinator>::operator()() const (in /home/ingo/rt5/build2/debug/rawtherapee)
==16285==    by 0xD00969: sigc::adaptor_functor<sigc::bound_mem_functor0<void, rtengine::ImProcCoordinator> >::operator()() const (adaptor_trait.h:251)
==16285==    by 0xD0054C: sigc::internal::slot_call0<sigc::bound_mem_functor0<void, rtengine::ImProcCoordinator>, void>::call_it(sigc::internal::slot_rep*) (slot.h:103)
==16285==    by 0x5CD7A4C: ??? (in /usr/lib/x86_64-linux-gnu/libglibmm-2.4.so.1.3.0)

[agriggio]

@heckflosse as you suspected, there seems to be something wrong in RawImageSource::getImage in the fuji case. Some of the RGB values are NaN. I suppose there is an off-by-one error or something similar in the code. This (very crude) patch makes the crash go away here. If I comment out the first part of the patch, I get an assertion failure down below (where I added the checks for !isnan).
I hope this could help you in finding a proper solution :smile:

diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -700,6 +700,17 @@
         imheight = maximheight;
     }

+    // AG
+    if (fuji) {
+        for (int i = 0; i < image->getHeight(); ++i) {
+            for (int j = 0; j < image->getWidth(); ++j) {
+                image->r(i, j) = 0;
+                image->g(i, j) = 0;
+                image->b(i, j) = 0;
+            }
+        }
+    }
+
     int maxx = this->W, maxy = this->H, skip = pp.getSkip();

     // raw clip levels after white balance
@@ -864,6 +875,19 @@
             image->g(image->getHeight() - 1, j) = (image->g(image->getHeight() - 2, j) + image->g(image->getHeight() - 1, j + 1) + image->g(image->getHeight() - 1, j - 1)) / 3;
             image->b(image->getHeight() - 1, j) = (image->b(image->getHeight() - 2, j) + image->b(image->getHeight() - 1, j + 1) + image->b(image->getHeight() - 1, j - 1)) / 3;
         }
+
+        // AG
+#ifndef NDEBUG
+        {
+            for (int i = 0; i < image->getHeight(); ++i) {
+                for (int j = 0; j < image->getWidth(); ++j) {
+                    assert(!isnan(image->r(i, j)));
+                    assert(!isnan(image->g(i, j)));
+                    assert(!isnan(image->b(i, j)));
+                }
+            }
+        }
+#endif // NDEBUG
     }

     // Flip if needed

[heckflosse]

@agriggio I already found a (hopefully) proper solution. I valgrinded before/after and now the valgrind log looks better :)

Patch follows soon :)

[agriggio]

awesome! :+1:

[heckflosse]

Here's the patch which avoids the crash. I'll post a new one without black pixels at the corners soon :)

diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
index 9678fca..80cee48 100644
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -822,12 +822,19 @@ void RawImageSource::getImage (const ColorTemp &ctemp, int tran, Imagefloat* ima
         int a = ((tran & TR_ROT) == TR_R90 && image->getWidth() % 2 == 0) || ((tran & TR_ROT) == TR_R180 && image->getHeight() % 2 + image->getWidth() % 2 == 1) || ((tran & TR_ROT) == TR_R270 && image->getHeight() % 2 == 0);

         // first row
+
+        for (int j = 0; j < 1 + a; ++j) {
+            image->r(0, j) = image->g(0, j) = image->b(0, j) = 0.f;
+        }
+
         for (int j = 1 + a; j < image->getWidth() - 1; j += 2) {
             image->r(0, j) = (image->r(1, j) + image->r(0, j + 1) + image->r(0, j - 1)) / 3;
             image->g(0, j) = (image->g(1, j) + image->g(0, j + 1) + image->g(0, j - 1)) / 3;
             image->b(0, j) = (image->b(1, j) + image->b(0, j + 1) + image->b(0, j - 1)) / 3;
         }

+        image->r(0, image->getWidth() - 1) = image->g(0, image->getWidth() - 1) = image->b(0, image->getWidth() - 1) = 0.f;
+
         // other rows
         for (int i = 1; i < image->getHeight() - 1; i++) {
             for (int j = 2 - (a + i + 1) % 2; j < image->getWidth() - 1; j += 2) {
@@ -859,11 +866,17 @@ void RawImageSource::getImage (const ColorTemp &ctemp, int tran, Imagefloat* ima
         // last row
         int b = (a == 1 && image->getHeight() % 2) || (a == 0 && image->getHeight() % 2 == 0);

+        for (int j = 0; j < 1 + b; ++j) {
+            image->r(image->getHeight() - 1, j) = image->g(image->getHeight() - 1, j) = image->b(image->getHeight() - 1, j) = 0.f;
+        }
+
         for (int j = 1 + b; j < image->getWidth() - 1; j += 2) {
             image->r(image->getHeight() - 1, j) = (image->r(image->getHeight() - 2, j) + image->r(image->getHeight() - 1, j + 1) + image->r(image->getHeight() - 1, j - 1)) / 3;
             image->g(image->getHeight() - 1, j) = (image->g(image->getHeight() - 2, j) + image->g(image->getHeight() - 1, j + 1) + image->g(image->getHeight() - 1, j - 1)) / 3;
             image->b(image->getHeight() - 1, j) = (image->b(image->getHeight() - 2, j) + image->b(image->getHeight() - 1, j + 1) + image->b(image->getHeight() - 1, j - 1)) / 3;
         }
+
+        image->r(image->getHeight() - 1, image->getWidth() - 1) = image->g(image->getHeight() - 1, image->getWidth() - 1) = image->b(image->getHeight() - 1, image->getWidth() - 1) = 0.f;
     }

     // Flip if needed

[heckflosse]

Here's the patch with better corner pixels. I didn't waste coding time to interpolate them from neighboring pixels. I just copied them from line above/below ;)

diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
index 9678fca..092b0c8 100644
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -856,14 +856,36 @@ void RawImageSource::getImage (const ColorTemp &ctemp, int tran, Imagefloat* ima
             }
         }

+        for (int j = 0; j < 1 + a; ++j) {
+            image->r(0, j) = image->r(1, j);
+            image->g(0, j) = image->g(1, j);
+            image->b(0, j) = image->b(1, j);
+        }
+
+        image->r(0, image->getWidth() - 1) = image->r(1, image->getWidth() - 1);
+        image->g(0, image->getWidth() - 1) = image->g(1, image->getWidth() - 1);
+        image->b(0, image->getWidth() - 1) = image->b(1, image->getWidth() - 1);
+
+
         // last row
         int b = (a == 1 && image->getHeight() % 2) || (a == 0 && image->getHeight() % 2 == 0);

+
         for (int j = 1 + b; j < image->getWidth() - 1; j += 2) {
             image->r(image->getHeight() - 1, j) = (image->r(image->getHeight() - 2, j) + image->r(image->getHeight() - 1, j + 1) + image->r(image->getHeight() - 1, j - 1)) / 3;
             image->g(image->getHeight() - 1, j) = (image->g(image->getHeight() - 2, j) + image->g(image->getHeight() - 1, j + 1) + image->g(image->getHeight() - 1, j - 1)) / 3;
             image->b(image->getHeight() - 1, j) = (image->b(image->getHeight() - 2, j) + image->b(image->getHeight() - 1, j + 1) + image->b(image->getHeight() - 1, j - 1)) / 3;
         }
+
+        for (int j = 0; j < 1 + b; ++j) {
+            image->r(image->getHeight() - 1, j) = image->r(image->getHeight() - 2, j);
+            image->g(image->getHeight() - 1, j) = image->g(image->getHeight() - 2, j);
+            image->b(image->getHeight() - 1, j) = image->b(image->getHeight() - 2, j);
+        }
+
+        image->r(image->getHeight() - 1, image->getWidth() - 1) = image->r(image->getHeight() - 2, image->getWidth() - 1);
+        image->g(image->getHeight() - 1, image->getWidth() - 1) = image->g(image->getHeight() - 2, image->getWidth() - 1);
+        image->b(image->getHeight() - 1, image->getWidth() - 1) = image->b(image->getHeight() - 2, image->getWidth() - 1);
     }

     // Flip if needed

[heckflosse]

I found another issue when rotating fuji super-ccd files by 90°. I will fix that too...

Edit: I will most likely fix that tomorrow. Now I will give precedence to pixelshift work :)

[heckflosse]

@agriggio

I hope this could help you in finding a proper solution

It didn't help, but only because I was so busy searching the issue using valgrind while you also worked on finding the culprit in parallel ;-)
We should coordinate our work better in future to avoid wasting time :)
Thank you very much for your effort to find the source of the Issue, greatly appreciated :+1:

Ingo

[agriggio]

@heckflosse don't worry, it was a waste of time only in a very technical sense. I did it because I wanted to, and learnt a lot in the process. This is never a bad thing :smile:

[agriggio]

@heckflosse have you pushed this to dev already?

[heckflosse]

@agriggio not yet. I want to solve the other issue I mentioned above too. Then I will push.

[heckflosse]

@agriggio Alberto, after a second look at the code I detected that my fix from above does not cover all cases. There are a lot of cases (rotation combined with width and height of preview) which would have all to be tested. So, for now, I tend to use your solution to fill the image with zeros a priori (only for the fuji case of course). That's the safe solution for this case and not worse than before concerning output. What do you think?

Ingo

[heckflosse]

Further remark:

With the merge of pixelshift rt now is able to access up to 4 sub-frames of raw files.
Fuji Super-CCD raw files contain 2 sub-frames. But they have different size. For that reason I disabled access to second frame for Fuji Super-CCD raw files here.
If we ever make an approach to improve support for this files, we should take that into account.

[agriggio]

sounds good to me! :+1:
if you feel like, you might perhaps just make the zero filling loop a bit less naive than my version, e.g. by parallelizing it, although I'm not sure the difference will be noticeable...

[heckflosse]

@agriggio I took the simple approach for this rare species of raw files ;-)

[agriggio]

:+1: