-3

This code (which I edited from code I found on a previous Stack Overflow question) is able to take the input of a ppm image and output the same image, but with all of the colors complementary. In my assignment, I am asked to do this, but then to swap the red and green components (basically make img->data[i].red into img->data[i].green and vice versa). However, I am unsure of how to go about doing this. I can't just make two equal statements, as the first would essentially nullify the second. The only thing that I can think of is to create a temporary array of the PPMImage type (a struct created earlier). However, I am a bit unsure of how the code and logic for this would work.

Here is the code. The 'for' loop in the changecolorPPM function is where the colors are complemented, and where I am pretty sure the swapping needs to occur.

#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 CREATOR "RPFELGUEIRAS"
#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] != '6') {
     fprintf(stderr, "Invalid image format (must be 'P6')\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, 3 * 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, "P6\n");

//comments
fprintf(fp, "# Created by %s\n",CREATOR);

//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, 3 * img->x, img->y, fp);
fclose(fp);
}

void changeColorPPM(PPMImage *img)
{
int i;

if(img){

     for(i=0;i<img->x*img->y;i++){
          img->data[i].red=RGB_COMPONENT_COLOR-img->data[i].red;
          img->data[i].green=RGB_COMPONENT_COLOR-img->data[i].green;
          img->data[i].blue=RGB_COMPONENT_COLOR-img->data[i].blue;
     }
}
}

int main(int argc, char* argv[]){
PPMImage *image;
char* filename = argv[1];
image = readPPM(filename);
changeColorPPM(image);
writePPM("OutputFile.ppm",image);
printf("Press Enter");
getchar();
}
Austin
  • 73
  • 1
  • 5
  • Search for "swap two values in C". – luser droog Apr 12 '13 at 23:41
  • 2
    I don't like this. You have an assignment and you are just copy-pasting it. It is bad on so many layers... But here is what you need: create temporary variable `int t` to store `img->data[i].red` then set `red` to `green` and then `green` to `t`. – Piotr Jaszkowski Apr 12 '13 at 23:41
  • This would work even though img->data[i].red/green needs to be an array? – Austin Apr 12 '13 at 23:43

1 Answers1

0

From your question, I believe you are looking for a swap function which could easily be implemented as

img->data[i].red = img->data[i].red + img->data[i].green;
img->data[i].green = img->data[i].red - img->data[i].green;
img->data[i].red = img->data[i].red - img->data[i].green;

Update:

This section is fine if there is no overflow from the addition operation. However, if the precision of the destination is not sufficient enough to hold the bits, then there could be potential loss of data. Hence, in these cases, as Mats and other experts suggested, the traditional temporary variable method would be the best way forward

temp = img->data[i].red;
img->data[i].red = img->data[i].green;
img->data[i].green = temp;

Note:

If you are into image processing, then this swap may not be pleasing from a visual perspective. Usually, Green contains the maximum intensity i.e. amount of brightness information as compared to Red and Blue and human eye can catch the variations in intensity very easily. It will interesting to check, how the final picture looks like after the swap.

Ganesh
  • 5,880
  • 2
  • 36
  • 54
  • Ah, I can see your logic here. Not to mention, this could come in handy in a lot of other situations as well. To be honest, I never even though it could be that simple lol. Thanks a lot! – Austin Apr 12 '13 at 23:46
  • @Austin.. Nice.. Please check my updated answer with a note at the end. This could be interesting :) – Ganesh Apr 12 '13 at 23:47
  • 3
    It is very unlikely that this is more efficient than `t = img->data[i].red; img->data[i].red = img->data[i].green; img->data[i].green = t;" - this is how a normal `swap` function is typically written. – Mats Petersson Apr 12 '13 at 23:50
  • @Mats This also sounds like a good alternative. As far as this logic looks, both swaps will do the same thing. Thanks for the alternative! – Austin Apr 12 '13 at 23:54
  • 1
    @Ganesh I implemented your function and it appears as if it didn't do anything too drastic. I used a ppm image of the Ireland flag that I made for another assignment. It turned light green to hot pink white to black and light orange to light blue. Thanks again! – Austin Apr 12 '13 at 23:58
  • 1
    Yes, my concern is with performance, not "what it actually does". And white should not to black from removing green. It should become a purplish colour - you're probably doing something wrong... – Mats Petersson Apr 13 '13 at 00:02
  • @Austin.. Just out of curiosity.. If you try the temporary variable method as suggested by Mats, do you observe a different result? – Ganesh Apr 13 '13 at 00:04
  • @MatsPetersson.. I have updated my answer with your recommendation also. – Ganesh Apr 13 '13 at 00:08
  • @Ganesh - Nope, no difference. Same result as your method. – Austin Apr 13 '13 at 03:46