Heyho,

FRIGN wrote:
> char *
> readimage(int fd, int *w, int *h)

I like returning a pointer to the data instead of using a separate argument for
data.

>       buf[9] = buf[19] = buf[29] = '\0';

Why do you overwrite the blanks before checking if they are blanks? If the file
format specifies these characters as blanks, they should be checked for.
Otherwise the format specification should not say „blank“ but „any one byte“.

>       *w = atoi(buf+10);
>       *h = atoi(buf+20);
>       total = (*w) * (*h) * 4;

The assignments could be included in the calculation, but this would be less
readable. I probably would have done it anyways.

> Now, what puzzles me is why no explanation is given on how the data
> itself should be stored. It says RGBA, so I suppose he meant
> 
> | 8 bit | 8 bit | 8 bit | 8 bit |
> |  red  | green |  blue | alpha |
> 
> for each pixel. This is rather trivial then, given an unsigned
> 8-bit-integer can be expressed with his according ASCII-char, basically
> turning this image file-format into a string:
> 
> imagefile 2 1 5%4&4"31

ASCII only uses 7bit and since you probably want to use this one extra bit as
well, I would at least consider the data portion to be binary and not a string.

> Let me know what you think. I suppose anything more complex than that will
> limit our possibilities and the flexiblity and simplicity of this format.

I also think it is a well defined format. If someone want's to transfer an image
over the network, he can easily use an arbitrary compression algorithm,
therefore we do not need one inside the format and also get rid of different
pixel formats (peek at the possibilities of DirectX if you want to throw up).
One problem could be if someone needs more than 8 bit per color channel, but I
think 8 bit are enough.

--Markus

Reply via email to