Jump to content
  • Advertisement
Sign in to follow this  
Bozebo

Unity Confusing issue while loading .bmp

This topic is 3071 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

edit:
(old thread title: "Array contains more bytes than it should (C++)")
Scroll down to my 2nd post for the updated issue.
---------------------
Hi. I am making a console application to open a 24bpp windows bitmap which will then convert it into a custom greyscale format which I am going to use as terrain.

Essentially I am having a play around to make a toolkit chain for the purpose of learning etc.

My program is trying to access an array location which is out of range, in trying to fix that I discovered some curious information about the arrays in use that doesn't make any sense.

Firstly, here is the full code (read below before looking too deeply into it):
#include <stdint.h>
#include <iostream>

#define DEBUG

#ifdef DEBUG
#define DEBUG_INPIXELS
#endif

typedef struct fileHeaderMagic{
unsigned char magic[2];
} fileHeaderMagic;

typedef struct fileHeader{
uint32_t filesz;
uint16_t creator1;
uint16_t creator2;
uint32_t bmp_offset;
} fileHeader;

typedef struct fileBmpHeader{
uint32_t header_sz;
uint32_t width;
uint32_t height;
uint16_t nplanes;
uint16_t bitspp;
uint32_t compress_type;
uint32_t bmp_bytesz;
uint32_t hres;
uint32_t vres;
uint32_t ncolors;
uint32_t nimpcolors;
} fileBmpHeader;

//24 bpp colour format
typedef struct col24{
unsigned char b;
unsigned char g;
unsigned char r;
} col24;

using namespace std;

bool convert(char* file){
//make the image object
FILE * image;
//open the image file for reading
image = fopen(file,"r");
//check that it was opened
if(image == NULL){
cout << "could not open file";
return false;
}

//make the magic, file header and bitmap header
fileHeaderMagic magic;
fileHeader header;
fileBmpHeader bmpHeader;

//read the magic
fread(&magic,sizeof(magic),1,image);
//check the magic
if(magic.magic[0] != char(66) || magic.magic[1] != char(77)){
cout << "file is not a Windows bitmap";
return false;
}

//read the header
fread(&header,sizeof(header),1,image);

//read the bitmap header
fread(&bmpHeader,sizeof(bmpHeader),1,image);
//check header attributes
if(bmpHeader.header_sz != 40){
cout << "unexpected header size";
return false;
}
if(bmpHeader.nplanes != 1){
cout << "unexpected colour planes";
return false;
}
if(bmpHeader.width < 1 || bmpHeader.height < 1){
cout << "bitmap has an invalid dimension";
return false;
}
if(bmpHeader.bitspp != 24){
cout << "bitmap must be 24 bits per pixel";
return false;
}
if(bmpHeader.compress_type != 0){
cout << "bitmap must be uncompressed";
return false;
}
if(bmpHeader.ncolors != 0){
cout << "bitmap has an invalid number of colours (" <<
int(bmpHeader.ncolors) << ")";
return false;
}

//calculate bytes per row
int bytesPerRow = bmpHeader.bmp_bytesz/bmpHeader.height;
#ifdef DEBUG
cout << "bytesPerRow: " << bytesPerRow << endl;
#endif
//calculate padding size
int padding = bytesPerRow - bmpHeader.width*3; //byte per row - byte of pixels
#ifdef DEBUG
cout << "padding: " << padding << endl;
#endif
char* scrap[padding]; //bytes of padding scrap
//make an array of 24 bit pixels
col24* inPixels[bmpHeader.width*bmpHeader.height];
#ifdef DEBUG
cout << bmpHeader.width*bmpHeader.height << " pixels" << endl;
cout << sizeof(col24) << " bytes per pixel" << endl;
cout << sizeof(inPixels) << " bytes in inPixels array" << endl;
cout << bmpHeader.width*bmpHeader.height*sizeof(col24) <<
" bytes (expected)" << endl;
#endif
//current pixel
int currentPixel = 0;
//loop through each row bottom to top
for(int row = bmpHeader.height - 1;row >= 0;row --){
#ifdef DEBUG
cout << "row " << row << endl;;
#endif
//loop through each column
for(int col = 0;col < bmpHeader.width;col ++){
#ifdef DEBUG_INPIXELS
cout << "currentPixel: " << currentPixel << endl;
#endif
fread(inPixels[currentPixel],3,1,image); //read 3 byte as a colour
#ifdef DEBUG_INPIXELS
cout << int(inPixels[currentPixel]->b) << ",";
cout << int(inPixels[currentPixel]->g) << ",";
cout << int(inPixels[currentPixel]->r) << " ";
#endif
currentPixel ++; //move to the next pixel
}
fread(&scrap,padding,1,image); //read the padding as scrap
#ifdef DEBUG
cout << endl;
#endif
}

#ifdef DEBUG
system("pause");
#endif

return true;
}

//application entry point
int main(int argc, char **argv, char **envp){
char * open; //path to file to convert
bool err; //if an error has occurred
if(argc >= 2){
open = argv[1];
err = !convert(open);
} else {
#ifdef DEBUG
err = !convert("image.bmp");
#endif
#ifndef DEBUG
cout << "please specify a file";
err = true;
#endif
}

if(err){
cout << endl;
system("pause");
}

//exit application
return 0;
}

















Now to isolate what I believe may be the fault areas.
I have a 24bpp colour format to read pixels from the bitmap data:
//24 bpp colour format
typedef struct col24{
unsigned char b;
unsigned char g;
unsigned char r;
} col24;















This struct must be 3 bytes in size, no?

Here is the section that trying to do all the work (reading the bitmap data into the col24 array) and is causing the access violation:
  //calculate bytes per row
int bytesPerRow = bmpHeader.bmp_bytesz/bmpHeader.height;
#ifdef DEBUG
cout << "bytesPerRow: " << bytesPerRow << endl;
#endif
//calculate padding size
int padding = bytesPerRow - bmpHeader.width*3; //byte per row - byte of pixels
#ifdef DEBUG
cout << "padding: " << padding << endl;
#endif
char* scrap[padding]; //bytes of padding scrap
//make an array of 24 bit pixels
col24* inPixels[bmpHeader.width*bmpHeader.height];
#ifdef DEBUG
cout << bmpHeader.width*bmpHeader.height << " pixels" << endl;
cout << sizeof(col24) << " bytes per pixel" << endl;
cout << sizeof(inPixels) << " bytes in inPixels array" << endl;
cout << bmpHeader.width*bmpHeader.height*sizeof(col24) <<
" bytes (expected)" << endl;
#endif
//current pixel
int currentPixel = 0;
//loop through each row bottom to top
for(int row = bmpHeader.height - 1;row >= 0;row --){
#ifdef DEBUG
cout << "row " << row << endl;;
#endif
//loop through each column
for(int col = 0;col < bmpHeader.width;col ++){
#ifdef DEBUG_INPIXELS
cout << "currentPixel: " << currentPixel << endl;
#endif
fread(inPixels[currentPixel],3,1,image); //read 3 byte as a colour
#ifdef DEBUG_INPIXELS
cout << int(inPixels[currentPixel]->b) << ",";
cout << int(inPixels[currentPixel]->g) << ",";
cout << int(inPixels[currentPixel]->r) << " ";
#endif
currentPixel ++; //move to the next pixel
}
fread(&scrap,padding,1,image); //read the padding as scrap
#ifdef DEBUG
cout << endl;
#endif
}
















That is giving me the following output:


The thing that confuses me is that the inPixels array is, for whatever reason, 892528 bytes in size, when it should clearly be 669396 bytes! The figure of 892528 bytes would come from width*height*4 not width*height*3; the col24 struct is indeed 3 bytes in size and the array is of width*height col24 structs, hence it should be 669396 bytes and not 892528.

My intention is to read the bitmap data into the array of col24 structs so that the array contains the pixels 1 dimensionally from bottom left to top right so that I can then make them greyscale and output a custom file format to load into my OpenGL application as the heightmap.

To fix my access violation I will need to understand what is going on there, what am I doing wrong?

edit:
While you are here, what do you think of my coding style? Should I use more full lines of whitespace?

[Edited by - Bozebo on July 14, 2010 4:10:13 PM]

Share this post


Link to post
Share on other sites
Advertisement
The inPixels array is 4 * 223132 because it holds POINTERS to col24. A pointer is 4 bytes on a 32 bit system, so that is perfectly al right.

Why it has an access violation is not because it's out of bounds, its because you dereference a pointer you never initialised (probably a NULL pointer, as that's the standard value pointers get when not being initialised). You need to remove the star and make the line read:

col24 inPixels[bmpHeader.width * bmpHeader.height];


Also the scrap should remove the star, I suppose. Remember:

int *array[n] will store an array of pointers-to-integers accessed as *(array)
int array[n] will store an array of integers accessed as array

EDIT: better use the heap like Zipster says

Share this post


Link to post
Share on other sites
The compiler is padding out the struct to 4 bytes for optimization reasons. I won't go in to too much detail, but basically it is typically much faster and easier for a modern CPU to access objects on a 4 byte boundary than a 3 byte boundary.

If you need to have the struct be exactly 3 bytes you'll have to use some compiler specific options to tell it to pack the structure.

For Visual Studio:

#pragma pack(push,1)
//24 bpp colour format
typedef struct col24{
unsigned char b;
unsigned char g;
unsigned char r;
} col24;
#pragma pack(pop)




For GCC:

//24 bpp colour format
typedef struct col24{
unsigned char b;
unsigned char g;
unsigned char r;
} col24 __attribute__ ((__packed__));




Edit: Woops. I misread part of your post. Nothing to see here!

Share this post


Link to post
Share on other sites
Quote:
Original post by Hyrcan
Quote:
This struct must be 3 bytes in size, no?

No. See: Data structure alignment

No, the structure size is in fact 3. It's an uninitialized pointer issue:

col24* inPixels = new col24[bmpHeader.width*bmpHeader.height];

But really a std::vector would be the best solution here:

std::vector<col24> inPixels(bmpHeader.width*bmpHeader.height);

Access also becomes inPixels.r, since you're no longer treating it as an array of pointers.

Share this post


Link to post
Share on other sites
Unless you tell it otherwise the compiler will round up the size of structures.
You can see this by prining the size of col24. To get stop this padding you need to place round you structure

#pragma pack (1) // set alignement to 1 byte

place structures here

#pragma pack(pop) // undo the last pragma pack

Loking at your screen shot , you display the current pixel number , then nothing You code shows ( cut down a bit )

cout << "currentPixel: " << currentPixel << endl;
fread(inPixels[currentPixel],3,1,image); //read 3 byte as a colour
cout << int(inPixels[currentPixel]->b) << ",";

As you dont see the next output its due to the line reading the pixels.

My first thought is that inPixels[currentPixel] should be &inPixels[currentPixel], BUT looking at the start of the code you create the array as such

col24* inPixels[bmpHeader.width*bmpHeader.height];

This means you have pointers to all the pixels in the image , not actual col24. This will also be the problem with scrap[padding]. These values never get initialise so they will point all over the place, so in the fread it will read the col24 into a random piece of memory and kaboom.

what you need is

char* scrap = new char[padding];
col24 *inPixels = new col24[bmpHeader.width*bmpHeader.height];

then in the reading
fread(&inPixels[currentPixel],3,1,image); //read 3 byte as a colour
cout << int(inPixels[currentPixel].b) << ",";
cout << int(inPixels[currentPixel].g) << ",";
cout << int(inPixels[currentPixel].r) << ",";

fread(scrap,padding,1,image); //read the padding as scrap
or
fseek(SEEK_CUR,padding);



I see others have replied Doh too slow :D





Share this post


Link to post
Share on other sites
Quote:
Original post by Decrius
(...) (probably a NULL pointer, as that's the standard value pointers get when not being initialised).

An uninitialized pointer usually just holds some garbage value. There's no guarantee it will be set to 0.

Share this post


Link to post
Share on other sites
Ah ok. Fixed :D edit: (no, read below)
  //calculate bytes per row
int bytesPerRow = bmpHeader.bmp_bytesz/bmpHeader.height;
#ifdef DEBUG
cout << "bytesPerRow: " << bytesPerRow << endl;
#endif
//calculate padding size
int padding = bytesPerRow - bmpHeader.width*3; //byte per row - byte of pixels
#ifdef DEBUG
cout << "padding: " << padding << endl;
#endif
char scrap[padding]; //bytes of padding scrap
//make an array of 24 bit pixels
col24 inPixels[bmpHeader.width*bmpHeader.height];
#ifdef DEBUG
cout << bmpHeader.width*bmpHeader.height << " pixels" << endl;
cout << sizeof(col24) << " bytes per pixel" << endl;
cout << sizeof(inPixels) << " bytes in inPixels array" << endl;
cout << bmpHeader.width*bmpHeader.height*sizeof(col24) <<
" bytes (expected)" << endl;
#endif
//current pixel
int currentPixel = 0;
//loop through each row bottom to top
for(int row = bmpHeader.height - 1;row >= 0;row --){
#ifdef DEBUG
cout << "row " << row << endl;;
#endif
//loop through each column
for(int col = 0;col < bmpHeader.width;col ++){
#ifdef DEBUG_INPIXELS
cout << "currentPixel: " << currentPixel << endl;
#endif
fread(&inPixels[currentPixel],3,1,image); //read 3 byte as a colour
#ifdef DEBUG_INPIXELS
cout << int(inPixels[currentPixel].b) << ",";
cout << int(inPixels[currentPixel].g) << ",";
cout << int(inPixels[currentPixel].r) << " ";
#endif
currentPixel ++; //move to the next pixel
}
fread(&scrap,padding,1,image); //read the padding as scrap
#ifdef DEBUG
cout << endl;
#endif
}










I was trying (something like) that initially and some issues lead me to believe it would be better to use pointers.

I can see that opting for a vector would make the program nice and simple and any tiny performance overheads really wouldn't matter.

----------
edit:
ok, I changed the sample image and now it gets so far through then thinks every pixel after then is just 0,0,0

with an image of only one colour (i dunno, 200,10,44) it works. But if there are a bunch of different colours present in the image it starts picking up everything as 0,0,0 after a certain point :S What...
This image makes it happen (programmer art yay):

Whereas if the image had only the yellow background and none of the coloured bars, it goes through the whole thing without a fuss - picking up the yellow all the way. Considering I am doing nothing with palettes or anything like that, how is this even possible?

edit:
Look at this:

Always at the same pixel...

And look, the following image works fine without it ever reporting 0,0,0

Image dimensions: 240x175
If I make the 240x175 image look like this instead, it starts always reporting 0,0,0 at pixel 874.

I am confused.

[Edited by - Bozebo on July 12, 2010 5:10:12 AM]

Share this post


Link to post
Share on other sites
OK, I put some more error checking things in:
//loop through each row bottom to top
for(int row = bmpHeader.height - 1;row >= 0;row --){
#ifdef DEBUG
cout << "row " << row << endl;;
#endif
//loop through each column
for(int col = 0;col < bmpHeader.width;col ++){
#ifdef DEBUG_INPIXELS
cout << "currentPixel: " << currentPixel << endl;
#endif
//read the next 3 bytes into the current target pixel
pixelsRead = fread(&inPixels[currentPixel],3,1,image);
if(pixelsRead == 0){
cout << "unable to read pixel " << currentPixel << " from file";
if(feof(image) != 0){
cout << ", end of file encountered";
return false;
}
return false;
}
#ifdef DEBUG_INPIXELS
if(inPixels[currentPixel].b == 0 && inPixels[currentPixel].g == 0 &&
inPixels[currentPixel].r == 0) system("pause");

cout << int(inPixels[currentPixel].b) << ",";
cout << int(inPixels[currentPixel].g) << ",";
cout << int(inPixels[currentPixel].r) << " ";
#endif
currentPixel ++; //move to the next pixel
//check that EOF was not encountered
if(feof(image) != 0){
cout << "unexpectedly encountered end of file";
return false;
}
}
//skip the padding
if(padding != 0 && fseek(image,padding,SEEK_CUR) == 0){
cout << "failed to skip padding";
return false;
}
#ifdef DEBUG
cout << endl;
#endif
}



It is halting with the message "unable to read pixel 873, end of file encountered". Which explains why it starts to read 0 values, but it doesn't explain why there is an end of file at all. How can the eof exist just because of the particular colours of the bitmap? Seeing as some work and some don't.

edit:
Perhaps I should read a whole row at a time. That would simplify things for a start, that should have been the first method >_<

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!