Convolution function gives segmentation error while getting pixel value of relative image coordinates

Question

I'm trying to write a convolution function in C for my computer vision study. In this function, every pixel in the convolved image is a sum of product of original image and filter kernel like in this image and this gif.

In the code below pixel values are float. get_pixel() function gets the pixel value at given indexes. set_pixel() function sets the value to given indexes.

image convolve(image im, image filter) {
    // imx, imy, imc: indexes of image pixels
    // fix, fiy: indexes of filter pixels
    // rx, ry: relative indexes of pixels
    
    image convolved_img = make_image(im.w, im.h, im.c); // image with same dimensions
    float value = 0;                                    // pixel value
    int oxo = floor(filter.w / 2);                      // half of the kernel width
    int xox = floor(filter.h / 2);                      // half of the kernel height

    // Convolution Loop
    for(int imc = 0; imc < im.c; imc++) {               // for every channel
        for(int imx = 0; imx < im.w; imx++) {
            for(int imy = 0; imy < im.h; imy++) {       // for every pixel
                value = 0;
                for(int fix = 0; fix < filter.w; fix++) {
                    for(int fiy = 0; fiy < filter.h; fiy++) {
                        int rx = imx - oxo + fix;
                        int ry = imy - xox + fiy;
                        value += get_pixel(filter, fix, fiy, 0) * get_pixel(im, rx, ry, imc);
                    }
                }
                set_pixel(convolved_img, imx, imy, imc, value);
            }
        }
    }
    return convolved_img;
}

I'm getting segmentation fault (core dumped) error. After debugging I realized its because of line:

value += get_pixel(filter, fix, fiy, 0) * get_pixel(im, rx, ry, imc);

When I gave fixed values of rx and ry, the program executes successfully. Inside the loop I printed the values of imx, imy, fix, fiy, rx, ry and everything works until a portion of the image has processed; after uncertain time of loop then program crushes without any reason.

I'm sure that it cannot be a index bounds related because I truncated indexes inside get_pixel() function below which I get stored value from a long array of floats.

float get_pixel(image im, int x, int y, int c) {
    if(x > im.w) {x = im.w;}
    else if(y > im.h) {y = im.h;}
    else if(c > im.c) {c = im.c;}
    else if(x < 0) {x = 0;}
    else if(y < 0) {y = 0;}
    else if(c < 0) {c = 0;}

    int index = (c * (im.h * im.w)) + (y * im.w) + x;
    return im.data[index];
}

Here is my thought about this operation as pseudo-code:

create convolved_image with same dimensions
for every pixel (imx, imy) in image {
    float value = 0;
    for every pixel (fix, fiy) in filter {
        // calculate relative pixel coordinates
        int rx = imx - (filter / 2) + fix;
        int ry = imy - (filter / 2) + fiy;
        value += filter(fix, fiy) * image(rx, ry);
    }
    set pixel of convolved_image to value
}

Am I missing something? What is the fault in my approach? Or is there a better way for this operation?

Have you used a debugger? It will tell you immediately exactly which line of code triggers the seg fault. And have you actually examined the value of `index` when the program crashes? [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems). If you still need help, please provide a [minimal verifiable example](https://stackoverflow.com/help/minimal-reproducible-example). — kaylum, Jul 22 '20 at 12:19
"because I truncated indexes" No, you don't. 1. `{y = im.h;}` is already off by 1. 2. All parameters can be out of bounds. You only fix at most 1 of them. You must remove the `else` keywords and check all conditions. — Gerhardh, Jul 22 '20 at 12:49
Not related to your segfault, but your approach will cause edge pixels taken multiple times. Instead of skipping pixels that are outside the image, you take the edge pixel instead. Not sure if that is what you want. — Gerhardh, Jul 22 '20 at 13:04

score 1 · Accepted Answer · answered Jul 22 '20 at 12:55

This is clearly an out of bounds access:

                for(int fix = 0; fix < filter.w; fix++) {
                    for(int fiy = 0; fiy < filter.h; fiy++) {
                        int rx = imx - oxo + fix;
                        int ry = imy - xox + fiy;
                        value += get_pixel(filter, fix, fiy, 0) * get_pixel(im, rx, ry, imc);
                    }
                }

With imx going up to im.x and fix going up to 2*oxo you are clearly larger than im.x. Same for imy.

You try to limit the range but that is not correct:

float get_pixel(image im, int x, int y, int c) {
    if(x > im.w) {x = im.w;}
    else if(y > im.h) {y = im.h;}
    else if(c > im.c) {c = im.c;}
    else if(x < 0) {x = 0;}
    else if(y < 0) {y = 0;}
    else if(c < 0) {c = 0;}

    int index = (c * (im.h * im.w)) + (y * im.w) + x;
    return im.data[index];
}

You forgot that all parameters can be wrong. You stop after first. Also you limit to size+1 which also is wrong.

Change like this:

float get_pixel(image im, int x, int y, int c) {
    if(x >= im.w) {x = im.w-1;}
    else if(x < 0) {x = 0;}

    if(y >= im.h) {y = im.h-1;}
    else if(y < 0) {y = 0;}

    if(c >= im.c) {c = im.c-1;}
    else if(c < 0) {c = 0;}

    int index = (c * (im.h * im.w)) + (y * im.w) + x;
    return im.data[index];
}

Thanks. This was a rookie mistake :) It work know :) – enesdemirag Jul 22 '20 at 13:15 — enesdemirag, Jul 22 '20 at 13:15

Convolution function gives segmentation error while getting pixel value of relative image coordinates

1 Answers1