Merge pull request BradLarson#1550 from erysaj/fix-memory-access

fix memory access issues when working with CVPixelBuffer-backed framebuffer
sisufo · May 26, 2014 · 9bc3de2 · 9bc3de2
2 parents e8aa04e + 7d66360
commit 9bc3de2
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 4 deletions.
diff --git a/framework/Source/GPUImageFramebuffer.h b/framework/Source/GPUImageFramebuffer.h
@@ -50,6 +50,8 @@ typedef struct GPUTextureOptions {
 - (void)restoreRenderTarget;
 
 // Raw data bytes
+- (void)lockForReading;
+- (void)unlockAfterReading;
 - (NSUInteger)bytesPerRow;
 - (GLubyte *)byteBuffer;
 

diff --git a/framework/Source/GPUImageFramebuffer.m b/framework/Source/GPUImageFramebuffer.m
@@ -7,6 +7,7 @@ @interface GPUImageFramebuffer()
 #if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
     CVPixelBufferRef renderTarget;
     CVOpenGLESTextureRef renderTexture;
+    NSUInteger readLockCount;
 #else
 #endif
     NSUInteger framebufferReferenceCount;
@@ -330,7 +331,7 @@ - (CGImageRef)newCGImageFromFramebufferContents;
 
             glFinish();
             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
-            CVPixelBufferLockBaseAddress(renderTarget, 0);
+            [self lockForReading];
             rawImagePixels = (GLubyte *)CVPixelBufferGetBaseAddress(renderTarget);
             dataProvider = CGDataProviderCreateWithData((__bridge_retained void*)self, rawImagePixels, paddedBytesForImage, dataProviderUnlockCallback);
             [[GPUImageContext sharedFramebufferCache] addFramebufferToActiveImageCaptureList:self]; // In case the framebuffer is swapped out on the filter, need to have a strong reference to it somewhere for it to hang on while the image is in existence
@@ -372,7 +373,7 @@ - (CGImageRef)newCGImageFromFramebufferContents;
 - (void)restoreRenderTarget;
 {
 #if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
-    CVPixelBufferUnlockBaseAddress(renderTarget, 0);
+    [self unlockAfterReading];
     CFRelease(renderTarget);
 #else
 #endif
@@ -381,6 +382,35 @@ - (void)restoreRenderTarget;
 #pragma mark -
 #pragma mark Raw data bytes
 
+- (void)lockForReading
+{
+#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
+    if ([GPUImageContext supportsFastTextureUpload])
+    {
+        if (readLockCount == 0)
+        {
+            CVPixelBufferLockBaseAddress(renderTarget, 0);
+        }
+        readLockCount++;
+    }
+#endif
+}
+
+- (void)unlockAfterReading
+{
+#if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
+    if ([GPUImageContext supportsFastTextureUpload])
+    {
+        NSAssert(readLockCount > 0, @"Unbalanced call to -[GPUImageFramebuffer unlockAfterReading]");
+        readLockCount--;
+        if (readLockCount == 0)
+        {
+            CVPixelBufferUnlockBaseAddress(renderTarget, 0);
+        }
+    }
+#endif
+}
+
 - (NSUInteger)bytesPerRow;
 {
     if ([GPUImageContext supportsFastTextureUpload])
@@ -400,9 +430,9 @@ - (NSUInteger)bytesPerRow;
 - (GLubyte *)byteBuffer;
 {
 #if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
-    CVPixelBufferLockBaseAddress(renderTarget, 0);
+    [self lockForReading];
     GLubyte * bufferBytes = CVPixelBufferGetBaseAddress(renderTarget);
-    CVPixelBufferUnlockBaseAddress(renderTarget, 0);
+    [self unlockAfterReading];
     return bufferBytes;
 #else
     return NULL; // TODO: do more with this on the non-texture-cache side

diff --git a/framework/Source/GPUImageRawDataOutput.m b/framework/Source/GPUImageRawDataOutput.m
@@ -106,6 +106,8 @@ - (void)renderAtInternalSize;
     if(lockNextFramebuffer)
     {
         retainedFramebuffer = outputFramebuffer;
+        [retainedFramebuffer lock];
+        [retainedFramebuffer lockForReading];
         lockNextFramebuffer = NO;
     }
 
@@ -297,6 +299,7 @@ - (void)lockFramebufferForReading;
 
 - (void)unlockFramebufferAfterReading;
 {
+    [retainedFramebuffer unlockAfterReading];
     [retainedFramebuffer unlock];
     retainedFramebuffer = nil;
 }