debayer algorithms cause bad-access-exceptions under some circumstances

[bakercp]

Under heavy multi-camera load, I am getting seemingly random bad access exceptions on one line of the demosaicing code (for each of the gray/rgb/bgr). I believe I traced it to the following code:

https://github.com/inspirit/PS3EYEDriver/blob/master/src/ps3eye.cpp#L630-L642

for (int y = 0; y < frame_height-1; source_row += source_stride, dest_row += dest_stride, ++y)
{
    const uint8_t* source = source_row;
    const uint8_t* source_end = source + (source_stride-2); // -2 to deal with the fact that we're starting at the second pixel of the row and should end at the second-to-last pixel of the row (first and last are filled separately)
    uint8_t* dest = dest_row;       

    // Row starting with Green
    if (y % 2 == 0)
    {
        // Fill first pixel (green)
        dest[-1*swap_br]    = (source[source_stride] + source[source_stride + 2] + 1) >> 1;
        dest[0]             = source[source_stride + 1];
        dest[1*swap_br]     = (source[1] + source[source_stride * 2 + 1] + 1) >> 1; // THIS LINE
/// ...

I believe, for example, for a 640x480 image, when y == 478 (y only goes from 0 -> 479) then source[source_stride * 2 + 1] will yield an invalid memory address if the FrameBuffer source is the last frame in the ring buffer. By luck, the memory address is not invalid 50% of the time in a double frame buffer and increasing the number of buffers makes it less likely, but it still happens on occasion.

One quick and dirty method to fix it is to just loop over y < frame_height - 2. I'm in the process of digging into the demosaicing code to see if that makes sense.

@rovarma What do you think?

[rovarma]

Hey,

I'm honestly not sure anymore. It's been quite some time since I wrote the code, I'd have to go through it in detail again. Your analysis makes sense (at first glance, it definitely appears to be an out-of-bounds read), but I think your proposed fix will lead to the last two lines in the image being garbage (in the current code, the last line in the image is filled by copying the second-to-last line, which will be skipped in the loop now). I don't have time to look at the debayer code itself in detail right now.

[bakercp]

If I get a chance, I'll dig into the bayer-code a bit more and it may give me a chance to look at #46 too. Thanks for the feedback and great optimizations @rovarma !

[marcel303]

Hi, I ran into this too and came up with the same fix as @bakercp. The fix is correct, as the loop produces two rows per iteration. It starts at the 2nd row (the first row is also copied, so with the fix it decodes into rows [1, height-2] and row 0 and row height-1 are copied. I also verified the fix visually, checking pixels get written by first writing some noise and seeing if it got overwritten, which it did.