CS50 pset4 filter - Image Filters in C

Question

I started the CS50 and am at pset4. I completed both the less and more parts for the filter app that takes an image and applies a filter to it. In this pset we need to write the filters code only(greyscale, sepia, reflect, blur and edges). The edges function should apply the Sobel operator to an image so that it detects its edges.

It works by taking each pixel and modifying it based on the 3x3 grid of pixels that surrounds that pixel. Each neighbour is multiplied by the its correspondent Gx kernel value and added to the sum. The same is done for Gy. the middle pixel is also included. In this case bmp images are used and are represented by a 2d array of values for Red Green and Blue. So we need to do the process above for all three colour channels. The new pixels get the value of the square root of Gx^2 + Gy^2 to a max of 255; Pixels past the edge should be treated as solid black (0 0 0 RGB value);

Gx and Gy kernels:

Link if not very clear explanation : https://cs50.harvard.edu/x/2020/psets/4/filter/more

After 2 days I completed the task but I am sure my way is not the best way to do it, so I'm asking for some improvements that can be implement by a beginer.

Files bmp.h and filter.c were written for us and we are not allowed to modify them in any way. Code for the 2 files below.

bmp.h

// BMP-related data types based on Microsoft's own

#include <stdint.h>

/**
 * Common Data Types
 *
 * The data types in this section are essentially aliases for C/C++
 * primitive data types.
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/cc230309.aspx.
 * See http://en.wikipedia.org/wiki/Stdint.h for more on stdint.h.
 */
typedef uint8_t  BYTE;
typedef uint32_t DWORD;
typedef int32_t  LONG;
typedef uint16_t WORD;

/**
 * BITMAPFILEHEADER
 *
 * The BITMAPFILEHEADER structure contains information about the type, size,
 * and layout of a file that contains a DIB [device-independent bitmap].
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/dd183374(VS.85).aspx.
 */
typedef struct
{
    WORD   bfType;
    DWORD  bfSize;
    WORD   bfReserved1;
    WORD   bfReserved2;
    DWORD  bfOffBits;
} __attribute__((__packed__))
BITMAPFILEHEADER;

/**
 * BITMAPINFOHEADER
 *
 * The BITMAPINFOHEADER structure contains information about the
 * dimensions and color format of a DIB [device-independent bitmap].
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/dd183376(VS.85).aspx.
 */
typedef struct
{
    DWORD  biSize;
    LONG   biWidth;
    LONG   biHeight;
    WORD   biPlanes;
    WORD   biBitCount;
    DWORD  biCompression;
    DWORD  biSizeImage;
    LONG   biXPelsPerMeter;
    LONG   biYPelsPerMeter;
    DWORD  biClrUsed;
    DWORD  biClrImportant;
} __attribute__((__packed__))
BITMAPINFOHEADER;

/**
 * RGBTRIPLE
 *
 * This structure describes a color consisting of relative intensities of
 * red, green, and blue.
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/aa922590.aspx.
 */
typedef struct
{
    BYTE  rgbtBlue;
    BYTE  rgbtGreen;
    BYTE  rgbtRed;
} __attribute__((__packed__))
RGBTRIPLE;

filter.c

#include <getopt.h>
#include <stdio.h>
#include <stdlib.h>

#include "helpers.h"

int main(int argc, char *argv[])
{



    // Define allowable filters
    char *filters = "begr";

    // Get filter flag and check validity
    char filter = getopt(argc, argv, filters);
    if (filter == '?')
    {
        fprintf(stderr, "Invalid filter.\n");
        return 1;
    }

    // Ensure only one filter
    if (getopt(argc, argv, filters) != -1)
    {
        fprintf(stderr, "Only one filter allowed.\n");
        return 2;
    }

    // Ensure proper usage
    if (argc != optind + 2)
    {
        fprintf(stderr, "Usage: filter [flag] infile outfile\n");
        return 3;
    }

    // Remember filenames
    char *infile = argv[optind];
    char *outfile = argv[optind + 1];

    // Open input file
    FILE *inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", infile);
        return 4;
    }

    // Open output file
    FILE *outptr = fopen(outfile, "w");
    if (outptr == NULL)
    {
        fclose(inptr);
        fprintf(stderr, "Could not create %s.\n", outfile);
        return 5;
    }

    // Read infile's BITMAPFILEHEADER
    BITMAPFILEHEADER bf;
    fread(&bf, sizeof(BITMAPFILEHEADER), 1, inptr);

    // Read infile's BITMAPINFOHEADER
    BITMAPINFOHEADER bi;
    fread(&bi, sizeof(BITMAPINFOHEADER), 1, inptr);

    // Ensure infile is (likely) a 24-bit uncompressed BMP 4.0
    if (bf.bfType != 0x4d42 || bf.bfOffBits != 54 || bi.biSize != 40 ||
        bi.biBitCount != 24 || bi.biCompression != 0)
    {
        fclose(outptr);
        fclose(inptr);
        fprintf(stderr, "Unsupported file format.\n");
        return 6;
    }

    int height = abs(bi.biHeight);
    int width = bi.biWidth;

    // Allocate memory for image
    RGBTRIPLE(*image)[width] = calloc(height, width * sizeof(RGBTRIPLE));
    if (image == NULL)
    {
        fprintf(stderr, "Not enough memory to store image.\n");
        fclose(outptr);
        fclose(inptr);
        return 7;
    }

    // Determine padding for scanlines
    int padding = (4 - (width * sizeof(RGBTRIPLE)) % 4) % 4;

    // Iterate over infile's scanlines
    for (int i = 0; i < height; i++)
    {
        // Read row into pixel array
        fread(image[i], sizeof(RGBTRIPLE), width, inptr);

        // Skip over padding
        fseek(inptr, padding, SEEK_CUR);
    }

    // Filter image
    switch (filter)
    {
        // Blur
        case 'b':
            blur(height, width, image);
            break;

        // Edges
        case 'e':
            edges(height, width, image);
            break;

        // Grayscale
        case 'g':
            grayscale(height, width, image);
            break;

        // Reflect
        case 'r':
            reflect(height, width, image);
            break;
    }

    // Write outfile's BITMAPFILEHEADER
    fwrite(&bf, sizeof(BITMAPFILEHEADER), 1, outptr);

    // Write outfile's BITMAPINFOHEADER
    fwrite(&bi, sizeof(BITMAPINFOHEADER), 1, outptr);

    // Write new pixels to outfile
    for (int i = 0; i < height; i++)
    {
        // Write row to outfile
        fwrite(image[i], sizeof(RGBTRIPLE), width, outptr);

        // Write padding at end of row
        for (int k = 0; k < padding; k++)
        {
            fputc(0x00, outptr);
        }
    }

    // Free memory for image
    free(image);

    // Close infile
    fclose(inptr);

    // Close outfile
    fclose(outptr);

    return 0;
}

I created the functions for the filters in the file bellow

helpers.c

#include "helpers.h"
#include <math.h>


void grayscale(int height, int width, RGBTRIPLE image[height][width])
{
    //declare a variable average to store the average colour
    float average;
    //loop through image rows
    for (int i = 0; i < height; i++)
    {
        //loop through image pixels
        for (int j = 0; j < width; j++)
        {
            //calculate average value for each pixel
            average = 1.0 * (image[i][j].rgbtBlue + image[i][j].rgbtGreen + image[i][j].rgbtRed) / 3;
            //assign the average value to all values of each pixel
            image[i][j].rgbtBlue = round(average);
            image[i][j].rgbtGreen = round(average);
            image[i][j].rgbtRed = round(average);
        }
    }
}

// Reflect image horizontally
void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    //loop through image rows
    for (int i = 0; i < height; i++)
    {
        //loop through image pixels
        for (int j = 0; j < width / 2; j++)
        {
            int swapBlue = image[i][j].rgbtBlue;
            int swapGreen = image[i][j].rgbtGreen;
            int swapRed = image[i][j].rgbtRed;
            image[i][j].rgbtBlue = image[i][width - j - 1].rgbtBlue;
            image[i][j].rgbtGreen = image[i][width - j - 1].rgbtGreen;
            image[i][j].rgbtRed = image[i][width - j - 1].rgbtRed;
            image[i][width - j - 1].rgbtBlue = swapBlue;
            image[i][width - j - 1].rgbtGreen = swapGreen;
            image[i][width - j - 1].rgbtRed = swapRed;
        }
    }
}

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    //initialise a variable average for each colour, a new array to copy to and a counter
    float averageGreen;
    float averageRed;
    float averageBlue;
    RGBTRIPLE image2[height][width];
    int count;

    //loop through array
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            //set counter and averages to 0
            count = 0;
            averageBlue = 0;
            averageGreen = 0;
            averageRed = 0;
            //loop to get pixels above and below height
            for (int r = i - 1; r <= i + 1 && r < height; r++)
            {
                //if index out of bounds make it 0
                if (r < 0)
                {
                    continue;
                }

                //loop to get pixels to the left and right of width
                for (int c = j - 1; c <= j + 1 && c < width; c++)
                {
                    //if index out of bounds make it 0
                    if (c < 0)
                    {
                        continue;
                    }

                    //add values to the average and increment count
                    averageBlue += image[r][c].rgbtBlue;
                    averageGreen += image[r][c].rgbtGreen;
                    averageRed += image[r][c].rgbtRed;
                    count++;
                }
            }
            //udpdate copy array with average values divided by counter
            image2[i][j].rgbtBlue = round(averageBlue / count);
            image2[i][j].rgbtGreen = round(averageGreen / count);
            image2[i][j].rgbtRed = round(averageRed / count);
        }
    }

    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            image[i][j] = image2[i][j];
        }
    }
}


// Detect edges
void edges(int height, int width, RGBTRIPLE image[height][width])
{
    //delcare variables for Gx and Gy to calculate values for each colour
    int GxBlue;
    int GxGreen;
    int GxRed;
    int GyBlue;
    int GyGreen;
    int GyRed;

    //initialise the Gx and Gy kernels
    int Gx[3][3] =
    {
        {-1, 0, 1},
        {-2, 0, 2},
        {-1, 0, 1}
    };
    int Gy[3][3] =
    {
        {-1, -2, -1},
        {0, 0, 0},
        {1, 2, 1}
    };

    //delcare new array of bigger size to pad original array with 0's
    RGBTRIPLE image2[height + 2][width + 2];
    //delcare new array to store new values
    RGBTRIPLE image3[height][width];

    //loop big aray to add 0 padding and original array
    for (int i = 0 ; i < height + 2; i ++)
    {
        for (int j = 0; j < width + 2; j++)
        {
            //if on edges add 0
            if (i == 0 || j == 0 || i == height + 1 || j == width + 1)
            {
                image2[i][j].rgbtBlue = 0;
                image2[i][j].rgbtGreen = 0;
                image2[i][j].rgbtRed = 0;
            }
            //else add original array values
            if (i > 0 && i < height + 1  && j > 0 && j < width + 1)
            {
                image2[i][j].rgbtBlue = image[i - 1][j - 1].rgbtBlue;
                image2[i][j].rgbtGreen = image[i - 1][j - 1].rgbtGreen;
                image2[i][j].rgbtRed = image[i - 1][j - 1].rgbtRed;
            }
        }
    }

    //loop inner array
    for (int i = 1; i < height + 1; i++)
    {
        for (int j = 1; j < width + 1; j++)
        {
            //initialise variables to 0 every time we switch pixel
            GxBlue = 0;
            GxGreen = 0;
            GxRed = 0;
            GyBlue = 0;
            GyGreen = 0;
            GyRed = 0;

            //loop all neighbours of inner array
            for (int row = i - 1; row <= i + 1; row++)
            {
                for (int col = j - 1; col <= j + 1; col++)
                {
                    //add values of the neighbours multiplied to the corresponded Gx and Gy values to the sum
                    GxBlue += image2[row][col].rgbtBlue * Gx[row - i + 1][col - j + 1];
                    GxGreen += image2[row][col].rgbtGreen * Gx[row - i + 1][col - j + 1];
                    GxRed += image2[row][col].rgbtRed * Gx[row - i + 1][col - j + 1];

                    GyBlue += image2[row][col].rgbtBlue * Gy[row - i + 1][col - j + 1];
                    GyGreen += image2[row][col].rgbtGreen * Gy[row - i + 1][col - j + 1];
                    GyRed += image2[row][col].rgbtRed * Gy[row - i + 1][col - j + 1];
                }
            }
            //make sure each value does not exceed 255
            //calculte the square root of the squared sums
            if (sqrt(GxBlue * GxBlue + GyBlue * GyBlue) > 255)
            {
                image3[i - 1][j - 1].rgbtBlue = 255;
            }
            else
            {
                image3[i - 1][j - 1].rgbtBlue = round(sqrt(GxBlue * GxBlue + GyBlue * GyBlue));
            }

            if (sqrt(GxGreen * GxGreen + GyGreen * GyGreen) > 255)
            {
                image3[i - 1][j - 1].rgbtGreen = 255;
            }
            else
            {
                image3[i - 1][j - 1].rgbtGreen = round(sqrt(GxGreen * GxGreen + GyGreen * GyGreen));
            }

            if (sqrt(GxRed * GxRed + GyRed * GyRed) > 255)
            {
                image3[i - 1][j - 1].rgbtRed = 255;
            }
            else
            {
                image3[i - 1][j - 1].rgbtRed = round(sqrt(GxRed * GxRed + GyRed * GyRed));
            }
        }
    }

    //copy values in original array
    for (int i = 0; i < height; i++)
    {
        for (int j = 0; j < width; j++)
        {
            image[i][j].rgbtBlue = image3[i][j].rgbtBlue;
            image[i][j].rgbtGreen = image3[i][j].rgbtGreen;
            image[i][j].rgbtRed = image3[i][j].rgbtRed;
        }
    }

}

If you want to see the effects of the edge filter:

original picture:

picture after filter:

So I have 2 questions:

1. Is there any way of writing the edge function code cleaner and more efficient for a beginer

2. In the blur function it took me waaaaaay to long to make all the for loops work and I almost gave up. Is there a good way to find write this type of loops easier if I clearly know what to do(ie: I know I need to access the i - 1 to i + 1 element but how do I write this without going out of bounds). I could probably rewrite the function using the 0 padding as well, like for the edge one but this seems cleaner.

3.For the edge function I admit the padding with 0 on edges on a bigger array idea isn't mine and I found it on stackoverflow, but it wasn't implemented there, so I just used the idea. Is it a bad practice or something that is usually done?

Thanks you and sorry if unclear.

PS: my first post here :)

Answer 1

What a cool topic!

Here are my comments on your code:

---

Please post everything next time! You didn't post helpers.h, so I made my own helpers.h out of helpers.c and some guesses:

#include <math.h>
#include "bmp.h"

void grayscale(int height, int width, RGBTRIPLE image[height][width]);
void reflect(int height, int width, RGBTRIPLE image[height][width]);
void blur(int height, int width, RGBTRIPLE image[height][width]);
void edges(int height, int width, RGBTRIPLE image[height][width]);

Hopefully I'm not missing anything! But this compiles, and even with -Wall -Wextra -pedantic there are no warnings. Nice :)

---

I tried the Sobel filter on a few BMPs. It seems to mostly work (yay!), but I got a segmentation fault on the large ish images I tried (few MB). That's because you do this:

RGBTRIPLE image2[height + 2][width + 2];

This will take up a lot of stack space -- too much for my computer to handle. A better idea is to allocate your images on the heap. Look at filter.c for an example. You want something like:

RGBTRIPLE(*image2)[width + 2] = calloc(height + 2, sizeof(RGBTRIPLE) * (width + 2));

This makes the big BMPs work! You'll also have to free this memory since it's being allocated on the heap.

---

In the function greyscale:

float average;
....
average = 1.0 * (image[i][j].rgbtBlue + image[i][j].rgbtGreen + image[i][j].rgbtRed) / 3;

A lot of C programmers declare all of their variables at the beginning of functions. C89 required that local variables be declared at the beginning of the block they live in, and some people took that to the extreme and declared stuff as far up as they could go.

In 2020, you don't have to do this. You haven't followed C89 in a lot of other places anyway (compile with -std=c89 to see the 60 warnings and 3 errors). So you might as well do:

float average = ....

This way you avoid uninitialized variables, and you will never have to scroll up/down/up/down... while reading your C code.

As a nit, I think instead of 1.0 * (...) / 3 you could just as easily have written (...) / 3.0. Or better yet, just do integer division. You're about to round anyway. Does it matter if you are 0.33/255 off?

Either way, if you're going to round average, you might as well make it an int and then call round once instead of calling round(average) three times.

---

In the function reflect:

How about making a function void swap(BYTE* lhs, BYTE* rhs, int size);? Then you can write

for (int i  ....) {
    for (int j .....) {
        swap(image[i][j], image[i][width - j - 1], sizeof(RGBTRIPLE));
    }
}

instead of 9 bulky lines of code.

---

In the function blur:

float averageGreen;

Same comment as before about initializing at the top of functions.

RGBTRIPLE image2[height][width];

Same comment about stack vs. heap. This will fail for large images. Also, how about a better name like "blurredImage" or something to that effect. Most of your names are good, but it's a bad sign when you have to use a counter to distinguish your variables in human readable code.

for (int r = i - 1; r <= i + 1 && r < height; r++) 
{
    if (r < 0)
    {
        continue;
    }

This breaks my brain. Cognitive overload. No wonder you wrote "it took me way too long to make all the for loops work."

1. it looks like you did get it right (as far as I can tell...), so good job. Even though it's not as elegant as it could be, it's good you stuck with it.

2. As you write more code, you'll get used to writing loops/boolean logic, and this will get easier. That said there are some guidelines to make this easier.

I think my brain blew while I read this because you wrote && r < height in the for loop's condition and then put if (r < 0) in an if statement. You didn't separate concerns well enough.

Your first concern is to find i and j.

Your second concern is to find how far from i and j you need to stray.

And your final concern is to avoid going out of bounds.

How does this look?

for (int i = 0; i < height; ++i) {
    for (int j = 0; j < width; ++j) {
        for (int i_offset = -1; i_offset <= -1; ++i_offset) {
            for (int j_offset = -1; j_offset <= -1; ++j_offset) {
                int row = i + i_offset;
                int col = j + j_offset;
                if (0 <= row && row < height && ...) {
                    ...
                }
            }
        }
    }
}

I think this is relatively easy to read.

Some would argue this is less efficient since you'll run through all the col values when row is out of bounds. Don't listen to these arguments though -- the compiler can hoist these checks for you (at least Clang 10.0.0 with -O does this). As a rule, write the clear thing and only optimize manually if you have to.

I could probably rewrite the function using the 0 padding as well, like for the edge one but this seems cleaner.

I agree that in this case it's cleaner not to use 0 padding. How could you figure out count if you use 0 padding?

for (int i = 0; i < height; i++)
{
    for (int j = 0; j < width; j++)
    {
        image[i][j] = image2[i][j];
    }
}

You basically want to do image = image2. This can be done more elegantly.

First idea (very common idiom): use memcpy. It will be faster and it's easier to read.

Second idea (maybe controversial):

swap(image, image2); // swap the pointers
free(image2); // free the original image

Faster and simpler! This only works if you know you can call free on the memory passed into the function -- that's a strong assumption! In this case you can (assuming you fix your declaration of image2 to use calloc or similar), but you should at the very least add a comment to the function definition saying you plan to free the argument.

---

In the function edges:

int GxBlue;

Same comment.

//initialise the Gx and Gy kernels

Good to see Gx and Gy written out so clearly! An uglier implementation would have -1 * blah + 1 * bleh + .... Good that you didn't do that.

RGBTRIPLE image2[height + 2][width + 2];

Same comment.

//if on edges add 0

If you use calloc, then you can assume the memory you receive is zeroed out.

//else add original array values
if (i > 0 && i < height + 1  && j > 0 && j < width + 1)

How about just else instead of this comment/a written out negation of the condition? This isn't just a style tip -- you don't want to make it hard for the compiler to prove it only needs to compute the condition once.

if (sqrt(GxBlue * GxBlue + GyBlue * GyBlue) > 255)
{
    image3[i - 1][j - 1].rgbtBlue = 255;
}
else
{
    image3[i - 1][j - 1].rgbtBlue = round(sqrt(GxBlue * GxBlue + GyBlue * GyBlue));
}

How about

image3[i - 1][j - 1].rgbtBlue = edgeHelper(GxBlue);

Where edgeHelper is a function that looks like:

BYTE edgeHelper(float Gx, float Gy) {
    float magnitude = round(euclideanDistance(Gx, Gy));
    return magnitude > 255 ? 255 : magnitude; 
}

Note that I've rounded before doing the comparison rather than after. I don't think it matters much, but it's a difference all the same. You'll have to figure out euclideanDistance but I think you'll have no issue with it!

image[i][j].rgbtBlue = image3[i][j].rgbtBlue;
image[i][j].rgbtGreen = image3[i][j].rgbtGreen;
image[i][j].rgbtRed = image3[i][j].rgbtRed;

Earlier in your program you wrote image[i][j] = image2[i][j] .. that was better than this! but the same comments apply regardless. Since you [height + 2][width + 2] and relied on having 0s in memory rather than doing bound checks, you can no longer do that swap/free hack.

---

Architectural tip:

How about adding (and using) these:

// could store height/width in the image
struct Image { ... };

Image* createImage(int height, int width);

void destroyImage(Image*);

void swapImage(Image*, Image*);

// this could return 0 for out of bounds
RGBTRIPLE getPixel(Image*, int i, int j);

setPixel(Image*, int i, int j, RGBTRIPLE);

// this would help you get rid of a lot of the for loops!
forEachImageIndex(Image*, void(*callback)(Image*, int j, int j));

Properly using these would probably mean modifying filter.c, but maybe it's worth thinking about.

---

Is there any way of writing the edge function code cleaner and more efficient for a beginer

I think I already answered about cleanliness, but I'll answer here about efficiency. Your code is pretty efficient. One of the beautiful things about C is that it encourages writing simple code, and the compiler can do a really good job optimizing simple code for you. I played around with different compiler flags, and Clang can optimize/vectorize this pretty well.

---

For the edge function I admit the padding with 0 ... I found on stackoverflow, ... I just used the idea. Is it a bad practice or something that is usually done?

There are a few concerns here:

• Can you legally copy stuff from SO? I'll link you to an answer that can explain better than I can: https://meta.stackoverflow.com/questions/339152/plagiarism-and-using-copying-code-from-stack-overflow-and-submitting-it-in-an-as.

• Is it a good idea to copy ideas from SO? In my opinion, it's OK as long as you understand what you're copying. You should copy a link to what you're copying (except for the most generic ideas).

• Is it a good idea to copy code from SO? Copying code is so dangerous. I would avoid it if you can.

• "Is copying code from SO common" It is so common that people have made memes about it. But that doesn't make it a good idea...

TL;DR: I think what you did is OK, but you should cite SO.com somewhere.

---

General tips:

• Learn more of stdlib.h like memcpy and when to use malloc/free.
• Don't be afraid to make helper functions to avoid repetitive code.
• Use a more modern C standard/more modern C style guide.
• Test your code with lots of files. Seek out big ones/small ones/weird ones etc. You could have caught that segfault.
• Keep practicing!

Answer 2

    #pragma once
#include "helpers.h"
#include <math.h>
#include <stdlib.h>
#include <stdio.h>
void swap(BYTE* c1, BYTE* c2){
    BYTE temp;
    temp = *c1;
    *c1 = *c2;
    *c2 = temp;
}
int get_blue(int i,int j,int height,int width,int* matrix,int which,RGBTRIPLE image[height][width]){
    int sum = 0;
    int k = 0;
    int count = 0;
    for(int m = i; m < i+3 ; m++){
        for(int n = j; n < j+3; n++){
            if((m < 0) || (m > (height -1)) || (n < 0) || (n > (width - 1))){
            }
            else {
                sum = sum + (image[m][n].rgbtBlue)*(*(matrix+k));
                count++;
            }
            k++;
        }
    }
    if (which == 0)
        return sum/count;
    else
        return sum;
}
int get_green(int i,int j,int height,int width,int* matrix,int which,RGBTRIPLE image[height][width]){
    int sum = 0;
    int k = 0;
    int count = 0;
    for(int m = i; m < i+3 ; m++){
        for(int n = j; n < j+3; n++){
            if((m < 0) || (m > (height -1)) || (n < 0) || (n > (width - 1))){
            }
            else {
                sum = sum + (image[m][n].rgbtGreen)*(*(matrix+k));
                count++;
            }
            k++;
        }
    }
    if (which == 0)
        return sum/count;
    else
        return sum;
}
int get_red(int i,int j,int height,int width,int* matrix, int which,RGBTRIPLE image[height][width]){
    int sum = 0;
    int k = 0;
    int count = 0;
    for(int m = i; m < i+3 ; m++){
        for(int n = j; n < j+3; n++){
            if((m < 0) || (m > (height -1)) || (n < 0) || (n > (width - 1))){
            }
            else {
                sum = sum + (image[m][n].rgbtRed)*(*(matrix+k));
                count++;
            }

            k++;
        }
    }
    if (which == 0)
        return sum/count;
    else
        return sum;

}
// Convert image to grayscale
void grayscale(int height, int width, RGBTRIPLE image[height][width])
{
    //need to take average 
    //take each pixel to that value
    int average;
    for(int i = 0 ; i < height ; i++){
        for(int j =0 ; j < width ; j++){
            average = (image[i][j].rgbtBlue + image[i][j].rgbtGreen + image[i][j].rgbtRed)/3;
            image[i][j].rgbtBlue = average;
            image[i][j].rgbtGreen = average;
            image[i][j].rgbtRed = average;
        }
    }
    return;
}

// Reflect image horizontally
void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    int middle = width/2;
    for(int i =0 ; i < height ; i++){
        //swap image[0][0] with image[0][width - 1]
        for(int j = 0; j < middle; j++){
            swap(&image[i][j].rgbtBlue, &image[i][width-j-1].rgbtBlue);
            swap(&image[i][j].rgbtGreen, &image[i][width-j-1].rgbtGreen);
            swap(&image[i][j].rgbtRed, &image[i][width-j-1].rgbtRed);
        }
    }
    return;

}

// Blur image
void blur(int height, int width, RGBTRIPLE image[height][width])
{
    RGBTRIPLE(*newimage)[width]=calloc(height, width*sizeof(RGBTRIPLE));
    int matrix[] = {1,1,1,1,1,1,1,1,1};
    for(int i = 0; i < height ; i++){
        for(int j = 0; j < width ; j++){
            newimage[i][j].rgbtBlue = round(get_blue(i-1,j-1,height,width,matrix,0,image));
            newimage[i][j].rgbtGreen= round(get_green(i-1,j-1,height,width,matrix,0,image));
            newimage[i][j].rgbtRed = round(get_red(i-1,j-1,height,width,matrix,0,image));
        }
    }
    //copy
    for(int i = 0; i < height ; i++){
        for(int j = 0; j < width ; j++){
            image[i][j].rgbtBlue = newimage[i][j].rgbtBlue;
            image[i][j].rgbtGreen= newimage[i][j].rgbtGreen;
            image[i][j].rgbtRed= newimage[i][j].rgbtRed;

        }
    }
    free(newimage);
    return;
}

// Detect edges
void edges(int height, int width, RGBTRIPLE image[height][width])
{
    blur(height,width,image);
    int bluegx,bluegy,greengx,greengy,redgx,redgy;
    int bluegxy,greenxy,redxy;
    int gx[] = {1,0,-1,2,0,-2,1,0,-1};
    int gy[] = {1,2,1,0,0,0,-1,-2,-1};
    RGBTRIPLE(*newimage)[width]=calloc(height, width*sizeof(RGBTRIPLE));
    for(int i = 0; i < height ; i++){
        for(int j = 0; j < width ; j++){
            bluegx= get_blue(i-1,j-1,height,width,gx,1,image);
            greengx= get_green(i-1,j-1,height,width,gx,1,image);
            redgx= get_red(i-1,j-1,height,width,gx,1,image);

            bluegy= get_blue(i-1,j-1,height,width,gy,1,image);
            greengy= get_green(i-1,j-1,height,width,gy,1,image);
            redgy= get_red(i-1,j-1,height,width,gy,1,image);

            bluegxy = (round(sqrt(bluegx*bluegx + bluegy*bluegy)));
            greenxy= (round(sqrt(greengx*greengx+ greengy*greengy)));
            redxy = (round(sqrt(redgx*redgx+ redgy*redgy)));

            newimage[i][j].rgbtBlue = (bluegxy> 255)?255:bluegxy;
            newimage[i][j].rgbtGreen= (greenxy > 255)?255:greenxy;
            newimage[i][j].rgbtRed = (redxy> 255)?255:redxy;
        }
    }
    for(int i = 0; i < height ; i++){
        for(int j = 0; j < width ; j++){
            image[i][j].rgbtBlue = newimage[i][j].rgbtBlue;
            image[i][j].rgbtGreen= newimage[i][j].rgbtGreen;
            image[i][j].rgbtRed= newimage[i][j].rgbtRed;
        }
    }
    free(newimage);
    return;
}

This is the code one can implement as it contains fewer loops, and is well implemented. The result from the above code is as follows(for Sobel operator):