Problem Reading Bmp's ..

[Jm.GlezdeRueda]

Hi all,

Im trying to read a 24bit bmp with fread, and i have some problems.. I
want to read the whole structure in one time, but i dont know why, it
only reads the first member well..

I have two questions..

1- why if i change fread(bmp1, sizeof(bmp1), 1, fin); to fread(bmp1,
sizeof(struct bmp), 1, fin); i have a Segment violation ??

2- can i read the whole structure in one time ?? ( i can read the
structure well if i read it member by member)


Code>>


#include <stdio.h>
#include <stdlib.h>



typedef unsigned char BYTE,uc;
typedef unsigned short WORD,us;
typedef unsigned long DWORD,ul;
typedef unsigned long LONG;

struct bmp{

WORD FileType; /* File type, always 4D42h ("BM") */
DWORD FileSize; /* Size of the file in bytes */
WORD Reserved1; /* Always 0 */
WORD Reserved2; /* Always 0 */
DWORD BitmapOffset; /* Starting position of image data in bytes
*/
DWORD Size; /* Size of this header in bytes */
LONG Width; /* Image width in pixels */
LONG Height; /* Image height in pixels */
WORD Planes; /* Number of color planes */
WORD BitsPerPixel; /* Number of bits per pixel */
DWORD Compression; /* Compression methods used */
DWORD SizeOfBitmap; /* Size of bitmap in bytes */
LONG HorzResolution; /* Horizontal resolution in pixels per
meter */
LONG VertResolution; /* Vertical resolution in pixels per meter
*/
DWORD ColorsUsed; /* Number of colors in the image */
DWORD ColorsImportant; /* Minimum number of important colors */
};

int main(){

struct bmp *bmp1;

ul i;
FILE *fin;

fin=fopen("1.bmp","rb");


if (fin == NULL) return 0;
fread(bmp1, sizeof(bmp1), 1, fin);
printf("\n Propiedades de la imagen\n");
printf("
______________________________________________________________\n");
printf("\n Tipo de fichero: %d\n",bmp1->FileType);
printf("\n Tamaño del fichero en bytes: %d\n",bmp1->FileSize);
printf("\n Zona reservada: %d\n",bmp1-> Reserved1);
printf("\n Zona reservada: %d\n",bmp1-> Reserved2);
printf("\n Offset hasta el mapa de bits:%d\n",bmp1-

BitmapOffset);

printf("\n Tamaño de la cabecera: %d\n",bmp1->Size);
printf("\n Ancho de la imagen: %d \n",bmp1->Width);
printf("\n Altura de la imagen: %d \n",bmp1->Height);
printf("\n Número de planos: %d \n",bmp1->Planes);
printf("\n Bits por pixel: %d \n",bmp1->BitsPerPixel);
printf("\n Compresión: %d \n",bmp1->Compression);
printf("\n Tamaño de la imagen en bytes: %d \n",bmp1-

SizeOfBitmap);

printf("\n Resolución horizontal: %d \n",bmp1->HorzResolution);
printf("\n Resolución vertical: %d \n",bmp1->VertResolution);
printf("\n Tamaño de la paleta: %d \n",bmp1->ColorsUsed);
printf("\n Colores importantes: %d \n\n",bmp1->ColorsImportant);

unsigned char *puntero=malloc(sizeof(uc)*((bmp1->SizeOfBitmap)*3));




FILE *fout;


fout=fopen("2.bmp","wb");

fwrite(bmp1,sizeof(struct bmp),1,fout);


fclose(fin);
fclose(fout);
free(puntero);

};

__________________________________________

Console Result>>




Propiedades de la imagen
______________________________________________________________

Tipo de fichero: 19778 ***** (this is BM, its ok)

Tamaño del fichero en bytes: -1208460616 ******** ( ???)

Zona reservada: 6944 ********* ( ??? )

Zona reservada: 47097

Offset hasta el mapa de bits:-1209676762

Tamaño de la cabecera: -1209347184

Ancho de la imagen: -1209676730

Altura de la imagen: -1209676714

Número de planos: 52326

Bits por pixel: 47077

Compresión: -1209676682

Tamaño de la imagen en bytes: -1209676666

Resolución horizontal: 0

Resolución vertical: 0

Tamaño de la paleta: 0

Colores importantes: 0

 

[Malcolm McLean]

Im trying to read a 24bit bmp with fread, and i have some problems.. I
want to read the whole structure in one time, but i dont know why, it
only reads the first member well..

If you declare a structure and fred it it might match up and it might not.
It will also break on a different compiler.
The way to treat a binary file format is to declare functions that load the
fields byte by byte from the stream and reassemble them. For instance image
width is a 32 bit integer, I think little-endian.

Wrap up the entire read into one function. Probably you only want image
dimensions and raster data. However you might not want the bgr-padding
format Microsoft use, and there is no reason you should return the raster in
that order.

 

[Jm.GlezdeRueda]

The way to treat a binary file format is to declare functions that load the

fields byte by byte from the stream and reassemble them. For instance image
width is a 32 bit integer, I think little-endian.

Wrap up the entire read into one function. Probably you only want image
dimensions and raster data. However you might not want the bgr-padding
format Microsoft use, and there is no reason you should return the raster in
that order.

I use gcc.. so.. do you think its better to read it member by member
or there is something to fix it?

Thanks

 

[Jm.GlezdeRueda]

Here its a sample of my old code.. do you mean something like this?
can i fix the problem with #pragma or something like that? Thanks

CODE:

int openBmp(char *nombre,unsigned char *puntero) {

struct bmp *bmp1;
ul i;
FILE *fin;

fin=fopen("1","rb");


if (fin == NULL) return 0;
fopen(&bmp1->FileType, sizeof(bmp1->FileType), 1, fout);
fopen(&bmp1->FileSize, sizeof(bmp1->FileSize), 1, fout);
fopen(&bmp1->Reserved1, sizeof(bmp1->Reserved1), 1, fout);
fopen(&bmp1->Reserved2, sizeof(bmp1->Reserved2), 1, fout);
fopen(&bmp1->BitmapOffset, sizeof(bmp1->BitmapOffset), 1, fout);
fopen(&bmp1->Size, sizeof(bmp1->Size), 1, fout);
fopen(&bmp1->Width, sizeof(bmp1->Width), 1, fout);
fopen(&bmp1->Height, sizeof(bmp1->Height), 1, fout);
fopen(&bmp1->Planes, sizeof(bmp1->Planes), 1, fout);
fopen(&bmp1->BitsPerPixel, sizeof(bmp1->BitsPerPixel), 1, fout);
fopen(&bmp1->Compression, sizeof(bmp1->Compression), 1, fout);
fopen(&bmp1->SizeOfBitmap, sizeof(bmp1->SizeOfBitmap), 1, fout);
fopen(&bmp1->HorzResolution, sizeof(bmp1->HorzResolution), 1, fout);
fopen(&bmp1->VertResolution, sizeof(bmp1->VertResolution), 1, fout);
fopen(&bmp1->ColorsUsed, sizeof(bmp1->ColorsUsed), 1, fout);
fopen(&bmp1->ColorsImportant, sizeof(bmp1->ColorsImportant), 1, fout);


puntero=malloc(sizeof(uc)*(SizeOfBitmap*3));


for (i=0;i<(bmp1->SizeOfBitmap*3);i++) {
fread(puntero,sizeof(uc),1,fin);
};

fclose(fin);

};

 

[Jm.GlezdeRueda]

FREAD INSTEAD OF FOPEN

 

[Malcolm McLean]

Here its a sample of my old code.. do you mean something like this?
can i fix the problem with #pragma or something like that? Thanks

CODE:

int openBmp(char *nombre,unsigned char *puntero) {

struct bmp *bmp1;
ul i;
FILE *fin;

fin=fopen("1","rb");


if (fin == NULL) return 0;
fopen(&bmp1->FileType, sizeof(bmp1->FileType), 1, fout);
fopen(&bmp1->FileSize, sizeof(bmp1->FileSize), 1, fout);
fopen(&bmp1->Reserved1, sizeof(bmp1->Reserved1), 1, fout);
fopen(&bmp1->Reserved2, sizeof(bmp1->Reserved2), 1, fout);
fopen(&bmp1->BitmapOffset, sizeof(bmp1->BitmapOffset), 1, fout);
fopen(&bmp1->Size, sizeof(bmp1->Size), 1, fout);
fopen(&bmp1->Width, sizeof(bmp1->Width), 1, fout);
fopen(&bmp1->Height, sizeof(bmp1->Height), 1, fout);
fopen(&bmp1->Planes, sizeof(bmp1->Planes), 1, fout);
fopen(&bmp1->BitsPerPixel, sizeof(bmp1->BitsPerPixel), 1, fout);
fopen(&bmp1->Compression, sizeof(bmp1->Compression), 1, fout);
fopen(&bmp1->SizeOfBitmap, sizeof(bmp1->SizeOfBitmap), 1, fout);
fopen(&bmp1->HorzResolution, sizeof(bmp1->HorzResolution), 1, fout);
fopen(&bmp1->VertResolution, sizeof(bmp1->VertResolution), 1, fout);
fopen(&bmp1->ColorsUsed, sizeof(bmp1->ColorsUsed), 1, fout);
fopen(&bmp1->ColorsImportant, sizeof(bmp1->ColorsImportant), 1, fout);


puntero=malloc(sizeof(uc)*(SizeOfBitmap*3));


for (i=0;i<(bmp1->SizeOfBitmap*3);i++) {
fread(puntero,sizeof(uc),1,fin);
};

fclose(fin);

};

Eek.
No.
Like this

/*
We want to load an image as a raster. Probably we want rgb values, but we
can easily change to bga-dummy, or an array of COLORREFs if required just be
altering the code a bit.
The code is a unit. Take the name of the .bmp file, return the raster and
the image dimensions.
On error return NULL. If you need more error information then pass in
another integer. However generally you just need to know if the function
failed or succeeded.
*/

unsigned char *loadbmp(const char *fname, int *width, int *height)
{
FILE *fp;
int err;
unsigned char *answer;
/* open file in binary mode */
fp = fopen(fname, "rb");
if(!fp)
return 0;

/* get all the information we need for the header */
err = loadheader(fp, &width, &height);

/*
in this code we allocate memory for the raster, and load it.
we have to be careful to clean up if we encounter error condtions.
*/
if(err == 0)
{
answer = malloc(width * height * 3);
if(answer)
err = loadraster(fp, answer, width, height);
if(err != 0)
{
free(answer);
answer = 0;
}
}
else
answer = 0;

fclose(fp);
}

/*
load the header information. Probably all we need is the width and the
height,
but you might want other fields like image physical size.
*/
int loadheader(FILE *fp, int *width, int *height)
{
int i;
/* skip a bit of useless information - number made up */
for(i=0;i<123;i++)
fgetc(fp);
/* load width - I think it is a 32 bit little endian. Note this code
could
be tightened up a bit */
*width = fgetc();
*width += fgetc() << 8;
*width += fgetc() << 16;
*width += fgetc() << 24;
/* same for height */
*height = fgetc();
*height += fgetc() << 8;
*height += fgetc() << 16;
*height+= fgetc() << 24;
/* skip trailing useless information */
for(i=0;i<123;i++)
fgetc(fp);

/* now sanity test width and height, also you might want to check the
"useless" fields for integrity */
if(*width <= 0 || *width > 16000)
return -1;
return 0
}

/*
now load the raster information into our buffer.
*/
int loadraster(FILE *fp, unsigned char *out, int width, int height)
{
int i;
int ii;
unsigned char red, green, blue;

for(i=0;i<height;i++)
{
for(ii=0;ii<width;ii++)
{
blue = fgetc(fp);
green = fgetc(fp);
red = fgetc(fp);
fgetc(fp); /* skip padding byte */
*out++ = red;
*out++ = green;
*out++ = blue;
}
/* might need to skip a few bytes at end of line */
}
/* if we ran out of data fail */
if(feof(fp))
return -1;
return 0;
}

This is skeleton code. It won't work exactly as typed in because I can't
remember all the details of the BMP format. Also I haven't tried compiling
it. It is simply intended to show you how to approach the problem and to
provide a framework.

 

[Ernie Wright]

Im trying to read a 24bit bmp with fread, and i have some problems.. I
want to read the whole structure in one time, but i dont know why, it
only reads the first member well..

I have two questions..

1- why if i change fread(bmp1, sizeof(bmp1), 1, fin); to fread(bmp1,
sizeof(struct bmp), 1, fin); i have a Segment violation ??

bmp1 is a pointer, and sizeof( bmp1 ) is the size of the pointer, not
the structure it points to.

sizeof( bmp1 ) != sizeof( struct bmp )
sizeof( *bmp1 ) == sizeof( struct bmp )
sizeof( bmp1 ) == sizeof( struct bmp * )

You get a segment fault because you haven't initialized bmp1. You don't
allocate any memory for it to point to.

struct bmp *bmp1;
bmp1 = malloc( sizeof *bmp1 );
if ( !bmp1 ) ... /* malloc() failed */

That only the second version leads to a segment fault is accidental.
Both of them might, or neither of them might.

2- can i read the whole structure in one time ?? ( i can read the
structure well if i read it member by member)

The answer to this is complicated.

You probably can't read your struct bmp all at once, as it is currently
defined. You can modify your struct bmp so that it will work, but you
should only do this with the understanding that it's not, in general,
the best way to approach reading binary files.

struct bmp{

WORD FileType; /* File type, always 4D42h ("BM") */
DWORD FileSize; /* Size of the file in bytes */

The second field is a DWORD, which is typically aligned on a 4-byte
boundary, meaning that the compiler will insert 2 bytes of padding after
the first field. This padding doesn't exist in the file, so your entire
structure will be misaligned with the contents of the file.

If you remove the first field, however, it's pretty likely that you will
be able to read in the rest of the structure in one call to fread(),
after reading the first two bytes separately.

unsigned char *puntero=malloc(sizeof(uc)*((bmp1->SizeOfBitmap)*3));

The extra parentheses are unnecessary. You can write

sizeof uc * bmp1->SizeOfBitmap * 3

Unless you're using a compiler that supports C99, or writing in C++
rather than C, you can't declare puntero at this point in the code.

If "uc" means "unsigned char", this will always be 1. You don't have to
multiply by this.

SizeOfBitmap will either contain the number of bytes required to hold
all three color channels, so that you don't have to multiply by 3, or it
will (very often for 24-bit BMPs) contain 0, so that you must calculate
the storage requirement some other way.

In my BMP reading code, I find the storage requirement by subtracting
the header size from the file size. I also use the BITMAPINFOHEADER
structure provided by the Win32 headers, rather than creating my own
struct bmp.

- Ernie http://home.comcast.net/~erniew

 

[Flash Gordon]

Malcolm McLean wrote, On 13/05/07 01:59:

Eek.
No.
Like this

/*
We want to load an image as a raster. Probably we want rgb values, but
we can easily change to bga-dummy, or an array of COLORREFs if required
just be altering the code a bit.
The code is a unit. Take the name of the .bmp file, return the raster
and the image dimensions.
On error return NULL. If you need more error information then pass in
another integer. However generally you just need to know if the function
failed or succeeded.
*/

unsigned char *loadbmp(const char *fname, int *width, int *height)
{
FILE *fp;
int err;
unsigned char *answer;
/* open file in binary mode */
fp = fopen(fname, "rb");
if(!fp)
return 0;

/* get all the information we need for the header */
err = loadheader(fp, &width, &height);

Always make sure you have a prototype in scope for a function before
calling it. Then the compiler will tell you that you are passing values
of the wrong type in the above line.

/*
in this code we allocate memory for the raster, and load it.
we have to be careful to clean up if we encounter error condtions.
*/
if(err == 0)
{
answer = malloc(width * height * 3);
if(answer)
err = loadraster(fp, answer, width, height);

It will tell you about a type mismatch here as well.

if(err != 0)
{
free(answer);
answer = 0;
}
}
else
answer = 0;

fclose(fp);

Having defined the function as returning an unsigned char* would not
returning one be a good idea? Possibly return answer since otherwise you
have lost the memory?

}

/*
load the header information. Probably all we need is the width and the
height,
but you might want other fields like image physical size.
*/
int loadheader(FILE *fp, int *width, int *height)
{
int i;
/* skip a bit of useless information - number made up */
for(i=0;i<123;i++)
fgetc(fp);
/* load width - I think it is a 32 bit little endian. Note this code
could
be tightened up a bit */
*width = fgetc();
*width += fgetc() << 8;
*width += fgetc() << 16;
*width += fgetc() << 24;

You should document your assumption that int is at least 32 bits (since
the C standard does not guarantee it) or use a type that is guaranteed
to e large enough.

/* same for height */
*height = fgetc();
*height += fgetc() << 8;
*height += fgetc() << 16;
*height+= fgetc() << 24;
/* skip trailing useless information */
for(i=0;i<123;i++)
fgetc(fp);

/* now sanity test width and height, also you might want to check the
"useless" fields for integrity */
if(*width <= 0 || *width > 16000)
return -1;

You should probably be using unsigned types rather than signed types,
since that is proably how the image format is defined. Also, a lot of
cameras produce images larger than that.

return 0
}

/*
now load the raster information into our buffer.
*/
int loadraster(FILE *fp, unsigned char *out, int width, int height)
{
int i;
int ii;
unsigned char red, green, blue;

for(i=0;i<height;i++)
{
for(ii=0;ii<width;ii++)
{
blue = fgetc(fp);
green = fgetc(fp);
red = fgetc(fp);
fgetc(fp); /* skip padding byte */
*out++ = red;
*out++ = green;
*out++ = blue;
}
/* might need to skip a few bytes at end of line */
}
/* if we ran out of data fail */
if(feof(fp))
return -1;
return 0;
}

This is skeleton code. It won't work exactly as typed in because I can't
remember all the details of the BMP format. Also I haven't tried
compiling it. It is simply intended to show you how to approach the
problem and to provide a framework.

Even with a framework you should avoid the obvious mistakes you made
here like passing incorrect parameter types. That is not going to help
someone who is learning.

 

[Malcolm McLean]

Malcolm McLean wrote, On 13/05/07 01:59:



Always make sure you have a prototype in scope for a function before
calling it. Then the compiler will tell you that you are passing values of
the wrong type in the above line.


It will tell you about a type mismatch here as well.


Having defined the function as returning an unsigned char* would not
returning one be a good idea? Possibly return answer since otherwise you
have lost the memory?


You should document your assumption that int is at least 32 bits (since
the C standard does not guarantee it) or use a type that is guaranteed to
e large enough.


You should probably be using unsigned types rather than signed types,
since that is proably how the image format is defined. Also, a lot of
cameras produce images larger than that.


Even with a framework you should avoid the obvious mistakes you made here
like passing incorrect parameter types. That is not going to help someone
who is learning.

It was a mistake to write so much skeleton code without compiling it. I
agree.

 

[Army1987]

Hi all,

Im trying to read a 24bit bmp with fread, and i have some problems.. I
want to read the whole structure in one time, but i dont know why, it
only reads the first member well..

I have two questions..

1- why if i change fread(bmp1, sizeof(bmp1), 1, fin); to fread(bmp1,
sizeof(struct bmp), 1, fin); i have a Segment violation ??

See Ernie Wright's answer.

2- can i read the whole structure in one time ?? ( i can read the
structure well if i read it member by member)

[code snipped]

Portably you don't. You dont know wheter there is any padding in
the struct. Even if you read it member by member beware of this:
1. On some other systems, unsigned char, unsigned short, unsigned
long might have different sizes. If you have a C99 compiler, use
uint8_t, uint16_t and uint_32t found in <inttypes.h> and
<stdint.h>.
2. Different machines have different endiannesses. The best
solution I can see, unfortunately (other than the "twisty maze of
#ifdef's" approach) is:
for (i=0; i<4; i++) {
n <<= CHAR_BIT;
n |= getchar();
}
(obviously this is just the idea, don't copy and paste it).

If you want to do things portably, use the ppm format, it is much
easier. It looks like that

P3
# comments
640 480
255
0 0 0 0 0 0 1 1 1...
(P3 is the header, 640 and 480 are the resolution, 255 is the depth
and the next are triples showing colors (0 0 0 = #000000, 0 0 1 =
#000001... 255 255 255 = #FFFFFF).

Or:

P6
# comments
640 480
255
<binary data>

e.g.
struct Pixel {unsigned char Red, Green, Blue} p;
putc(p.Red, file); putc(p.Green, file); putc(p.Blue, file);

 

[christian.bau]

I use gcc.. so.. do you think its better to read it member by member
or there is something to fix it?

First you have to make it clear to yourself what the data format of
that file actually is.

You have a struct definition, and the data format of the file is that
struct definition, compiled with one specific version of some old
Microsoft compiler for one specific processor. So forget about the
struct. What you have in the file is a sequence of bytes. Where you
have a "WORD" in that struct, what you actually have is two unsigned
eight bit chars, with the least significant one stored first.

So what you can do: Leave the struct as it is. Write a function to
read the struct from a file; you might also write a function to write
it to a file. In that function: Count exactly how many bytes that
struct has (when compiled with that version of the Microsoft
compiler). It is _not_ sizeof (struct), because that will tell you the
size on _your_ compiler, but you want the size on a _MS_ compiler.
Define an array of unsigned char with exactly that size and read it.
Then if you have a struct member that is for example a "WORD" starting
at offset 8, you set the value to (a [8]) + (a [9] << 8). That will
read the data portably on any implementation.

 

[Jm.GlezdeRueda]

Thanks all, many interesting things in all these posts. i think Ernie
Wright has the clue about the structure problem.. Ive read something
like that in other posts, but i didnt understood very well ..


I will post the new code version, trying to jump the first member. But
why reading member by member it works fine??

Thanks all -

 

[Chad]

Malcolm McLean wrote, On 13/05/07 01:59:







Always make sure you have a prototype in scope for a function before
calling it. Then the compiler will tell you that you are passing values
of the wrong type in the above line.


It will tell you about a type mismatch here as well.



Having defined the function as returning an unsigned char* would not
returning one be a good idea? Possibly return answer since otherwise you
have lost the memory?

I don't understand the comment Flash made. Can someone please
elaborate a bit more on this?

Chad

 

[Chad]

The part I'm drawing a blank on is

"Having defined the function as returning an unsigned char* would not
returning one be a good idea? Possibly return answer since otherwise
you
have lost the memory?"

 

[Ernie Wright]

Thanks all, many interesting things in all these posts. i think Ernie
Wright has the clue about the structure problem.. Ive read something
like that in other posts, but i didnt understood very well ..

The first few bytes of a BMP file look like this:

byte contents
0 WORD FileType; /* File type, always 4D42h ("BM") */
1 :
2 DWORD FileSize; /* Size of the file in bytes */
3 :
4 :
5 :
6 WORD Reserved1; /* Always 0 */
7 :
8 WORD Reserved2; /* Always 0 */
9 :
...

In particular, the FileSize field begins at an offset of 2 bytes.

But when the compiler reserves storage for your structure, it will
usually add padding bytes:

byte contents
0 WORD FileType; /* File type, always 4D42h ("BM") */
1 :
2 ***** pad byte inserted by the compiler *****
3 ***** pad byte inserted by the compiler *****
4 DWORD FileSize; /* Size of the file in bytes */
5 :
6 :
7 :
8 WORD Reserved1; /* Always 0 */
9 :
10 WORD Reserved2; /* Always 0 */
11 :
...

Notice that the compiler put FileSize at an offset of 4 rather than 2,
and added 2 bytes of padding before it.

The layout of your structure in memory therefore doesn't match the
layout of the file. You'll also find that sizeof struct bmp is
(probably) 56 rather than 54.

The compiler does this in order to get the FileSize member to begin at
an offset that's a multiple of 4. In general, the most common alignment
puts structure members that are n bytes long at offsets divisible by n.

Some computers will actually crash if the compiler doesn't do this. On
most others, misaligned access is merely very slow.

I will post the new code version, trying to jump the first member. But
why reading member by member it works fine??

This is one reason that reading one member at a time, or better still,
one byte at a time, is the more robust and portable way to read a binary
file. You don't have to rely on the compiler laying out a structure in
memory in a way that exactly matches the layout in the file.

- Ernie http://home.comcast.net/~erniew