Thread: I'm getting a segmentation fault when implementing a blur effect on a .bmp image

  1. #1
    Registered User
    Join Date
    Aug 2024
    Posts
    1

    I'm getting a segmentation fault when implementing a blur effect on a .bmp image

    This is the promt:
    There are a number of ways to create the effect of blurring or softening an image. For this problem, we’ll use the “box blur,” which works by taking each pixel and, for each color value, giving it a new value by averaging the color values of neighboring pixels.

    • Consider the following grid of pixels, where we’ve numbered each pixel.
      https://cs50.harvard.edu/x/2024/pset.../less/grid.png
    • The new value of each pixel would be the average of the values of all of the pixels that are within 1 row and column of the original pixel (forming a 3x3 box). For example, each of the color values for pixel 6 would be obtained by averaging the original color values of pixels 1, 2, 3, 5, 6, 7, 9, 10, and 11 (note that pixel 6 itself is included in the average). Likewise, the color values for pixel 11 would be be obtained by averaging the color values of pixels 6, 7, 8, 10, 11, 12, 14, 15 and 16.
    • For a pixel along the edge or corner, like pixel 15, we would still look for all pixels within 1 row and column: in this case, pixels 10, 11, 12, 14, 15, and 16.
    • I know that blurring one pixel ends up affecting the blur of another pixel, so I created a copy of the origianl array of pixels called "copy". Then I passed all the modified pixels to the origianl array like this:



    Code:
    • // Blur image void blur(int height, int width, RGBTRIPLE image[height][width]) { // Create a copy of image RGBTRIPLE copy[height][width]; for (int i = 0; i < height; i++) { for (int j = 0; j < width; j++) { copy[i][j] = image[i][j]; } } // Blur each pixel for (int i = 0; i < height; i++) { for (int j = 0; j < width; j++) { if(i - 1 >= 0 && i + 1 < height && j - 1 >= 0 && j + 1 < width) { copy[i][j].rgbtRed = round((image[i - 1][j - 1].rgbtRed + image[i - 1][j].rgbtRed + image[i - 1][j + 1].rgbtRed + image[i][j - 1].rgbtRed + image[i][j].rgbtRed + image[i][j + 1].rgbtRed + image[i + 1][j - 1].rgbtRed + image[i + 1][j].rgbtRed + image[i + 1][j + 1].rgbtRed)/9); copy[i][j].rgbtGreen = round((image[i - 1][j - 1].rgbtGreen + image[i - 1][j].rgbtGreen + image[i - 1][j + 1].rgbtGreen + image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen + image[i + 1][j - 1].rgbtGreen + image[i + 1][j].rgbtGreen + image[i + 1][j + 1].rgbtGreen)/9); copy[i][j].rgbtBlue = round((image[i - 1][j - 1].rgbtBlue + image[i - 1][j].rgbtBlue + image[i - 1][j + 1].rgbtBlue + image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue + image[i + 1][j - 1].rgbtBlue + image[i + 1][j].rgbtBlue + image[i + 1][j + 1].rgbtBlue)/9); } else if(j - 1 < 0) { copy[i][j].rgbtRed = round((image[i - 1][j].rgbtRed + image[i - 1][j + 1].rgbtRed + image[i][j].rgbtRed + image[i][j + 1].rgbtRed + image[i + 1][j].rgbtRed + image[i + 1][j + 1].rgbtRed)/6); copy[i][j].rgbtGreen = round((image[i - 1][j].rgbtGreen + image[i - 1][j + 1].rgbtGreen + image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen + image[i + 1][j].rgbtGreen + image[i + 1][j + 1].rgbtGreen)/6); copy[i][j].rgbtBlue = round((image[i - 1][j].rgbtBlue + image[i - 1][j + 1].rgbtBlue + image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue + image[i + 1][j].rgbtBlue + image[i + 1][j + 1].rgbtBlue)/6); } else if(j + 1 >= width) { copy[i][j].rgbtRed = round((image[i - 1][j - 1].rgbtRed + image[i - 1][j].rgbtRed + image[i][j - 1].rgbtRed + image[i][j].rgbtRed + image[i + 1][j - 1].rgbtRed + image[i + 1][j].rgbtRed)/6); copy[i][j].rgbtGreen = round((image[i - 1][j - 1].rgbtGreen + image[i - 1][j].rgbtGreen + image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen + image[i + 1][j - 1].rgbtGreen + image[i + 1][j].rgbtGreen)/6); copy[i][j].rgbtBlue = round((image[i - 1][j - 1].rgbtBlue + image[i - 1][j].rgbtBlue + image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue + image[i + 1][j - 1].rgbtBlue + image[i + 1][j].rgbtBlue)/6); } else if(i - 1 < 0) { copy[i][j].rgbtRed = round((image[i][j - 1].rgbtRed + image[i][j].rgbtRed + image[i][j + 1].rgbtRed + image[i + 1][j - 1].rgbtRed + image[i + 1][j].rgbtRed + image[i + 1][j + 1].rgbtRed)/6); copy[i][j].rgbtGreen = round((image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen + image[i + 1][j - 1].rgbtGreen + image[i + 1][j].rgbtGreen + image[i + 1][j + 1].rgbtGreen)/6); copy[i][j].rgbtBlue = round((image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue + image[i + 1][j - 1].rgbtBlue + image[i + 1][j].rgbtBlue + image[i + 1][j + 1].rgbtBlue)/6); } else if(i + 1 >= height) { copy[i][j].rgbtRed = round((image[i - 1][j - 1].rgbtRed + image[i - 1][j].rgbtRed + image[i - 1][j + 1].rgbtRed + image[i][j - 1].rgbtRed + image[i][j].rgbtRed + image[i][j + 1].rgbtRed)/6); copy[i][j].rgbtGreen = round((image[i - 1][j - 1].rgbtGreen + image[i - 1][j].rgbtGreen + image[i - 1][j + 1].rgbtGreen + image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen)/6); copy[i][j].rgbtBlue = round((image[i - 1][j - 1].rgbtBlue + image[i - 1][j].rgbtBlue + image[i - 1][j + 1].rgbtBlue + image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue)/6); } else if(j - 1 < 0 && i - 1 < 0) { copy[i][j].rgbtRed = round((image[i][j].rgbtRed + image[i][j + 1].rgbtRed + image[i + 1][j].rgbtRed + image[i + 1][j + 1].rgbtRed)/4); copy[i][j].rgbtGreen = round((image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen + image[i + 1][j].rgbtGreen + image[i + 1][j + 1].rgbtGreen)/4); copy[i][j].rgbtBlue = round((image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue + image[i + 1][j].rgbtBlue + image[i + 1][j + 1].rgbtBlue)/4); } else if(j + 1 >= width && i - 1 < 0) { copy[i][j].rgbtRed = round((image[i][j - 1].rgbtRed + image[i][j].rgbtRed + image[i + 1][j - 1].rgbtRed + image[i + 1][j].rgbtRed)/4); copy[i][j].rgbtGreen = round((image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen + image[i + 1][j - 1].rgbtGreen + image[i + 1][j].rgbtGreen)/4); copy[i][j].rgbtBlue = round((image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue + image[i + 1][j - 1].rgbtBlue + image[i + 1][j].rgbtBlue)/4); } else if(j - 1 < 0 && i + 1 >= height) { copy[i][j].rgbtRed = round((image[i - 1][j].rgbtRed + image[i - 1][j + 1].rgbtRed + image[i][j].rgbtRed + image[i][j + 1].rgbtRed)/4); copy[i][j].rgbtGreen = round((image[i - 1][j].rgbtGreen + image[i - 1][j + 1].rgbtGreen + image[i][j].rgbtGreen + image[i][j + 1].rgbtGreen)/4); copy[i][j].rgbtBlue = round((image[i - 1][j].rgbtBlue + image[i - 1][j + 1].rgbtBlue + image[i][j].rgbtBlue + image[i][j + 1].rgbtBlue)/4); } else if(j + 1 >= width && i + 1 >= height) { copy[i][j].rgbtRed = round((image[i - 1][j - 1].rgbtRed + image[i - 1][j].rgbtRed + image[i][j - 1].rgbtRed + image[i][j].rgbtRed)/4); copy[i][j].rgbtGreen = round((image[i - 1][j - 1].rgbtGreen + image[i - 1][j].rgbtGreen + image[i][j - 1].rgbtGreen + image[i][j].rgbtGreen)/4); copy[i][j].rgbtBlue = round((image[i - 1][j - 1].rgbtBlue + image[i - 1][j].rgbtBlue + image[i][j - 1].rgbtBlue + image[i][j].rgbtBlue)/4); } } } // Pass the values through reference for (int i = 0; i < height; i++) { for (int j = 0; j < width; j++) { image[i][j] = copy[i][j]; } } }




    I am getting the following errors plus a segmentation fault:
    :( blur correctly filters middle pixel
    expected "127 140 149\n", not "126 140 149\n"
    :( blur correctly filters pixel in corner
    expected "70 85 95\n", not "55 62 65\n"
    :( blur correctly filters 3x3 image
    expected "70 85 95\n80 9...", not "55 62 65\n80 9..."
    :( blur correctly filters 4x4 image
    expected "70 85 95\n80 9...", not "56 70 73\n80 9..."

    Could somebody please help me understand what might be wrong with my code?

  2. #2
    Registered User
    Join Date
    Dec 2017
    Posts
    1,664
    Which line/statement is generating the seggy? That would've been useful information.
    Also, how are you compiling this? Visual Studio? Command line? Other? Perhaps not on Windows (although RGBTRIPLE sounds like windows)?
    And are you compiling it as C or C++ (usually depends on whether the file is .c or .cpp on Windows, I think).

    Still, your first "else if" will not work properly.
    What if i - 1 is less than 0 or i + 1 is greater than height - 1?
    Code:
                else if (j - 1 < 0)
                {
                    copy[i][j].rgbtRed = round((image[i - 1][j    ].rgbtRed
                                              + image[i - 1][j + 1].rgbtRed
                                              + image[i    ][j    ].rgbtRed
                                              + image[i    ][j + 1].rgbtRed
                                              + image[i + 1][j    ].rgbtRed
                                              + image[i + 1][j + 1].rgbtRed)
                                              / 6);
                    // same for green and blue...
                }
    Other "else ifs" have the same kind of problem.

    Note that the round() will not do anything here since you've used integer division which essentially truncates any fractional part.
    You would need to divide by 6.0 to make it floating point before the round.

    Also, you don't need to initialize copy first since you are replacing all the elements.

    Just for fun, see if the following works. I haven't tested it since you didn't show your entire program. You may need to fix small errors as I have not tried to compile it at all.
    Code:
    typedef struct RGBTripleSum {
        unsigned r, g, b;
    } RGBTripleSum;
    
    void addRGB(RGBTripleSum *sum, const RGBTRIPLE *rgb) {
        sum->r += rgb->rgbtRed;
        sum->g += rgb->rgbtGreen;
        sum->b += rgb->rgbtBlue;
    }
    
    void blur(int height, int width, RGBTRIPLE im[height][width]) {
        RGBTRIPLE copy[height][width];
        for (int h = 0; h < height; h++)
            for (int w = 0; w < width; w++) {
                RGBTripleSum sum = {0, 0, 0};
                addRGB(&sum, &im[h][w]);
                unsigned cnt = 1;
                if (h > 0) {
                                         addRGB(&sum, &im[h - 1][w    ]); ++cnt;
                    if (w > 0)         { addRGB(&sum, &im[h - 1][w - 1]); ++cnt; }
                    if (w < width - 1) { addRGB(&sum, &im[h - 1][w + 1]); ++cnt; }
                }
                if (h < height - 1) {
                                         addRGB(&sum, &im[h + 1][w    ]); ++cnt;
                    if (w > 0)         { addRGB(&sum, &im[h + 1][w - 1]); ++cnt; }
                    if (w < width - 1) { addRGB(&sum, &im[h + 1][w + 1]); ++cnt; }
                }
                if (w > 0)             { addRGB(&sum, &im[h    ][w - 1]); ++cnt; }
                if (w < width - 1)     { addRGB(&sum, &im[h    ][w + 1]); ++cnt; }
    
                copy[h][w].rgbtRed   = round((double)sum.r / cnt);
                copy[h][w].rgbtGreen = round((double)sum.g / cnt);
                copy[h][w].rgbtBlue  = round((double)sum.b / cnt);
            }
        memcpy(im, copy, height * width * 3);  // include <string.h> for this
    }
    Last edited by john.c; 08-16-2024 at 01:05 PM.
    All truths are half-truths. - A.N. Whitehead

  3. #3
    Registered User
    Join Date
    Apr 2021
    Posts
    148
    Two points:

    1. You are testing the values of i and j. There are two tests for each, <MIN and >MAX, which produce only three possible outcomes because <MIN and >MAX cannot both be true at once. Thus you have 3x3 possible states. But after your first test, you are only checking single conditions, while any of 3 possible conditions may apply. For example, if j <MIN applies, you don't handle the subcases of i<MIN, i>MIN, i>MAX. You just lump them all together.

    2. You are testing for cases inside a loop where you generate all the possible values. This is silly. Just generate the values knowing what the cases are, already. That is, instead of looping over all possible i, j pairs, try picking out the values of i and j that correspond to the various cases, and writing your code that way. (Basically, separate code sections for the corners, the edges, and the middle.)

  4. #4
    Registered User
    Join Date
    Dec 2017
    Posts
    1,664
    aghast makes a good point
    Code:
    // middle elements
    for (int h = 1; h < height - 1; ++h)
        for (int w = 1; w < width - 1; ++w)
            im[h][w] ...
    
    // left and right edges
    for (int h = 1; h < height - 1; ++h) {
        im[h][0] ...
        im[h][width - 1] ...
    }
    
    // top and bottom edges/
    for (int w = 1; w < width - 1; ++w) {
        im[0][w] ...
        im[height - 1][w] ...
    }
    
    // corners
    im[0][0] ...
    im[0][width - 1] ...
    im[height - 1][0] ...
    im[height - 1][width - 1] ...
    All truths are half-truths. - A.N. Whitehead

  5. #5
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,677
    > RGBTRIPLE copy[height][width];
    Another issue is running out of stack space.

    Even a small 600x600 image tops 1MB.

    Which is a bit of a problem when default stack size is 1MB
    Thread Stack Size - Win32 apps | Microsoft Learn

    It's 4MB if you're on 64-bit Windows, and typically 8MB if you're on Linux.
    Sure, you get a bit of extra wriggle room, but you're only a user choice between "it works" and "omg-segfault".
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  6. #6
    Registered User
    Join Date
    Dec 2017
    Posts
    1,664
    Salem makes a very important point!
    It is best to malloc the extra storage so that it is not on the stack.
    Note that below we simulate a 2D array with a 1D array.
    Combining salem's and aghast's ideas:
    Code:
    RGBTRIPLE *copy = malloc(height * width * sizeof(RGBTRIPLE));
    
    // middle elements (leaves out edges)
    for (int h = 1; h < height - 1; ++h)
        for (int w = 1; w < width - 1; ++w)
            copy[h * width + w] = ...
    
    // left and right edges (leaves out corners)
    for (int h = 1; h < height - 1; ++h) {
        // left edge
        copy[h * width] = ...
        // right edge
        copy[h * width + (width - 1)] = ...
    }
    
    // top and bottom edges (leaves out corners)
    for (int w = 1; w < width - 1; ++w) {
        // top edge
        copy[w] = ...
        // bottom edge
        copy[(height - 1) * width + w] = ...
    }
    
    // -- corners ----------------------
    // top-left corner
    copy[0] = ...
    // top-right corner
    copy[width - 1] = ...
    // bottom-left corner
    copy[(height - 1) * width] = ...
    // bottom-right corner
    copy[(height - 1) * width + (width - 1)] = ...
    
    memcpy(image, copy, height * width * sizeof(RGBTRIPLE));
    
    free(copy);
    All truths are half-truths. - A.N. Whitehead

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help making a blur effect in c
    By odachi in forum C Programming
    Replies: 2
    Last Post: 04-17-2020, 06:37 PM
  2. Replies: 1
    Last Post: 06-09-2016, 10:45 PM
  3. Image Blur Filter
    By Mcdom34 in forum C Programming
    Replies: 5
    Last Post: 05-01-2015, 08:36 AM
  4. Segmentation Fault -- Stack implementing a Linked List
    By jackalclone1 in forum C Programming
    Replies: 10
    Last Post: 02-15-2014, 12:44 AM
  5. A little trouble implementing a blur filter
    By omishompi in forum C++ Programming
    Replies: 2
    Last Post: 04-16-2006, 08:57 AM

Tags for this Thread