Reading PPM format images

I’m trying to read this PPM format image, which contains an ASCII header followed by binary pixel data. The header consists of something like this:

P6
650 652
255

P6 indicates that it is a PPM image. The next two fields are the width and height of the image. The last field gives the maximum pixel value. At the end of the header is a \n and then the binary pixel data. The image is in color so there are three bytes (red, green, blue). The goal of my readPPM function is to return the pixel data in a one-dimensional array of unsigned chars, plus the width and height of the image. The goal of my writePPM function (I haven't done anything for that function yet) is to write the PPM format image to an empty file from the given information returned from my readPPM function. I'm still not sure how to make this program work so I don't store the 650 in width and 652 in height. I'll worry about that once I can actually read and write the text files.


main.cc

Code:

---

  int main() {

        //char fileName[50] = "binary_pixels.txt";
        char fileName[50] = "test.ppm";
        char pSix[10];                // indicates this is a PPM image
        int width = 0;                // width of the image
        int height = 0;                // heigt of the image
        int maximum = 0;        // maximum pixel value
        int size = 128;                // size of the array

        // read the PPM file and store its contents inside an array and return the pointer to that array to pixelArray
        unsigned char* pixelArray = readPPM(fileName, pSix, &width, &height, &maximum);

---

readWritePPM.cc

Code:

---

    unsigned char* readPPM(const char* fileName, char* pSix, int* width, int* height, int* maximum) {

        // open the file to read just the header reading
        FILE* fr = fopen(fileName, "r");

        // formatted read of header
        fscanf(fr, "%s", pSix);

        // check to see if it's a PPM image file
        if (strncmp(pSix, "P6" , 10) != 0) {
                printf("They are not the same\n");
        } else {
                printf("They are the same\n");
        }

        // read the rest of header
        fscanf(fr, "%d\n %d\n", width, height);

    fscanf(fr, "%d\n", maximum);

    // check to see if they were stored properly
    printf("PSix: %s\n", pSix);
    printf("Width: %d\n", *width);
    printf("Height: %d\n", *height);
    printf("maximum: %d\n", *maximum);

    //int size = width * height;
    int size = 423800;

    // allocate array for pixels
    unsigned char* pixels = new unsigned char[size];

        // unformatted read of binary pixel data
        while (fread(pixels, sizeof(int), 128, fr)) {
                printf("%s", pixels);
        } // end of for loop

        // close file
        fclose(fr);

        // return the array
        return pixels;
       
} // end of readPPM

---

Code:

---

    while (fscanf(fr, "%c", &pixels) != EOF) {

---

An issue is here. pixels is a pointer to type of unsigned char* - pointer to unsigned char. The fscanf() will read one char from the stream and put it into the memory location pointed to by the last parameter. In this case the address of a pointer to the pointer to memory of unsigned char - which is not what is wanted. Also the memory location to which the char is read isn't changed upon reading chars - so the same location is repeatedly overwritten. Consider (not tried)

Code:

---

unsigned char* readPPM(const char* fileName, int* width, int* height) {
    unsigned char *pixels = new unsigned char[128];

    FILE* fr = fopen(fileName, "r");

    for (unsigned char *ptr = pixels; fscanf(fr, "%c", ptr) != EOF; ++ptr)
        printf("%c", *ptr);

    fclose(fr);
    return pixels;
}

---

Also note that there is no checking to make sure that the file opened OK before being used.

Code:

---

    unsigned char* pixelArray[size] = readPPM(fileName, &width, &height);

---

See previous thread. This should be

Code:

---

    unsigned char* pixelArray = readPPM(fileName, &width, &height);

---

Code:

---

    printf("%s\n", pixelArray[i]);

---

Shouldn't this be

Code:

---

    printf("%c\n", pixelArray[i]);

---

as the elements of pixelArray are of type unsigned char?

I've successfully stored the header (P6, 650, 652, and 255)) in their right variables (I know because I tried printing their values and they came out right).
My output is shown below.

Output:
PSix: P6

Width: 650

Height: 652

maximum: 255

I was wondering what those nonsense characters below the maximum were. Are those the binary pixel data? If so, then I just need to write my writePPM function. I also updated my codes above if you're curious.

[Please don't retrospectively amend posts. It alters the flow of the threads. If code has been altered, then just post the revised code in a new post.

Also, the 'nonsense' characters posted were interfering with the operation of the edit boxes so I've removed them.]

Cheers!

These characters are part of the non-printable ASCII characters that form the binary pixel data.

A couple of points re the revised code.

Code:

---

    //int size = width * height;
    int size = 423800;

---

The size for the binary data for a P6 format file is width * height * 3 - as for each pixel there is 1 byte for each of RGB colours. So

Code:

---

    int size = width * height * 3;

---

To then read the binary data it's much easier to just use

Code:

---

    if (fread(pixels, 1, size, fr) != size)
      puts("Error reading binary data");

---

Code:

---

  fscanf(fr, "%s", pSix);

---

This really is a big no:thumbd:-no! This will read an undermined number of characters from the file into memory starting at location pSix and continue until a white-space character is found. In a correct format file, this should read 2 chars, but in an incorrect format file could read any number. For the correct way is to specify the maximum number of chars to be read consider

Code:

---

    fscanf(fr, "%5s", pSix);

---

This will read no more than 5 chars into the memory pointed to by pSix.

Why are you passing pSiz to readPPM()? As its only used within readPPM(), why not just define it within that function? You are also not freeing the allocated memory, neither are you checking for errors.

Consider c code

Code:

---

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

unsigned char* readPPM(const char* const fileName, unsigned int* const width, unsigned int* const height, unsigned int* const maximum)
{
        const char p6[] = "P6";
        char header[10];
        unsigned char *pixels = NULL;

        *width = *height = *maximum = 0;

        FILE* fr = fopen(fileName, "rb");
        if (fr == NULL) {
                printf("Unable to open file %s\n", fileName);
                return NULL;
        }

        fscanf(fr, "%9s", header);
        if (strcmp(header, p6))
                printf("Incorrect header %s found\n", header);
        else
                if (fscanf(fr, "%d %d\n%d\n", width, height, maximum) != 3)
                        puts("Invalid header values");
                else {
                        const unsigned int size = *width * *height * 3;

                        if ((pixels = malloc(size)) == NULL)
                                puts("Error allocating memory");
                        else {
                                unsigned int read = 0;

                                if ((read = fread(pixels, 1, size, fr)) != size) {
                                        printf("Error reading binary data. Wanted %i, got %i\n", size, read);
                                        free(pixels);
                                        pixels = NULL;
                                }
                        }
                }

        fclose(fr);
        return pixels;
}

int main()
{
        //char fileName[50] = "binary_pixels.txt";
        char fileName[500] = "c:\\Program Files\\Git\\mingw64\\lib\\tk8.6\\demos\\images\\teapot.ppm";
        unsigned int width = 0;
        unsigned int height = 0;
        unsigned int maximum = 0;

        unsigned char* pixelArray = readPPM(fileName, &width, &height, &maximum);

        if (pixelArray) {
                printf("Width: %d\n", width);
                printf("Height: %d\n", height);
                printf("maximum: %d\n", maximum);
                free(pixelArray);
        }
}

---

Hey 2kaud,

I just have a few questions to your code above.

What exactly is happening in this part?

unsigned int read = 0;

if ((read = fread(pixels, 1, size, fr)) != size) {...}

return pixels;


1. What do you store in read and why do you need it?

2. And what are you doing with pixels?
For me it looks like the data/content of the picture is in pixels?

I am confused because you store width, height, maximum(rgb?) and this 3 are also the only output of printf.
Nothing happens with the data/content.
Is it also possible to store the data outside the function?

Just for me to understand.
This Part is to read and store the header

Code:

---

fscanf(fr, "%9s", header);
        if (strcmp(header, p6))
                printf("Incorrect header %s found\n", header);
        else
                if (fscanf(fr, "%d %d\n%d\n", width, height, maximum) != 3)
                        puts("Invalid header values");
                else {
                        const unsigned int size = *width * *height * 3;

---

and this part is for the data/content

Code:

---

                        if ((pixels = malloc(size)) == NULL)
                                puts("Error allocating memory");
                        else {
                                unsigned int read = 0;

                                if ((read = fread(pixels, 1, size, fr)) != size) {
                                        printf("Error reading binary data. Wanted %i, got %i\n", size, read);
                                        free(pixels);
                                        pixels = NULL;
                                }
                        }
                }

        fclose(fr);
        return pixels;

---

But i dont see/understand what you are doing with pixels? :(

It's an old post and if you read this and help me understand i owe you more than one beer.

Thank you and br

1) fread() returns the number of items actually read so this value is assigned to read. It is used in the next statement if an error occurs with fread() ie the number actually read is not the same as that requested. If no error occurs, the value of read is still assigned but not used.

2) The code to read the header obtains the width and height. The number of pixels is width * height. The value of each pixel is stored as a char in the file. Once the number of pixels is known from the header, memory of the required size is allocated to the variable pixels. The contents of the file is then read into this memory, the file is closed and a pointer to this memory is returned. If there was an error, then the allocated memory is freed and NULL is returned to indicate an error condition.

3) printf() is only used in an error situation and just shows the header values.

4) as the header values are returned as part of the function call and the pixel data is returned from the function, the data is meant to be used outside of the function. Note in this case it is the function callers responsibility to free the returned memory.

Hope this clarifies things.