Problem with pointer arithmetic for a filtering algorithm

I have this piece of code:

Mat expanded_image = Mat::zeros(image.rows + 2, image.cols + 2, image.type());
copyMakeBorder(image, expanded_image, 1, 1, 1, 1, BORDER_REPLICATE);
Mat new_image = Mat::zeros(image.size(), image.type());

for (int y = 1; y < expanded_image.rows-1; y++) {
    const uchar* previous = expanded_image.ptr<uchar>(y - 1);
    const uchar* current = expanded_image.ptr<uchar>(y);
     const uchar* next = expanded_image.ptr<uchar>(y + 1);
     uchar* output;
     for (int x = image.channels(); x < (expanded_image.cols-1) * image.channels(); x++) {
         output = new_image.ptr<uchar>(y-1, x - image.channels());
         *output =
                 saturate_cast<uchar>((4*current[x] + 2*current[x+1]+2*current[x-1] +
                     previous[x-1] + 2*previous[x] + previous[x-1] + 
                     next[x-1] + 2*next[x] + next[x+1]) / 16);
   }
}

And when I am running my program in debug mode I get a error thrown at line 30 which is this line:
output = new_image.ptr<uchar>(y-1, x - image.channels());

Any help on what am I missing to see?

Thanks!

You get an error at line 30. What does the error say? (I’m guessing seg fault or similar)

The code isn’t entirely clear to me, but it looks like your problem is that your inner loop goes from image.channels() to (exanded_image.cols - 1) * image.channels(), and then you index into new_image with (x-image.channels()) which will be out of range. I think (maybe) multipying by image.channles() is wrong in your inner loop. Also the way you index with x-1 and x + 1 might be wrong - not sure what you are trying to do there (is that to access previous pixel values? If so you probably need to have a factor of channles() in there to get to the corresponding color component of the previous / next pixel.

Also have you tried inspecting the values in the debugger. What value does y-1 have? What about x - image.channels()? Are they in range of new_image.size() ?

1 Like

your code does not work at all for img.channels() > 1
(all x+1, x-1, x++ things need to be reconsidered, then)

tl;dr: don’t write per-pixel code like that, please.

it looks like your code is trying to do a simple 3x3 convolution:

K = {
    1, 2, 1,
    2, 4, 2,
    1, 2, 1
} // /16

use filter2D() instead of writing loops, please

1 Like

your code does not work at all for img.channels() > 1
(all x+1, x-1, x++ things need to be reconsidered, then)

Why it won’t work for img.channels() > 1 ? Doesn’t the loops cycle channel by channel for all pixels this way?
The idea I got from this tutorial: OpenCV: Mask operations on matrices . Please see the Sharpen function.

Plus filter2D is good for a convolution but for some custom pixel algorithm I need access to pixels. For example like in the case of custom createBorder function using specific prediction algorithm.

Hi Steve,

I have this printed in the console:

OpenCV(4.7.0-dev) Error: Assertion failed ((unsigned)i1 < (unsigned)size.p[1]) in cv::Mat::ptr, file C:\opencv-4.x\Build-4.x\install\include\opencv2\core\mat.inl.hpp, line 752

Which makes me think that I simply try to fetch a pointer to an element that does not exist in the image Mat.

I got this as exception message in IDE:

Unhandled exception at 0x00007FF8C697FDEC in ImageFileFiltering.exe: Microsoft C++ exception: cv::Exception at memory location 0x0000006A7CAFE7A0.

And the PC points to line 1294 of C:\opencv-4.x\opencv-4.x\modules\core\src\system.cpp
which is the error processing function that printed the above message to console I think.

I think I should check this code and see what it does. The error message itself is to some degree a hint.

There are different ways to store multi-channel images. In OpenCV usually they are stored in a packed representation - one sequence of pixels, with B, G and R stored sequentially for a given pixel. In some cases the B/G/R data is stored as three separate sequences, so all of the B pixels stored in sequence, then all of the G, then all of the R. This is often referred to as a planar representation. The way you ask your question “Doesn’t the loop cycle channel by channel for all the pixels” suggests that you believe you are operating with planar data, but I suspect you are actually working with packed data.

The assertion error you are getting makes sense. Your inner loop has a limit of (expanded_image.cols - 1) * image.channels. For a BGR image with 100 columns, this would be 297, but wyhen you try to get your output pointer, your x value is out of range for that image (which has 100 columns).

You can make this code work, but you have to understand the structure of the image, etc. You would also need another loop to handle the different color channels.

The key is to understand what you get back from new_image.ptr(y,x) - it is a pointer to the first element (channel value) for the pixel with index (y,x). The other elements for that pixel (in a multi-channel image) come next.

For example, for a BGR image:

BGRBGRBGRBGR
______________^
When you call output = new_image.ptr(0,3) you will get a pointer which points to the first channel of the pixel in row 0, column 3 (the 4th pixel in the image).

So output points to the B value for that pixel (assuming a BGR image), so you can access it with output[0], and you can access the G value with output[1], and the R value with output[2]

To access the next pixel on that row, you would use:
B: output[channels]
G: output[channels + 1]
R: output[channels + 2]

And the previous pixel would be:
B: output[0 - channels];
G: output[1 - channels];
R: output[2 - channels];

I think it is helpful to work through this to understand and learn, but for code you intend to run and use, you are much better off using the provided functions. This kind of code is very error prone, and the functions that OpenCV provides have been tested, optimized, and written to handle all of the corner cases - including the ones you haven’t thought about yet. Many of the functions are able to be parallelized, take advantage of advanced instruction sets or other hardware-specific optimizations. So unless you are doing something very specific, you are almost certainly better off using the functions that already exist.

1 Like

Hi Steve, thank you very much for the reply!

After reading it few times and looking at my code with refreshed eyes I think I understood what your are saying. But this code is exactly for leraning purposes, in production software I would probably will use filter2D.

But there always might be the need to implement custom processing algorithm so knowing how to work with pixels is always great in my humble opinion.

Have a nice day!