EXC_BAD_ACCESS on ofPixels.swapRgb() in ofImage.loadImage() with 640x480 img

Ok, I’m completely puzzled at this one, and it is so simple I’m sure I’ve lost perspective.

On trying to load a 640x480 image of I get an EXC_BAD_ACCESS in swapRgb(), precisely in std::swap

  
  
template<typename PixelType>  
void ofPixels_<PixelType>::swapRgb(){  
	if (channels >= 3){  
		int sizePixels		= width*height;  
		int cnt				= 0;  
		PixelType * pixels_ptr = pixels;  
                 
		while (cnt < sizePixels){  
                        // EXC_BAD_ACCESS on the next line  
			std::swap(pixels_ptr[0],pixels_ptr[2]);  
			cnt++;  
			pixels_ptr+=channels;  
		}  
	}  
}  
  

I’m printing the values of width, height and channels inside ofPixels::swapRgb and everything seems to be fine, but I get a crash every time I try to load a 640x480 image. Now, the really puzzling thing is that I don’t get the crash at other image sizes. But just 640x480. I’m attaching 4 test pngs. The 640x480s crash consistently, while the 640x481s load and display consistently using an unmodified imageLoaderExample in of007 (official zip) on osx 10.6. the only code I’m using is

  
bikers.loadImage("images/test640x480psd.png");  

640x480, saved with of grabScreen. crashes

640x480, created with photoshop. crashes

640x481, saved with of grabScreen. doesn’t crash

640x481, created with photoshop. doesn’t crash

could anyone give a try at loading these images with the imageSaverExample and see if they get the same results?
or does anyone have a clue, what else can be going on? I’m at a complete loss.

Thanks!
j

hey j,

I’ve run into this same problem! and had no idea how to fix it… I ended up commenting out swapRgb from the code.

It has something to do with the particular images I was loading though, it would crash on some, and not on others.

If you print out your image type do you get OF_IMAGE_UNDEFINED. i found that to be the case when this bug occurred.

hey thanks obviousjim. nice to see I’m not the only one.
I get OF_IMAGE_COLOR for both crashing and non-crashing images, which is correct.
The weird thing is that trying now again, I’ve had a couple of times where the 640x480 images have worked, so it might be something else going on. still puzzled.

Thanks!
j

I’ve had this problem sometimes. It seems pixels_ptr[2] is out of bounds, so I changed the swapRgb() method to the following. This seems to work fine (although I can imagine it skips the last pixel).

  
  
void ofPixels_<PixelType>::swapRgb(){  
	if (channels >= 3){  
		int sizePixels		= width*height;  
		int cnt				= 0;  
		PixelType * pixels_ptr = pixels;  
  
		while (cnt < sizePixels - 1){  
			std::swap(pixels_ptr[0],pixels_ptr[2]);  
			cnt++;  
			pixels_ptr+=channels;  
		}  
	}  
  

hey thanks!
that indeed does the trick at the expense of a pixel.
I still would like to know what’s going on here. Did you guys find any common feature for the images that crashed for you? I doubt it has anything to do with the size of the image, but that’s the only thing I’ve got so far.

if anyone wants to try and see if the images that crash for me (top post) crash for them that’d be great.

Cheers,
j

Both your crashing pictures from the top post don’t crash for me, using Ubuntu and the oF master from Oct. 17th.
I can’t see what should be wrong with the original swapRgb, aside from minor cosmetic stuff (interesting addressing method, but it should work; and shouldn’t this method be called swapRb?)
Has this been reported as an issue on github?

thanks bilderbuchi. I wonder if it’s just a mac issue.

Still haven’t reported as I was not sure if it was only my problem. being on such a commonly used method I was surprised it hadn’t been posted on the forum prevously.
Will report now.
Thanks!
j

It is indeed a weird way to loop through the pixel array…when I rewrite it like this:

  
  
template<typename PixelType>  
void ofPixels_<PixelType>::swapRgb(){  
	if (channels >= 3){  
		int nrOfPixels = width*height*channels;  
        	for(int i = 0; i < nrOfPixels; i+=channels){  
           		std::swap(pixels[i], pixels[i+2]);  
        	}  
	}  
}  
  

it loads images nicely. Anyone care to test it? It should work for every pixel now. Don’t see what would be functionally different from the previous implementation though…

actually, I’ve just tried with a fresh git checkout and haven’t had a single crash. so it might be solved already.

@daanvanhasselt great! will test that in 007 in a second

no crashes with your implementation in 007 either. Nice one Daan.

and actually, based on that code, if I apply swap directly to pixels instead of pixels_ptr in the original code

  
  
std::swap(pixels[0],pixels[2]);  
  

I get no crashes either, so we might have a culprit.
Still no idea what is wrong, or why it doesn’t happen to all images or all the time. Anyway, I won’t post an issue to github unless I get a crash on the version there .

Thanks for the help everyone.
j

daan, now that you say it, yes it’s a pretty weird way of iterating through the pixels :slight_smile: i changed it from:

  
  
for(int i = 0; i < width*height; i++){  
      PixelType  tmp = pixels[i*channels];  
      pixels[i*channels] = pixels[i*channels+2]  
      pixels[i*channels+2] = tmp;  
}    
  

to make it faster, but did that super weird while instead of just a for loop. will change it in git.

But anyway i don’t see why it should be crashing. as for the fact that it’s crashing with some sizes and not with others or in some OS and not another, seg faults are like that : ) it depends of lots of things like how much memory have you used before doing a wrong access. so it really has nothing to do with the size of the image.

I had a similar problem, but-with-images-that-were-1024px-wide. 1023px wide was OK, but 1024 wide or wider would cause the same sort of crash. It didn’t matter what the height of the image was, and it didn’t seem to matter what was in the image.

Arturo’s version of swapRgb() above fixed the problem though. Thanks!

@daanvanhasselt thanks! your code solved the problem for me too. I’m using of_preRelease_v007_osx.

ive just ran into the same issue.
its an ios app and runs fine in debug mode but then crashes inside the swapRgb() method in release build :S

@arturo
interesting point about the segfault…
does that mean that some other part of the program has written to a part of memory that it wasn’t supposed to touch, and when swapRgb() is doing its thing, it assumes that that part of memory is available when its actually already used?

ive implemented the fix and it now works fine…
would the fix be checked into the master branch on github since its only small fix?
or does it go into the develop branch for next release?

no it actually seems that swapRgb is doing a wrong access. and yes you are right will add that fix to master too.

Arturo is this in the master branch of OF on github? Thanks

AFAIK, it’s fixed in the develop branch.

it’s fixed in master now too

This really is a weird bug…I fixed this in my ios version of 007 but my app is being rejected by Apple because they can’t launch the app on their iPad 2. I only have a first-gen iPad to test with and can’t replicate the issue. The testers at Apple were kind enough to send along two crash reports and they both pointed right toward the swapRgb method. I just sent the build to a buddy of mine with an iPad 2 (via the excellent free testflight service) so this is to be continued!

This is indeed a very strange error. I’ve encountered it recently when trying to build my ofxVolumetricsExample on Mac. The error happens from the initial loadImage for my checkered background which is incidently 1024 pixels wide. The rest of the program loops through and loads 100 more images without a problem if I comment out the background image.

I had trouble believing that there was anything wrong with the swapRgb method because this same code works just fine in linux.

I eventually came to a solution where if I specifically allocate the image to the correct dimensions first, then load the image, everything works just fine:

  
  
background.allocate(1024,768,OF_IMAGE_COLOR);  
background.loadImage("background.png");  
  

To me, this says that the problem with the swapPixels() routine may be a red herring, but rather the automated allocation step when loading an image into an unallocated image object could be the culprit. I haven’t looked into it further.