Reading In PPM Pixels As RGB Integers

**DGB_684** · 01-02-2022

So my code involves reading in a PPM image and then storing it in an array so it can be saved in an new ppm file. Though I think there is a problem with my pointers that means its not actually reading the file. Also my code ends after allocating the memory for the array. Any help is much appreciated.

Code:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    #define maxheight 1080
    #define maxwidth 1920
    #define RGB_COMPONENT_COLOUR 255
    #define pgmtype "P2"
    #define ppmtype "P3"
    
    typedef struct
    {
        int red, green, blue;
    } PPMPixel;
    
    typedef struct
    {
        int x, y;
    } PPMImage;
    
    typedef struct 
    {
        int rgb_comp_colour;
        char filetype[3];
        int height;
        int width;
    } PPMHead;
    
    
    /*PPMHead head[3];
    {
        head[3].filetype;
        head[3].height;
        head[3].width;
    }*/
    
    PPMHead head;
    PPMHead* head_ptr = &head;
    
    PPMPixel p;
    PPMPixel* p_ptr = &p;
    
    PPMPixel *data; //Defines pointer to PPMPixel
    int **Array; //Double pointer defines as a pointer pointing to a pointer that is pointing to an integer
    PPMPixel **RGBArray; //Double pointer defines as a pointer pointing to a pointer that is pointing to the PPMPixel structure
    FILE *fp;
    int r, g, b;
    
    void headercheck ()
    {
    
        fscanf(fp, "%s %d %d %d", head.filetype, &head.width, &head.height, &head.rgb_comp_colour);
        printf("%s %d %d %d", head.filetype, head.width, head.height, head.rgb_comp_colour);
    
        if (head.width > maxwidth || head.height > maxheight)
        {
            printf("\tInvalid image size. The maximum value of image is 1920x1080.\n");
            printf("\tImage size is %d x %d\n", head.width, head.height);
        }
        else
        {
            printf("\tImage size is valid\n");
            printf("\tImage size is %d x %d\n", head.width, head.height);
        }
    
        if ((strcmp (head.filetype, pgmtype)!=0) && (strcmp (head.filetype, ppmtype)!=0))
        {
            printf("\tInvalid filetype\n");
        }
        else
        {
            if(strcmp (head.filetype, pgmtype)==0)
            {
                printf("\t File is PGM type image\n");
            }
            else
            {
                if(strcmp (head.filetype, ppmtype)==0)
                {
                    printf("\t File is PPM type image\n");
                }
            }
        }
    
        if ((head.rgb_comp_colour == RGB_COMPONENT_COLOUR))
        {
            printf("\t Image is 8 bit\n");
        }
        else
        {
            if (head.rgb_comp_colour > RGB_COMPONENT_COLOUR)
            {
                printf("Maximum bit-depth is 8 bits\n");
            }
            else
            {
                printf("\tImage is not 8 bit\n");
            }
        }
    }
    
    int main(void)
    {
        char fname[100];
        printf("Enter file name: ");
        scanf("%s", fname);
        fseek(stdin,0,SEEK_END);
        fp = fopen(fname, "r");
        if (fp == NULL)
        {   
            printf("\tError while opening the file\n");
        }                        
        else    
        {
            printf("\tReading in %s\n", fname);
        }
    
        headercheck();
    
        if (strcmp (head.filetype, ppmtype)==0)
        {
            RGBArray = (PPMPixel **)malloc(head.height*sizeof(PPMPixel*)); //Points to malloc
                if((RGBArray == NULL))
                    {
                        printf("Error allocating memory to the array");
                    }
                    else
                    {
                        printf("Memory allocated to the PPM array sucessfully");
                    }
            for (int i=0;i<head.width;i++)
            {
                RGBArray[i] = (PPMPixel *)malloc(head.width*sizeof(PPMPixel));
            }
            
            printf("Error 2");
            //Initialising each element
            for (int j=0;j<head.height;j++)
            {
                for (int i=0;i<head.width;i++)
                {
                    fscanf(fp, "%3d %3d %3d ", &p.red, &p.green, &p.blue); //Scans in integers of the address pointer to PPMPixel
                    data = &RGBArray[i][j]; //Defines data pointer pointing to address of RGBArray[i][j]
                    data->red = p.red; //Access member of PPMPixel structure to equal one of the three RGB channels
                    data->green = p.green;
                    data->blue = p.blue;
                }
            }
        }
        fclose(fp);
        
        //Save PPM Array Into New PPM File
        FILE *pf;
        int i, j;
        char fname2[100];
        printf("Enter file name: ");
        scanf("%s", fname2);
        fseek(stdin,0,SEEK_END);
        pf = fopen(fname2, "w");
        if (pf == NULL)
        {   
            printf("\tError while opening the file\n");
        }
        else    
        {
            printf("\tWriting in %s\n", fname2);
        }
    
        for(j=0;j<head.height;j++)
        {
            fprintf(pf, "\n");
            for(i=0;i<head.width;i++)
            {
                fprintf(pf, "%3d ", RGBArray[i][j].red);
                fprintf(pf, "%3d ", RGBArray[i][j].green);
                fprintf(pf, "%3d ", RGBArray[i][j].blue);
                //fprintf(pf, "%3d ", (RGBArray+j*head.width + i)*r);
                //fprintf(pf, "%3d ", (RGBArray+j*head.width + i)*g);
                //fprintf(pf, "%3d ", (RGBArray+j*head.width + i)*b);
            }
        }
    
        fclose(pf);
        free(RGBArray);
        RGBArray = NULL;
        for(int i=0;i<head.width;i++)
            {
                free(RGBArray[i]);
                RGBArray[i] = NULL;
            }
        return 0;
    }

**thmm** · 01-02-2022

One obvious error is on lines 184-190. You delete your pointers in the wrong order. It should be in reverse order of allocating them.

**DGB_684** · 01-02-2022

Oh ok I overlooked that one thanks. The other errors still persist unfortunately...

**thmm** · 01-02-2022

Are you not getting confused by so many types and variables ? At least I get.
Why not just using 2 types and one var for the image?
Also 2 separate function to load and save the image would be much easier to understand.

Code:

typedef struct _ 
{
  unsigned char red, 
                green, 
                blue;
}PPM_Pixel;


typedef struct _ 
{
  char filetype[3];
  int rgb_comp_colour;
  int width, 
      height;
  PPM_Pixel** pixels;
}PPM_Image;

**DGB_684** · 01-02-2022

This is one part of a larger piece of code. In the main code is ~350 lines of code plus and I used functions in there. That's also why I have so many variables. Yes it is confusing but I can't really think of a better way to do it right now...

**john.c** · 01-02-2022

Search for //// to find all my comments.

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
 
#define maxheight 1080
#define maxwidth 1920
#define RGB_COMPONENT_COLOUR 255
#define pgmtype "P2"
#define ppmtype "P3"
 
typedef struct
{
    int red, green, blue;
} PPMPixel;
 
//// This is not used anywhere //// 
typedef struct
{
    int x, y;
} PPMImage;
 
typedef struct
{
    int rgb_comp_colour; //// what does "comp colour" mean? ////
    char filetype[3];
    int height;
    int width;
} PPMHead;
 
//// You should not have any of these global variables ////
PPMHead head;
PPMHead* head_ptr = &head; //// unused ////
 
PPMPixel p;
PPMPixel* p_ptr = &p; //// unused ////
 
PPMPixel *data; //Defines pointer to PPMPixel  //// well duh! useless comment ////
int **Array;  //// unused ////
PPMPixel **RGBArray;
FILE *fp;
int r, g, b; //// unused ////
 
void headercheck ()
{
    fscanf(fp, "%s %d %d %d", head.filetype, &head.width, &head.height, &head.rgb_comp_colour);
    printf("%s %d %d %d", head.filetype, head.width, head.height, head.rgb_comp_colour);
 
    //// Why is there a maximum image size? ////
    if (head.width > maxwidth || head.height > maxheight)
    {
        printf("\tInvalid image size. The maximum value of image is 1920x1080.\n");
        printf("\tImage size is %d x %d\n", head.width, head.height);
    }
    else
    {
        printf("\tImage size is valid\n");
        printf("\tImage size is %d x %d\n", head.width, head.height);
    }
 
    if ((strcmp (head.filetype, pgmtype)!=0) && (strcmp (head.filetype, ppmtype)!=0))
    {
        printf("\tInvalid filetype\n");
        //// and yet the program continues ////
    }
    else
    {
        if(strcmp (head.filetype, pgmtype)==0)
        {
            printf("\t File is PGM type image\n");
        }
        else
        {
            if(strcmp (head.filetype, ppmtype)==0)
            {
                printf("\t File is PPM type image\n");
            }
        }
    }
 
    if ((head.rgb_comp_colour == RGB_COMPONENT_COLOUR))
    {
        printf("\t Image is 8 bit\n");
    }
    else
    {
        if (head.rgb_comp_colour > RGB_COMPONENT_COLOUR)
        {
            printf("Maximum bit-depth is 8 bits\n");
            //// and yet the program continues ////
        }
        else
        {
            printf("\tImage is not 8 bit\n");
            //// and yet the program continues ////
        }
    }
}
 
int main(void)
{
    char fname[100];
    printf("Enter file name: ");
    
    //// scanf with %s cannot read a filename with a space in it ////
    scanf("%s", fname);
 
    //// This does nothing (at least on my system) ////
    fseek(stdin,0,SEEK_END);
 
    fp = fopen(fname, "r");
 
    if (fp == NULL)
    {   
        printf("\tError while opening the file\n");
        //// If the file didn't open, the program should end ////
    }                        
    else
    {
        printf("\tReading in %s\n", fname);
    }
 
    headercheck();
 
    if (strcmp (head.filetype, ppmtype)==0) //// What if it isn't ppmtype? ////
    {
        RGBArray = (PPMPixel **)malloc(head.height*sizeof(PPMPixel*)); //Points to malloc //// WTF?! ////
            if((RGBArray == NULL))
                {
                    printf("Error allocating memory to the array\n");
                    //// and yet the program continues ////
                }
                else
                {
                    //// These "successful" messages are not needed ////
                    printf("Memory allocated to the PPM array sucessfully\n");
                }
 
        //// This loop needs to be up to head.height (not width) ////
        for (int i=0;i<head.width;i++)
        {
            RGBArray[i] = (PPMPixel *)malloc(head.width*sizeof(PPMPixel));
        }
 
        //Initialising each element
        for (int j=0;j<head.height;j++)
        {
            for (int i=0;i<head.width;i++)
            {
                //// Should just be %d, not %3d ////
                //// Then you should check that none of the values are
                  // greater than the "comp_colour" ////
                fscanf(fp, "%3d %3d %3d ", &p.red, &p.green, &p.blue); //Scans in integers of the address pointer to PPMPixel
                //// This should be [j][i] since the height comes first ////
                data = &RGBArray[i][j]; //Defines data pointer pointing to address of RGBArray[i][j]
                data->red = p.red; //Access member of PPMPixel structure to equal one of the three RGB channels
                data->green = p.green;
                data->blue = p.blue;
            }
        }
    }
    fclose(fp);
     
    //Save PPM Array Into New PPM File
    FILE *pf;
    int i, j;
    char fname2[100];
    printf("Enter file name: ");
    scanf("%s", fname2);
    fseek(stdin,0,SEEK_END); //// does nothing ////
    pf = fopen(fname2, "w");
    if (pf == NULL)
    {   
        printf("\tError while opening the file\n");
        //// and yet the program continues ////
    }
    else
    {
        printf("\tWriting in %s\n", fname2);
    }
 
    //// You forgot to write the header! ////
 
    for(j=0;j<head.height;j++)
    {
        fprintf(pf, "\n"); //// This should be after the inner loop, not before ////
        for(i=0;i<head.width;i++)
        {
            //// should be [j][i] ////
            fprintf(pf, "%3d ", RGBArray[i][j].red);
            fprintf(pf, "%3d ", RGBArray[i][j].green);
            fprintf(pf, "%3d ", RGBArray[i][j].blue);
        }
    }
 
    fclose(pf);
 
    //// Obviously you can't free RGBArray first! ////
    free(RGBArray);
    RGBArray = NULL;
    //// This loop needs to be up to head.height (not width) ////
    for(int i=0;i<head.width;i++)
    {
        free(RGBArray[i]);
        RGBArray[i] = NULL; //// not needed ////
    }
 
    return 0;
}

**DGB_684** · 01-02-2022

I've made your suggested changes and it works now! Thanks a lot! The array doesn't look all that pretty but it works fine now!

Thread: Reading In PPM Pixels As RGB Integers

Thread Tools

Search Thread

Display

Reading In PPM Pixels As RGB Integers

Similar Threads

Reading Pixels

reading negative integers

reading integers

reading BMP files and pixels' colours

Reading pixels from + sending mouse clicks to another application window - how??

Tags for this Thread