Problems converting tga files to black and white

Question

I have been trying to make this program to convert a tga image for color into black and white. But i have no clue how to go about it. I am verry new to C and have yet to get the hang of the syntax and even proper usage of ubuntu.

I think my problem is somthing with tha tga files header cant be read. Because the result i get when trying this program on a tga file is an unopenable picture with no height. "height = 0".

Is there some good links for one to read up on C?

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

struct pixel {
    uint8_t r, g, b, a;
};

static uint8_t *load_image(char *filename, int *sizex, int *sizey)
{
    uint8_t *image;
    char buf[512];
    char *bufptr;
    int ret;

    FILE *fp = fopen(filename, "r");
    bufptr = fgets(buf, 512, fp);
    ret = fscanf(fp, "%d %d\n", sizex, sizey);
    bufptr = fgets(buf, 512, fp);

    image = malloc(*sizex * *sizey * 4);

    int i;
    uint8_t *ptr = image;
    for (i=0; i<*sizex * *sizey; ++i) {
        ret = fread(ptr, 1, 3, fp);
        ptr += 4;
    }

    fclose(fp);
    return image;
}

static int save_image(const char *filename, uint8_t *image, int sizex, int sizey)
{
    FILE *fp = fopen(filename, "w");
    fprintf(fp, "P6\n%d %d\n255\n", sizex, sizey);

    int i;
    uint8_t *ptr = image;
    for (i=0; i<sizex * sizey; ++i) {
        fwrite(ptr, 1, 3, fp);
        ptr += 4;
    }
    fclose(fp);

    return 1;
}

void convert_grayscale(uint8_t *input, uint8_t *output, int sizex, int sizey)
{
    // Y = 0.299 * R + 0.587 * G + 0.114 * B

    int i;

    for (i = 0; i < sizex * sizey; ++i)
        {
            struct pixel *pin = (struct pixel*) &input[i*4];
            struct pixel *pout = (struct pixel*) &output[i*4];

            float luma = 0.299 * pin->r + 0.587 * pin->g + 0.114 * pin->b;

            if (luma > 255)
                luma = 255;

            uint8_t intluma = (int) luma;

            pout->r = intluma;
            pout->g = intluma;
            pout->b = intluma;
            pout->a = 255;
        }

}

int main()
{
    uint8_t *inputimg, *outputimg;
    int sizex, sizey;

    inputimg = load_image("image.tga", &sizex, &sizey);

    outputimg = malloc(sizex * sizey * 4);

    convert_grayscale(inputimg, outputimg, sizex, sizey);

    save_image("output.tga", outputimg, sizex, sizey);
}

Answer 1

(Personal note: A longer answer after reading Why Stackoverflow sucks. That should be Required Reading for everyone who gets Moderator privileges.)

The problem is your load_image code seems designed to read PPM (ASCII-based) images:

Each PPM image consists of the following: 1. A "magic number" for identifying the file type. A ppm image's magic number is the two characters "P6". 2. Whitespace (blanks, TABs, CRs, LFs). 3. A width, formatted as ASCII characters in decimal. 4. Whitespace. 5. A height, again in ASCII decimal. 6. Whitespace. 7. The maximum color value (Maxval), again in ASCII decimal. Must be less than 65536 and more than zero. 8. A single whitespace character (usually a newline). 9. A raster of Height rows [...]

-- your first fgets reads, then discards, the "magic number" line, followed by reading the width and height, and then discarding the "maxval" line.

It ought to work for PPM images (and you could rename this routine load_ppm_image) were it not for a single important issue: after all that ASCII stuff, you switch to fread, and so here is Warning #1.

Before opening your file, decide whether you are going to read exclusively ASCII text, or might need to read binary data.

The problem is that 'text mode' "w" converts certain characters when reading and writing into others. That's built-in behavior in all common C libraries; it attempts to fix the end-of-line characters mess that a previous generation of programmers left us with. Now, reading text files in text mode got a bit simpler, but reading binary data is impossible. You can't be sure you got exactly what was in the file.

Let's get on with Warning #2: not all file formats are the same.

The above routine works (mostly) for PPM images, but it will fail on TGA because its header is organized differently. The TGA header is described rather well here (a random pick of Google results).

The specification describes bytes, so first thing to do is change your fopen line to

FILE *fp = fopen(filename, "rb");

and, by the way, a good practice is to test if it was successful:

if (fp == NULL)
{
   printf ("Opening the file '%s' failed\n", filename);
   return NULL;
}

Then you can use fgetc or fread to read one or more bytes. Here comes Warning #3: use fread with care.

fread reads multiple bytes in the order in which they are stored into the file, and so you would think it may read an item such as width and height -- each a 2-byte integer value -- in one 'read' operation. But fread does not know the order of the bytes in your system (nor in the file itself), and so it could be it reads "lo-hi", as in the specification I pointed to, while in your computer the order of bytes in an integer is "hi-lo". To clarify: if the file contains this

80 00

and you read, then store, this with fread (&width,1,2, fp), these 2 bytes get stored into computer memory in that same order. The bytes are in Big-Endian order; the "large" byte is at the end. But if your computer happens to be a Little-Endian order system, you would not get the value 0x0080 = 128 but 0x8000 = 32768 instead!

The way to circumvent this is to read one byte at a time:

width = fgetc(fp) + (fgetc(fp)<<8);

will always read the data in the correct order: low first, then high. Only the sum gets stored (in the order for your system, but that's now irrelevant!).

With the above, I think I'm out of warnings. Using the TGA specifications as a guide, you can now open the file, read the header one byte at a time until you have all information that's needed, and continue to fread your raw image data into memory. You can safely use fread to read your image bytes three at a time because they will appear in the same order into memory as they were read (they are not integers or larger, so the "memory order" is not an issue).

A good approach to ensure you are reading the correct information is this:

1. Read one byte at a time, to prevent endianness issues.
2. Add a comment in your code detailing what it is
3. Print out the value
4. Check with the specifications if the value is allowed.

To get you started, after the fopen line (and the required check if it worked):

int idLength = fgetc(fp); /* length of id string after header */
printf ("id length: %u bytes\n", idLength);
int colorMapType = fgetc(fp); /* 0 = RGB */
printf ("color map type: %u\n", colorMapType);
if (colorMapType != 0)
{
   printf ("unexpected color map type!\n");
   return NULL;
}
int imageType = fgetc(fp); /* 0 = None, 1 = Indexed, 2 = RGB, 3 = Greyscale */

.. and so on. When the entire header has been read and you didn't encounter surprises, you are ready to set up things to read the actual image data. No changes needed there, your existing code should work just fine.

---

Post-edit: I see I used

int colorMapType = fgetc(fp);

where the 'color map type' is in fact a byte, not an integer. That is to allow a belt-and-suspenders approach. If you encounter the end of the file while you are reading the header, the code that fgetc returns is EOF. EOF cannot be stored into a char, because it is an integer value: 0xFFFFFFFF (more accurately: (int)-1). If you store it into a char, you cannot distinguish it from the perfectly okay value 0x000000FF (the value 255).

The belt-and-suspender approach is to check each and every single byte:

if (colorMapType == EOF)
{
   printf ("encountered unexpected end of file!\n");
   return NULL;
}

Overkill if you are working with a known file, and you know it's a valid TGA (you can view and edit it with bitmap editors), but if you ever plan to work on files of which you don't know if they are valid, you might need this.