-1

My goal is to blur an image in C (not C++), by taking the average of all surrounding pixels. I have separated my calculations by:

  • If pixel is at the top left corner
  • If pixel is at top right corner
  • If pixel is at bottom left center
  • If pixel is at bottom right corner
  • If pixel is at top edge (no corner)
  • If pixel is at bottom edge (no corner)
  • All other pixels

I am having a lot of trouble with my code, which I fixed by creating a copy of the image. But, the my code starts to not work when it gets to the bottom few rows in a 3x3 and 4x4 image.

I need help with reviewing what is wrong with my code. Also, I know my code is very lengthy which is a way for me to write everything out and understand it more, but I would also like if someone could give input on another way to write the code. I am very new to this, so I definitely need a lot of explanation. I have searched and viewed other shortened, well written codes, but I have a hard time understanding it.

I'd really appreciate all the help!

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width]){

//top left corner variables
float ravg_lcorner = 0;
float gavg_lcorner = 0;
float bavg_lcorner = 0;

//top right corner variables
float ravg_rcorner = 0;
float gavg_rcorner = 0;
float bavg_rcorner = 0;

//bottom left corner variables
float ravg_blcorner = 0;
float gavg_blcorner = 0;
float bavg_blcorner = 0;

//bottom right corner variables
float ravg_brcorner = 0;
float gavg_brcorner = 0;
float bavg_brcorner = 0;

//top row center variables
float ravg_topcenter = 0;
float gavg_topcenter = 0;
float bavg_topcenter = 0;

//top row center variables
float ravg_btmcenter = 0;
float gavg_btmcenter = 0;
float bavg_btmcenter = 0;

//left side center variables
float ravg_lscenter = 0;
float gavg_lscenter = 0;
float bavg_lscenter = 0;

//right side center variables
float ravg_rscenter = 0;
float gavg_rscenter = 0;
float bavg_rscenter = 0;

//center variaables
float ravg_center = 0;
float gavg_center = 0;
float bavg_center = 0;

RGBTRIPLE temp;
RGBTRIPLE copy_image[height][width];

//create copy of image canvas
for (int i = 0; i < height; i++) 
{
    for (int j = 0; j < width; j++)
    {
        temp = image[i][j];
        image[i][j] = copy_image[i][j];
        copy_image[i][j] = temp;
    }
}


for (int i = 0; i < height; i++)
{
    for (int j = 0; j < width; j++)
    {
        
        //if pixel is at top row left corner 
        if (i == 0 && j == 0)
        {
            //calculate all red average
            ravg_lcorner = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j+1].rgbtRed) / 4.00);
            //calculate all green average
            gavg_lcorner = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j+1].rgbtGreen) / 4.00);
            //calculate all blue average
            bavg_lcorner = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j+1].rgbtBlue) / 4.00);
            
            //new pixel color

            image[i][j].rgbtRed = ravg_lcorner;
            image[i][j].rgbtGreen = gavg_lcorner;
            image[i][j].rgbtBlue = bavg_lcorner;
            
        }
        
        //if pixel is top row right corner
        if (i == 0 && j == width - 1)
        {
            //calculate all red average
            ravg_rcorner = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j-1].rgbtRed) / 4.00);
            //calculate all green average
            gavg_rcorner = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j-1].rgbtGreen) / 4.00);
            //calculate all blue average
            bavg_rcorner = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j-1].rgbtBlue) / 4.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_rcorner;
            image[i][j].rgbtGreen = gavg_rcorner;
            image[i][j].rgbtBlue = bavg_rcorner;
        }
        
        //if pixel is at bottom row left corner 
        if(i == height - 1 && j == 0)
        {
            //calculate all red average
            ravg_blcorner = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j+1].rgbtRed) / 4.00);
            //calculate all green average
            gavg_blcorner = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j+1].rgbtGreen) / 4.00);
            //calculate all blue average
            bavg_blcorner = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j+1].rgbtBlue) / 4.00);
            
            //new pixel color
            copy_image[i][j].rgbtRed = ravg_blcorner;
            copy_image[i][j].rgbtGreen = gavg_blcorner;
            copy_image[i][j].rgbtBlue = bavg_blcorner;
        }
        
        //if pixel is at bottom row right corner 
        if(i == height -1 && j == width - 1)
        {
            //calculate all red average
            ravg_brcorner = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j-1].rgbtRed) / 4.00);
            //calculate all green average
            gavg_brcorner = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j-1].rgbtGreen) / 4.00);
            //calculate all blue average
            bavg_brcorner = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j-1].rgbtBlue) / 4.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_rcorner;
            image[i][j].rgbtGreen = gavg_rcorner;
            image[i][j].rgbtBlue = bavg_rcorner;
        }
        
        //if pixel is top row center
        if(i == 0 && j < width - 1 && j > 0)
        {
            //calculate all red average
            ravg_topcenter = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i+1][j-1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j+1].rgbtRed) / 6.00);
            //calculate all green average
            gavg_topcenter = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i+1][j-1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j+1].rgbtGreen) / 6.00);
            //calculate all blue average
            bavg_topcenter = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i+1][j-1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j+1].rgbtBlue) / 6.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_topcenter;
            image[i][j].rgbtGreen = gavg_topcenter;
            image[i][j].rgbtBlue = bavg_topcenter;
        }
        
        //if pixel is bottom row center
        if(i == height - 1 && j < width - 1 && j > 0)
        {
            //calculate all red average
            ravg_topcenter = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i-1][j-1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j+1].rgbtRed) / 6.00);
            //calculate all green average
            gavg_topcenter = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i-1][j-1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j+1].rgbtGreen) / 6.00);
            //calculate all blue average
            bavg_topcenter = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i-1][j-1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j+1].rgbtBlue) / 6.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_btmcenter;
            image[i][j].rgbtGreen = gavg_btmcenter;
            image[i][j].rgbtBlue = bavg_btmcenter;
        }
        
        //if pixel is at left side center
        if (i < height - 1 && i > 0 && j == 0)
        {
            //calculate all red average
            ravg_lscenter = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j+1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j+1].rgbtRed) / 6.00);
            //calculate all green average
            gavg_lscenter = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j+1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j+1].rgbtGreen) / 6.00);
            //calculate all blue average
            bavg_lscenter = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j+1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j+1].rgbtBlue) / 6.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_lscenter;
            image[i][j].rgbtGreen = gavg_lscenter;
            image[i][j].rgbtBlue = bavg_lscenter;
        }
        
        //if pixel is at right side center
        if (i < height - 1 && i > 0 && j == width - 1)
        {
            //calculate all red average
            ravg_rscenter = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j-1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j-1].rgbtRed) / 6.00);
            //calculate all green average
            gavg_rscenter = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j-1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j-1].rgbtGreen) / 6.00);
            //calculate all blue average
            bavg_rscenter = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j-1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j-1].rgbtBlue) / 6.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_rscenter;
            image[i][j].rgbtGreen = gavg_rscenter;
            image[i][j].rgbtBlue = bavg_rscenter;
        }
        
        //if pixel is at center
        if (i < height - 1 && i > 0 && j < width - 1 && j > 0)
        {
            //calculate all red average
            ravg_center = (int) round((copy_image[i][j].rgbtRed + copy_image[i][j-1].rgbtRed + copy_image[i][j+1].rgbtRed + copy_image[i+1][j+1].rgbtRed + copy_image[i+1][j].rgbtRed + copy_image[i+1][j-1].rgbtRed + copy_image[i-1][j-1].rgbtRed + copy_image[i-1][j].rgbtRed + copy_image[i-1][j+1].rgbtRed) / 9.00);
            //calculate all green average
            gavg_center = (int) round((copy_image[i][j].rgbtGreen + copy_image[i][j-1].rgbtGreen + copy_image[i][j+1].rgbtGreen + copy_image[i+1][j+1].rgbtGreen + copy_image[i+1][j].rgbtGreen + copy_image[i+1][j-1].rgbtGreen + copy_image[i-1][j-1].rgbtGreen + copy_image[i-1][j].rgbtGreen + copy_image[i-1][j+1].rgbtGreen) / 9.00);
            //calculate all blue average
            bavg_center = (int) round((copy_image[i][j].rgbtBlue + copy_image[i][j-1].rgbtBlue + copy_image[i][j+1].rgbtBlue + copy_image[i+1][j+1].rgbtBlue + copy_image[i+1][j].rgbtBlue + copy_image[i+1][j-1].rgbtBlue + copy_image[i-1][j-1].rgbtBlue + copy_image[i-1][j].rgbtBlue + copy_image[i-1][j+1].rgbtBlue) / 9.00);
            
            //new pixel color
            image[i][j].rgbtRed = ravg_center;
            image[i][j].rgbtGreen = gavg_center;
            image[i][j].rgbtBlue = bavg_center;
        }
        
    }
}

return;}
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
Rebecca
  • 3
  • 2
  • 1
    Have you tried running your code line by line in a debugger while monitoring the values of all variables, in order to determine at which point your program stops behaving as intended? If you did not try this, then you probably want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). Even if this does not solve your problem, it should at least help you produce a [mre] of your problem. – Andreas Wenzel Oct 14 '20 at 03:10
  • 1
    Just as a side note: It seems inefficient to check for every pixel whether it is in one of the corners or on the edge. It would probably be more efficient to change the line `for (int j = 0; j < width; j++)` to `for (int j = 1; j < width - 1; j++)` and handle the cases of `j == 0` and `j == width - 1` outside the innermost loop. That way, you will no longer have to check for the left and right edges. You can do something similar for the top and bottom edges. However, this is only a performance consideration. This inefficiency should not be the reason why your code doesn't work. – Andreas Wenzel Oct 14 '20 at 03:31
  • 1
    You have a long list of `if` statements. If they are mutually exclusive, then it would probably be better to use `if` and then a chain of `else if` afterwards. In that case, the last condition would just be an `else`. That way, you can ensure that exactly one of your conditional blocks is executed. If you don't do it this way, then you run the risk of several `if` blocks being executed or none at all. You can always add a `//` comment after the else statement, to specify what condition you expect. In that case, you may want to verify that the condition is true using the `assert` macro. – Andreas Wenzel Oct 14 '20 at 03:41
  • Thank you so much! I will read up on the debugging, and I'll will try out the else if & else statement. For checking the edges, wouldn't I still need to check them since for corner pixels, I'm averaging a total of 4 pixels, edges a total of 6 pixels, and middle a total of 9? – Rebecca Oct 14 '20 at 03:46
  • You are right that you have to make sure that you apply the correct code for edge pixels and the correct code for center pixels. However, instead of checking for every pixel whether it is one of the edge cases, my proposal was to make your innermost loop only iterate over non-edge pixels, so it doesn't have to perform any checks for edge cases. But don't worry about this for now. My proposal will only make your program run faster. It will not make your program run more correctly. Your problem right now is not the speed of your program, but its correctness. So concentrate on that for now. – Andreas Wenzel Oct 14 '20 at 04:04
  • 1
    Your code for handling the "pixel is at bottom row left corner " is different from the code for handling all other cases. All other cases write the final result ("new pixel color") to `image`, but this case writes the final result to `image_copy` instead. I believe that this is not intended. – Andreas Wenzel Oct 14 '20 at 04:18
  • As pointed out in the answer below (written by someone else), your code for copying the canvas does not work. Instead of having three lines for swapping data, you should simply write `copy_image[i][j] = image[i][j];` to directly make a copy of the data. – Andreas Wenzel Oct 14 '20 at 04:26
  • @AndreasWenzel You nailed the problem in the original code. That's a classical copy and paste error. – Costantino Grana Oct 14 '20 at 04:35
  • Thank you for the pointers!! All are fixed, but the issue is still somewhere in the bottom row calculations? I updated my code in above :) – Rebecca Oct 14 '20 at 04:54
  • Rebecca, you should really avoid having duplicated code. That is going to be a problem. Check the answer below and see if it works for you. – Costantino Grana Oct 14 '20 at 09:13
  • @Rebecca Include an output so we can help in visually debugging it... – jackw11111 Oct 14 '20 at 11:57
  • 1
    Thank you everyone, I figured it out and there was just an erorr in bottom corner and bottom middle variable. – Rebecca Oct 14 '20 at 14:27
  • @Rebecca: I was not appropriate to overwrite your question with the solution, because this invalidated the answer. There is nothing wrong with invalidating my comments, but I am talking about the answer provided by someone else. Therefore, I have reverted it. If you want to post your solution, feel free to post your own answer. See the following official help page for further information: [Can I answer my own question?](https://stackoverflow.com/help/self-answer) You don't have to write the code for your solution again. You can copy&paste from the edit log of your answer (revision 2). – Andreas Wenzel Oct 14 '20 at 19:59

1 Answers1

2

Ok, next time include the definition of your types in the question, so people can avoid guessing.

I suggest making it simpler by avoiding copy and paste. This should be a rule: "if you copy and paste, you are doing it wrong".

So we can loop over the image, loop over the 3x3 neighborhood sum the values and count how many valid pixels we have. Then we divide by that count. I avoided rounding and stick to a simpler truncate, but if you really want to round, do it with integers and don't use floats just for that.

The three functions are there to make the code terser and could be substituted with just repeated code.

My opinion: don't use an RGB triple with names and prefer an array with 3 elements, so you can loop through channels much more easily. And I strongly suggest to avoid VLAs. They are unsupported under Visual Studio and are a pain to work with.

Finally, your "copy image" was uselessly swapping undefined data into the image.

And more finally, I didn't test this code!

#include <stdint.h>
#include <string.h>

typedef struct tagRGBTRIPLE {
    uint8_t rgbtRed, rgbtGreen, rgbtBlue;
} RGBTRIPLE;

RGBTRIPLE rgbtMake(int r, int g, int b) 
{
    RGBTRIPLE out = { r, g, b };
    return out;
}
void rgbtIntAdd(int *r, int *g, int *b, RGBTRIPLE *t)
{
    *r += t->rgbtRed;
    *g += t->rgbtGreen;
    *b += t->rgbtBlue;
}
void rgbtIntDiv(int *r, int *g, int *b, int div)
{
    *r /= div;
    *g /= div;
    *b /= div;
}

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE copy_image[height][width];

    //create copy of image canvas
    memcpy(copy_image, image, height * width * sizeof(RGBTRIPLE));

    for (int i = 0; i < height; i++) {
        for (int j = 0; j < width; j++) {
            int r = 0, g = 0, b = 0, count = 0;
            for (int m = -1; m <= 1; ++m) {
                if (i + m < 0 || i + m >= height) {
                    continue;
                }
                for (int n = -1; n <= 1; ++n) {
                    if (j + n < 0 || j + n >= width) {
                        continue;
                    }
                    rgbtIntAdd(&r, &g, &b, &copy_image[i + m][j + n]);
                    ++count;
                }
            }
            rgbtIntDiv(&r, &g, &b, count);

            image[i][j] = rgbtMake(r, g, b);
        }
    }
}
Costantino Grana
  • 3,132
  • 1
  • 15
  • 35
  • why do you consider VLAs to be "a pain"? – tstanisl Apr 19 '21 at 20:51
  • @tstanisl They look nice, but: 1) if they are too large to fit in the stack you are in UB (no return value to check); 2) you cannot return one, since they disappear with the block they are declared with; 3) they cannot grow dynamically, because they are "variable length" only at declaration time, not after that; 4) sizeof becomes a runtime operator; 5) C++ doesn't support them; 6) you are forced to pass the size parameters before the VLA. – Costantino Grana Apr 20 '21 at 19:34
  • 1) VLA can have dynamic storage `int (*a)[width] = malloc(height * sizeof *a);` 2) see answers to https://stackoverflow.com/questions/30438159/return-vla-and-usage 3) pointer to VLA works perfectly with realloc() 4) it's not a problem 5) it's C not C++ 6) it's actually good to force passing the size, GCC has extension to let pass size after VLA argument. https://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html – tstanisl Apr 20 '21 at 19:42
  • If the aim was getting a better syntax than using pointers, reverting to pointers to VLA misses the point. But I'm not arguing against it too much. I just prefer to avoid them. It's interesting that also the Linux kernel guys agree: https://linux.slashdot.org/story/18/10/29/1935253 – Costantino Grana Apr 21 '21 at 08:02