Thread: Blurring a ppm image.

I'm trying to blur a ppm image by averaging the components of the color(r, g, b) within a certain reach of a specific pixel. This is a picture and description:
In this diagram, we are trying to compute the color for the pixel in the center (the red element). Its neighbors (within a reach of 4) are all of the green elements. The pixels outside of this 9x9 square are not considered in the blurring calculation for this pixel. To compute the color for the center pixel, average the red, green, and blue components (independently) of every pixel in the 9x9 square (including the pixel itself; the red element in this diagram).

I have some code, but it is not working correctly. Can someone tell me what I'm doing incorrectly?

the function "computed_color" on line 52 is shown in the next block of code.

Code:

 
#include <stdlib.h>
#include <stdio.h>
#include "blur_calculations.h"
#define NOT_ENOUGH_ARGS 2
#define MIN_ARGS 3
#define PICTURE_FILE 1
#define REACH_NUMBER 4
#define OUTPUT_FILE "blurred.ppm"
#define PPM_NUMBER "P3"
#define MAX_ROWS 500
#define MAX_COLS 500

void solve(FILE *in_picture, FILE *out_picture, char *argv[])
{
    int r, g, b;
    int num_col, num_row, max_color;
    struct color pixel_data[MAX_ROWS][MAX_COLS];
    struct color blur_color;
    char ppm_num[3];

    fscanf(in_picture, "%s %d %d %d", ppm_num, &num_col, &num_row, &max_color);

    if(num_row > 500 || num_col > 500)
    {
        fprintf(stderr, "%s: dimensions are too large \n", argv[0]);
    exit(1);
    }

    fprintf(out_picture, "%s %d %d %d\n", PPM_NUMBER, num_col, num_row, 
                                  max_color);

    int ret = fscanf(in_picture, "%d %d %d", &r, &g, &b);

    while(ret == 3)
    {
        for(int row=0; row <num_row; row++) 
        {
            for(int col=0; col < num_col; col++)
            {
                pixel_data[row][col] = create_color(r, g, b);

                ret = fscanf(in_picture, "%d %d %d ", &r, &g, &b);
            }
        }
        ret = fscanf(in_picture, "%d %d %d ", &r, &g, &b);
    }

    for(int row=0; row < num_row; row++) 
    {
        for(int col=0; col < num_col; col++)
        {
            blur_color = computed_color(pixel_data, argv[2][0], row, col);

            fprintf(out_picture, "%d %d %d ", blur_color.r, blur_color.g, 
                                              blur_color.b);
        }
    }
}

FILE *open_file(const char name[], const char mode[])
{
    FILE *file = fopen(name, mode);
    if(file == NULL)
    {
        perror(name);
    exit(1);
    }
    return file;
}

void check_args(int argc, char *argv[])
{
    if(argc < NOT_ENOUGH_ARGS)
    {
        fprintf(stderr, "%s: not enough arguments \n", argv[0]);
    exit(1);
    }
    else if(argc < MIN_ARGS)
    {
        argv[2][0] = REACH_NUMBER;
    }

}

int main(int argc, char *argv[])
{
    check_args(argc, argv);
    FILE *in_picture = open_file(argv[PICTURE_FILE], "r");
    FILE *out_picture = open_file(OUTPUT_FILE, "w");

    solve(in_picture, out_picture, argv);
    fclose(in_picture);
    fclose(out_picture);

}

here is my code to calculate the average, which I use in my "solve" function:

Code:

 
#include "blur_calculations.h"

struct color create_color(int r, int g, int b)
{
    struct color colors;
    colors.r = r;
    colors.g = g;
    colors.b = b;
    return colors;
}

struct color computed_color(struct color input[][MAX_COLS], int reach, 
                            int rows, int cols)
{
    struct color pixel_col;

    int top_rows = rows - reach;
    int left_cols = cols - reach;

    if(top_rows < 0)
    {
        top_rows = 0;
    }
    if(left_cols < 0)
    {
        left_cols = 0;
    }

    int bot_rows = rows + reach;
    int right_cols = cols + reach;
    
    if(bot_rows > 500)
    {
        bot_rows = 500;
    }
    if(right_cols > 500)
    {
        right_cols = 500;
    }


    for(int row = top_rows; row <= bot_rows; row++) 
    {
        for(int col = left_cols; col <= right_cols; col++)
        {
            
            pixel_col.r += input[row][col].r;
            pixel_col.g += input[row][col].g;
            pixel_col.b += input[row][col].b;
        }
    }

    pixel_col.r = pixel_col.r/((bot_rows - top_rows + 1)*
                               (right_cols - left_cols + 1));
    pixel_col.g = pixel_col.g/((bot_rows - top_rows + 1)*
                               (right_cols - left_cols + 1));
    pixel_col.b = pixel_col.b/((bot_rows - top_rows + 1)*
                               (right_cols - left_cols + 1));

    return pixel_col;
}

!&@#*!Q@$#&!@#$*O&@#%$*&@#%*&()@#%$

I had a really nice, long reply with lots of good info, and I lost it all because some idiot (me) bumped a power cable. So here's the short version

• Your blur function looks generally right -- i.e. you correctly average the pixel values, with the exception of the clamp issue noted below.
• You defined constants, use them instead of magic numbers.
• A few constants could use better names. REACH_NUMBER should be DEFAULT_REACH or similar. NOT_ENOUGH_ARGS is wrong. 2 args is enough. That should be called MIN_ARGS. The other one (currently MIN_ARGS) should be renamed to MAX_ARGS or something.
• Don't clamp at 500. What if your image is 100x100?
• If argc is 2, then args you can access/modify are argv[0] and argv[1]. argv[argc] (which is argv[2] in this case) is null. Accessing argv[2][0] here tries to access memory at address null, which is undefined behavior and may crash your program. You should, instead, pass something to check_args that you can populate with either DEFAULT_REACH or what the user entered.
• Command line args are always treated as strings, even if they're numbers. Thus, if I call your program with reach of 3, your program gets arg[2] = "3". That is the character '3', which is not the same as the number 3. You need to use atoi (or better, strtol) to convert it to an int.
• You never check the value of ret in your inner loop (fscanf line 42). You call fscanf again, and overwrite the value of ret on line 45, before checking it in the while condition on line 34.
• I would define a struct that holds program options (input_filename and reach), and one for ppm_header_info (ppm_version, width, height, max_color). Pass them as needed to functions.

Originally Posted by winsticknova

thanks! I changed the NOT_ENOUGH_ARGS to 1. In regards to the clamping at 500, my professor has stated (but I'm not really sure what you mean by clamping. I am not well versed in programming terminology):

Generally clamping refers to ensuring a value doesn't exceed some limits. In your previous program, you multiplied the red value by 10, but ensured that if it did not exceed the max color value by saying

Code:

if (red_value > MAX_COLOR)
    red_value = MAX_COLOR;

I think I've sometimes heard that called "trimming a value" as well. Clamping is maybe not the best term to use here, but is the most accurate one that came to mind at the time of writing. It accurately describes your clamping bot_rows and right_cols to (somewhat) valid values.

What I meant, however, was:
You have absolute maximum dimensions of 500x500 for your image. Anything bigger you simply don't process at all, your program just quits. That is okay, and what the professor says you should do. You satisfy that requirement with lines 23-27 of the first code block.

You don't, however, properly handle smaller images. What if your image is only 100x100? As-is, your code would try to blend in row/col 100, 101, 102, etc, which is bad. Those array spots are within bounds, but don't have valid data, thus you would blend garbage values around the edges of your image. You need to use rows and cols as your "clamp" or maximum values, not just 500.

oh! I understand now. makes so much more sense. thank you so much!