Convert RGB to Grayscale in bare C

Question

I am applying a basic tutorial on image processing in C and I am having problems with this program to convert RGB into grayscale but the output pic is somehow corrupted and although the code runs with no errors, and I cant put my hands on the problem. The code is below.

FILE *fIn = fopen("tiger.bmp","rb");                    //Input File name
FILE *fOut = fopen("tiger_gray.bmp","wb");              //Output File name

int i,j,y;
unsigned char byte[54];

if(fIn==NULL)   
{
    printf("File does not exist.\n");
}

for(i=0;i<54;i++)               //read the 54 byte header from fIn
{
    byte[i] = getc(fIn);
}

fwrite(byte,sizeof(unsigned char),54,fOut);         //write the header back
// extract image height, width and bit Depth from image Header
int height = *(int*)&byte[18];
int width = *(int*)&byte[22];
int bitDepth = *(int*)&byte[28];

printf("width: %d\n",width);
printf("height: %d\n",height );

int size = height*width;                        

unsigned char buffer[size][3];              //to store the image data


for(i=0;i<size;i++)                                         //RGB to gray
{
    y=0;
    buffer[i][2]=getc(fIn);                                 //blue
    buffer[i][1]=getc(fIn);                                 //green
    buffer[i][0]=getc(fIn);                                 //red

    y=(buffer[i][0]*0.3) + (buffer[i][1]*0.59)  + (buffer[i][2]*0.11);          //conversion formula of rgb to gray

    putc(y,fOut);
    putc(y,fOut);
    putc(y,fOut);
}

fclose(fOut);
fclose(fIn);

Answer 1

There were two major problems in your code.

1. You had the width and height reversed.
2. You were not accounting for the required padding at the end of every row to make it a multiple of 4 bytes.

You were also allocating a large buffer that you did not need based on how the rest of the code was written. Generally I'd prefer to read/process either one full row at a time or even the full image at once, but to do that you want to use malloc or calloc because the data may be larger than the available stack. In this case, to keep things simple, I just process one pixel at a time.

I also got rid of getc/putc because I prefer fread/fwrite and you're never really dealing with 1 byte at a time.

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

int main()
{
    FILE *fIn = fopen("tiger.bmp", "rb");
    FILE *fOut = fopen("tiger_gray.bmp", "wb");
    if (!fIn || !fOut)
    {
        printf("File error.\n");
        return 0;
    }

    unsigned char header[54];
    fread(header, sizeof(unsigned char), 54, fIn);
    fwrite(header, sizeof(unsigned char), 54, fOut);

    int width = *(int*)&header[18];
    int height = abs(*(int*)&header[22]);
    int stride = (width * 3 + 3) & ~3;
    int padding = stride - width * 3;

    printf("width: %d (%d)\n", width, width * 3);
    printf("height: %d\n", height);
    printf("stride: %d\n", stride);
    printf("padding: %d\n", padding);

    unsigned char pixel[3];
    for (int y = 0; y < height; ++y)
    {
        for (int x = 0; x < width; ++x)
        {
            fread(pixel, 3, 1, fIn);
            unsigned char gray = pixel[0] * 0.3 + pixel[1] * 0.58 + pixel[2] * 0.11;
            memset(pixel, gray, sizeof(pixel));
            fwrite(&pixel, 3, 1, fOut);
        }
        fread(pixel, padding, 1, fIn);
        fwrite(pixel, padding, 1, fOut);
    }
    fclose(fOut);
    fclose(fIn);
    return 0;
}