1

Below is my program. I am trying to apply grayscale filter using bitmapdata class in visual c++. I am getting AccessViolationException at 11, tagged by the comment. I have tried using CLR:Safe and CLR:pure but no use. In c# this would be solved by using unsafe block. Any suggestions? None of the other solutions on related questions worked.

 Bitmap^ bmp = gcnew Bitmap(pictureBox1->Image);

BitmapData^ data = bmp->LockBits(Rectangle(0,0,bmp->Width,bmp->Height), ImageLockMode::ReadWrite, PixelFormat::Format24bppRgb);

        int blue=0, green=0, red=0; 

             System::IntPtr s = data->Scan0; 

            int* P = (int*)(void*)s;

            for (int i =0; i<bmp->Height;i++)

            {

                for (int j = 0; j < bmp->Width*3; j++)

                {

                    blue = (int)P[0];  //access violation exception

                    green =(int )P[1];

                    red = (int)P[2];

                    int avg = (int)((blue + green + red) / 3);

                    P[0] = avg;

                    P[1] = avg;

                    P[2] = avg;                       

                    P +=3;

                }

            }
            bmp->UnlockBits(data);
        pictureBox1->Image = bmp;
Ed S.
  • 122,712
  • 22
  • 185
  • 265
  • P += 3 is clearly incorrect, that offsets the pointer by 3 x sizeof(int) = 12 bytes. Using int* for a 24bpp image is not correct, use byte*. – Hans Passant Mar 07 '14 at 18:37

1 Answers1

1

You are using an int* when you should be using a byte*. Your pixels are three bytes each, one byte per channel. Your int is (likely) 4 bytes, so p[0] returns an entire pixel plus on byte past it. This is why you get an access violation; you are overrunning the bounds of the image buffer.

When you increment a pointer, you are adding sizeof *p bytes to it. In this case, P += 3 increments the pointer P by 12 bytes. Much too much, and you'll never be able to read a single pixel (or channel) of a 24bpp image with an int*. You are also assuming that your stride is Width * 3, which may or may not be correct (bitmaps are 4 byte aligned.)

Byte* base = (Byte*)data->Scan0;
int stride = data->Stride;

for(int y = 0; y < data->Height; ++y) {
    Byte* src = base + y * stride;
    for(int x = 0; x < data->Width; ++x, src += 3) {
        // bitmaps are stored in BGR order (though not really important here).
        // I'm assuming a 24bpp bitmap.
        Byte b = src[0];
        Byte g = src[1];
        Byte r = src[2];
        int average = (r + g + b) / 3;
        src[0] = src[1] = src[2] = (Byte)average;
    }
}
Ed S.
  • 122,712
  • 22
  • 185
  • 265
  • That's true. May also have to look at Bitmap.LockBits method, I thought it returned status - the pointer is returned via parameter (or is this C# not C++) ? – TonyWilk Mar 07 '14 at 18:37
  • @TonyWilk: `LockBits` returns a `BitmapData` object, which is what `data` is here. The call to `LockBits` is just not shown (here or in the OP's example). – Ed S. Mar 07 '14 at 18:38
  • @Ed S. It worked like a charm. It worked without stride reference too. – SSSpecialist Mar 08 '14 at 04:47
  • @SSSpecialist: You need to use stride when setting your pointer to the beginning of a row though. The stride is the width of a scan line, in bytes. The logical width of the image (in bytes) is not necessarily the stride. Bitmaps are 4 byte aligned, i.e., if the width is not a multiple of 4 (bytes) then they are padded. – Ed S. Mar 08 '14 at 05:27