Kaynağa Gözat

mac-avcapture: Fix crash issues on Intel-based Macs and older macOS

Fixes several possible crash issues that might occur on Intel-based Macs
and older macOS versions:

On modern macOS versions (13+) allocated memory is zero-allocated by
default which makes NULL pointer checks work correctly after allocation.

On older macOS versions this is not the case, so the OBSAVCaptureInfo
struct needs to be zero-allocated to ensure the guards in the tick and
render functions bail out correctly.

On Intel-based Macs and/or older macOS versions passing a reference to
the OBSAVCapture instance inside the OBSAVCapture struct can lead to a
crash because of a possible circular reference that cannot be resolved
at runtime.

Passing only a reference of the OBSAVCapture to libobs and incrementing
the retain count at source creation (decrementing in when the source is
destroyed) avoids this issue entirely.
PatTheMav 1 yıl önce
ebeveyn
işleme
03c42e5b93

+ 0 - 1
plugins/mac-avcapture/OBSAVCapture.h

@@ -36,7 +36,6 @@ typedef enum : NSUInteger {
 
 /// C struct for interaction with obs-module functions
 typedef struct av_capture {
-    id capture;
     IOSurfaceRef previousSurface;
     IOSurfaceRef currentSurface;
     OBSAVCaptureTexture *texture;

+ 34 - 32
plugins/mac-avcapture/plugin-main.m

@@ -16,7 +16,7 @@ const char *av_capture_get_text(const char *text_id)
 
 static void *av_capture_create(obs_data_t *settings, obs_source_t *source)
 {
-    OBSAVCaptureInfo *capture_data = bmalloc(sizeof(OBSAVCaptureInfo));
+    OBSAVCaptureInfo *capture_data = bzalloc(sizeof(OBSAVCaptureInfo));
     capture_data->isFastPath = false;
     capture_data->settings = settings;
     capture_data->source = source;
@@ -25,14 +25,12 @@ static void *av_capture_create(obs_data_t *settings, obs_source_t *source)
 
     OBSAVCapture *capture = [[OBSAVCapture alloc] initWithCaptureInfo:capture_data];
 
-    capture_data->capture = capture;
-
-    return capture_data;
+    return (void *) CFBridgingRetain(capture);
 }
 
 static void *av_fast_capture_create(obs_data_t *settings, obs_source_t *source)
 {
-    OBSAVCaptureInfo *capture_info = bmalloc(sizeof(OBSAVCaptureInfo));
+    OBSAVCaptureInfo *capture_info = bzalloc(sizeof(OBSAVCaptureInfo));
     capture_info->isFastPath = true;
     capture_info->settings = settings;
     capture_info->source = source;
@@ -47,17 +45,15 @@ static void *av_fast_capture_create(obs_data_t *settings, obs_source_t *source)
 
     OBSAVCapture *capture = [[OBSAVCapture alloc] initWithCaptureInfo:capture_info];
 
-    capture_info->capture = capture;
-
-    return capture_info;
+    return (void *) CFBridgingRetain(capture);
 }
 
-static const char *av_capture_get_name(void *capture_info_aliased __unused)
+static const char *av_capture_get_name(void *av_capture __unused)
 {
     return obs_module_text("AVCapture");
 }
 
-static const char *av_fast_capture_get_name(void *capture_info_aliased __unused)
+static const char *av_fast_capture_get_name(void *av_capture __unused)
 {
     return obs_module_text("AVCapture_Fast");
 }
@@ -79,9 +75,10 @@ static void av_fast_capture_set_defaults(obs_data_t *settings)
     obs_data_set_default_bool(settings, "enable_audio", true);
 }
 
-static obs_properties_t *av_capture_properties(void *capture_info_aliased)
+static obs_properties_t *av_capture_properties(void *av_capture)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
 
     obs_properties_t *properties = obs_properties_create();
 
@@ -106,11 +103,11 @@ static obs_properties_t *av_capture_properties(void *capture_info_aliased)
         bool isFastPath = capture_info->isFastPath;
 
         // Add Property Visibility and Callbacks
-        configure_property(device_list, true, true, properties_changed, capture_info);
+        configure_property(device_list, true, true, properties_changed, capture);
         configure_property(use_preset, !isFastPath, !isFastPath, (!isFastPath) ? properties_changed_use_preset : NULL,
-                           capture_info);
+                           capture);
         configure_property(preset_list, !isFastPath, !isFastPath, (!isFastPath) ? properties_changed_preset : NULL,
-                           capture_info);
+                           capture);
 
         configure_property(resolutions, isFastPath, isFastPath, NULL, NULL);
         configure_property(use_buffering, !isFastPath, !isFastPath, NULL, NULL);
@@ -123,18 +120,18 @@ static obs_properties_t *av_capture_properties(void *capture_info_aliased)
     return properties;
 }
 
-static void av_capture_update(void *capture_info_aliased, obs_data_t *settings)
+static void av_capture_update(void *av_capture, obs_data_t *settings)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
-    OBSAVCapture *capture = capture_info->capture;
-    capture_info->settings = settings;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    capture.captureInfo->settings = settings;
 
     [capture updateSessionwithError:NULL];
 }
 
-static void av_fast_capture_tick(void *capture_info_aliased, float seconds __unused)
+static void av_fast_capture_tick(void *av_capture, float seconds __unused)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
 
     if (!capture_info->currentSurface) {
         return;
@@ -174,9 +171,10 @@ static void av_fast_capture_tick(void *capture_info_aliased, float seconds __unu
     }
 }
 
-static void av_fast_capture_render(void *capture_info_aliased, gs_effect_t *effect __unused)
+static void av_fast_capture_render(void *av_capture, gs_effect_t *effect __unused)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
 
     if (!capture_info->texture) {
         return;
@@ -203,33 +201,36 @@ static void av_fast_capture_render(void *capture_info_aliased, gs_effect_t *effe
     gs_enable_framebuffer_srgb(previous);
 }
 
-static UInt32 av_fast_capture_get_width(void *capture_info_aliased)
+static UInt32 av_fast_capture_get_width(void *av_capture)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
 
     CGSize frameSize = capture_info->frameSize.size;
 
     return (UInt32) frameSize.width;
 }
 
-static UInt32 av_fast_capture_get_height(void *capture_info_aliased)
+static UInt32 av_fast_capture_get_height(void *av_capture)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
 
     CGSize frameSize = capture_info->frameSize.size;
 
     return (UInt32) frameSize.height;
 }
 
-static void av_capture_destroy(void *capture_info_aliased)
+static void av_capture_destroy(void *av_capture)
 {
-    OBSAVCaptureInfo *capture_info = capture_info_aliased;
+    OBSAVCapture *capture = (__bridge OBSAVCapture *) (av_capture);
 
-    if (!capture_info) {
+    if (!capture) {
         return;
     }
 
-    OBSAVCapture *capture = capture_info->capture;
+    OBSAVCaptureInfo *capture_info = capture.captureInfo;
+
     [capture stopCaptureSession];
     [capture.deviceInput.device unlockForConfiguration];
 
@@ -251,8 +252,9 @@ static void av_capture_destroy(void *capture_info_aliased)
         capture_info->sampleBufferDescription = NULL;
     }
 
-    capture_info->capture = NULL;
     bfree(capture_info);
+
+    CFBridgingRelease((__bridge CFTypeRef _Nullable)(capture));
 }
 
 #pragma mark - OBS Module API

+ 15 - 15
plugins/mac-avcapture/plugin-properties.h

@@ -15,54 +15,54 @@
 ///   - enable: Whether the source property should be enabled (user-changeable)
 ///   - visible: Whether the source property should be visible
 ///   - callback: Pointer to a function that will be called if this property has been modified or the properties are reloaded
-///   - callback_data: Optional payload data for the callback function
-void configure_property(obs_property_t *property, bool enable, bool visible, void *callback, void *callback_data);
+///   - capture: Optional reference to ``OBSAVCapture`` instance
+void configure_property(obs_property_t *property, bool enable, bool visible, void *callback, OBSAVCapture *capture);
 
 /// Generic callback handler for changed properties. Will update all properties of an OBSAVCapture source at once
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source (``OBSAVcaptureInfo``)
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - properties: Pointer to properties struct associated with the source
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_changed(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties, obs_property_t *property,
+bool properties_changed(OBSAVCapture *capture, obs_properties_t *properties, obs_property_t *property,
                         obs_data_t *settings);
 
 /// Callback handler for preset changes.
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - properties: Pointer to properties struct associated with the source
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_changed_preset(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties, obs_property_t *property,
+bool properties_changed_preset(OBSAVCapture *capture, obs_properties_t *properties, obs_property_t *property,
                                obs_data_t *settings);
 
 /// Callback handler for changing preset usage for an OBSAVCapture source. Switches between preset-based configuration and manual configuration
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - properties: Pointer to properties struct associated with the source
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_changed_use_preset(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties,
-                                   obs_property_t *property, obs_data_t *settings);
+bool properties_changed_use_preset(OBSAVCapture *capture, obs_properties_t *properties, obs_property_t *property,
+                                   obs_data_t *settings);
 
 /// Updates preset property with description-value-pairs of presets supported by the currently selected device
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_update_preset(OBSAVCaptureInfo *captureInfo, obs_property_t *property, obs_data_t *settings);
+bool properties_update_preset(OBSAVCapture *capture, obs_property_t *property, obs_data_t *settings);
 
 /// Updates device property with description-value-pairs of devices available via CoreMediaIO
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_update_device(OBSAVCaptureInfo *captureInfo, obs_property_t *property, obs_data_t *settings);
+bool properties_update_device(OBSAVCapture *capture, obs_property_t *property, obs_data_t *settings);
 
 /// Updates available values for all properties required in manual device configuration.
 ///
@@ -77,8 +77,8 @@ bool properties_update_device(OBSAVCaptureInfo *captureInfo, obs_property_t *pro
 ///  Frame rate ranges will be limited to ranges only available for a specific combination of resolution and color format.
 ///
 /// - Parameters:
-///   - captureInfo: Pointer to capture info struct associated with the source
+///   - capture: Pointer to ``OBSAVCapture`` instance
 ///   - property: Pointer to the property that the callback is attached to
 ///   - settings: Pointer to settings associated with the source
 /// - Returns: Always returns true
-bool properties_update_config(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties, obs_data_t *settings);
+bool properties_update_config(OBSAVCapture *capture, obs_properties_t *properties, obs_data_t *settings);

+ 29 - 32
plugins/mac-avcapture/plugin-properties.m

@@ -10,50 +10,50 @@
 
 extern const char *av_capture_get_text(const char *text_id);
 
-void configure_property(obs_property_t *property, bool enable, bool visible, void *callback, void *callback_data)
+void configure_property(obs_property_t *property, bool enable, bool visible, void *callback, OBSAVCapture *capture)
 {
     if (property) {
         obs_property_set_enabled(property, enable);
         obs_property_set_visible(property, visible);
 
         if (callback) {
-            obs_property_set_modified_callback2(property, callback, callback_data);
+            obs_property_set_modified_callback2(property, callback, (__bridge void *) (capture));
         }
     }
 }
 
-bool properties_changed(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties, obs_property_t *property __unused,
+bool properties_changed(OBSAVCapture *capture, obs_properties_t *properties, obs_property_t *property __unused,
                         obs_data_t *settings)
 {
+    OBSAVCaptureInfo *captureInfo = capture.captureInfo;
+
     obs_property_t *prop_use_preset = obs_properties_get(properties, "use_preset");
     obs_property_t *prop_device = obs_properties_get(properties, "device");
     obs_property_t *prop_presets = obs_properties_get(properties, "preset");
 
     obs_property_set_enabled(prop_use_preset, !captureInfo->isFastPath);
 
-    if (captureInfo && captureInfo->capture && settings) {
-        properties_update_device(captureInfo, prop_device, settings);
+    if (captureInfo && settings) {
+        properties_update_device(capture, prop_device, settings);
 
         bool use_preset = (settings ? obs_data_get_bool(settings, "use_preset") : true);
 
         if (use_preset) {
-            properties_update_preset(captureInfo, prop_presets, settings);
+            properties_update_preset(capture, prop_presets, settings);
         } else {
-            properties_update_config(captureInfo, properties, settings);
+            properties_update_config(capture, properties, settings);
         }
     }
 
     return true;
 }
 
-bool properties_changed_preset(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties __unused,
-                               obs_property_t *property, obs_data_t *settings)
+bool properties_changed_preset(OBSAVCapture *capture, obs_properties_t *properties __unused, obs_property_t *property,
+                               obs_data_t *settings)
 {
     bool use_preset = obs_data_get_bool(settings, "use_preset");
 
-    if (captureInfo && captureInfo->capture && settings && use_preset) {
-        OBSAVCapture *capture = captureInfo->capture;
-
+    if (capture && settings && use_preset) {
         NSArray *presetKeys =
             [capture.presetList keysSortedByValueUsingComparator:^NSComparisonResult(NSString *obj1, NSString *obj2) {
                 NSNumber *obj1Resolution;
@@ -123,7 +123,7 @@ bool properties_changed_preset(OBSAVCaptureInfo *captureInfo, obs_properties_t *
     }
 }
 
-bool properties_changed_use_preset(OBSAVCaptureInfo *captureInfo, obs_properties_t *properties,
+bool properties_changed_use_preset(OBSAVCapture *capture, obs_properties_t *properties,
                                    obs_property_t *property __unused, obs_data_t *settings)
 {
     bool use_preset = obs_data_get_bool(settings, "use_preset");
@@ -132,7 +132,7 @@ bool properties_changed_use_preset(OBSAVCaptureInfo *captureInfo, obs_properties
     obs_property_set_visible(preset_list, use_preset);
 
     if (use_preset) {
-        properties_changed_preset(captureInfo, properties, preset_list, settings);
+        properties_changed_preset(capture, properties, preset_list, settings);
     }
 
     const char *update_properties[5] = {"resolution", "frame_rate", "color_space", "video_range", "input_format"};
@@ -151,12 +151,10 @@ bool properties_changed_use_preset(OBSAVCaptureInfo *captureInfo, obs_properties
     return true;
 }
 
-bool properties_update_preset(OBSAVCaptureInfo *captureInfo, obs_property_t *property, obs_data_t *settings)
+bool properties_update_preset(OBSAVCapture *capture, obs_property_t *property, obs_data_t *settings)
 {
-    OBSAVCapture *captureInstance = captureInfo->capture;
-
-    NSArray *presetKeys = [captureInstance.presetList
-        keysSortedByValueUsingComparator:^NSComparisonResult(NSString *obj1, NSString *obj2) {
+    NSArray *presetKeys =
+        [capture.presetList keysSortedByValueUsingComparator:^NSComparisonResult(NSString *obj1, NSString *obj2) {
             NSNumber *obj1Resolution;
             NSNumber *obj2Resolution;
             if ([obj1 isEqualToString:@"High"]) {
@@ -202,7 +200,7 @@ bool properties_update_preset(OBSAVCaptureInfo *captureInfo, obs_property_t *pro
 
     if (device) {
         for (NSString *presetName in presetKeys) {
-            NSString *presetDescription = captureInstance.presetList[presetName];
+            NSString *presetDescription = capture.presetList[presetName];
 
             if ([device supportsAVCaptureSessionPreset:presetName]) {
                 obs_property_list_add_string(property, presetDescription.UTF8String, presetName.UTF8String);
@@ -213,7 +211,7 @@ bool properties_update_preset(OBSAVCaptureInfo *captureInfo, obs_property_t *pro
             }
         };
     } else if (deviceUUID.length) {
-        size_t index = obs_property_list_add_string(property, captureInstance.presetList[currentPreset].UTF8String,
+        size_t index = obs_property_list_add_string(property, capture.presetList[currentPreset].UTF8String,
                                                     currentPreset.UTF8String);
         obs_property_list_item_disable(property, index, true);
     }
@@ -221,7 +219,7 @@ bool properties_update_preset(OBSAVCaptureInfo *captureInfo, obs_property_t *pro
     return true;
 }
 
-bool properties_update_device(OBSAVCaptureInfo *captureInfo __unused, obs_property_t *property, obs_data_t *settings)
+bool properties_update_device(OBSAVCapture *capture __unused, obs_property_t *property, obs_data_t *settings)
 {
     obs_property_list_clear(property);
 
@@ -273,9 +271,8 @@ bool properties_update_device(OBSAVCaptureInfo *captureInfo __unused, obs_proper
     return true;
 }
 
-bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *properties, obs_data_t *settings)
+bool properties_update_config(OBSAVCapture *capture, obs_properties_t *properties, obs_data_t *settings)
 {
-    OBSAVCapture *captureInstance = capture->capture;
     AVCaptureDevice *device = [AVCaptureDevice deviceWithUniqueID:[OBSAVCapture stringFromSettings:settings
                                                                                        withSetting:@"device"]];
 
@@ -292,7 +289,7 @@ bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *prope
     prop_input_format = obs_properties_get(properties, "input_format");
     obs_property_list_clear(prop_input_format);
 
-    if (!captureInstance.isFastPath) {
+    if (!capture.isFastPath) {
         prop_color_space = obs_properties_get(properties, "color_space");
         prop_video_range = obs_properties_get(properties, "video_range");
 
@@ -303,12 +300,12 @@ bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *prope
     CMVideoDimensions resolution = [OBSAVCapture dimensionsFromSettings:settings];
 
     if (resolution.width == 0 || resolution.height == 0) {
-        [captureInstance AVCaptureLog:LOG_DEBUG withFormat:@"No valid resolution found in settings"];
+        [capture AVCaptureLog:LOG_DEBUG withFormat:@"No valid resolution found in settings"];
     }
 
     struct media_frames_per_second fps;
     if (!obs_data_get_frames_per_second(settings, "frame_rate", &fps, NULL)) {
-        [captureInstance AVCaptureLog:LOG_DEBUG withFormat:@"No valid framerate found in settings"];
+        [capture AVCaptureLog:LOG_DEBUG withFormat:@"No valid framerate found in settings"];
     }
 
     CMTime time = {.value = fps.denominator, .timescale = fps.numerator, .flags = 1};
@@ -324,7 +321,7 @@ bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *prope
     input_format = (int) obs_data_get_int(settings, "input_format");
     inputFormats = [[NSMutableArray alloc] init];
 
-    if (!captureInstance.isFastPath) {
+    if (!capture.isFastPath) {
         color_space = (int) obs_data_get_int(settings, "color_space");
         video_range = (int) obs_data_get_int(settings, "video_range");
 
@@ -338,13 +335,13 @@ bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *prope
     BOOL hasFoundResolution = NO;
     BOOL hasFoundFramerate = NO;
     BOOL hasFoundInputFormat = NO;
-    BOOL hasFoundColorSpace = captureInstance.isFastPath;
-    BOOL hasFoundVideoRange = captureInstance.isFastPath;
+    BOOL hasFoundColorSpace = capture.isFastPath;
+    BOOL hasFoundVideoRange = capture.isFastPath;
 
     if (device) {
         // Iterate over all formats reported by the device and gather them for property lists
         for (AVCaptureDeviceFormat *format in device.formats) {
-            if (!captureInstance.isFastPath) {
+            if (!capture.isFastPath) {
                 FourCharCode formatSubType = CMFormatDescriptionGetMediaSubType(format.formatDescription);
 
                 NSString *formatDescription = [OBSAVCapture stringFromSubType:formatSubType];
@@ -473,7 +470,7 @@ bool properties_update_config(OBSAVCaptureInfo *capture, obs_properties_t *prope
             obs_property_list_item_disable(prop_input_format, index, true);
         }
 
-        if (!captureInstance.isFastPath) {
+        if (!capture.isFastPath) {
             if (!hasFoundVideoRange) {
                 int device_range;
                 const char *range_description;