BradLarson / GPUImage

An open source iOS framework for GPU-based image and video processing
http://www.sunsetlakesoftware.com/2012/02/12/introducing-gpuimage-framework
BSD 3-Clause "New" or "Revised" License
20.25k stars 4.61k forks source link

Crash at GPUImageFilter::CFRetain(renderTarget); with latest framework #389

Closed CullenSUN closed 10 years ago

CullenSUN commented 12 years ago

Hi Brad,

I pulled the latest code.

The line of code:

UIImage *filteredImage=[myGroupfilter imageByFilteringImage:myUIImage];

used to work fine, but now always crash, and pointed to Thread 1: trace>

     CFRetain(renderTarget); // I need to retain the pixel buffer here and release in the data source callback to prevent its bytes from being prematurely deallocated during a photo write operation.

Thanks

brspurri commented 12 years ago

I've seen this before as well, but I just avoided whatever I was doing that caused this (can't remember now). So I'd be curious to see if anyone else has run into this as well.

On Wed, Aug 15, 2012 at 1:41 PM, CullenSUN notifications@github.com wrote:

Hi Brad,

I pulled the latest code.

The line of code:

UIImage *filteredImage=[myGroupfilter imageByFilteringImage:myUIImage];

used to work fine, but now always crash, and pointed to Thread 1: trace>

 CFRetain(renderTarget); // I need to retain the pixel buffer here and release in the data source callback to prevent its bytes from being prematurely deallocated during a photo write operation.

Thanks

— Reply to this email directly or view it on GitHubhttps://github.com/BradLarson/GPUImage/issues/389.

CullenSUN commented 12 years ago

Direct reason to crash: I have GPUImageGaussianSelectiveBlurFilter in the filtergroup. Removed it, no crash. However found another bug.

[myGroupfilter imageByFilteringImage:myUIImage]; has no effect at all. Realized that Brad changed a lot code for this class. Could you take a look? Thank a lot.

dmitric commented 12 years ago

I noticed I was getting this when using any camera size other then the full resolution on iPhone 4S when doing an imageCapture from live source. Even the back camera on an iPad gets me this one as well.

eaceto commented 11 years ago

Closed? It was fixed? I still got this error. I made a UIViewController with a central big image, and a small carrousel with a lot of filters previewing that image with the filter applied).

When I select a filter, i get "CGContextDrawImage: invalid context 0x0" but this does not crash my app.

When I select a filter, and in less than a second I select another filter, I get the crash at GPUImageFilter -> CFRetain(renderTarget); because renderTarget is 0x00000000.

It happens no matter the image orientation is portrait or landscape.

How can I fix this? preventing the CFRetain when renderTarget is nil?

thanks!

BradLarson commented 11 years ago

Sorry, mistakenly closed this instead of another one. This is still an issue that I'm tracking down.

dpestana commented 11 years ago

I'm experiencing the same problem as well.

BradLarson commented 11 years ago

@dpestana - This happens with the very latest code in the repository (the commit I made yesterday)? I was experimenting with something that I thought could fix this, but it might not be handling all cases.

If you still see this, when exactly does it happen, and with what kind of a filter chain? Can you modify one of the example applications to exhibit this behavior?

CullenSUN commented 11 years ago

I remember: I got this when I used a DoingNothingFilter addTarget colorFilter1, processImage, then remove colorFilter1, addTarget colorFilter2, processImage, CRASH here!! Worked around by release DoingNothingFilter first, create a new DoingNothingFilter addTarget colorFilter2, processImage

dpestana commented 11 years ago

Hi @BradLarson. I had to go back to the old code, because when I solved the problem with taking pictures I had problems with my ImagePicker (crashing after selecting more than one picture and applying filters to them). Now it's working great but I did a number of things to get it to work. It's an "Occam's Razor solution", but it worked. I removed the [filter prepareForImageCapture] and then on the Capture I added forceProcessingAtSize 1936 * 1936. It solved my problem. No memory issues or anything like that. I tried hard to have the full resolution (2448 * 2448) but it keeps crashing, so 1936 * 1936 looks great to me. The Blur still crashes with that resolution, but that's a minor issue.

rexeisen commented 11 years ago

@BradLarson We are seeing the same crash on the retainTarget using a very similar setup as others have expressed. Is there anything that we can do or send that may help you solve this problem?

dpestana commented 11 years ago

Yes @rexeisen . My crash is also retainTarget. I believe it has something to do with [filter prepareForImageCapture]. When I removed it, it started to work ok. I wish I could help more.

karlvr commented 11 years ago

I've encountered the same issue. For me it occurs in the GPUImageLanczosResamplingFilter. It is because of code like below, which appears in various places (setFilterFBO and createFilterFBOofSize) in various classes (incl. GPUImageTwoPassFilter, GPUImageLanczosResamplingFilter):

        if ([GPUImageOpenGLESContext supportsFastTextureUpload] && preparedToCaptureImage)
        {
            preparedToCaptureImage = NO;
            [super createFilterFBOofSize:currentFBOSize];
            preparedToCaptureImage = YES;
        }

This turns off the preparedToCaptureImage flag when calling createFilterFBOofSize which means that renderTarget is not set. In other classes this appears to be because renderTarget then goes on to be created (eg. GPUImageTwoPassFilter), or because that FBO is the first of two and we use renderTarget for the second one.

For my case I believe the solution to this issue is to remove the preparedToCaptureImage fiddling in the Lanczos filter. That appears to work. I'm not sure if this is right so I haven't created a pull request. Happy to do so though if it is.

Note that in GPUImageJFAVoronoiFilter the similar code is commented out, so perhaps it is double-creating renderTarget?

yeahphil commented 11 years ago

@BradLarson thanks for all your work on GPUImage. It is a truly awesome library.

I've run into the issue described above as well. This branch contains a single commit modifying the SimpleImageFilter example to crash when running on an iOS device (it does work in the simulator):

https://github.com/yeahphil/GPUImage/tree/crash_repro

I've also confirmed that @karlvr is correct. Several filter classes set preparedToCaptureImage = NO, and this prevents renderTarget from being initialized, as here at GPUImageTwoPassFilter.m:146:

        if ([GPUImageOpenGLESContext supportsFastTextureUpload] && preparedToCaptureImage)
        {
            // renderTarget would get initialized here
        }

Seems like changing the condition there would resolve the issue.

However, I haven't yet read deeply enough to grok the intent behind setting that preparedToCaptureImage flag, but it sure looks like it's intended to prevent something. So I'm also reluctant to submit a pull request.

karlvr commented 11 years ago

I have got to the bottom of what the issue is for me. I started images flowing down the GPUImage pipeline before I called prepareForImageCapture.

eg. [source processImage] then [filter prepareForImageCapture]

That meant that the pipeline created the FBO (createFilterFBOofSize:) before prepareForImageCapture was called (race condition), so renderTarget was not set. However prepareForImageCapture ran before newCGImageFromCurrentlyProcessedOutputWithOrientation so preparedForImageCapture was set, it tries to use renderTarget and dies as it's NULL.

It sounds like this might be similar to what others are experiencing? It is a race condition, and appears to be more consistent on single-core or slower devices. I have solved it in my code by ensuring I call prepareForImageCapture before the pipeline starts.

I think there should be a solution in GPUImage to protect against this. Either simply a warning or assert in prepareForImageCapture. I have committed a possible solution as an addition to pull request #880.

I'm happy to discuss this further and hopeful that this might be the solution we're looking for for the renderTarget issues!