Thread: Help: Find Darkest point in ppm File using RGB in C

  1. #1
    Registered User
    Join Date
    Mar 2014
    Posts
    4

    Question Help: Find Darkest point in ppm File using RGB in C

    Hi!

    I have write a program to read ppm file and write a new ppm file. However, I'm not sure how to find the darkest point in this ppm file.

    Note:
    1. there are 610200 RGB point, but my new file only created 176196 RGB point.
    2. RGB range: 0-255. (0,0,0)= black; (255,255,255)= white
    3. When reporting the location of your dark pixels, assume the upper left corner of the image is location 1, 1

    any help is appreciate!

    Below is my code:

    Code:
    #include<stdio.h>#include<stdlib.h>
    
    
    typedef struct {
         unsigned char red,green,blue;
    } pixel_t;
    
    
    typedef struct {
         int x, y;
         pixel_t *data;
    } PPMImage;
    
    
    
    
    #define RGB_COMPONENT_COLOR 255
    
    
    static PPMImage *readPPM(const char *filename)
    {
             char buff[16];
             PPMImage *img;
             FILE *fp;
             int c, rgb_comp_color;
             //open PPM file for reading
             fp = fopen(filename, "rb");
              if (!fp) {
                  fprintf(stderr, "Unable to open file '%s'\n", filename);
                  exit(1);
             }
    
    
         //read image format
         if (!fgets(buff, sizeof(buff), fp)) {
              perror(filename);
              exit(1);
         }
    
    
    //check the image format
    if (buff[0] != 'P' || buff[1] != '3') {
         fprintf(stderr, "Invalid image format (must be 'P3')\n");
         exit(1);
    }
    
    
    //alloc memory form image
    img = (PPMImage *)malloc(sizeof(PPMImage));
    if (!img) {
         fprintf(stderr, "Unable to allocate memory\n");
         exit(1);
    }
    
    
    //check for comments
    c = getc(fp);
    while (c == '#') {
    while (getc(fp) != '\n') ;
         c = getc(fp);
    }
    
    
    ungetc(c, fp);
    //read image size information
    if (fscanf(fp, "%d %d", &img->x, &img->y) != 2) {
         fprintf(stderr, "Invalid image size (error loading '%s')\n", filename);
         exit(1);
    }
    
    
    //read rgb component
    if (fscanf(fp, "%d", &rgb_comp_color) != 1) {
         fprintf(stderr, "Invalid rgb component (error loading '%s')\n", filename);
         exit(1);
    }
    
    
    //check rgb component depth
    if (rgb_comp_color!= RGB_COMPONENT_COLOR) {
         fprintf(stderr, "'%s' does not have 8-bits components\n", filename);
         exit(1);
    }
    
    
    while (fgetc(fp) != '\n') ;
    //memory allocation for pixel data
    img->data = (pixel_t*)malloc(img->x * img->y * sizeof(pixel_t));
    
    
    if (!img) {
         fprintf(stderr, "Unable to allocate memory\n");
         exit(1);
    }
    
    
    //read pixel data from file
    if (fread(img->data, sizeof(pixel_t) * img->x, img->y, fp) != img->y) {
         fprintf(stderr, "Error loading image '%s'\n", filename);
         exit(1);
    }
    
    
    fclose(fp);
    return img;
    }
     void writePPM(const char *filename, PPMImage *img)
    {
    FILE *fp;
    //open file for output
    fp = fopen(filename, "wb");
    if (!fp) {
         fprintf(stderr, "Unable to open file '%s'\n", filename);
         exit(1);
    }
    
    
    //write the header file
    //image format
    fprintf(fp, "P3\n");
    
    
    
    
    //image size
    fprintf(fp, "%d %d\n",img->x,img->y);
    
    
    // rgb component depth
    fprintf(fp, "%d\n",RGB_COMPONENT_COLOR);
    
    
    // pixel data
    fwrite(img->data, sizeof(pixel_t) * img->x, img->y, fp);
    fclose(fp);
    }
    
    
    
    int main(int argc,const char *filename, PPMImage *img){
    
    
    filename = "n44f.pgm";
        FILE *fp;
    //open file for output
    fp = fopen(filename, "wb");
    if (!fp) {
         fprintf(stderr, "Unable to open file '%s'/n", filename);
         exit(1);
    }
    
    
    printf("image name: %s \n", filename);
    
    
    //write the header file
    //image format
    printf("magic number  = P3\n");
    
    
    
    //image size
    printf("width, height = %d, %d\n",img->x,img->y);
    
    
    // rgb component depth
    printf( "color val     = %d\n",RGB_COMPONENT_COLOR);
    
    
    PPMImage *image;
    
    
    image = readPPM("n44f.ppm");
    
    writePPM("n44fnew.ppm",image);
    printf("Press Enter to continue");
    getchar();
    
    }




    Here is ppm file:
    https://drive.google.com/file/d/0B1u...it?usp=sharing




  2. #2
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Some very quick observations.

    I wouldn't be using fgets() with a binary file -- even for the 'header' section if there is no possibility of there being a zero byte in that section. fread() would be more appropriate.

    Your line (which I've suggetsed changing to use fread())

    Code:
    if (!fgets(buff, sizeof(buff), fp))
    Is not what I'd call "robust". You're assuming that buff is an array of characters, which may or may not be an appropriate assumption since you can just scroll up a few lines to check, but for the sake of caution I'd modify the size of the buffer to be calculated as sizeof buff / sizeof buff[0] since this way it doesn't matter if you change the buffer to, say for example, an array of ints. I'd modify your other calculations that are used in a similar manner as well.

    Also, don't cast the return value of malloc (a void * can be assigned to any valid pointer type without a cast).

    You're not free()ing the memory you allocated.

    Fix these first and maybe the problem will disappear (probably not, but who knows).

  3. #3
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by Hodor View Post
    I wouldn't be using fgets() with a binary file -- even for the 'header' section if there is no possibility of there being a zero byte in that section. fread() would be more appropriate.
    Except it's not a binary file, so fgets is appropriate, and fread is not. Here's the PPM formats. 6globe is using P3 which is ASCII format.

    @6globe:
    fread is actually incorrect to use here.

    Quote Originally Posted by Hodor View Post
    Is not what I'd call "robust". You're assuming that buff is an array of characters, which may or may not be an appropriate assumption since you can just scroll up a few lines to check, but for the sake of caution I'd modify the size of the buffer to be calculated as sizeof buff / sizeof buff[0] since this way it doesn't matter if you change the buffer to, say for example, an array of ints. I'd modify your other calculations that are used in a similar manner as well.
    Except that fgets only takes a pointer to char. Any other type of pointer (such as pointer to int) would be incorrect to ever use with fgets, even if you cast it. Thus, no need to divide out the size of a single element for fgets.
    Quote Originally Posted by Hodor View Post
    Also, don't cast the return value of malloc (a void * can be assigned to any valid pointer type without a cast).

    You're not free()ing the memory you allocated.
    These two are correct however.

  4. #4
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by 6globe View Post
    I have write a program to read ppm file and write a new ppm file. However, I'm not sure how to find the darkest point in this ppm file.

    Note:
    1. there are 610200 RGB point, but my new file only created 176196 RGB point.
    2. RGB range: 0-255. (0,0,0)= black; (255,255,255)= white
    3. When reporting the location of your dark pixels, assume the upper left corner of the image is location 1, 1
    So you know that the darkest possible pixel is black, (0, 0, 0). The brightest is white (255, 255, 255). So obviously the smaller the red, green and blue components, the darker the pixel. So just look for the pixel with the smallest R, G and B components. There's just one small catch. You have to figure out how to handle comparing pixels in a way that makes sense since there are 3 parts to each pixel. For example, which is darker between
    (1, 1, 1) and (0, 0, 255)?
    Is it (1, 1, 1) because the total of the components is lower, or is it (0, 0, 255) because the first two components are smaller?

    Think about it, and see what you come up with. Create a small test image of all (1, 1, 1) and all (0, 0, 255) to visually compare and check your guess. That should be easy since you have a program to write PPM files.

    EDIT: You don't give a very thorough description of your assignment requirements, but you may not need to read the whole file into memory, you may be able to process it one pixel at a time.

  5. #5
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,791
    Quote Originally Posted by anduril462 View Post
    Except it's not a binary file, so fgets is appropriate, and fread is not. Here's the PPM formats. 6globe is using P3 which is ASCII format.
    Anduril. I guess that's what you (I) get when you quickly scan through code.

    I saw

    Code:
    //read pixel data from file
    if (fread(img->data, sizeof(pixel_t) * img->x, img->y, fp) != img->y) {
         fprintf(stderr, "Error loading image '%s'\n", filename);
         exit(1);
    }
    And the rest is history.




    Except that fgets only takes a pointer to char. Any other type of pointer (such as pointer to int) would be incorrect to ever use with fgets, even if you cast it. Thus, no need to divide out the size of a single element for fgets.
    I disagree with this. Sure, fgets() can only use a pointer to char, this has nothing to do with the buffer size in my opinion.

  6. #6
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by Hodor View Post
    I disagree with this. Sure, fgets() can only use a pointer to char, this has nothing to do with the buffer size in my opinion.
    For general buffer sizes, you're right, you must divide. My point was, the lack of division and supposed lack of robustness is not an issue with char arrays; the division is rather pointless. Per the standard, sizeof(char) is always guaranteed to be 1, thus

    sizeof(any_char_array) / sizeof(any_char_array[0])
    is really just
    sizeof(any_char_array) / 1
    which is just
    sizeof(any_char_array)

    It doesn't hurt to divide it out, it just (IMO) adds unnecessary clutter to the code. It doesn't really provide robustness to changes since, if one changes the type to array of int, fgets is no longer a valid function to use, so a whole new line of code must be there, not just changing a sizeof expression.

    Of course, sizeof doesn't work if you have a pointer (e.g. a buffer passed into a function) instead of an actual array, but that applies with or without the division.

  7. #7
    Registered User
    Join Date
    Mar 2014
    Posts
    4

    Question

    Quote Originally Posted by anduril462 View Post
    For general buffer sizes, you're right, you must divide. My point was, the lack of division and supposed lack of robustness is not an issue with char arrays; the division is rather pointless. Per the standard, sizeof(char) is always guaranteed to be 1, thus

    sizeof(any_char_array) / sizeof(any_char_array[0])
    is really just
    sizeof(any_char_array) / 1
    which is just
    sizeof(any_char_array)

    It doesn't hurt to divide it out, it just (IMO) adds unnecessary clutter to the code. It doesn't really provide robustness to changes since, if one changes the type to array of int, fgets is no longer a valid function to use, so a whole new line of code must be there, not just changing a sizeof expression.

    Of course, sizeof doesn't work if you have a pointer (e.g. a buffer passed into a function) instead of an actual array, but that applies with or without the division.


    Hi!

    What should I change for this code??


    Code:
    int main(int argc,char* argv[], PPMImage *img){
    
    
    char* filename = argv[1];
    
    
        FILE *fp;
    //open file for output
    fp = fopen(filename, "wb");
    if (!fp) {
         fprintf(stderr, "Unable to open file '%s'/n", filename);
         exit(1);
    }
    
    
    printf("image name: %s \n", filename);
    
    
    //write the header file
    //image format
    printf("magic number  = P3\n");
    
    
    
    
    //image size
    printf("width, height = %d, %d\n",img->x,img->y);
    
    
    // rgb component depth
    printf( "color val     = %d\n",RGB_COMPONENT_COLOR);
    
    
    
    
    
    
    
    
    PPMImage *image;
    
    
    image = readPPM("n44f.ppm");
    changeColorPPM(image);
    writePPM("n44fnew.ppm",image);
    printf("Press Enter");
    getchar();
    }

  8. #8
    Registered User
    Join Date
    Mar 2014
    Posts
    4
    Quote Originally Posted by Hodor View Post
    Anduril. I guess that's what you (I) get when you quickly scan through code.

    I saw

    Code:
    //read pixel data from file
    if (fread(img->data, sizeof(pixel_t) * img->x, img->y, fp) != img->y) {
         fprintf(stderr, "Error loading image '%s'\n", filename);
         exit(1);
    }
    And the rest is history.






    I disagree with this. Sure, fgets() can only use a pointer to char, this has nothing to do with the buffer size in my opinion.



    Hi!

    What should I change for this code??

    Code:
    int main(int argc,char* argv[], PPMImage *img){
    
    
    char* filename = argv[1];
    
    
        FILE *fp;
    //open file for output
    fp = fopen(filename, "wb");
    if (!fp) {
         fprintf(stderr, "Unable to open file '%s'/n", filename);
         exit(1);
    }
    
    
    printf("image name: %s \n", filename);
    
    
    //write the header file
    //image format
    printf("magic number  = P3\n");
    
    
    
    
    //image size
    printf("width, height = %d, %d\n",img->x,img->y);
    
    
    // rgb component depth
    printf( "color val     = %d\n",RGB_COMPONENT_COLOR);
    
    
    
    
    
    
    
    
    PPMImage *image;
    
    
    image = readPPM("n44f.ppm");
    changeColorPPM(image);
    writePPM("n44fnew.ppm",image);
    printf("Press Enter");
    getchar();
    }

  9. #9
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by 6globe View Post
    Below is my code:
    Your code looks a lot like somebody else's. That is called plagiarism, and the consequences can be quite severe. You are having trouble because you didn't write the code nor do you understand how it works. The only thing you've learned so far is how to copy code off the internet and post it in a forum as your own. If you actually spent any time reading and understanding the PPM format, and writing this yourself, you probably wouldn't have any trouble with it. The PPM format isn't difficult, nor is using fscanf and fprintf to read/write PPM files.


    Quote Originally Posted by 6globe View Post
    What should I change for this code??
    Nothing, it's perfect! Or maybe it isn't...I don't know. You haven't said what the new code is for or what it's supposed to do.

    Honestly, since it's not your code, I would change all of it. Start from scratch and write it yourself. Figure out a solution "by hand" before you start coding -- come up with a solid plan. Then, work systematically turning that solution into pseudo code and then to actual code. Work in small steps, 5-10 lines of code at a time. Compile and test often, fixing all errors, warnings and bugs. Repeat until finished.

  10. #10
    Registered User
    Join Date
    Mar 2014
    Posts
    4
    Quote Originally Posted by anduril462 View Post
    Your code looks a lot like somebody else's. That is called plagiarism, and the consequences can be quite severe. You are having trouble because you didn't write the code nor do you understand how it works. The only thing you've learned so far is how to copy code off the internet and post it in a forum as your own. If you actually spent any time reading and understanding the PPM format, and writing this yourself, you probably wouldn't have any trouble with it. The PPM format isn't difficult, nor is using fscanf and fprintf to read/write PPM files.



    Nothing, it's perfect! Or maybe it isn't...I don't know. You haven't said what the new code is for or what it's supposed to do.

    Honestly, since it's not your code, I would change all of it. Start from scratch and write it yourself. Figure out a solution "by hand" before you start coding -- come up with a solid plan. Then, work systematically turning that solution into pseudo code and then to actual code. Work in small steps, 5-10 lines of code at a time. Compile and test often, fixing all errors, warnings and bugs. Repeat until finished.

    Hi! anduril462,

    Thank you for your advise. I'm very new to ppm format file, and I'd like to learn more about how to read/write PPM files using C programming?

    Can you help me with this? Do I need to any functions to do this or just use a main function will do the work?

    Thank you!

  11. #11
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by 6globe View Post
    I'm very new to ppm format file, and I'd like to learn more about how to read/write PPM files using C programming?

    Can you help me with this
    There is no built-in support in C for PPM formats. Learning to read/write PPM files in C involves learning the PPM file format and learning how to read/write to file in C, then combining the two so you read/write the right information to/from the file.
    In post #3, I gave you a link that describes PPM file formats (that red text is a link, click it). It even gives you some sample PPM files and shows you the images they produce. You're using P3 which is a plain-text/ASCII format, so you can read in files by using fscanf and write files with fprintf.
    Quote Originally Posted by 6globe View Post
    ? Do I need to any functions to do this or just use a main function will do the work?
    You never need functions to solve a problem, you can do it all in main, but it's a very bad idea. Breaking your code up into functions helps you keep things organized and makes it easier for you to find and fix mistakes. It also allows you to reuse common code more easily.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Find closest point on a line?
    By xArt in forum C Programming
    Replies: 18
    Last Post: 04-14-2013, 12:50 PM
  2. Replies: 5
    Last Post: 11-08-2011, 11:29 AM
  3. Replies: 2
    Last Post: 04-27-2011, 04:34 AM
  4. How can i find one point inside an arc..?
    By jana.malli in forum C++ Programming
    Replies: 1
    Last Post: 02-09-2011, 01:46 AM
  5. program to find the saddle point of a matrix ??
    By intermediatech in forum C Programming
    Replies: 28
    Last Post: 05-05-2010, 05:26 PM

Tags for this Thread