3

I'm using OpenCV to iterate through an image and find the colour of each pixel, here's some of the code I'm using:

IplImage* img = cvLoadImage("c:\\test.png");

int pixels = img->height * img->width;
int channels = img->nChannels;

for (int i = 0; i < pixels*channels; i+= channels)
{

    unsigned char red = img->imageData[i + 2];
    unsigned char green = img->imageData[i + 1];
    unsigned char blue = img->imageData[i];

    outputRGBValues(red, green, blue);
    if (red == REDCOLOUR && green == GREENCOLOUR && blue == BLUECOLOUR)
    {
        count++;
    }
}
cvReleaseImage(&img);

When I run it, it outputRGBValues outputs negative values. Most often R, G, B = -1, but occasionally other negative numbers and a handful of positive numbers. I've heard something about uninitialized memory contents, and pixels not being allocated to the memory properly. But I'm don't really understand it and definitely don't know how to fix it. What am I doing wrong and how can I fix it?

UPDATE

After fixing the code (as above) with fschmitt's changes, I'm a bit closer. This is the image I'm using, if it's any help. Quite hard to see, but it's just a 5*3 'V' of black pixels with one green one at the bottom.

Running the code on it, I get this output:

0 0 0
255 255 255
255 255 255
255 255 255
0 0 0
255 255 186
0 0 255
255 255 0
And it continues

The first 5 lines are fine, exactly what they should be. That's the top row of the image. The next row, 6th line onwards is wrong. It should be:

255 255 255
255 255 255
0 0 0

I'm not sure what's causing this. How can it work for the top line and not the second? Is there some sort of mismatch, and its taking the value one bit to the left of where it should?

JBenson
  • 293
  • 2
  • 5
  • 13
  • If red green and blue are supposed to be from 0 to 255 why are you using an int? Why not an unsigned char? – JoshD Oct 04 '10 at 21:23

4 Answers4

4

Try this:

IplImage* img = cvLoadImage("c:\\test.png");

for (int i = 0; i < img->height; i++)
{
    for (int j = 0; j < img->width; j += img->nChannels)
    {
        unsigned char red = img->imageData[i * img->widthStep + j + 2];
        unsigned char green = img->imageData[i * img->widthStep + j + 1];
        unsigned char blue = img->imageData[i * img->widthStep + j];

        outputRGBValues(red, green, blue);
        if (red == REDCOLOUR && green == GREENCOLOUR && blue == BLUECOLOUR)
        {
            count++;
        }
    }
}
cvReleaseImage(&img);
Paul R
  • 208,748
  • 37
  • 389
  • 560
  • do note widthstep is member of IplImage, instead of instance variable in given code. – YeenFei Oct 05 '10 at 05:44
  • @YeenFei: thanks for the correction - now edited accordingly - not sure what you mean about byte alignment in your second comment though ? – Paul R Oct 05 '10 at 07:10
  • i mistaken that you are incrementing pointer offset by nChannels, bad mistake. Deleted my second reply. – YeenFei Oct 06 '10 at 01:57
2

I don't understand why you are accessing pixel data blindly (by 1D index) while you already know the image dimension (proved by the usage of IplImage::height and IplImage::width) and opencv does provide function to access pixel value (cvGet*D).

You are getting odd values in the direct pointer access because you did not factor in the byte padding performed by OpenCV. You may find in 8 bit images, (width*nChannels <= widthStep) for this reason.

A quick revised code can be as follow:

IplImage* img = cvLoadImage("c:\\test.png");
for(int iRow=0; iRow<img->height; ++iRow)
{
    for(int iCol=0; iCol<img->width; ++iCol)
    {
        CvScalar pixelVal = cvGet2D( img, iRow, iCol);
        unsigned char red = pixelVal.val[2];
        unsigned char green = pixelVal.val[1];
        unsigned char blue = pixelVal.val[0];

        outputRGBValues(red, green, blue);
        if (red == REDCOLOUR && green == GREENCOLOUR && blue == BLUECOLOUR)
        {
            count++;
        }
    }
}
cvReleaseImage(&img);

Also, note that usually OpenCV arrange data in BGR format, which can verified at IplImage::channelSeq. Do not confused by IplImage::colorModel, which tells the color model.

I'll go with Paul R suggestion that you should find a reference and understand the library before attempting to use them.

YeenFei
  • 3,180
  • 18
  • 26
1

I've never used OpenCV, but I would imagine that cvCreateImage doesn't initialise the contents of the image, and certainly not to anything meaningful.

Oliver Charlesworth
  • 267,707
  • 33
  • 569
  • 680
1

Quite some issues here:

  • You need to fill the image with meaningful data
  • Depending on how the image was constructed, you may have swapped red and blue
  • You should use unsigned char red
  • You declare a vector for each pixel and don't use it
fschmitt
  • 3,478
  • 2
  • 22
  • 24
  • I thought that pixel colour was stored as BGR, not RGB in imageData? Thanks for picking those things up, I'll edit with the changes. – JBenson Oct 04 '10 at 21:32
  • It depends on how you construct the image, but indeed you are right, the default is BGR, I edited the answer. – fschmitt Oct 04 '10 at 21:38
  • Okay, I'm getting closer. Post updated, the first row of pixels is found correctly, but it's wrong from the second row onwards – JBenson Oct 04 '10 at 21:55
  • @JBenson: your method for accessing pixel data is incorrect, as it does not take into account the difference between `width` and `widthStep` (there may be padding at the end of each row, hence the difference). Recommend reading O'Reilly "Learning OpenCV" which covers the basics such as this in detail. – Paul R Oct 04 '10 at 22:06