1

I am trying to make my own classes to manipulate BMP images..

I started off with this simple code:

Everything has been done as given in this wikepedia link.

I am having two problems:

  1. When I try to open the image it's giving error

    Premature end of file detected
    
  2. The actual file I created exceeds the file size thats supposed to be by two bytes.

Here's the code:

#include <fstream>
#include <iostream>

using namespace std;

struct BMP_Header
{
    public :

    char id[2];
    unsigned int total_image_size;
    short int app_data1,app_data2;
    unsigned int offset;
};

struct DIB_Header 
{
    public :

    unsigned int dib_header_size;
    int image_width;
    int image_height;
    short int no_colour_planes;
    short int colour_depth;
    unsigned int compression_method;
    unsigned int raw_image_size;
    unsigned int horizontal_resolution;
    unsigned int vertical_resolution;
    unsigned int num_colours_palette;
    unsigned int imp_colours_used;
};


int main ()
{
    int index=0;

    BMP_Header bmp_header;
    DIB_Header dib_header;
    char pixel_array[16];

    bmp_header.total_image_size=70;
    bmp_header.offset=54;
    bmp_header.id[0]='B';
    bmp_header.id[1]='M';
    bmp_header.app_data1=0;
    bmp_header.app_data2=0;

    dib_header.dib_header_size=40;
    dib_header.image_width=2;
    dib_header.image_height=2;
    dib_header.colour_depth=24;
    dib_header.raw_image_size=16;
    dib_header.horizontal_resolution=2835;
    dib_header.vertical_resolution=2835;
    dib_header.no_colour_planes=1;
    dib_header.compression_method=0;
    dib_header.num_colours_palette=0;   
    dib_header.imp_colours_used=0;

    pixel_array[index++]=0;
    pixel_array[index++]=0;
    pixel_array[index++]=255;

    pixel_array[index++]=255;
    pixel_array[index++]=255;
    pixel_array[index++]=255;

    pixel_array[index++]=0;
    pixel_array[index++]=0;

    pixel_array[index++]=255;
    pixel_array[index++]=0;
    pixel_array[index++]=0;

    pixel_array[index++]=0;
    pixel_array[index++]=255;
    pixel_array[index++]=0;

    pixel_array[index++]=0;
    pixel_array[index++]=0;

// Image Made
    fstream file;
    file.open ("abc.bmp",ios::out | ios::binary);
    file.write ((char*)&bmp_header,sizeof (bmp_header));
    file.write ((char*)&dib_header,sizeof (dib_header));
    file.write ((char*)pixel_array,sizeof (pixel_array));

    file.close ();
return 0;
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
Rohith R
  • 1,309
  • 2
  • 17
  • 36
  • This code only ever puts one entry into the `pixel_array` array here. You probably want to use a loop here so that you cover every `index` in the array. Also I would strongly suggest using the c++ standard library containers, such as `std::vector` or `std::array`, over these raw arrays. – shuttle87 Jun 26 '14 at 17:16
  • i have initialized all the elements of the pixel array....pls have a look... – Rohith R Jun 26 '14 at 17:18
  • 1
    Consider using `struct` to make it abundantly clear that `BMP_Header` and `DIB_Header` should be POD. Also consider putting the member-initializers directly on the member-declarations, so you can remove the custom constructor... Next, are you absolutely sure you need those casts to `char*`? – Deduplicator Jun 26 '14 at 17:21
  • Ok...will try doing so..btw what is POD....? – Rohith R Jun 26 '14 at 17:22
  • @PRP http://stackoverflow.com/questions/146452/what-are-pod-types-in-c – shuttle87 Jun 26 '14 at 17:23
  • are you sure sizeof (pixel_array) is returning sizeof(char)? sometimes in c++, object sizes are padded in order to better fit the architecture of the computer. It's very possible that sizeof(pixel_array) is actually returning a number much larger than sizeof(char) – clearlyspam23 Jun 26 '14 at 17:25
  • @Deduplicator After Making Them Structs Still Getting The Same Error....!!! – Rohith R Jun 26 '14 at 17:26
  • @clearlyspam23 sizeof (pixel_array) is returning 16 as expected... – Rohith R Jun 26 '14 at 17:28
  • 1
    Besides all the rest, you should be using fixed width types, otherwise on another platform/compiler you risk messing up the memory layout due to different sizes types. – Matteo Italia Jun 26 '14 at 17:32
  • @PRP Are you making a generic class that deals with BMP files (read/write...)? Or do you just want to generate image data programatically and output it to a file? – cubuspl42 Jun 27 '14 at 12:43
  • i am making a generic class.... – Rohith R Jun 28 '14 at 09:21

2 Answers2

3

You should not use C structs to read/write things like a BMP header, because the precise binary layout is not guaranteed: specially because of fields alignment and padding. There is also the issue of little-big endianess, which you must be aware.

A workaround is to include the compiler pragmas/settings to disable structs padding; a better alternative is to write each field explicitly.

Added: Even worse, as pointed in a comment, it's wrong to use C plain types (int...) for fixed width fields; the actual width will depend on the platform. You should use int32_t or something like that.

leonbloy
  • 73,180
  • 20
  • 142
  • 190
  • Actually, you can (mostly). Still, adding some static assertions would be a good idea... – Deduplicator Jun 26 '14 at 17:33
  • i read the article in wikepedia....all data that have to be entered in little-endian...so that should not be a problem here...so u mean to say that I wite each field separately in to the file....? – Rohith R Jun 26 '14 at 17:34
  • @Deduplicator : you're right, I changed "cannot" to "should not" – leonbloy Jun 26 '14 at 17:35
  • "that should not be a problem here" But your program would not work on a different architecture, that's (normally) bad. "so u mean to say that I write each field separately in to the file?" Yes – leonbloy Jun 26 '14 at 17:37
  • Padding stuff seems to be the problem that i identified....will try writing each field separately – Rohith R Jun 26 '14 at 17:53
  • Wrote Each field explicitly...still same problem...although the problem of file size exceeding the actual size by 2 bytes seems solved...! – Rohith R Jun 26 '14 at 18:30
  • "still same problem" which is the problem? how are you reading it? – leonbloy Jun 26 '14 at 18:33
  • Adding the __attribute__ ((packed)) to the struct (aka. struct hdr {...} __attribute__ ((packed));) will eliminate padding problems. YMMV depending on compiler used. – Aumnayan Jun 26 '14 at 20:24
1
struct DIB_Header 
{
    public :

    unsigned int dib_header_size;
    int image_width;
    int image_height;
    short int no_colour_planes;
    short int colour_depth;
    unsigned int compression_method;
    unsigned int raw_image_size;
    unsigned int horizontal_resolution;
    unsigned int vertical_resolution;
    unsigned int num_colours_palette;
    unsigned int imp_colours_used;


} __attribute__ ((packed));

This should eliminate the padding problem you are reporting, provided your elements are the appropriate size for the header (I didn't cross check). There is no problems handling bmp's in C/C++. YMMV depending on the compiler as each can handle the packed attribute slightly differently. You should be fins in this case however.

Also, be aware of the row padding that BMP files expect in the data elements. As per the format guidelines from http://en.wikipedia.org/wiki/BMP_file_format:

The bits representing the bitmap pixels are packed in rows. The size of each row is rounded up to a multiple of 4 bytes (a 32-bit DWORD) by padding.

Aumnayan
  • 671
  • 3
  • 12