How to write image data to binary PGM file format(P5)?

Hi all,
I fail to write the image data to binary PGM format (P5).
Let's say i have to write a image data into PGM P5 format.
I have the information for width, height, Y_min, Y_max, XL, XR and the TEMPLATE1 (data matrix) ==>A region of interest from an image.
Y_min & Y_max = minimum and maximum of the height value.
XL & XR = min and max of the width value for the image.

First, i call the subroutine,

Code:

---

write_data_grey_binary(width1,height1,XL1,XR1,Y_min1,Y_max1,filename99[total],TEMPLATE1);

void write_data_grey_binary(width,height,XL,XR,Y_min,Y_max,filename,output)
int width,height,XL,XR,Y_min,Y_max;
char *filename;
unsigned char** output;
{
    int i,j,nread;
    FILE *fp;
    unsigned char* temp;

    if((fp = fopen(filename,"w")) == NULL){
        printf("File 333 output data  can not open\n");
        exit(1);
    }
       
        fprintf(fp,"P5\n%d  %d\n%d\n",width,height,(GRYSCL-1));

        temp = (unsigned char*)calloc(height*width, sizeof(unsigned char));

        for(i=Y_min;i<=Y_max;i++){
                  for(j=XL;j<=XR;j++){ 
                    temp[(i*width)+j]= (unsigned char)output[i][j];
                }
        }
        nread = fwrite((void*)temp, sizeof(unsigned char), (height*width), fp);
        free(temp);
                fclose(fp);
        return(0);
}

---

However , i have an error at the line " temp[(i*width)+j]= (unsigned char)output[i][j]; ".
However, this coding can be ran when i want to write the full image like:
write_data_grey_binary(width,height,0,width-1,0,height-1,filename99[total],inpgrey);

Why not just copy temp then increment it in your loop? It would be faster and easier to read. IE:

Code:

---

unsigned char *ptr = temp;
for(i=Y_min;i<=Y_max;i++)
{
    for(j=XL;j<=XR;j++, ptr++)
    { 
        *ptr = (unsigned char)output[i][j];
    }
}

---

[edit] What error messages do you get? [/edit]

Is this part of an existing bit of old code? The old-style function prototype tells me that it is.

I don't see the point of the cast here:

Code:

---

temp[(i*width)+j]= (unsigned char)output[i][j];

---

Both sides are already given as "unsigned char", so the cast doesn't change anything.

Also, there's no need to cast the return value of calloc:

Code:

---

temp = (unsigned char*)calloc(height*width, sizeof(unsigned char));

---

This just leads to problems if you don't include the correct header files.

You also seem to mix text (fprintf()) with binary output to the file. That's generally not a good plan.

--
Mats

Thanks for the advices guys. I have solve the problem.

Why are you using temporary space? There's no need for it...

Code:

---

for(i=Y_min;i<=Y_max;i++){
        fwrite(output[i] + XL, 1, XL - XR + 1, fp);
}

---

Also, your loops are based on XL..XR and Y_min..Y_max, and yet the values for width and height you output to the file come from somewhere else. This means that if somebody passes a width that is not exactly XL - XR + 1, or a height which is not exactly Y_max - Y_min + 1, things blow up. Why pass the width and height at all, since they can be computed from the rectangle?

Quote:

---

Originally Posted by matsp

You also seem to mix text (fprintf()) with binary output to the file. That's generally not a good plan.

---

No problem mixing fprintf() and fwrite(). Writing to files would be a true pain if it were otherwise.

Thanks a lot for these advices. I have learned a lot from here though the problem has been solved.
Thanks guys.