Displaying histogram pixel values from PGM image (p5) using Pure C without any image processing library

This question is challenging to get more understanding on Image Processing using pure C. I have done a simple program reading non-binary PGM file using C compiled with GCC. Now, it is becoming a problem when I try to read binary PGM file. This binary PGM file can be acquired by converting JPG to PGM using IrvanView.

NOTE: Please don't answer with any image processing library (such as: OpenCV).

My current code is:

#include    <stdio.h> 
#include    <stdlib.h>
#define WIDTH 1024  
#define HEIGHT 768
#define READ_IMAGE_NAME "MY_PGM_FILE_NAME.pgm"

void print_histogram_table(int *histog);

main() {
  FILE *fp;

  int i,j, height= HEIGHT, width=WIDTH;
  char line[100];

  // Color depth is 255. 
  unsigned char pixel_value;

  fp = fopen(READ_IMAGE_NAME,"r");

  // get the first four lines.
  fgets (line,100,fp); 
  fgets (line,100,fp);
  fgets (line,100,fp);
  fgets (line,100,fp);

  // Histogram helper
  int histo[65536];
  int x;
  for ( x =0; x < 65536; x++) {    
        histo[x] = 0;
  }

  for(j=0;j<height;j++) {
     for(i=0;i<width;i++) {
         fread(&pixel_value, sizeof(unsigned char), 1, fp);
         // Turn on the code below, if you want to check color on specific row and column.
//             printf("row num. %d column num. %d    pixel value=%d\n",j,i,pixel_value);  

       histo[pixel_value]++;
     }
  }

  // Make histogram
  print_histogram_table(histo);

  fclose(fp);       
  getch();
}

void print_histogram_table(int *histog)
{
  int x; 
  for (x= 0; x < 255; x++) {
     if ( histog[x] != 0)
        printf("Color number %d count %d\n", x, histog[x]); 
  }
}

I have read some pages related to my problem [开发者_C百科How to read .PGM format file?] , but I cannot find any clear and simple answer. I apologize for any mistake in my code. Any suggestion and critic regarding my code would be appreciated.

My script above could not display correct color histogram, because if you think rationally, you might get pixel color above 100 (not only below 100). So, the main question is How to fix this problem?

EDIT I

• Add PGM image. PGM Image Sample
• Add function to create image histogram.

EDIT II

• Add one more fgets, see sample image.

My script above could not display correct color histogram, because if you think rationally, you might get pixel color above 100 (not only below 100). So, the main question is How to fix this problem?

I'm assuming you mean:

Right now, running this program on the linked image only displays non-zero histogram entries for pixel values below 100. I know of at least one pixel value in the image above 100, so there's a bug in my code. Can anyone help me figure out why this happens?

I suggest you re-phrase your question to make this clear.

The code superficially looks OK, assuming you only want it to work on the image you linked to. However, there a lots of small mistakes that may add up. Here are a few suggestions:

Histogram size

First, you can't print your entire histogram. Consider passing the histogram size to the function that prints the histogram. Moreover, even if the sizes did match (both 256), you would still have an error. You never print the 256th value.

int histo[65536];
// ...
print_histogram_table(histo);
// ...
void print_histogram_table(int *histog)
{
    int x; 
    for (x= 0; x < 255; x++) {
       if ( histog[x] != 0)
          printf("Color number %d count %d\n", x, histog[x]); 
}

Binary I/O

You need to specify "binary" I/O when opening binary files. On UNIX, this traditionally makes no difference because it is the default mode. However, on Windows (I'm assuming your running Windows since you're using Irfan View), you need to explicitly state that you want binary I/O.

This is a common mistake when dealing with binary files for the first time. Basically, as soon as the fread() call catches an EOF byte, it will stop reading the file and you will get garbage values for all subsequent reads (probably copies of the last real byte), which means you're not actually reading your entire image.

fp = fopen(READ_IMAGE_NAME,"r");
// change to:
fp = fopen(READ_IMAGE_NAME,"rb");

Other minor concerns

There are loads of things not handled by your code:

1. PNM files may have comments at the beginning of the file
2. Lines may have more than 100 characters.
3. Image size is not forcibly 1024x768
4. It's not much use to hardcode the name of the file in the program, even for just testing your code.
5. If you actually get a 16-bit-per-pixel grayscale image, your histogram is large enough, but you should be reading 2-byte values.

And... what's the question? Your code looks almost fine for reading binary PGM (P5?), it's just missing newline check for each pixel data row (according to wikipedia article, both ASCII and binary format have newlines at the end of each pixel data row).

Some comments:

• Don't hardcode the width and height, you should use information from the header instead
• Check the magic number, throw/return error if it's not P5 (or handle it correctly)
• The file could have line comments, consider that

A few notes about your program:

• you should read the header instead of hardcoding the width, height and max pixel value.
• it's way more efficient to load the whole image into memory and do the processing from memory
• PGM files are binary, you should use "rb" in the fopen (in case you ever run this on Windows, where binary vs. text are different)
• your histogram print function is missing the last pixel (255 is a valid pixel value)
• not sure why your histogram array goes to 64K, are you planning to support 16-bit PGMs as well?
• you should have error handling to avoid crashes or other unexpected behavior

Below is an improved version of your program with the above points addressed. I hope it helps.

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

int main(int argc, char* argv[])
{
    char line[100];
    int width, height, maxval;
    unsigned char* image;
    unsigned int* histo;
    int i;
    FILE *fd;

    fd = fopen(argv[1], "rb");
    if (fd == NULL) {
        printf("could not open image file\n");
        exit(1);
    }

    // magic number
    fgets(line, sizeof(line), fd);
    if (strcmp(line, "P5\n") != 0) {
        printf("image is not in PGM format!\n");
        fclose(fd);
        exit(1);
    }

    // skip comment
    fgets(line, sizeof(line), fd);

    // read header
    if (fscanf(fd, "%d %d %d\n", &width, &height, &maxval) != 3) {
        printf("invalid header\n");
        fclose(fd);
        exit(1);
    }
    if (maxval > 255) {
        printf("sorry, only 8-bit PGMs are supported at this time!\n");
        fclose(fd);
        exit(1);
    }

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

    // read image data
    image = malloc(width * height);
    if (fread(image, sizeof(unsigned char), width * height, fd) != width * height) {
        printf("could not read image\n");
        fclose(fd);
        exit(1);
    }
    fclose(fd);

    // compute histogram
    histo = (int*)calloc(maxval+1, sizeof(int));
    for (i = 0; i < width * height; i++)
        histo[image[i]]++;

    // print histogram
    for (i = 0; i <= maxval; i++)
        printf("Color number %d count %d\n", i, histo[i]);

    // release memory
    free(image);
    free(histo);
}