2

I am trying to write a program that converts a p3 PPM file to P6 using C. But I am running into two problems. 1, I am getting a segmentation fault in my code. and 2, the header isn't being properly read into the converted p6 file. Here is my header file.

#ifndef PPM_UTILS
#define PPM_UTILS

#include "stdio.h"
#include "stdlib.h"
#include "string.h"

// First meaningful line of the PPM file
typedef struct header {
    char MAGIC_NUMBER[3];
    unsigned int HEIGHT, WIDTH, MAX_COLOR;
} header_t;

// Represents an RGB pixel with integer values between 0-255
typedef struct pixel {
    unsigned int R, G, B;
} pixel_t;

// PPM Image representation
typedef struct image {
    header_t header;
    pixel_t* pixels;
} image_t;

header_t read_header(FILE* image_file);
image_t* read_ppm(FILE* image_file);
image_t* read_p6(FILE* image_file, header_t header);
image_t* read_p3(FILE* image_file, header_t header);

void write_header(FILE* out_file, header_t header);
void write_p6(FILE* out_file, image_t* image);
void write_p3(FILE* out_file, image_t* image);

#endif

And I have the main part of the code split between two files that I combine in the compiler

#include <stdio.h>
#include "ppm_utils.h"

header_t read_header(FILE* image_file)
{

    header_t header;

    fscanf(image_file, "%c %d %d %d",header.MAGIC_NUMBER, &header.WIDTH, &header.HEIGHT, &header.MAX_COLOR);
    return header;
}

void write_header(FILE* out_file, header_t header)
{

    fprintf(out_file, "%c %d %d %d", *header.MAGIC_NUMBER,header.HEIGHT,header.WIDTH,header.MAX_COLOR);
}

image_t* read_ppm(FILE* image_file)
{

    header_t header = read_header(image_file);

    image_t* image = NULL;
    if (strcmp("P3", header.MAGIC_NUMBER) == 0)
    {
        image = read_p3(image_file, header);
    }
    else if (strcmp("P6", header.MAGIC_NUMBER) == 0)
    {
        image = read_p6(image_file, header);
    }
    return image;
}

image_t* read_p6(FILE* image_file, header_t header)
{

    int size;
    size = header.HEIGHT * header.WIDTH;
    image_t* image = (image_t*) malloc (sizeof(image_t));
    image -> header = header;
    image -> pixels = (pixel_t*) malloc (sizeof(pixel_t)* size);
    char r,g,b;
    r = 0;
    g = 0;
    b = 0;
    int i;
    for (i=0;i<size;i++){
        fscanf(image_file, "%c%c%c", &r, &g, &b);
        image -> pixels -> R = (int) r;
        image -> pixels -> G = (int) g;
        image -> pixels -> B = (int) b;
    }
    return image;
}
image_t* read_p3(FILE* image_file, header_t header)
{
    int size;
    size = header.HEIGHT * header.WIDTH;
    image_t* image = (image_t*) malloc (sizeof(image_t));
    image -> header = header;
    image -> pixels = (pixel_t*) malloc (sizeof(pixel_t)* size);
    int r,g,b;
    r = 0;
    g = 0;
    b = 0;
    int i;
    for (i=0;i<size;i++){
        fscanf(image_file, "%d %d %d ", &r, &g, &b);
        image -> pixels -> R = (int) r;
        image -> pixels -> G = (int) g;
        image -> pixels -> B = (int) b;
    }
    return image;

}

void write_p6(FILE* out_file, image_t* image)
{
    header_t header = image -> header;
    header.MAGIC_NUMBER[1]=6;
    write_header(out_file, header);
    int size = header.HEIGHT * header.WIDTH;
    int i;
    for (i=0;i<size;i++){
        fprintf(out_file,"%c%c%c", (char) image->pixels->R, (char) image->pixels->G, (char) image->pixels->B);
    }

}
void write_p3(FILE* out_file, image_t* image)
{
    header_t header = image -> header;
    header.MAGIC_NUMBER[1]=3;
    write_header(out_file, header);
    int size = header.HEIGHT * header.WIDTH;
    int i;
    for (i=0;i<size;i++){
        fprintf(out_file,"%d %d %d ",image->pixels->R,image->pixels->G,image->pixels->B);

    }

}

..

#include <stdio.h>
#include "ppm_utils.h"

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

    if (argc != 3)
    {
        printf("The program needs two arguments");
        return 1;
    }

    FILE *fr;
    fr = fopen(argv[1],"r");

    if (fr == NULL)
    {
        printf("The opening failed");
    }

    FILE *fw;
    fw = fopen(argv[2],"w");

    if (fw == NULL)
    {
        printf("The opening failed");
    }
    image_t* image = read_ppm(fr);
    write_p6(fw,image);
    free(image);
    free(image->pixels);
    fclose(fr);
    fclose(fw);
    return 0;
}

Do you guys have any solutions?

Pablo
  • 13,271
  • 4
  • 39
  • 59
Harrison
  • 23
  • 1
  • 6
  • 1
    You get the segmentation fault because you free first an structure (image) and then try to access/free a member (image->pixels) – Paul Götzinger Jan 29 '18 at 23:50
  • 2
    One thing I noticed. In your header file you are including standard library headers in quotes (`"stdio.h"` etc.), instead of angle brackets (``). Quotes are only for if those headers are in the same directory as yours. – Tanner Babcock Jan 30 '18 at 00:02

3 Answers3

1

When reading the header you need to specify the number of characters of the magic number to read:

fscanf(image_file, "%2c %d %d %d",header.MAGIC_NUMBER, &header.WIDTH, &header.HEIGHT, &header.MAX_COLOR);

To be used with strcmp()the last byte in the array must be set to 0:

header.MAGIC_NUMBER[2] = 0;

When writing the header you need to write it as an string:

fprintf(out_file, "%s %d %d %d", header.MAGIC_NUMBER,header.HEIGHT,header.WIDTH,header.MAX_COLOR);
  • This helped clear a lot of things up, Thanks!! But, for some reason, whenever I check the text file of the p6 ppm that I create, it shows up as a empty box character rather than a 6. Do you have any solutions for this? – Harrison Jan 30 '18 at 00:09
  • do you mean the second character of the magic number? the '6'-character? EDIT: I think I fixed it please remove the '*' preceding `header.MAGIC_NUMBER` when writing the header (fixed in answer) – Paul Götzinger Jan 30 '18 at 00:13
  • Thanks! I did that but it's still happening. I'm very unsure of what's causing it do that because everything else gets copied fine. And, it's causing my ppm image to not open properly due to it saying its an improper header. Even when I try changing it manually to p6 it still doesn't work. My header looks like P 256 256 255. edit: oh it seems the box didn't copy. I feel like maybe I have some sort of character wrong because maybe its caused by it trying to copy a int into a character array? But I don't know how to fix that. – Harrison Jan 30 '18 at 00:20
  • I'm not quite sure, but you could try to use `%2s` in scanf instead of `%2c` I would recommend to try debugging or outputing the magic number with printf to check if it read without problems – Paul Götzinger Jan 30 '18 at 00:28
0
for (i=0;i<size;i++){
    fscanf(image_file, "%c%c%c", &r, &g, &b);
    image -> pixels -> R = (int) r;
    image -> pixels -> G = (int) g;
    image -> pixels -> B = (int) b;
}

Repeat assigning the same memory

Fox
  • 1
  • 1
0

The way you parse your PPM format header, using fscanf(), is never going to work.

Like I explain in the first point of this related answer, the PPM header may contain comments.

Note that this answer applies equally to all PNM formats, P1 to P6 (PBM, PGM, PPM, PBM, PGM, PPM, and PAM, respectively). The format of their headers is the same, except for the number of nonnegative integer values listed in the header (2 for PBM, 3 for PGM and PPM, and 4 for PAM).

(P7, Portable AnyMap or PAM, has an extra parameter, tupletype, that is an uppercase string. It is less often implemented than the others, although it is a very nice general image format. The below approach will work for it too, if you add parsing for the tupletype yourself.)

It seems many people have trouble constructing the "read one PNM format header value" part. I have seen too many examples of code that works for some, but not all PNM format files, hurting inoperability and causing unwarranted headaches to developers and users alike, so I think it is warranted to show an example how one could go about it correctly.

The correct operation to read the headers of any of the PNM formats is simple: You use fgetc() to read the fixed part of the header (P, the format digit, and the following whitespace character, one of '\t', '\n', '\v', '\f', '\r', or ' '), and a helper function to parse the nonnegative integer value. (For P1 and P4 formats, there are just two values, width and height in pixels, in that order; for the P2, P3, P5, and P6 formats, there are three values: width, height, and maxval, where maxval is the maximum component value used in the file.)

I suggest you go read the pseudocode in the answer I linked to above, if you actually care about learning, and try to implement your own first. If you get stuck, you can compare to the known working version below.


For simplicity, let's use a helper function, pnm_is_whitespace(character), that returns true (nonzero) if character is one of those whitespace characters:

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

static inline int pnm_is_whitespace(const int c)
{
    return (c == '\t' || c == '\n' || c == '\v' ||
            c == '\f' || c == '\r' || c == ' ');
}

Note that PNM files are not locale aware, so the above will work for all PNM formats on all operating systems. The above function is just a shorthand helper function you won't need to modify even when porting the code.

Because the values in the PNM headers are nonnegative integers, we can use a simple function that returns either the value, or -1 if an error occurs:

static inline int pnm_header_value(FILE *src)
{
    int  c;

    /* Skip leading whitespace and comments. */
    c = fgetc(src);
    while (1) {
        if (c == EOF) {
            /* File/stream ends before the value. */
            return -1;
        } else
        if (c == '#') {
            /* Comment. Skip the rest of the line. */
            do {
                c = fgetc(src);
            } while (c != EOF && c != '\r' && c != '\n');
        } else
        if (pnm_is_whitespace(c)) {
            /* Skip whitespace. */
            c = fgetc(src);
        } else
            break;
    }

    /* Parse the nonnegative integer decimal number. */
    if (c >= '0' && c <= '9') {
        int  result = (c - '0');

        c = fgetc(src);
        while (c >= '0' && c <= '9') {
            const int  old = result;

            /* Add digit to number. */
            result = 10*result + (c - '0');

            /* Overflow? */
            if (result < old)
                return -1;

            /* Next digit. */
            c = fgetc(src);
        }

        /* Do not consume the separator. */
        if (c != EOF)
            ungetc(c, src);

        /* Success. */
        return result;
    }

    /* Invalid character. */
    return -1;
}

The above function parses the input in a very similar fashion that %d does in the scanf() family of functions, except that it correctly skips any PNM comment lines preceding the value. If there is an error, it returns -1, in which case the file is not really in PNM format. Otherwise it returns the nonnegative (0 or larger) value specified in the file, without consuming the character that followes the number.

Using the above, we can trivially create a function, that supports the P1-P6 format PNM (PBM, PGM, and PPM formats; ASCII and binary variants) file format headers:

enum {
    PNM_P1      = 1, /* ASCII PBM */
    PNM_P2      = 2, /* ASCII PGM */
    PNM_P3      = 3, /* ASCII PPM */
    PNM_P4      = 4, /* BINARY PBM */
    PNM_P5      = 5, /* BINARY PGM */
    PNM_P6      = 6, /* BINARY PPM */
    PNM_UNKNOWN = 0
};

static inline int pnm_header(FILE *src,
                             int  *width,
                             int  *height,
                             int  *maxdepth)
{
    int  format = PNM_UNKNOWN;
    int  value;

    if (!src)
        return PNM_UNKNOWN;

    /* The image always begins with a 'P'. */
    if (fgetc(src) != 'P')
        return PNM_UNKNOWN;

    /* The next character determines the format. */
    switch (fgetc(src)) {
    case '1': format = PNM_P1; break;
    case '2': format = PNM_P2; break;
    case '3': format = PNM_P3; break;
    case '4': format = PNM_P4; break;
    case '5': format = PNM_P5; break;
    case '6': format = PNM_P6; break;
    default:
        errno = 0;
        return PNM_UNKNOWN;
    }

    /* The next character must be a whitespace. */
    if (!pnm_is_whitespace(fgetc(src)))
        return PNM_UNKNOWN;

    /* Next item is the width as a decimal string. */
    value = pnm_header_value(src);
    if (value < 1)
        return PNM_UNKNOWN;
    if (width)
        *width = value;

    /* Next item is the height as a decimal string. */
    value = pnm_header_value(src);
    if (value < 1)
        return PNM_UNKNOWN;
    if (height)
        *height = value;

    /* Maxdepth, for all but P1 and P4 formats. */
    if (format == PNM_P1 || format == PNM_P4) {
        if (maxdepth)
            *maxdepth = 1;
    } else {
        value = pnm_header_value(src);
        if (value < 1)
            return PNM_UNKNOWN;
        if (maxdepth)
            *maxdepth = value;
    }

    /* The final character in the header must be a whitespace. */
    if (!pnm_is_whitespace(fgetc(src)))
        return PNM_UNKNOWN;

    /* Success. */
    return format;
}

If you supply a FILE handle to pnm_header(), and pointers to the width, height, and maxval (set to 1 for PBM format headers), it will return PNM_P1 to PNM_P6 with the three variables filled with the header information, or PNM_UNKNOWN if the header cannot be parsed correctly.

I have verified that this function can correctly parse the headers of all PBM, PGM, and PPM files I have (including all 210 or so icon files on my Linux laptop).

Nominal Animal
  • 38,216
  • 5
  • 59
  • 86