--------------------------------------------------------------------------

> I don't understand which functions go where.  You say all of the ppm
> functions should be in a file called ppm.c.  Does that include all of
> the function prototypes you listed?  So the menu functions go in im.c?

No -- the ppm functions that should go in ppm.c are the ppm file
reading/writing functions.  The other functions, including the
menu functions, lighten/darken functions, flip, and rotate, go in
im.c.


> And would the structs go in ppm.h?

The structs contain data values that are not necessarily exclusive
to ppm files, so they would be better suited for im.h.

--------------------------------------------------------------------------

> Why 8 numbers per line instead of 9 for writing the ascii pixel data?

The specs say 8 RGB values, meaning 8 sets of 3 short ints (24 values total).
I chose 8 because it was a power of 2 and because 24 values makes a line in a
file of reasonable size for viewing.

--------------------------------------------------------------------------

> ... For some
> reason in the conversion from ascii to binary (using xv's 'save as' ppm
> raw format) it changes the ascii header values of the file. After saving a
> raw ppm file from some ascii ppm file the width, height and max_colors (before
> even being run in my program) is different from what its supposed to be.
> Although when I open the new raw file in xv it looks ok, how could it look
> the same when the height and width are multiples of what they are supposed
> to be?
>
> the ascii file starts
>
> P3
> 4 24
> 15

This is a very unusual image -- it's incredibly small (4x24) and the max_color
value doesn't really make sense.  Use the handin command to put the image in
your directory; I'd like to see it.


> and the binary file starts - keep in  mind this has nothing to do with the
> way my program is processing it, this is just what the binary text file
> looks like when it is converted.
>
> P6
> 68 24
> 255

xv probably has some conventions for the smallest size a file can be,
along with the smallest max_color value possible.  Since the binary form
of ppm files uses 8 bits to store a color, the max value has to be 255.

The images look OK when viewed because xv probably performs some data
modifications to get it in the form it wants.


> Since that happens, my program reads in this information and the output is
> completely off. When I read in an ascii file and write it as binary it
> works just fine since it has the right height, width, and max_color
> values. Am I missing something??

Test your program with the test image from the webpage and other valid
images you can find, perhaps in jpeg format.  Then, use xv to convert
them to put them in proper ppm format before your program reads them in.

--------------------------------------------------------------------------

> Another thing- if we increase black (0,0,0) by 10%, we still get black- how
> should we handle this?

Just keep it at (0, 0, 0).


> Also, when rounding, is it ok to just round the calculated values down
> (truncating), since this is how C converts a float calculation to a int type
> anyway?

That's fine.

--------------------------------------------------------------------------

> With the extra credit should comments in the header be ignored, you just
> want us to look at the pixel values themselves?

Right -- ignore the comments and any white space (like newlines).  Just make
sure the file data (height, width, max_color, and all the pixels) are the same.

--------------------------------------------------------------------------

> Any idea why rotating an image once would work, then when I try to rotate
> the image that has already been rotated it rotates it all the way back to
> the original state ? It will rotate 90 degrees to the right (CW) then when
> I rotate that... it rotates 270 degrees back to the begining ?!? On the
> third try it rotates that image to the right like before but messes up
> again on the fourth try.

Are you really rotating the image, or just exchanging the rows and
columns?  There is a difference.

--------------------------------------------------------------------------

> i found that if the RGB value continues to be multiplied by 1.1, then it
> makes the round back to 0...254, 255, 0, 1, 2...
>
> so i just want to confirm that 255 is the max max_color

Right, so before assigning the new value, check to make sure that it
doesn't overflow (that is, > 255).  If so, set the value to 255 since
that's as big as it can get.  We won't be testing files that have a
max_color greater than 255.

--------------------------------------------------------------------------

> In ppm.h, I assume we are supposed to do some kind of structure "prototype,"
> but I couldn't figure out how to do it.  I tried things like "typedef struct
> IMAGE_T;" but nothing worked, so I just included im.h.  I later looked at the
> book, and it basically said to do that (bottom of p.271).  The problem comes
> when I try to include im.h in more than one file.  I get errors like this:
> im.h:5: conflicting types for `COLOR_T'
> im.h:5: previous declaration of `COLOR_T'

Including im.h is the right thing to do.

Your problem with the file being included multiple times was mentioned
in class.  There's an example of how to do this properly in the handout 
I passed out -- try to get a copy of it if you don't already have it.

--------------------------------------------------------------------------

> Is there any way to check if we've properly freed all of
> the memory we used?

There are some ways to do this, but they're too complicated to explain
by e-mail.

--------------------------------------------------------------------------

> Do we have to do any error checking on the filenames the user
> types in (that would get pretty nasty, so I'm assuming no)?

No.

--------------------------------------------------------------------------

>         in my lighten and darken I think the decimals that I am getting
> are slightly off from the ones on your websites... the images look almost
> identical but when u switch from one to the other you can see that there
> are very tiny changes- so I think the way I am rounding and the way you
> are rounding are two different ways- any way I should handle this ??

The way I handled it in my code was simply to multiply the R, G, and B
values by 0.9 (for darken) and assign them to the original fields.
Casting is done implicitly in this case, but you could also explicitly
cast the resulting float with (unsigned char).

--------------------------------------------------------------------------

> You say you want the values within the COLOR_T struct to be
> unsigned characters, but wouldn't they need to be integers to hold the
> values read in from the file?  I was assigning integer values to chars
> earlier, which is where my problem was.  Anyway, maybe you can help me
> on this?  At this point, I'm using the following struct:
>
> typedef struct {
>   int R, G, B;
> } COLOR_T;

Keep in mind that char's are really int's.  The unsigned char is a
perfect data type to hold our R, G, B values because they're just
the right size: 8 bits (color values run from 0..255).

To read in values from the ascii ppm file, you can use fscanf with
%hd format strings, or read them into int's and cast them to
unsigned char's when assigning them.

--------------------------------------------------------------------------

> When freeing the memory of the 2-d array that I created how do I need to do
> this do I have to go back  and free every one that I malloced? If so how do
> it get at the pointer I need? I tried to free it in a loop using
> free(im->pixels[i][j]); but it just gave me errors. This was similar to how
> I malloced the space the first time. Can you give me any pointers?

Nice pun.

Yes, you have to free all of the memory you allocated when you no
longer need it.  The pointers can be accessed the same way you
accessed them when you allocated the memory (im -> pixels [i]).

--------------------------------------------------------------------------

> The problem I have stumbled upon is when I try to free memory in the 
> rotate method.  If I comment out the portion of code intended to free 
> the imprisoned memory, the rotate method works beautifully.  When I do 
> actually try to free memory, I get a seg fault.  Obviously I am not 
> getting something here -  could you please advise.

You should free your space in the same way you malloc'ed it -- one full
row at a time.  That's the general rule: you can only free memory that
you malloc'ed.  You didn't malloc space for each pixel, so you can't free
space for each pixel either.  Plus, each pixel is a struct and not a pointer,
and you can only pass pointers to free().

--------------------------------------------------------------------------

> I don't have a binary example to read in to see if there should in fact 
> be a visible difference in the appearance. Kind of confused on this matter.

There should be no visible differences between the two files.  If you'd
like to test on a binary file, use xv to save out a binary (raw) ppm
files (just load it up with the balloon image and save it as binary).

When you're confident things are working, read in the binary file that
your program wrote out for further testing.

--------------------------------------------------------------------------

> Could you explain a little more about the extra credit and the format you
> want it in. Should we just print a message to the screen if their the same
> or not? Also I'm assuming we can use functions in ppm.c in the ppmdiff.c
> file, will the graders know to link these two files?

You can just print a message stating that the files are the same or
they're different.  Keep in mind that comments don't count.

And yes, you should re-use your ppm code for the ppmdiff.  Place
instructions in the header comments on how to compile for the graders.

--------------------------------------------------------------------------

> I'm having some trouble reading in the ASCII RGB values.  What is the
> right way to do this?

I think the easiest way to handle this problem is to read the values into
temporary int variables using fscanf().  You can use %d in the control
string, as in

  fscanf (fp, "%d %d %d", &R, &G, &B);

where R, G, and B are local int's.

Then, you can assign the R, G, and B fields in the corresponding pixel
to these int's.  You will probably want to use a cast:

  im -> pixels [y] [x].R = (unsigned char) R;

For writing, you can just use:

  fprintf (fp, "%d %d %d ", im -> pixels [y] [x].R, ...);

--------------------------------------------------------------------------

> My flip_image() function is giving me a seg fault.  I have allocate memory
> for my temp IMAGE_T variable and tried using GDB but GDB gives me a "no
> debugging symbols found".

First of all, you don't really need to allocate or re-allocate memory in 
flip_image since the flipped image is going to have the same dimensions.


> Here's the new temporary image.
> 
>  IMAGE_T *temp;

The second problem is that even if you do create new space, you don't need
a whole new variable of IMAGE_T.  You just need a new variable of type
COLOR_T **.  You can allocate space for that, then just assign the pixels
pointer in your original IMAGE_T structure to point to the new space (after
you've free'd the old memory, of course).

Third, even if you do create a new IMAGE_T variable, it shouldn't be a 
pointer; it should just be a variable of that type of structure.  If you
were, however, to create a variable of IMAGE_T *, you would need to use
malloc to allocate space for it before accessing any of the fields (e.g.,
temp -> pixels).

--------------------------------------------------------------------------

> When trying to read in my binary file (which i created using xv) i am
> getting an address out of bounds seg fault.
>
> Here is my code:
> ...
>   char line[im->width];
>   unsigned char b_line[im->width*3+1];

C does not support dynamic arrays, so you should not use them.
(A dynamic array is one in which the dimension cannot be
determined until runtime.)  The array dimension must be a
constant.

--------------------------------------------------------------------------

> I have a question regarding the lighten_image function also. I traverse the
> two-dimensional pixel array and I multiply each <RGB> value by 1.1 by doing 
> the following:
> ...
>             im->pixels[y][x].R *= 1.1;
> ...

The problem is that you're overflowing the RGB values before checking them.  So,
you need to assign the product of 1.1 and each RGB value to a temporary variable,
then check that before assigning it to the RGB unsigned char's.

--------------------------------------------------------------------------