compare images:best method..

[dave2k]

i have two images which are 4 bytes per pixel. i need to compare them to make sure that if the colour black, i.e. oxff,oxff,oxff occurs in either image, it occurs in the other image and the same coorindates. so far i am looping through using memcmp, and my code i below. But i have heard that because each pixel is a 32 bit value, i could use a simple int comparison. How could i change my code below to do this? And which method is quicker: mine or the nit comparison? Thanks.

bool IsBlack(BYTE* byt)
{
BYTE black[] = {0x00,0x00,0x00};
return memcmp(black, byt, 3) == 0;
} // 1st image
HANDLE hBm1 = ::LoadImage(0, "C:\\Documents and Settings\\ftwyman.MARINEDATA\\Desktop\\1.bmp",
IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE );
BITMAP bm1;
::GetObject(hBm1, sizeof(BITMAP), &bm1);
int size1 = bm1.bmHeight * bm1.bmWidth * bm1.bmBitsPixel / 8;
BYTE *bytes1 = new BYTE[size1];
::GetBitmapBits((HBITMAP)hBm1, size1, bytes1);

// 2nd image
HANDLE hBm2 = ::LoadImage(0, "C:\\Documents and Settings\\ftwyman.MARINEDATA\\Desktop\\2.bmp",
IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE );
BITMAP bm2;
::GetObject(hBm2, sizeof(BITMAP), &bm2);
int size2 = bm2.bmHeight * bm2.bmWidth * bm2.bmBitsPixel / 8;
BYTE *bytes2 = new BYTE[size2];
::GetBitmapBits((HBITMAP)hBm2, size2, bytes2);

for(int i=0; i < size1; i+=4)
{
if(IsBlack(&bytes1[i]) ^ IsBlack(&bytes2[i]))
{
break;
}
}

if(i == size1)
std::cout << "matching!";

[Desade]

You've already encapsulated your pixel comparison in IsBlack, so the change is very simple. IsBlack should change to:


bool IsBlack(BYTE* byt)
{
int black = 0;
return memcmp(&black, byt, 3) == 0;
}


Or, an alternate strategy that's a little uglier but avoids memcmp:


bool IsBlack(BYTE* byt)
{
return *((int *)byt) == 0;
}


Now as for your other question... is this faster? I doubt it, but it's such an easy change you might as well try it and see what you think. You might also want to try explicitly inlining or turning into a macro this IsBlack function to avoid stack allocation. That MIGHT speed things up a bit, but I suspect the compiler is already inlining it when you build for Release anyhow.

One thing that concerns me, though, is that you are querying the Bitmap object to get the bits/pixel and translating that into bytes/pixel when you allocate your array, but you are then assuming exactly 4 bytes later on (increment of your indexer and IsBlack itself). If the bytes/pixel on these images changes in the future, this bug might sneak through. Just a heads-up.

[dave2k]

how can i explicitly inline the function? also is there any way of telling whetehr compiler inlining the function for me?

[VladimirF]

As Desade noted, your code only works for 32-bit images.
If that is OK with you, you can optimize your code this way:
- instead of BYTE arrays use int; this will let you use simple increment in a loop instead of +=4; of course, adjust the size of these arrays (remove "* bmX.bmBitsPixel / 8" from your formulas); I would at least check that bmBitsPixel is 32.
- if you limit your tests to black color (value of 0x00000000), you do not need IsBlack() at all, inlined or otherwise; 32-bit test for black is:
if(ints[i] == 0)
...

- since you are using XOR, you could test for "NOT black" as well, which looks cleaner:if( ints1[i] ^ ints2[i])
break;
Also, your test for "if(i == size1)" after the loop is WRONG - the value of "i" is undefined outside the loop, and according to the C++ standard is not even visible there. Instead, use bool flag, like bMatch; set it to true before the loop, and set to false right before the break, when you determined that bitmaps don't match. Then just teat this flag after your loop.

[dave2k]

if i wanted to test for non-black value like yellow, which i think is 0xff,0xff,0x00, i will still need a function for this?

[VladimirF]

if i wanted to test for non-black value like yellow, which i think is 0xff,0xff,0x00, i will still need a function for this?
No, you do not NEED it. Functions help you to break your code into manageable pieces, reuse your code, etc. You can code your functionality right where it's needed:
if( (ints1[i] == 0xffff00) ^ (ints2[i] == 0xffff00) )
break;
Or use macros as was suggested by Desade.