C - rgb values - Calculating the average of rgb values for a blur filter

Question

The first two ones were not so difficult but the third one makes me mad. The blur filter has to calculate an average of the rgb values of certain pixel groups in order to replace the values of the centered pixel. Imagine a 3x3 grid where the pixel in the center has to be manipulated with the rgb values of the average of the eight surrounding pixels and the center-pixel itself.

What I have done so far is the following:

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    int n;
    int m;
    int averageRed;
    int averageBlue;
    int averageGreen;

    //For each row..
    for (int i = 0; i < height; i++)
    {
        //..and then for each pixel in that row...
        for (int j = 0; j < width; j++)
        {

            //...if i and j equal 0...         
            if (i == 0 && j == 0)
            {
                for (m = i; m <= 1; m++)
                {
                    for (n = j; n <= 1; n++)
                    {
                        averageRed = averageRed + image[m][n].rgbtRed;
                        averageBlue = averageBlue + image[m][n].rgbtBlue;
                        averageGreen = averageGreen + image[m][n].rgbtGreen;

                        printf("%i\n", averageRed);
                        printf("%i\n", averageBlue);
                        printf("%i\n", averageGreen); 
                    }
                }

                image[i][j].rgbtRed = round((float)averageRed / 4);
                image[i][j].rgbtBlue = round((float)averageBlue / 4);
                image[i][j].rgbtGreen = round((float)averageGreen / 4);

                printf("%i\n", image[i][j].rgbtRed);
                printf("%i\n", image[i][j].rgbtBlue);
                printf("%i\n", image[i][j].rgbtGreen);
            }


            //If i equals 0 and j is greater than 0...
            else if (i == 0 && j > 0)
            {
                //..take the line that equals i..
                for (m = i; m <= 1; m++)
                {
                    //..and take from each pixel ot that line...
                    for (n = j - 1; n <= 1; n++)
                    {
                        //..the color values and add them to the average-variables
                        averageRed = averageRed + image[m][n].rgbtRed;
                        averageBlue = averageBlue + image[m][n].rgbtBlue;
                        averageGreen = averageGreen + image[m][n].rgbtGreen;
                    }
                }

                //Set the current pixel values to the averages
                image[i][j].rgbtRed = round((float)averageRed / 6);
                image[i][j].rgbtBlue = round((float)averageBlue / 6);
                image[i][j].rgbtGreen = round((float)averageGreen / 6);

                printf("%i\n", image[i][j].rgbtRed);
                printf("%i\n", image[i][j].rgbtBlue);
                printf("%i\n", image[i][j].rgbtGreen);
            }


            else if (i > 0 && j == 0)
            {
                for (m = i - 1; m <= 1; m++)
                {
                    for (n = j; n <= 1; n++)
                    {
                        averageRed = averageRed + image[m][n].rgbtRed;
                        averageBlue = averageBlue + image[m][n].rgbtBlue;
                        averageGreen = averageGreen + image[m][n].rgbtGreen;
                    }
                }

                image[i][j].rgbtRed = round((float)averageRed / 6);
                image[i][j].rgbtBlue = round((float)averageBlue / 6);
                image[i][j].rgbtGreen = round((float)averageGreen / 6);
            }


            else if (i > 0 && j > 0 )
            {

                // ..take every line from i - 1 to i + 1...
                for (m = i - 1; m <= 1; m++)
                {

                    //...and in each line take every pixel from j - 1 to j + 1...
                    for (n = j - 1; n <= 1; n++)
                    {

                        //...and add the RGB value to average-variables
                        averageRed = averageRed + image[m][n].rgbtRed;
                        averageBlue = averageBlue + image[m][n].rgbtBlue;
                        averageGreen = averageGreen + image[m][n].rgbtGreen;
                    }
                }

                //Set current value to the rounded average
                image[i][j].rgbtRed = ((float)averageRed / 9);
                image[i][j].rgbtBlue = ((float)averageBlue / 9);
                image[i][j].rgbtGreen = ((float)averageGreen / 9);
            }  


        }

    }
    return;

}

The compiling works without any complaint, but the results are slightly strange (especially the first four blocks) - the Test.bmp ist just a 55px x 55px black/white bmp-file:

> ~/pset4/filter/ $ ./filter -b images/test.bmp blur.bmp0 38118032 0 0
> 38118032 0 0 38118032 0 0 38118032 0 helpers.c:93:40: runtime error:
> 9.52951e+06 is outside the range of representable values of type 'unsigned char' 0 164 0 helpers.c:120:40: runtime error: 6.35303e+06
> is outside the range of representable values of type 'unsigned char' 0
> 137 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0
> 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160
> 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0
> 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0
> 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160
> 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0 160 0 0
> 160 0 0 160 0 helpers.c:142:40: runtime error: 6.35311e+06 is outside
> the range of representable values of type 'unsigned char'
> helpers.c:167:40: runtime error: 4.23546e+06 is outside the range of
> representable values of type 'unsigned char' ~/pset4/filter/ $

Thanks very much in advance for any advise!

Greetz

Answer 1

Note that average* variables are uninitialized, so when you sum to them, you have UB. They need to be preset to 0, certainly at the outset, but possibly before each major loop.

---

Also, aside from your other issues that others have noted, you may need to do saturation math.

That's because for rgbt* (e.g. rgbtRed) is a byte, so the value can be clipped incorrectly.

You are doing:

image[i][j].rgbtRed = round((float)averageRed / 6);

This can be rewritten as:

averageRed = round((float)averageRed / 6);
image[i][j].rgbtRed = averageRed;

But, if (e.g.) averageRed was 256, then rgbtRed would end up as 1 [because the assignment to image is [effectively]:

image[i][j].rgbtRed = averageRed & 0xFF;

So, instead of storing bright red, you're storing nearly black. The final would need to be 255, the "saturated" maximum color value.

So, to fix that [or merely to guard against it], do:

averageRed = round((float)averageRed / 6);
if (averageRed > 255)
    averageRed = 255;
image[i][j].rgbtRed = averageRed;

Edit: Upon further reflection, you only need to do this if the right hand side can exceed 255, but I'm [now] not sure that it can. To check against this, you could add (e.g.):

if (averageRed > 255) {
    fprintf(stderr,"value overflow\n");
    exit(1);
}

You could wrap this in an #ifdef, do tests, and if it doesn't trigger, you can remove it later.

---

UPDATE:

As stupid as the question might sound, but how can the value ever reach 256? Even if every pixel is white, none of the values can reach 256 or where is my mistake? (1 white Px: 255 255 255 -> 10 white Px: 2550 2550 2550 / 10 --> .....

Yes, per my "Edit:" above, it may not. I recently answered a similar question where the value could exceed 255.

But, your runtime error shows that the value does exceed the capacity of a byte (i.e. unsigned char).

That is probably due to the uninitialized sum variables.

But, also it is because the sum/average variables are not being reset at the start of a loop. You never reset them, so they just continue to grow and grow.

They need to be reset after you complete each 3x3 convolution kernel (i.e. after you store each output pixel).

And, I don't think your for (n = j; n <= 1; n++) loops are correct. You're mixing up absolute coordinate values (from j) and coordinate offsets.

You probably want something like:

for (m = -1; m <= 1; m++) {
    for (n = -1; n <= 1; n++) {
        averageRed += image[i + m][j + n].rgbtRed;
    }
}

---

UPDATE #2:

It may be easier to have a single set of loops, using some extra limit variables.

Also, on a per pixel basis, using floating point (i.e. round) can be slow. Although, I didn't do it, it can be replaced with integer math easily enough.

Further, using more descriptive names instead of i, j, m, n can help make the code a bit easier to understand and maintain.

Anyway, here's a somewhat refactored version of your function that is a bit simpler:

#include <math.h>

#if 1
typedef struct {
    unsigned char rgbtRed;
    unsigned char rgbtGreen;
    unsigned char rgbtBlue;
} __attribute__((__packed__)) RGBTRIPLE;
#endif

// Blur image
void
blur(int height, int width,
    RGBTRIPLE image[height][width],
    RGBTRIPLE imgout[height][width])
{
    int wid = width - 1;
    int hgt = height - 1;
    RGBTRIPLE *pixel;

    // For each row..
    for (int ycur = 0;  ycur <= hgt;  ++ycur) {
        int ylo = (ycur == 0) ? 0 : -1;
        int yhi = (ycur == hgt) ? 0 : 1;

        // ..and then for each pixel in that row...
        for (int xcur = 0;  xcur <= wid;  ++xcur) {
            int xlo = (xcur == 0) ? 0 : -1;
            int xhi = (xcur == wid) ? 0 : 1;

            int avgRed = 0;
            int avgGreen = 0;
            int avgBlue = 0;

            for (int yoff = ylo;  yoff <= yhi;  ++yoff) {
                for (int xoff = xlo;  xoff <= xhi;  ++xoff) {
                    pixel = &image[ycur + yoff][xcur + xoff];
                    avgRed += pixel->rgbtRed;
                    avgGreen += pixel->rgbtGreen;
                    avgBlue += pixel->rgbtBlue;
                }
            }

            int tot = ((yhi - ylo) + 1) * ((xhi - xlo) + 1);

            pixel = &imgout[ycur][xcur];
            pixel->rgbtRed = roundf((float) avgRed / tot);
            pixel->rgbtGreen = roundf((float) avgGreen / tot);
            pixel->rgbtBlue = roundf((float) avgBlue / tot);
        }
    }
}

Answer 2

For correctness you need to preserve the original values.

For speed, you only need to preserve the original values until they're no longer needed; and the horizontal sums can be recycled to minimize additions.

More specifically, ignoring top/bottom/left/right edges (which need extra care) and pretending it's monochrome (for RGB you just do it all 3 times), for each row of pixels:

• for each pixel in the row, do buffer[next_buffer_row][x] = image[y+2][x-1] + image[y+2][x] + image[y+2][x+1] to store the horizontal sums in a buffer.

• for each pixel in the row calculate the blurred values, like image[y][x] = (buffer[previous_buffer_row][x] + buffer[current_buffer_row][x] + buffer[next_buffer_row][x]) / 9

• advance to the next line in the image (y++); and rotate the buffer (previous_buffer_row++; if(previous_buffer_row>= 3) previous_buffer_row = 0; and current_buffer_row++; if(current_buffer_row>= 3) current_buffer_row = 0; and next_buffer_row++; if(next_buffer_row>= 3) next_buffer_row = 0;)

To handle left/right edges, you want to "peel off" the first iteration of the "for each pixel in row" loops, and the last iteration of the "for each pixel in row" loops; then modify them to suit. E.g. for the first pixel you want to do buffer[next_buffer_row][x] = image[y+2][x] + image[y+2][x+1] (because the pixel at image[y+2][x-1] doesn't exist) and image[y][x] = (buffer[previous_buffer_row][x] + buffer[current_buffer_row][x] + buffer[next_buffer_row][x]) / 6 (because there were only 6 pixels being averaged because 3 were past the left edge of the image).

Note: When I say "peel off", I mean that instead of doing (e.g.) for(i = 0; i < something; i++) { you copy and past the middle of the loop so its duplicated before and after the loop and do for(i = 1; i < something-1; i++) {.

To handle top/bottom edges, you want to "peel off" the first iteration of the "for each row" loop, and last iteration of the "for each row" loop; then modify them to suit. E.g. for the very first row of pixels you want generate the horizontal sums for 2 lines (not one) and then do image[y][x] = (buffer[current_buffer_row][x] + buffer[next_buffer_row][x]) / 6 because one row (3 pixels) doesn't exist (because it's past the top edge). Note that this will actually end up giving you 9 cases ("left/middle/right for horizontals * top/middle/bottom for verticals").

For averaging, with integer divisions the result will be slightly darker (due to rounding/truncation) than it should be. To avoid that (if you care), use result = (max * (sums + max/2)) / (9 * max) (e.g. if the maximum value is 255, then result = 255 * (sums + 127) / 2295. However, this adds overhead and complexity, and most people won't notice that the image is slightly darker, so whether this is good or bad depends on your use case.

For better quality blurring, you can use weights so that pixels further from the center pixel have less effect on the final value of a pixel. The problem here is that burring should be done with a circle but you're using a square; which will make diagonal edges look "more blurred" than horizontal/vertical edges. Usually the selected weights are described as a matrix. For an example:

| 1 2 1 |
| 2 4 2 |
| 1 2 1 |

... would mean that the center pixel's weight is 4 (so you multiply the values for the middle pixel by 4), the weight for the pixel above it is 2, etc. In this case you would divide by the sum of the weights, which happens to be 16 (and means that the division can be done with a faster "shift right").

The approach I've described (of having a buffer of "horizontal sums" for 3 lines only) can be applied to some weights easily (e.g. the weights I showed above) because the middle row of weights are a multiple of the top/bottom weights (2 4 2 is 2 times 1 2 1). If this isn't the case then the approach I've described needs an extra separate buffer for the middle row (which can be 2 pixels and not a whole row of pixels); and you won't be able to re-use the "horizontal sum (of weighted values)" for the middle row.

Finally; for extremely accurate results you need to realize that RGB values are usually gamma encoded (see https://en.wikipedia.org/wiki/Gamma_correction ). This means doing "gamma decode", then blur, then "gamma re-encode". However, gamma encoding/decoding is expensive (even when you use lookup tables to avoid pow()); and if you care about this level of perfection then it's best to design the entire pipeline (including storage and/or generation of the image/s that will be blurred) for raw values (without gamma encoding) and then do gamma encoding once at the end.