• Welcome to Overclockers Forums! Join us to reply in threads, receive reduced ads, and to customize your site experience!

I'm having trouble importing a text file into a 2D array with C

Overclockers is supported by our readers. When you click a link to make a purchase, we may earn a commission. Learn More.

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
Hi guys,

I'm working on a project that involves importing pgm files into a 2D array and manipulating the data with functions.
Basically, I have a file with:

P2
256 256
255

on the first three lines that needs to be discarded. I've decided to "dummy" read them into variables so I can access the data that I want. After that there are plain integers in rows and columns, with 256 rows, and 256 columns. The problem is that I can't figure out how to actually get the data I need into the required columns and rows. I'm having trouble with the fscanf function; it's just not working.

This is my source code so far:
Code:
#include <stdio.h>
#include <stdlib.h>


int main ()
{
FILE *inFile;
char filename[13];
int imagearray[256][256];
double dummy1;
int dummy2, dummy3, dummy4;


printf("Enter an image file to be processed:\n ");
gets(filename);

inFile = fopen(filename, "r"); //Open the file

if (inFile == NULL)
{
printf("The file that you entered does not exist\n");
printf("Please check the name and try again\n");
exit(1);
}

else
{
printf("The file is ready for processing\n");
printf("Starting image processing");

fscanf(inFile, "%lf %d %d %d %d", &dummy1, &dummy2, &dummy3, &dummy4, imagearray); //Scan the dummy variables and place data in to the array
}

fclose(inFile);

return 0;

}
Any ideas on what I should be doing?
 

marker

Member
Joined
Mar 20, 2005
You can't read in the entire array at once like that, you need to read an entry at a time. Here is the basic idea:

Code:
	#define SKIPLINES (3)
	#define ROWS (255)
	#define COLS (255)
	
	FILE *inFile;
	char filename[255];
	char line[255];  // Make the assumtion that a line < 255 chars
	int imagearray[ROWS][COLS];

	//
	// SETUP CODE ...
	//
	
	// Read in and discard first three lines
	for (int i = 0; i < SKIPLINES; i++) fgets(line, sizeof(line), inFile);
	
	for (int row = 0; row < ROWS; row++)
	{
		for (int col = 0; col < COLS; col++)
		{
			fscanf(inFile, "%d", &imagearray[row][col]);
			printf("%d\n", imagearray[row][col]);
		}
	}
	
	fclose(inFile);
 

mccoyn

Senior Member
Joined
Nov 17, 2003
Location
Michigan, USA
I have some tips about reading binary data in C.

First, when you use fopen on binary files you should open in binary mode. The default ascii mode will do funny things if you ever encounter and EOF byte.

Code:
infile = fopen(filename, "rb");

Second, fscanf looks for numbers encoded as a series of ascii characters, but in a binary file like PNG the numbers are encoded as binary data. The fread function gives a good way to get at this binary data.

Code:
fread(&imagearray[row][col], sizeof(int), 1, inFile);
 

marker

Member
Joined
Mar 20, 2005
If it is in binary, and you know the size and order of the array (row major or column major), you should be able to read the entire array in one statement

Code:
fread(imagearray, sizeof(int), ROWS * COLS, inFile);
 

mccoyn

Senior Member
Joined
Nov 17, 2003
Location
Michigan, USA
I'm sorry. Having reread the post I realize that its a pmg file, not a png file like I thought. So the whole binary business is wrong. The easiest way will be to call fscanf for each element in a loop as marker showed.
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
I've run into another pretty odd problem. I think my logic is good, but I can't get my program to do what I want. It will run, but it crashes once it hits the nested If statements. Basically, I want it to comb through the array from [0-255][0-255] looking for pixels that are black (0). If all of them are black, then the pixel is changed from black to white (255).

My code is as follows and please excuse the terrible presentation style. I'm still learning how to present my code.

Code:
for (row = 0; row < ROWS; row++)
		{																	 
				for (col = 0; col < COLS; col++)			
				{														
							if (imagearray2[row][col] == 0)
							{
									if (imagearray2[row + 1 ][col + 1] == 0)
									{
											
											
									}
										else if (imagearray2[row + 1][col - 1] == 0)
											{
													
																
											}
												else if (imagearray2[row - 1][col + 1] == 0)
												{
														
													
												}
													else if (imagearray2[row - 1][col - 1] == 0)
													{
															
														
													}
														else if (imagearray2[row + 1][col] == 0)
														{
															
																
														}
															else if (imagearray2[row - 1][col] == 0)
															{
																	
																	
															}
																else if (imagearray2[row][col + 1] == 0)
																{
																		
																		
																}
																	else if (imagearray2[row][col - 1] == 0)
																	{
																	
																			
																	}
								fprintf(outf2, "%d ", imagearray2[row][col]);
				} 
				
					
								else 
								{
										imagearray2[row][col] = 255;
										fprintf(outf2, "%d ", imagearray2[row][col]);
								}
								
		}
 

mccoyn

Senior Member
Joined
Nov 17, 2003
Location
Michigan, USA
I think the reason it crashes is because you have some out of bounds array indexes. Look at this if statement:
Code:
            else if (imagearray2[row - 1][col - 1] == 0)
When row is 0 and col is 0, then both indexes in this array will be -1. This can cause the program to crash.

There are a couple ways to deal with this, depending on how you want to treat out of bounds values. You can assume they are all non-zero. In this case, none of the edge cases will be set to white and you can just work on the interior region by changing the bounds of your loops.
Code:
for (row = 1; row < ROWS-1; row++)
{	 
    for (col = 1; col < COLS-1; col++)
    {

Its a little trickier if you want to assume the out-of-bounds elements are zero. In that case you will have to check the bounds before doing the comparison.

Code:
            else if (  (row >= 1)
                     &&(col >= 1)
                     &&(imagearray2[row - 1][col - 1])  )

When this if statement is compilied it is short circuited. If the first condition is false it never evaluates the second and third conditions. So it will never evaluate the last condition when it is out of bounds.

All these if statements inside a loop really slow down the CPU. I recently optimized a bit of 3D filter code that was very similar to what you are doing. The original code had bounds checking like my last example. I found that if I wrote a separate piece of code for the interior of the volume that didn't do bounds checking (like the first example) that ran much faster. The exterior used the code with bounds checking. All and all it ran 3 times faster since 99% of the elements didn't need any bounds checking of any kind.
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
*EDIT*

Original text: Hmm, I see what you mean. I never noticed that before. Since I'm primarly concerned with getting the programing working, I think I might sacrifice some speed in order to check the outer bounds. I'll impliment your suggestion and I'll see how it goes.

Thanks mccoyn

I've been messing around with the code for the last 2 hours and I still can't get the thing to do what I want. I've even switched over to the 'inner function' that mccoyn provided. The picture comes out stretched and distorted.

This is what I have so far:

Code:
for (row = 1; row < ROWS-1; row++)					
	{	 
    		for (col = 1; col < COLS-1; col++)
    		{
					if (imagearray2[row][col] == 0)
					{
						if (imagearray2[row + 1][col + 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row - 1][col - 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row + 1][col - 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row - 1][col + 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row][col + 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row][col - 1] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if	(imagearray2[row + 1][col] == 0)
						{
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						else if (imagearray2[row - 1][col] == 0)
						{
							imagearray2[row][col] = 255;
							fprintf(outf2, "%d ", imagearray2[row][col]);
						}
						
							imagearray2[row][col] = 0;
							fprintf(outf2, "%d ", imagearray2[row][col]);
					
					}
						
				else if (imagearray2[row][col] == 255)
				{
						imagearray2[row][col] = 255;
						fprintf(outf2, "%d ", imagearray2[row][col]);
				}
				
			}
			
			
	}
	
	return;
	
}
 
Last edited:

Adak

Senior Member
Joined
Jan 9, 2006
CAPiTA said:
*EDIT*



I've been messing around with the code for the last 2 hours and I still can't get the thing to do what I want. I've even switched over to the 'inner function' that mccoyn provided. The picture comes out stretched and distorted.

This is what I have so far:

Code:
for (row = 1; row < ROWS-1; row++)	  {	 
   for (col = 1; col < COLS-1; col++)   {

      if (imagearray2[row][col] == 0)    {
         if (imagearray2[row + 1][col + 1] == 0)   {
	imagearray2[row][col] = 0;
	fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row - 1][col - 1] == 0)  {
            imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row + 1][col - 1] == 0)   {
	imagearray2[row][col] = 0;
	fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row - 1][col + 1] == 0)   {
            imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row][col + 1] == 0)   {
            imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row][col - 1] == 0)   {
            imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row + 1][col] == 0)   {
            imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         else if (imagearray2[row - 1][col] == 0)   {
            imagearray2[row][col] = 255;
            fprintf(outf2, "%d ", imagearray2[row][col]);
         }
         imagearray2[row][col] = 0;
            fprintf(outf2, "%d ", imagearray2[row][col]);
      }
      else if (imagearray2[row][col] == 255)  {
         imagearray2[row][col] = 255;
         fprintf(outf2, "%d ", imagearray2[row][col]);
      }
   }
}
return;
}

OK, now that we've got the code all in the same time zone! :D

(just my idea on formatting: 'C' is a concise language. Your coding style should reflect that, within normal bounds of ease of understanding. Indenting 10 spaces for each level is a big waste of valuable screen space. 3 or 4 should suffice.)

What you've got here is the if statement from hell. :p Always to be avoided when you're first working up a program. It may seem "foolish" to do a number of if statements in the middle of the inner loop, but "foolish" can look very smart indeed, when you have only ONE set of inner statements to look at to debug, for every "state" that you want your program to do something with.

Really cuts down on de-bugging time.

This part is certainly hard to follow:

imagearray2[row][col] = 0;
fprintf(outf2, "%d ", imagearray2[row][col]);

Because you have it indented in your code as if it was part of an else if statement, and it is actually depending on an if statement far higher up, and not so indented.

What are the specific states that your program is looking for? Please list them fully, and number each one.

Now in your code, please number each block of code that is to work with each state of the array value.

Your code looks a lot like my chess program to generate moves by the pieces. My advice is to get it all ORGANIZED, and noted with helpful remarks.

//tests for state #1, value is white and ...
if(yadda > zappa)
...

//tests for state #2, value is black and the value at 4 o'clock is white.
if(whatever....

You get the idea.

Now as you step through your code, you'll be able to see what's going on, and what should NOT be going on. I'd bet $$$ that your code logic is "over-lapping", that is, some code is being handled once by your program, and then it's being handled (mishandled), again, because it's new value inadvertently falls into that logic "bucket".

Last but not least, you may have a problem with your aspect ratio on your screen. When I changed over from Win98 to Win2000, I also changed my monitor.

And now ALL my old console programs are distorted, because the new monitor has a different aspect ratio. If you're taking a graphics array which was displayed in graphics mode, and printing it out again in text mode, of course, it may appear all weird and distorted, even though your program is working just fine.

Hope that helps.

Adak
 

marker

Member
Joined
Mar 20, 2005
I can't really help unless (like Adak requested) you fully list and describe the specific states that your program is looking for.
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
Thanks for the follow ups. I'm not sure what you mean by states, but I'll try to interpret it as best as I can. I've modified the code again, and hopefully the comments will clear up some confusion. As of this instant, the code works but it changes everything to white.

e.g. This is what I start with:


and this is what I'm supposed to end up with



This is the revised code:

Code:
for (row = 1; row < ROWS-1; row++) 		           
{	 						//Starts scanning through the array that was imported, ignoring the outer rows and columns
    	for (col = 1; col < COLS-1; col++)		// Array has 256 rows, 256 columns, with either black (0) or white (255) pixels.
    	{
		if (imagearray2[row][col] == 0) //Checks to see if a pixel is black, if so then start checking the pixels around it
		{
					
								
			if (imagearray2[row + 1 ][col + 1] == 0)    // Checks right upper diagonal pixel             
			{													        //	 xxx
			}														//       x0x   e.g. If 0 is black, and all x's are black, then 0 is turned to white
			else if (imagearray2[row + 1][col - 1] == 0)   // Checks left upper diagonal pixel		 		//	 xxx
			{
			}
			else if (imagearray2[row - 1][col + 1] == 0)	//  etc.
			{
			}
			else if (imagearray2[row - 1][col - 1] == 0)
			{
			}
			else if (imagearray2[row + 1][col] == 0)
			{
			}
			else if (imagearray2[row - 1][col] == 0)
			{
			}
			else if (imagearray2[row][col + 1] == 0)
			{
			}
			else if (imagearray2[row][col - 1] == 0)
			{
			}
							
			imagearray2[row][col] = 255; //If all of the pixels below are black, then the pixel that was originally being checked is turned to white
			fprintf(outf2, "%d ", imagearray2[row][col]); //Overwrites the old pixel value with the new one (white if all conditions are met)		
																	
		}//end main if statement
						
	}// end second for loop
	
} //end initial for loop
 

Adak

Senior Member
Joined
Jan 9, 2006
I'm not sure what you mean by states ...

A "state" is a condition of the data that your program is testing for and/or changing.

State #1 could be when a black value is surrounded by white values, and you want to change the white values in this state, to black, and the black values, to white.

Is this just for pics? Just to create a negative image?

Do you (this is a killer, perhaps) KNOW THAT C HAS A COMMAND TO DO THIS ?? :mad:

If you want to do something else with the pic, you'll need your big-time loops. But not for a simple change to a negative of the original pic. C has your back on that one.

To continue though with your loop logic. What you want to do when coding such is to add the "continue" command to your if(), or else if() statements.

That prevents values that you've already changed, from falling inadvertantly into another if() or else if() block of code, and being processed AGAIN, by mistake.

Which I'm sure is your problem, here. You have no "defensive" code logic, and a big-time for loop, to tackle.

Capita, I'll do whichever you choose here:

1) I'll take your code, and correct it, using your for loops, and the "brain scan" image you have linked to.

2) I'll post up code using C's built in negative to positive color changer.

Whichever you like, mate.

I'm off running around for the afternoon (Pacific Time), but let me know.

Adak
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
Thanks for the help Adak. Unfortunately, it's not a simple negative image convert ;). When you said that though, I got pretty excited until I realized that it's not the same thing. Bah. I guess the state could be described as taking a black pixel and changing it to white IFF it it surrounded by 8 black pixels. I posted this basic ASCII image, but I'll post it again to clarify things:

Code:
xxx
x0x             where 0 is the black pixel in question, and x are its surrounding 
xxx             pixels. So if all the Xs are black, then 0 is changed to white.           
                 The other pixels are unchanged.

I think I'm going to mess around with that continue if() statement that you were talking about. I'll do some research on the net and see if I can figure out how to use it. As for your help, I think I'm going to mess around with that statement first before I come running :)

That's not to say I'm not extremely grateful for your help; I really appreciate it.
 

Adak

Senior Member
Joined
Jan 9, 2006
CAPiTA said:
Thanks for the help Adak. Unfortunately, it's not a simple negative image convert ;). When you said that though, I got pretty excited until I realized that it's not the same thing. Bah. I guess the state could be described as taking a black pixel and changing it to white IFF it it surrounded by 8 black pixels. I posted this basic ASCII image, but I'll post it again to clarify things:

Code:
xxx
x0x             where 0 is the black pixel in question, and x are its surrounding 
xxx             pixels. So if all the Xs are black, then 0 is changed to white.           
                 The other pixels are unchanged.

I think I'm going to mess around with that continue if() statement that you were talking about. I'll do some research on the net and see if I can figure out how to use it. As for your help, I think I'm going to mess around with that statement first before I come running :)

That's not to say I'm not extremely grateful for your help; I really appreciate it.


Yes, I looked back at the pic you posted, and saw that it was not a simple reversal of the image.

Your x's and 0 really clarified the algorithm, GREATLY!! :cool:

May I make a suggestion? The state you're looking for is just the one where the black value is surrounded by the black values.

So we could break this down as:

#1. The sub-state where the black value has three black's above it in the array. (which could break down further into sub-states, but not necessary unless you want to)


AND

#2. The sub-state where the black value has three black values below it in the array.
(Which could break down into further sub-states, but we don't have to go there unless we want to, since we can test for all three in one if()

AND

#3. The sub-state where the black value has a black value on it's left side

AND

#4. The sub-state where the black value has a black value on it's right side.

We COULD put this all in one large (OK, huge! :p) if statement, but we could also use 4 flags and keep it simple.

Flag #1 - high_ok: high_ok is 1 (true), when the row over the black value is all black's value.

Flag #2 - lower_ok: == 1 when the row lower than the value being tested is all black's value.

Flag #3 - side_ok: == 1 when both sides are black to the value being tested.

So now if high_ok AND lower_ok AND side_ok are true, then we want to change the value being tested to white.

Your algorithm becomes very complex or fails unless you have two arrays for this. One array is your original, which you refer to for getting the proper values to be tested, and one array is for the values that have already been tested.

Otherwise, you'll be making changes, and then making tests of those values that have already been changed, and that ruins the logic.

Say you test the value at array[10][10], and change it to white. Now when you test array[11][10], and use the NEW value for [10][10], your algo will not work. You have to refer to the ORIGINAL values of [10][10], not the NEW value.

See what I mean?

I would have butted out of this, but I don't think you'll find the help you're looking for on Google. If I'm wrong, let me know.

Adak
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
Ah! You made a really good point when talking about the need for a second array. I never thought about the consequences of using a the same array and getting conflicts from earlier changes. I remember trying the one huge if statement, and it didn't seem to work either. It would crash before writing in the new values. But I'm going to take it into consideration once again and hope I can get it to work. I'm pretty excited now, I think it's going to work.
 
OP
CAPiTA

CAPiTA

Member
Joined
Mar 7, 2004
Location
Canada
It works! I made the huge if statement, but I was still getting weird results. I broke the code down into seperate parts and messed with every variable I could. I turns out the second for statement was incorrect :
for (col = 1; col < COLS-1; col++) when it should have been for (col = 0; col < COLS; col++).

Once again, thanks a lot guys!
 

Adak

Senior Member
Joined
Jan 9, 2006
CAPiTA said:
It works! I made the huge if statement, but I was still getting weird results. I broke the code down into seperate parts and messed with every variable I could. I turns out the second for statement was incorrect :
for (col = 1; col < COLS-1; col++) when it should have been for (col = 0; col < COLS; col++).

Once again, thanks a lot guys!

That statement form: for(variable = 0; variable < array_max; variable++), is the standard form - memorize that, dream about it, stamp it onto your forehead. :p

Glad it all came together, Capita.


Adak
 

stui_nz

Member
Joined
Feb 8, 2003
Location
New Zealand
sorry for waiding in here a little late....


if i understand your algorithm correctly you dont need all those if statements..

just sum all the pixel values in the block of nine... if the sum is zero, then all pixels must be zero...

shorter code, faster execution, less confustion...



Code:
//untested code
int sum;
for (row = 1; row < ROWS-1; row++}{	 					
    for (col = 1; col < COLS-1; col++){
        sum=0;
        for (int i=-1; i<=1, i++){
            for (int j=-1; j<=1, j++){
                sum+=imagearray2[row+i][col+j];
            }
        }
        if (sum==0) newimagearray[row][col]=255;
    }
}
 

Adak

Senior Member
Joined
Jan 9, 2006
stui_nz said:
sorry for waiding in here a little late....


if i understand your algorithm correctly you dont need all those if statements..

just sum all the pixel values in the block of nine... if the sum is zero, then all pixels must be zero...

shorter code, faster execution, less confustion...



Code:
//untested code
int sum;
for (row = 1; row < ROWS-1; row++}{	 					
    for (col = 1; col < COLS-1; col++){
        sum=0;
        for (int i=-1; i<=1, i++){
            for (int j=-1; j<=1, j++){
                sum+=imagearray2[row+i][col+j];
            }
        }
        if (sum==0) newimagearray[row][col]=255;
    }
}

Thanks for adding this. Conceptually, a fine block of code. In this case though, all the if statements can be reduced to just one large one. Which has the speed advantage because it will "short circuit" and break out when the condition is no longer true. Saving a lot of time. Something along the lines of this, where r represents the row variable, and c represents columns, and a[] represents original array[].

Code:
/* already inside the double for () loops, with row and col NOT on the edges 
    this code is untested! */

if ((!a[r-1][c-1]) && (!a[r-1][c]) && (!a[r-1][c+1]) && (!a[r][c-1]) && (!a[r][c+1]) && (!a[r+1 ][c-1]) && (!a[r+1][c]) && (!a[r+1][c+1]))
   newimagearray[r][c] = 255;

Although this code is faster I belive, you can see why I hesitate to recommend it! :D :D

To handle the edges of the array, I'd think about making the array I was putting the pic array into larger, so it had a border all the way around it, ie, array[257][257], and then using a loop to set the border cells a value of -1 or some such. Then the above code should work with no special "border" logic to slow it down.

We do this in chess programming to help our pieces stay on the board. It's a hell of a laugh when the pawn on the far right hand column, takes an enemy piece, and miraculously "travels" to the far left hand column of the board! "My Program is a CHEATER!!" :clap: :clap:

I'm glad you brought this up, because the OP didn't show just what he was doing now inside the double for loops, and obviously that is a fertile ground to look for substantial speed up's.

There is a still faster way to do this, called "0X88", but it's rather arcane to understand. It's the fastest "pure" board representation in chess programming. If you'd like to read up on it (warning: not simple), surf to:
http://brucemo.com/compchess/programming/index.htm

Adak