MacOS getDevice() optimization, set CI env var, & add getVideoDevices() test (#1691)

* Revert "settings: revert 01b09d9 / fix device list (#1609)"

This reverts commit 4434fda.

* set target 12.0, add video test, replay-buffer test skip CI, getDevices()

* rename legacy getDevices() -> getDevicesUsingDummySource(). Adding
more error checking
* set CI flag for MacOS test runner so it can use obs.isCI()

Author: sandboxcoder

.github/workflows/main.yml

@@ -151,6 +151,7 @@ jobs:
           OSN_ACCESS_KEY_ID: ${{secrets.AWS_RELEASE_ACCESS_KEY_ID}}
           OSN_SECRET_ACCESS_KEY: ${{secrets.AWS_RELEASE_SECRET_ACCESS_KEY}}
           RELEASE_NAME: ${{matrix.ReleaseName}}
+          CI: true
       # Run even after test failures so the PR still gets the flaky summary.
       - name: Publish flaky test check
         if: ${{ always() }}

CMakeLists.txt

@@ -29,15 +29,15 @@ ENDIF()
 
 # Comfigure macos architecture and min version
 iF(APPLE)
-	set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "11.0")
-	set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "10.15")
+	set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=arm64] "12.0")
+	set(CMAKE_XCODE_ATTRIBUTE_MACOSX_DEPLOYMENT_TARGET[arch=x86_64] "12.0")
 	if (NOT CMAKE_OSX_ARCHITECTURES)
 		set(CMAKE_OSX_ARCHITECTURES "${CMAKE_HOST_SYSTEM_PROCESSOR}")
 	endif()
 	if ("${CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64")
-		set(CMAKE_OSX_DEPLOYMENT_TARGET "11.0")
+		set(CMAKE_OSX_DEPLOYMENT_TARGET "12.0")
 	else()
-		set(CMAKE_OSX_DEPLOYMENT_TARGET "10.15")
+		set(CMAKE_OSX_DEPLOYMENT_TARGET "12.0")
 	endif()
 	# See CMake issue # 21854
 	# https://gitlab.kitware.com/cmake/cmake/-/issues/21854

ci/configure-osn-osx.sh

@@ -8,7 +8,7 @@ cd ..
 [ -n "${ARCHITECTURE}" ] && CMAKE_OSX_ARCHITECTURES_PARAM="-DCMAKE_OSX_ARCHITECTURES=${ARCHITECTURE}" || CMAKE_OSX_ARCHITECTURES_PARAM=""
 
 cmake .. \
--DCMAKE_OSX_DEPLOYMENT_TARGET=10.15 \
+-DCMAKE_OSX_DEPLOYMENT_TARGET=12.0 \
 -DCMAKE_INSTALL_PREFIX=$PWD/../${SLFullDistributePath}/${InstallPath} \
 -DSTREAMLABS_BUILD=OFF \
 -DNODEJS_NAME=${RuntimeName} \

obs-studio-server/source/nodeobs_settings.cpp

@@ -20,6 +20,7 @@
 #include "osn-error.hpp"
 #include "nodeobs_api.h"
 #include "shared.hpp"
+#include <sstream>
 #include "memory-manager.h"
 #include "osn-video.hpp"
 #include "osn-encoders.hpp"
@@ -3864,46 +3865,87 @@ void OBS_settings::saveGenericSettings(std::vector<SubCategory> genericSettings,
 	config_save_safe(config, "tmp", nullptr);
 }
 
-void getDevices(const char *source_id, const char *property_name, std::vector<ipc::value> &rval)
+// Gets the devices using a "dummy" source object that does not exist. For most
+// plugins this is fine but should be avoided for plugins like mac-coreaudio
+// because it will create a reconnect_thread. Triggers the following callbacks
+// on the obs_source_info: get_properties, get_defaults, and get_updates.
+void getDevicesUsingDummySource(const char *source_id, const char *property_name, std::vector<ipc::value> &rval)
 {
-	auto settings = obs_get_source_defaults(source_id);
-	if (!settings)
-		return;
+	OBSDataAutoRelease settings = obs_get_source_defaults(source_id);
+	if (!settings) {
+		std::ostringstream ss;
+		ss << "Could not get settings for source id: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
+	}
 
 	const char *dummy_device_name = "does_not_exist";
 	obs_data_set_string(settings, property_name, dummy_device_name);
-	if (strcmp(source_id, "dshow_input") == 0) {
-		obs_data_set_string(settings, "video_device_id", dummy_device_name);
-		obs_data_set_string(settings, "audio_device_id", dummy_device_name);
-	}
 
 	// Create a dummy source so that the "device" property can be init
-	auto dummy_source = obs_source_create(source_id, dummy_device_name, settings, nullptr);
+	OBSSourceAutoRelease dummy_source = obs_source_create(source_id, dummy_device_name, settings, nullptr);
 	if (!dummy_source) {
-		obs_data_release(settings);
-		return;
+		std::ostringstream ss;
+		ss << "Could not create source: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
 	}
 
 	auto props = obs_source_properties(dummy_source);
 	if (!props) {
-		obs_source_release(dummy_source);
-		obs_data_release(settings);
-		return;
+		std::ostringstream ss;
+		ss << "Could not get source properties for source id: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
 	}
 
 	auto prop = obs_properties_get(props, property_name);
 	if (!prop) {
 		obs_properties_destroy(props);
-		obs_source_release(dummy_source);
-		obs_data_release(settings);
-		return;
+		std::ostringstream ss;
+		ss << "Could not get source property " << property_name << " for source id: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
 	}
 
+	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
 	size_t items = obs_property_list_item_count(prop);
-	if (rval.size() > 1)
-		rval[1].value_union.ui64 += items;
-	else
-		rval.push_back(ipc::value((uint64_t)items));
+	rval.push_back(ipc::value((uint64_t)items));
+
+	for (size_t idx = 0; idx < items; idx++) {
+		const char *description = obs_property_list_item_name(prop, idx);
+		const char *device_id = obs_property_list_item_string(prop, idx);
+
+		if (!description || !strcmp(description, "") || !device_id || !strcmp(device_id, "")) {
+			rval[1].value_union.ui64--;
+			continue;
+		}
+
+		rval.push_back(ipc::value(description));
+		rval.push_back(ipc::value(device_id));
+	}
+
+	obs_properties_destroy(props);
+}
+
+// Gets the devices from the source using a property list without triggering
+// obs_source_info.get_updates callback.
+void getDevices(const char *source_id, const char *property_name, std::vector<ipc::value> &rval)
+{
+	auto props = obs_get_source_properties(source_id);
+	if (!props) {
+		std::ostringstream ss;
+		ss << "Could not get source properties for source id: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
+	}
+
+	auto prop = obs_properties_get(props, property_name);
+	if (!prop) {
+		obs_properties_destroy(props);
+		std::ostringstream ss;
+		ss << "Could not get source property " << property_name << " for source id: " << source_id;
+		PRETTY_ERROR_RETURN(ErrorCode::Error, ss.str());
+	}
+
+	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
+	size_t items = obs_property_list_item_count(prop);
+	rval.push_back(ipc::value((uint64_t)items));
 
 	for (size_t idx = 0; idx < items; idx++) {
 		const char *description = obs_property_list_item_name(prop, idx);
@@ -3919,8 +3961,6 @@ void getDevices(const char *source_id, const char *property_name, std::vector<ip
 	}
 
 	obs_properties_destroy(props);
-	obs_data_release(settings);
-	obs_source_release(dummy_source);
 }
 
 #ifdef WIN32
@@ -4057,9 +4097,8 @@ void enumAudioDevices(std::vector<ipc::value> &rval, EDataFlow dataFlow)
 
 void OBS_settings::OBS_settings_getInputAudioDevices(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval)
 {
-	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
-
 #ifdef WIN32
+	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
 	rval.push_back(ipc::value((uint32_t)1));
 	rval.push_back(ipc::value("Default"));
 	rval.push_back(ipc::value("default"));
@@ -4074,9 +4113,8 @@ void OBS_settings::OBS_settings_getInputAudioDevices(void *data, const int64_t i
 
 void OBS_settings::OBS_settings_getOutputAudioDevices(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval)
 {
-	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
-
 #ifdef WIN32
+	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
 	rval.push_back(ipc::value((uint32_t)1));
 	rval.push_back(ipc::value("Default"));
 	rval.push_back(ipc::value("default"));
@@ -4091,15 +4129,12 @@ void OBS_settings::OBS_settings_getOutputAudioDevices(void *data, const int64_t
 
 void OBS_settings::OBS_settings_getVideoDevices(void *data, const int64_t id, const std::vector<ipc::value> &args, std::vector<ipc::value> &rval)
 {
-	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
-
 #ifdef WIN32
+	rval.push_back(ipc::value((uint64_t)ErrorCode::Ok));
 	rval.push_back(ipc::value((uint32_t)0));
 	enumVideoDevices(rval);
 #elif __APPLE__
-	const char *source_id = "macos_avcapture";
-	const char *property_name = "device";
-	getDevices(source_id, property_name, rval);
+	getDevicesUsingDummySource("macos_avcapture", "device", rval);
 #endif
 
 	AUTO_DEBUG;

tests/osn-tests/src/test_osn_advanced_replayBuffer.ts

@@ -131,6 +131,9 @@ describe(testName, () => {
     });
 
     it('Start advanced replay buffer - Use Recording', async function() {
+        if (obs.isDarwin() && obs.isCI()) {
+            this.skip();
+        }
         replayBuffer = osn.AdvancedReplayBufferFactory.create();
         replayBuffer.path = path.join(path.normalize(__dirname), '..', 'osnData');
         replayBuffer.format = osn.ERecordingFormat.MP4;
@@ -258,6 +261,9 @@ describe(testName, () => {
     });
 
     it('Start advanced replay buffer - Use Stream through Recording', async function() {
+        if (obs.isDarwin() && obs.isCI()) {
+            this.skip();
+        }
         replayBuffer = osn.AdvancedReplayBufferFactory.create();
         replayBuffer.path = path.join(path.normalize(__dirname), '..', 'osnData');
         replayBuffer.format = osn.ERecordingFormat.MP4;

tests/osn-tests/src/test_osn_audio.ts

@@ -70,7 +70,10 @@ describe(testName, () => {
                 foundDefaultDevice = true;
             }
         }
-        if (!obs.isDarwin()) { // On virtual mac the default output device is not included in the list of output devices
+        // On Darwin CI, skip the default-device assertion only when there are no audio devices.
+        // mac-coreaudio only returns the default device when there is at least one audio device, but on CI there may be no audio devices, so the default device is not returned.
+        const shouldSkipDefaultDeviceAssertion = obs.isDarwin() && obs.isCI() && devices.length === 0;
+        if (!shouldSkipDefaultDeviceAssertion) {
             expect(foundDefaultDevice).to.equal(true, GetErrorMessage(ETestErrorMsg.DefaultDeviceNotFound));
         }
     });

tests/osn-tests/src/test_osn_video.ts

@@ -191,4 +191,15 @@ describe(testName, () => {
 
         context.destroy();
     });
+
+    it('Get video capture devices', function() {
+        const devices = osn.NodeObs.OBS_settings_getVideoDevices();
+        expect(devices).to.not.equal(undefined, GetErrorMessage(ETestErrorMsg.VideoDevices));
+        expect(Array.isArray(devices)).to.equal(true, GetErrorMessage(ETestErrorMsg.VideoDevicesIsArray));
+        for (const device of devices) {
+            expect(device).to.have.property('id');
+            expect(device).to.have.property('description');
+            logInfo(testName, `Video Capture Device Found: ${device.description} with id: ${device.id}`);
+        }
+    });
 });

tests/osn-tests/util/error_messages.ts

@@ -203,6 +203,8 @@ export const enum ETestErrorMsg {
     AudioDevices = 'Failed to get audio devices',
     AudioDevicesIsArray = 'Returned audio devices value is not an array',
     DefaultDeviceNotFound = 'Did not find the default audio device in the list of audio devices',
+    VideoDevices = 'Failed to get video devices',
+    VideoDevicesIsArray = 'Returned video devices value is not an array',
 }
 
 export function GetErrorMessage(message: string, value1?: string, value2?: string, value3?: string): string {

tests/osn-tests/util/obs_handler.ts

@@ -553,7 +553,7 @@ export class OBSHandler {
 
     isDarwin()
     {
-        // Wrapped this in a function- just incase we want to add more conditions later or disable only within the build agent.
+        // Wrapped this in a function- just in case we want to add more conditions later or disable only within the build agent.
         return this.os === 'darwin';
     }