-2

I'm creating small BMP files for my application and some of them, depending on number of pixels, crashes my app and Windows sees them corrupted. An example of working size is 60 x 60px, but i.e. 61 x 61 is not (variables m_width and m_height).

The structs used (#pragma pack is applied to BMP-related ones):

struct Rgb /// vector's content
{
    uint8_t r;
    uint8_t g;
    uint8_t b;
};

#pragma pack(push, 1)
struct FileHeader
{
    int16_t bfType;
    int32_t bfSize;
    int16_t bfReserved1;
    int16_t bfReserved2;
    int32_t bfOffBits;
};

struct BitMapInfoHeader
{
    int32_t biSize;   
    int32_t biWidth;
    int32_t biHeight;
    int16_t biPlanes; 
    int16_t biBitCount;
    int32_t biCompression;
    int32_t biSizeImage; 
    int32_t biXPelsPerMeter;
    int32_t biYPelsPerMeter;
    int32_t biClrUsed;
    int8_t  biClrImportant;
    int8_t  biClrRotation;
    int16_t biReserved;
};

struct RGBQuad
{
    int8_t rgbBlue;
    int8_t rgbGreen;
    int8_t rgbRed;
    int8_t rgbReserved;
};
#pragma pack(pop)

I allocate memory for whole BMP file, assign pointers to each file region, fill in the structs, copy pixel data from other array and save the file. Code:

    int m_width = 60, m_height = 60;

    uint8_t* data = new uint8_t[ m_width * m_height ];
    memset( data, 0, m_width * m_height );
    data[ 2 ] = 1;             /// one pixel differs

    std::vector< Rgb > RGBVec = { { 223, 223, 123 }, { 230, 0, 12 } };
    int numberOfSymbols = RGBVec.size();

    FileHeader* fileHeader;
    BitMapInfoHeader* infoHeader;
    RGBQuad* colorTable;
    uint8_t* m_pBMPFile;        /// pointer to bitmap in memory
    uint8_t* m_BMPData;         /// begin of pixel data

    int m_BMPFileLength = sizeof( FileHeader ) + sizeof( BitMapInfoHeader )
        + numberOfSymbols * sizeof( RGBQuad ) + m_width * m_height;

/// assign pointers to specific parts of bitmap:
    m_pBMPFile = new uint8_t[ m_BMPFileLength ];
    memset( m_pBMPFile, 0, m_BMPFileLength );
    fileHeader = reinterpret_cast< FileHeader* >( m_pBMPFile );
    infoHeader = reinterpret_cast< BitMapInfoHeader* >( m_pBMPFile + sizeof( FileHeader ) );
    colorTable =
        reinterpret_cast< RGBQuad* >( m_pBMPFile + sizeof( FileHeader ) + sizeof( BitMapInfoHeader ) );
    m_BMPData = reinterpret_cast< uint8_t* >( m_pBMPFile + sizeof( FileHeader ) + sizeof( BitMapInfoHeader )
        + numberOfSymbols * sizeof( RGBQuad ) );

///////////
/// FileHeader:
    fileHeader->bfType = 0x4d42;            /// magic number
    fileHeader->bfSize = m_BMPFileLength;
    fileHeader->bfOffBits = int( m_BMPData - m_pBMPFile );

/// BitMapInfoHeader:
    infoHeader->biSize = 40;
    infoHeader->biWidth = m_width;
    infoHeader->biHeight = -m_height;       /// multiplied by -1 so pixels are displayed top-down
    infoHeader->biPlanes = 1;
    infoHeader->biBitCount = 8;
    infoHeader->biCompression = 0;
    infoHeader->biSizeImage = 0;
    infoHeader->biXPelsPerMeter = 2835;
    infoHeader->biYPelsPerMeter = 2835;
    infoHeader->biClrUsed = numberOfSymbols;
    infoHeader->biClrImportant = 0;
    infoHeader->biClrRotation = 0;
/// palette:
    int i = 0;
    for( auto& s : RGBVec )
    {
        ( &colorTable[ i ] )->rgbRed = s.r;
        ( &colorTable[ i ] )->rgbGreen = s.g;
        ( &colorTable[ i ] )->rgbBlue = s.b;
        ++i;
    }

/// apply pixel data:
    memcpy( m_BMPData, data, m_width * m_height );
/// save:
    std::ofstream file2( "out.bmp", std::ios::binary | std::ios::trunc );
    file2.write( ( char* )m_pBMPFile, m_BMPFileLength );
    file2.close();

    delete[] m_pBMPFile;

Compiled on VS2015, 64 bit.

  • 2
    Did you remember to [add padding](https://stackoverflow.com/a/2601392/560648)? – Lightness Races in Orbit Nov 19 '18 at 13:48
  • 2
    Did you run your code through a _debugger_ to determine the cause of the crash? – Lightness Races in Orbit Nov 19 '18 at 13:49
  • 1
    Agree with Lightness. Each scan line of the image needs to be padded on a 4 byte boundary. Even if your program didn't crash, more than likely your BMP file when viewed would have the "staircase" effect, showing you that the scanlines were off. – PaulMcKenzie Nov 19 '18 at 13:53
  • You have major bugs in the above code.. First off, you allocate `data` as [Width * Height].. but each pixel can be 3 bytes in size or 4 bytes in size.. + you didn't include padding.. + you didn't `delete[] data`.. oh man.. – Brandon Nov 19 '18 at 14:30

1 Answers1

0

The main issue is that m_width should be padded so that the width in bytes is divisible by 4. You can use the formula below for width_in_bytes which guarantees that. In this case it changes width from 61 to 64. The extra bytes can be ignored.

Furthermore you can simplify your code by avoiding overuse of pointers. Declaring headers as local variables is enough. Use std::vector to allocate data instead of new/delete. And use a single RGBQ header that can manage the data. Example:

struct RGBQ { uint8_t rgbBlue, rgbGreen, rgbRed, rgbReserved; };

int m_width = 61, m_height = 61;
int bitcount = 8;
int width_in_bytes = ((m_width * bitcount + 31) / 32) * 4;
int imagesize = width_in_bytes * m_height;

std::vector<RGBQ> color_table{ { 223, 223, 123 }, { 230, 0, 12 } };
std::vector<uint8_t> data(imagesize);
data[2] = 1;

FileHeader fileHeader = { 0 };
BitMapInfoHeader infoHeader = { 0 };
fileHeader.bfType = 0x4d42;            
fileHeader.bfSize = sizeof(fileHeader) + sizeof(infoHeader) + color_table.size()
    * sizeof(RGBQ) + data.size();
fileHeader.bfOffBits = sizeof(fileHeader) + sizeof(infoHeader);

infoHeader.biSize = 40;
infoHeader.biWidth = m_width;
infoHeader.biHeight = -m_height;       
infoHeader.biPlanes = 1;
infoHeader.biBitCount = bitcount;
infoHeader.biClrUsed = color_table.size();

std::ofstream file2("out.bmp", std::ios::binary | std::ios::trunc);
file2.write((char*)&fileHeader, sizeof(fileHeader));
file2.write((char*)&infoHeader, sizeof(infoHeader));
file2.write((char*)color_table.data(), color_table.size() * sizeof(RGBQ));
file2.write((char*)data.data(), data.size());
file2.close();
Barmak Shemirani
  • 30,904
  • 6
  • 40
  • 77