help writing to locked bitmap

William Labbett

Hi guys,

I've written some code to merge two black and white bitmaps. The code crashes when
writing to the output bitmap.

I've done some detective work but not found out what's wrong. The bad code is somewhere near the end this code :

#SelectExpand
   1int merge_black_and_white(ALLEGRO_CONFIG *cfg)
   2{
unsigned char mr, mg, mb; /*                 */
ALLEGRO_BITMAP *bw1;
ALLEGRO_BITMAP *bw2;
const char *bw1_name;
const char *bw2_name;
const char *config_string;
ALLEGRO_LOCKED_REGION *lock1;
ALLEGRO_LOCKED_REGION *lock2;
ALLEGRO_LOCKED_REGION *lock3;
ALLEGRO_BITMAP *output;
int blend_type;
int x, y;
unsigned char *ptr1, *ptr2, *ptr3;
int r1, g1, b1, r2, g2, b2, a1, a2;
unsigned char new_r, new_g, new_b;
unsigned char new_gray;

uint32_t new_colour;


/***** Load the first black and white bitmap. *****************************************/

bw1_name = al_get_config_value( cfg, NULL, "bw_picture1" );
  if(bw1_name == NULL)
  {
   cout << "No value for \"bw_picture1\" in config file.\n";
   eek();
  }
  else
  {
   cout << "Name of bitmap 1 is " << bw1_name << "\n\n";   
  }
  
  
cout << "Attempting to load " << bw1_name << "\n\n";

bw1 = al_load_bitmap(bw1_name);
 
  if(bw1 == NULL)
  {
  cout << "Loading failed.\n"; 
  eek(); 
  }
  else
  {
  cout << "Loaded it.\n";  
  } 

/***** Load the second black and white bitmap. ****************************************/

  
  bw2_name = al_get_config_value( cfg, NULL, "bw_picture2");
  if( bw2_name  == NULL )
  {
   cout << "No value for \"bw_picture2\" in config file.\n";
   eek();
  }
  else
  {
   cout << "Name of bitmap 2 is " << bw2_name << "\n\n";   
  }
  
  cout << "Attempting to load " << bw2_name << "\n\n";
 
  bw2 = al_load_bitmap( bw2_name );
 
  if(bw2 == NULL)
  {
   cout << "Loading failed.\n"; 
   return 1;
  }
  else
  {
   cout << "Loaded it.\n";  
  } 
  
  /*********************************************************************************************/



int bw = al_get_bitmap_width( bw1 );
int bh = al_get_bitmap_height( bw1 );

if( bw != al_get_bitmap_width(bw2) || bh != al_get_bitmap_height(bw2) )
{
   
   cout << "Bitmaps are not the same dimensions.\n";
  
   return 1;  
  }
  else
  {
    cout << "bw1 : " << bw << " * " << bh << "\n\n bw2 : " << al_get_bitmap_width(bw2) << " * " << al_get_bitmap_height(bw2) << "\n\n";  
}


output = al_create_bitmap( bw, bh );

if(output == NULL)
{
   cout << "couldn't create the output bitmap.\n";
   return 1;  
  }


lock1 = al_lock_bitmap( bw1, ALLEGRO_PIXEL_FORMAT_RGBA_8888,  ALLEGRO_LOCK_READONLY );

if(lock1 == NULL)
{
   cout << "Couldn't lock bitmap 1.\n";
   return 1;
  }

cout << " locked bitmap 1.\n\n";

lock2 = al_lock_bitmap( bw2, ALLEGRO_PIXEL_FORMAT_RGBA_8888, ALLEGRO_LOCK_READONLY );

if(lock2 == NULL)
{
  cout << "Couldn't lock bitmap 1.\n";
  return 1;
}

cout << " locked bitmap 2.\n\n";



output = al_create_bitmap( bw, bh );
if(output == NULL)
{
   cout << "Couldn't create the new bitmap.\n";
   return 1;  
  }




cout << "Getting blend type.\n";

/* Using bw1_name out of context here instead of using another variable. */

bw1_name = al_get_config_value( cfg, NULL, "black_and_white_merge_blend_type" );


if( strcmp( bw1_name, "average" ) == 0 )
{
  blend_type = BLEND_MODE_AVERAGE;  
}
else if( strcmp( bw1_name, "darkest" ) == 0 )
{
  blend_type = BLEND_MODE_DARKEST;
}
else
{
   cout << "No blend mode specified in configuration file.\n\n";
   return 1;  
}


lock3 = al_lock_bitmap( output, ALLEGRO_PIXEL_FORMAT_RGBA_8888,  ALLEGRO_LOCK_WRITEONLY );
if(lock3 == NULL)
{
   cout << "Couldn't lock output bitmap.\n\n";
   return 1;  
}
  
  /***************************************************************************************************/
  /****************** Get the colour which is the mask colour. ***************************************/
  /***************************************************************************************************/
  
  
  if( get_mask_colour(cfg, &mr, &mg, &mb) != 0 )
{
     cout << "Couldn't get mask colour from configuration file.\n\n";
     return 1;
}

/***************************************************************************************************/

cout << "processing bitmaps.\n\n";

for( y = 0; y < bh; ++y )
{
   for( x = 0; x < bw; ++x )
   {  
    cout << "y = " << y << "x = " << x << "\n\n";
    ptr1 = ((unsigned char *) lock1) + lock1->pitch * y + x * 4;
    ptr2 = ((unsigned char *) lock2) + lock2->pitch * y + x * 4;
    
    ptr3 = ((unsigned char *) lock3) + lock3->pitch * y + x * 4;
    
    //cout << "got pointers.\n\n";
    
    r1 = (int) *(ptr1 + 3);
    g1 = (int) *(ptr1 + 2);
    b1 = (int) *(ptr1 + 1);
      a1 = (int) *(ptr1 + 0);
      
      r2 = (int) *(ptr2 + 3);
      g2 = (int) *(ptr2 + 2);
      b2 = (int) *(ptr2 + 1);
      a2 = (int) *(ptr2 + 0);
      
      /*
      if(a1 != 255 || a2 != 255)
      {
     cout << "Alpha of at least one pixel on bitmap is not 255. Aborting in case this matters.\n\n";
     cout << "x = " << x << "y = " << y << " a1 = " << a1 << " a2 = " << a2 << "\n\n";
     return 1;  
    }
      else 
      * 
      */
      /*
      if( (r1 != g1 || g1 != b1 || r1 != b1) )
      {
     cout << "A pixel on bitmap 1 is not grey.\n\n";  
     return 1;
    }
    else if( (r2 != g2 || g2 != b2 || r2 != b2) )
      {
     cout << "A pixel on bitmap 2 is not grey.\n\n";  
     return 1;
    }
    */
    
    switch(blend_type)
    {
       case BLEND_MODE_AVERAGE:
       
          new_gray = (unsigned char) ((((float) r1 + (float) r2) / 2.0) + 0.5);
      
          break;
       case BLEND_MODE_DARKEST:
          
          new_gray = (unsigned char) (r1 > r2) ?  r1 : r2;
         
            break;
      }
      
      new_colour = ((new_gray) << 24) | ((new_gray) << 16) | ((new_gray) << 8) | 255;
      
      
      if( (r1 == mr && g1 == mg && b1 == mb) || (r2 == mr && g2 == mg && b2 == mb) )
      {
     /* Put */
      
      cout << "white.\n";
     *((uint32_t *) ptr3) = 0xffffffff;
      }
      else
      {
     cout << "grey.\n";
         *((uint32_t *) ptr3) = new_colour;
      }
   }
}

cout << "unlocking bitmaps.\n";


al_unlock_bitmap( bw1 );
al_unlock_bitmap( bw2 );
al_unlock_bitmap( output );



al_save_bitmap("output.png", output);

 272}

ph03nix

I just skimmed over, but two things caught my eye:

lock3 = al_lock_bitmap( output, ALLEGRO_PIXEL_FORMAT_RGBA_8888, ALLEGRO_LOCK_WRITEONLY );

if you are reading and writing to the bitmap, shouldn't you use ALLEGRO_LOCK_READWRITE? Not sure if that's causing the crash.

Also, this at the end:

*((uint32_t *) ptr3) = 0xffffffff;

It seems like you're setting the alpha to the combined value of the colour. It should be more like:

*((uint32_t *) ptr3) = 0xff; //a
*((uint32_t *) ptr3+1) = 0xff; //b
*((uint32_t *) ptr3+2) = 0xff; //g
*((uint32_t *) ptr3+3) = 0xff; //r

What I think is happening is that the value you set for the pixel is out of range, causing the crash.

Trent Gamblin

ph03nix said:

Also, this at the end:

*((uint32_t *) ptr3) = 0xffffffff;

It's perfectly ok to do that. That would set the pixel to white.

I'm not sure what the problem is William, did you try running it through a debugger?

ph03nix

Doesn't that set the value of the array at position ptr3 to 0xffffffff (4294967295)? Let's say I actually wanted to set unsigned char at that position to that value, that's how I would do it, right? (although it would become 0 or something)

Trent Gamblin

Yeah that's what it does.

This:

unsigned char *ptr;
*((uint32_t *)ptr) = 0xaabbccdd;

Is identical to this:

unsigned char *ptr;
*(ptr+0) = 0xdd;
*(ptr+1) = 0xcc;
*(ptr+2) = 0xbb;
*(ptr+3) = 0xaa;

ph03nix

I wrote this test code:

unsigned char* test = new unsigned char[4];
test[0]=0;
test[1]=0;
test[2]=0;
test[3]=0;
*((unsigned char*)test) = 0xffffffff;
std::cout<<(int)test[0]<<", "<<(int)test[1]<<", "<<(int)test[2]<<", "<<(int)test[3]<<std::endl;
delete []test;

and the output was 255, 0, 0, 0

Trent Gamblin

That's because you casted to unsigned char instead of uint32_t.

ph03nix

Oh, I see now...

William Labbett

Program received signal SIGSEGV, Segmentation fault.
0x004024fd in _fu63___ZSt4cout ()
(gdb)

I've rewritten the code a bit :

#SelectExpand
   1int merge_black_and_white(ALLEGRO_CONFIG *cfg)
   2{
unsigned char mr, mg, mb; /*                 */
ALLEGRO_BITMAP *bw1;
ALLEGRO_BITMAP *bw2;
const char *bw1_name;
const char *bw2_name;
const char *config_string;
ALLEGRO_LOCKED_REGION *lock1;
ALLEGRO_LOCKED_REGION *lock2;
ALLEGRO_LOCKED_REGION *lock3;
ALLEGRO_BITMAP *output;
int blend_type;
int x, y;
unsigned char *ptr1, *ptr2, *ptr3;
int r1, g1, b1, r2, g2, b2, a1, a2;
unsigned char new_r, new_g, new_b;
unsigned char new_gray;

uint32_t new_colour;


/***** Load the first black and white bitmap. *****************************************/

bw1_name = al_get_config_value( cfg, NULL, "bw_picture1" );
  if(bw1_name == NULL)
  {
   cout << "No value for \"bw_picture1\" in config file.\n";
   eek();
  }
  else
  {
   cout << "Name of bitmap 1 is " << bw1_name << "\n\n";   
  }
  
  
cout << "Attempting to load " << bw1_name << "\n\n";

bw1 = al_load_bitmap(bw1_name);
 
  if(bw1 == NULL)
  {
  cout << "Loading failed.\n"; 
  eek(); 
  }
  else
  {
  cout << "Loaded it.\n";  
  } 

/***** Load the second black and white bitmap. ****************************************/

  
  bw2_name = al_get_config_value( cfg, NULL, "bw_picture2");
  if( bw2_name  == NULL )
  {
   cout << "No value for \"bw_picture2\" in config file.\n";
   eek();
  }
  else
  {
   cout << "Name of bitmap 2 is " << bw2_name << "\n\n";   
  }
  
  cout << "Attempting to load " << bw2_name << "\n\n";
 
  bw2 = al_load_bitmap( bw2_name );
 
  if(bw2 == NULL)
  {
   cout << "Loading failed.\n"; 
   return 1;
  }
  else
  {
   cout << "Loaded it.\n";  
  } 
  
  /*********************************************************************************************/



int bw = al_get_bitmap_width( bw1 );
int bh = al_get_bitmap_height( bw1 );

if( bw != al_get_bitmap_width(bw2) || bh != al_get_bitmap_height(bw2) )
{
   
   cout << "Bitmaps are not the same dimensions.\n";
  
   return 1;  
  }
  else
  {
    cout << "bw1 : " << bw << " * " << bh << "\n\n bw2 : " << al_get_bitmap_width(bw2) << " * " << al_get_bitmap_height(bw2) << "\n\n";  
}





lock1 = al_lock_bitmap( bw1, ALLEGRO_PIXEL_FORMAT_RGBA_8888,  ALLEGRO_LOCK_READONLY );

if(lock1 == NULL)
{
   cout << "Couldn't lock bitmap 1.\n";
   return 1;
  }

cout << " locked bitmap 1.\n\n";

lock2 = al_lock_bitmap( bw2, ALLEGRO_PIXEL_FORMAT_RGBA_8888, ALLEGRO_LOCK_READONLY );

if(lock2 == NULL)
{
  cout << "Couldn't lock bitmap 1.\n";
  return 1;
}

cout << " locked bitmap 2.\n\n";




output = al_create_bitmap( bw, bh );
if(output == NULL)
{
   cout << "Couldn't create the new bitmap.\n";
   return 1;  
  }


cout << "output bitmap : " << al_get_bitmap_width(output)  << " * " << al_get_bitmap_height(output) << "\n\n";

cout << "Getting blend type.\n";

/* Using bw1_name out of context here instead of using another variable. */

bw1_name = al_get_config_value( cfg, NULL, "black_and_white_merge_blend_type" );


if( strcmp( bw1_name, "average" ) == 0 )
{
  blend_type = BLEND_MODE_AVERAGE;  
}
else if( strcmp( bw1_name, "darkest" ) == 0 )
{
  blend_type = BLEND_MODE_DARKEST;
}
else
{
   cout << "No blend mode specified in configuration file.\n\n";
   return 1;  
}


lock3 = al_lock_bitmap( output, ALLEGRO_PIXEL_FORMAT_RGBA_8888,  ALLEGRO_LOCK_READWRITE );
if(lock3 == NULL)
{
   cout << "Couldn't lock output bitmap.\n\n";
   return 1;  
}
  
  /***************************************************************************************************/
  /****************** Get the colour which is the mask colour. ***************************************/
  /***************************************************************************************************/
  
  
  if( get_mask_colour(cfg, &mr, &mg, &mb) != 0 )
{
     cout << "Couldn't get mask colour from configuration file.\n\n";
     return 1;
}

/***************************************************************************************************/

cout << "processing bitmaps.\n\n";

for( y = 0; y < bh; ++y )
{
   for( x = 0; x < bw; ++x )
   {  
    //cout << "y = " << y << "x = " << x << "\n\n";
    ptr1 = ((unsigned char *) lock1) + lock1->pitch * y + x * 4;
    ptr2 = ((unsigned char *) lock2) + lock2->pitch * y + x * 4;
    
    ptr3 = ((unsigned char *) lock3) + lock3->pitch * y + x * 4;
    
    //cout << "got pointers.\n\n";
    
    r1 = (int) *(ptr1 + 3);
    g1 = (int) *(ptr1 + 2);
    b1 = (int) *(ptr1 + 1);
      a1 = (int) *(ptr1 + 0);
      
      r2 = (int) *(ptr2 + 3);
      g2 = (int) *(ptr2 + 2);
      b2 = (int) *(ptr2 + 1);
      a2 = (int) *(ptr2 + 0);
      
      /*
      if(a1 != 255 || a2 != 255)
      {
     cout << "Alpha of at least one pixel on bitmap is not 255. Aborting in case this matters.\n\n";
     cout << "x = " << x << "y = " << y << " a1 = " << a1 << " a2 = " << a2 << "\n\n";
     return 1;  
    }
      else 
      * 
      */
      /*
      if( (r1 != g1 || g1 != b1 || r1 != b1) )
      {
     cout << "A pixel on bitmap 1 is not grey.\n\n";  
     return 1;
    }
    else if( (r2 != g2 || g2 != b2 || r2 != b2) )
      {
     cout << "A pixel on bitmap 2 is not grey.\n\n";  
     return 1;
    }
    */
    
    switch(blend_type)
    {
       case BLEND_MODE_AVERAGE:
       
          new_gray = (unsigned char) ((((float) r1 + (float) r2) / 2.0) + 0.5);
      
          break;
       case BLEND_MODE_DARKEST:
          
          new_gray = (unsigned char) (r1 > r2) ?  r1 : r2;
         
            break;
      }
      
      //cout << "new_gray = " << (int) new_gray << "\n";
      
      new_colour = ((new_gray) << 24) | ((new_gray) << 16) | ((new_gray) << 8) | 255;
      
      
      if( (r1 == mr && g1 == mg && b1 == mb) || (r2 == mr && g2 == mg && b2 == mb) )
      {
     /* Put */
      
      //cout << "white.\n";
     *((uint32_t *) ptr3) = 0xffffffff;
      }
      else
      {
     //cout << "grey.\n";
     //cout << "x = " << x << "y = " << y << "\n\n";
         *((uint32_t *) ptr3) = new_colour;
      }
   }
}

cout << "unlocking bitmaps.\n";


al_unlock_bitmap( bw1 );
al_unlock_bitmap( bw2 );
al_unlock_bitmap( output );



al_save_bitmap("output.png", output);

 270}

Trent Gamblin

The symbol name suggests it's crashing in cout. Compile with debugging (use -g command line switch to gcc/g++). Then try again, it should tell you a line number. Also run "bt" in gdb after it crashes.

William Labbett

g++ colourer.cpp shade_coloured_picture.cpp merge_black_and_white.cpp text_output_management.cpp -o colourer.exe -lallegro-5.0.5-monolith-md

Where do I put the -g in this line please ?

Trent Gamblin

Doesn't really matter, but let's just say "g++ -g ..." .

William Labbett

Thanks.

Got some better output :-

Program received signal SIGSEGV, Segmentation fault.
0x00402530 in _fu64___ZSt4cout () at merge_black_and_white.cpp:228
228 r2 = (int) *(ptr2 + 3);
(gdb)

Trent Gamblin

So that means line 228 in merge_black_and_white.cpp is what's crashing. Is that what we're looking at? If so I don't think your code listing is up to date because that's not what line 228 looks like.

EDIT: make sure your code and executable are matching.

William Labbett

Okay. nm the code so far.

This code :

#SelectExpand
   1cout << "pitch members of locks :";
cout << "lock1->pitch = " << lock1->pitch << "\n";
cout << "lock2->pitch = " << lock2->pitch << "\n";
cout << "lock3->pitch = " << lock3->pitch << "\n";
  
  /***************************************************************************************************/
  /****************** Get the colour which is the mask colour. ***************************************/
  /***************************************************************************************************/
  
  
  if( get_mask_colour(cfg, &mr, &mg, &mb) != 0 )
{
     cout << "Couldn't get mask colour from configuration file.\n\n";
     return 1;
}

/***************************************************************************************************/

cout << "processing bitmaps.\n\n";

for( y = 0; y < bh; ++y )
{
   for( x = 0; x < bw; ++x )
   {  
    //cout << "y = " << y << "x = " << x << "\n\n";
    
    if(x > al_get_bitmap_width(output))
    {
      cout << "problem 1000.\n";
      return 1;
      }
      
      if(x >= lock1->pitch)
      {
     cout << "x outside bitmap (1).\n";
     return 1;  
    }
    
    if(x >= lock2->pitch)
    {
     cout << "x outside bitmap (2).\n";
     return 1;  
    } 
    
    if(x >= lock3->pitch)
    {
     cout << "x outside bitmap (3).\n";
     
     cout << "pitch of lock3 is " << lock3->pitch << " x is " << x << "\n\n";
     
     return 1;  
    }
    
    
    ptr1 = ((unsigned char *) lock1) + lock1->pitch * y + x * 4;
    ptr2 = ((unsigned char *) lock2) + lock2->pitch * y + x * 4;
    ptr3 = ((unsigned char *) lock3) + lock3->pitch * y + x * 4;
   
    
    r1 = (int) *(ptr1 + 3);
    g1 = (int) *(ptr1 + 2);
    b1 = (int) *(ptr1 + 1);
      a1 = (int) *(ptr1 + 0);
      
      r2 = (int) *(ptr2 + 3);
      g2 = (int) *(ptr2 + 2);
      b2 = (int) *(ptr2 + 1);
      a2 = (int) *(ptr2 + 0);
      
      /*
      if(a1 != 255 || a2 != 255)
      {
     cout << "Alpha of at least one pixel on bitmap is not 255. Aborting in case this matters.\n\n";
     cout << "x = " << x << "y = " << y << " a1 = " << a1 << " a2 = " << a2 << "\n\n";
     return 1;  
    }
      else 
      * 
      */
      /*
      if( (r1 != g1 || g1 != b1 || r1 != b1) )
      {
     cout << "A pixel on bitmap 1 is not grey.\n\n";  
     return 1;
    }
    else if( (r2 != g2 || g2 != b2 || r2 != b2) )
      {
     cout << "A pixel on bitmap 2 is not grey.\n\n";  
     return 1;
    }
    */
    
    switch(blend_type)
    {
       case BLEND_MODE_AVERAGE:
       
          new_gray = (unsigned char) ((((float) r1 + (float) r2) / 2.0) + 0.5);
      
          break;
       case BLEND_MODE_DARKEST:
          
          new_gray = (unsigned char) (r1 > r2) ?  r1 : r2;
         
            break;
      }
      
      //cout << "new_gray = " << (int) new_gray << "\n";
      
      new_colour = ((new_gray) << 24) | ((new_gray) << 16) | ((new_gray) << 8) | 255;
      
      
      if( (r1 == mr && g1 == mg && b1 == mb) || (r2 == mr && g2 == mg && b2 == mb) )
      {
     /* Put */
      
      //cout << "white.\n";
     *((uint32_t *) ptr3) = 0xffffffff;
      }
      else
      {
    
         *((uint32_t *) ptr3) = new_colour;
      }
   }
}

cout << "unlocking bitmaps.\n";


al_unlock_bitmap( bw1 );
al_unlock_bitmap( bw2 );
al_unlock_bitmap( output );

...gives this output :-

#SelectExpand
  1Name of bitmap 1 is tree1_bw.png
  2
  3Attempting to load tree1_bw.png
  4
  5[New Thread 4644.0xa6c]
  6Loaded it.
  7Name of bitmap 2 is tree2_bw.png
  8
  9Attempting to load tree2_bw.png
 10
 11Loaded it.
 12bw1 : 1938 * 3486
 13
 14 bw2 : 1938 * 3486
 15
 16 locked bitmap 1.
 17
 18 locked bitmap 2.
 19
 20output bitmap : 1938 * 3486
 21
 22Getting blend type.
 23pitch members of locks :lock1->pitch = 7752
 24lock2->pitch = 7752
 25lock3->pitch = 7752
 26processing bitmaps.
 27
 28x outside bitmap (3).
 29pitch of lock3 is 255 x is 255
 30
 31Press any key to continue . . .
 32Program received signal SIGSEGV, Segmentation fault.

Which means the pitch of lock3 is changed to 255 at some point.

Any ideas what this could be ?

Memory corruption ?

EDIT :

Would this do anything :-

#include <stdint.h>

It's at the top of the source file.
I tried doing

#include <cstdint>

..but the compiler complained.

Trent Gamblin

Oop! I didn't notice this... instead of using ((unsigned char *)lock1) etc, use lock1->data.

William Labbett

Many thanks Trent!

Thread #613082. Printed from Allegro.cc