Bläddra i källkod

mac-virtualcam: Fix memory access issues for shared IOSurfaces

The DAL plugin-based virtualcamera shares data between OBS and the
plugin using an IOSurface. IOSurface locks are necessary to ensure
race conditions between data generation (OBS side) and consumption
(virtual camera side) and also that an IOSurface is not offloaded to
GPU memory when it is indeed needed in CPU memory.

Also moves the invalidation of the NSMachPort for the frames to after
the IOSurface data has been converted into a pixelbuffer and added to
the frame queue of the virtual camera, as an early invalidation will
cut off access to the pixel data shared with the DAL plugin.

(cherry picked from commit 447adfbe385249ba721dec2ad9d2e4aaa9f0376a)
PatTheMav 2 år sedan
förälder
incheckning
00b4c9c88a

+ 47 - 42
plugins/mac-virtualcam/src/dal-plugin/OBSDALMachClient.mm

@@ -93,6 +93,8 @@
 
 - (void)handlePortMessage:(NSPortMessage *)message
 {
+	__strong id<MachClientDelegate> strongDelegate = self.delegate;
+
 	VLogFunc(@"");
 	NSArray *components = message.components;
 	switch (message.msgid) {
@@ -113,57 +115,60 @@
 
 			IOSurfaceRef surface = IOSurfaceLookupFromMachPort(
 				[framePort machPort]);
-			[framePort invalidate];
-			mach_port_deallocate(mach_task_self(),
-					     [framePort machPort]);
 
-			if (!surface) {
-				ELog(@"Failed to obtain IOSurface from Mach port");
-				return;
-			}
-
-			/*
-			 * IOSurfaceLocks are only necessary on non Apple Silicon devices, as those have
-			 * unified memory. On Intel machines, the lock ensures that the IOSurface is copied back
-			 * from GPU memory to CPU memory so we can process the pixel buffer.
-			 */
+			if (surface) {
+				/*
+                 * IOSurfaceLocks are only necessary on non Apple Silicon devices, as those have
+                 * unified memory. On Intel machines, the lock ensures that the IOSurface is copied back
+                 * from GPU memory to CPU memory so we can process the pixel buffer.
+                 */
 #ifndef __aarch64__
-			IOSurfaceLock(surface, kIOSurfaceLockReadOnly, NULL);
+				IOSurfaceLock(surface, kIOSurfaceLockReadOnly,
+					      NULL);
 #endif
-			CVPixelBufferRef frame;
-			CVPixelBufferCreateWithIOSurface(kCFAllocatorDefault,
-							 surface, NULL, &frame);
+				CVPixelBufferRef frame;
+				CVPixelBufferCreateWithIOSurface(
+					kCFAllocatorDefault, surface, NULL,
+					&frame);
 #ifndef __aarch64__
-			IOSurfaceUnlock(surface, kIOSurfaceLockReadOnly, NULL);
+				IOSurfaceUnlock(surface, kIOSurfaceLockReadOnly,
+						NULL);
 #endif
-			CFRelease(surface);
-
-			uint64_t timestamp;
-			[components[1] getBytes:&timestamp
-					 length:sizeof(timestamp)];
-
-			VLog(@"Received frame data: %zux%zu (%llu)",
-			     CVPixelBufferGetWidth(frame),
-			     CVPixelBufferGetHeight(frame), timestamp);
-
-			uint32_t fpsNumerator;
-			[components[2] getBytes:&fpsNumerator
-					 length:sizeof(fpsNumerator)];
-			uint32_t fpsDenominator;
-			[components[3] getBytes:&fpsDenominator
-					 length:sizeof(fpsDenominator)];
-
-			[self.delegate receivedPixelBuffer:frame
-						 timestamp:timestamp
-					      fpsNumerator:fpsNumerator
-					    fpsDenominator:fpsDenominator];
-
-			CVPixelBufferRelease(frame);
+				CFRelease(surface);
+
+				uint64_t timestamp;
+				[components[1] getBytes:&timestamp
+						 length:sizeof(timestamp)];
+
+				VLog(@"Received frame data: %zux%zu (%llu)",
+				     CVPixelBufferGetWidth(frame),
+				     CVPixelBufferGetHeight(frame), timestamp);
+
+				uint32_t fpsNumerator;
+				[components[2] getBytes:&fpsNumerator
+						 length:sizeof(fpsNumerator)];
+				uint32_t fpsDenominator;
+				[components[3] getBytes:&fpsDenominator
+						 length:sizeof(fpsDenominator)];
+
+				[strongDelegate
+					receivedPixelBuffer:frame
+						  timestamp:timestamp
+					       fpsNumerator:fpsNumerator
+					     fpsDenominator:fpsDenominator];
+
+				CVPixelBufferRelease(frame);
+			} else {
+				ELog(@"Failed to obtain IOSurface from Mach port");
+			}
+			[framePort invalidate];
+			mach_port_deallocate(mach_task_self(),
+					     [framePort machPort]);
 		}
 		break;
 	case MachMsgIdStop:
 		DLog(@"Received stop message");
-		[self.delegate receivedStop];
+		[strongDelegate receivedStop];
 		break;
 	default:
 		ELog(@"Received unexpected response msgid %u",

+ 7 - 0
plugins/mac-virtualcam/src/obs-plugin/OBSDALMachServer.mm

@@ -148,6 +148,9 @@
 			       length:sizeof(fpsDenominator)];
 
 		IOSurfaceRef surface = CVPixelBufferGetIOSurface(frame);
+#ifndef __aarch64__
+		IOSurfaceLock(surface, 0, NULL);
+#endif
 
 		if (!surface) {
 			blog(LOG_ERROR,
@@ -174,6 +177,10 @@
 					 ]];
 
 		mach_port_deallocate(mach_task_self(), framePort);
+
+#ifndef __aarch64__
+		IOSurfaceUnlock(surface, 0, NULL);
+#endif
 	}
 }