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?

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.