livoxGen1🐛 Call stop() on all producers before deleting in _exit

stimBuffApis/lcameraBuff/lcameraBuff.cpp

@@ -14,7 +14,47 @@ namespace smo::stim_buff::lcamera_buff {
 const SmoCallbacks *lcameraBuffSmoHooksPtr = nullptr;
 SmoThreadingModelDesc lcameraBuffThreadingModelDesc;
 LcameraDevDllState lcameraDevDll;
-std::vector<std::shared_ptr<YuvStimProducer>> attachedStimulusProducers;
+std::vector<std::shared_ptr<YuvStimProducer>> managedStimulusProducers;
+
+void addManagedStimulusProducer(
+	const std::shared_ptr<YuvStimProducer>& producer)
+{
+	managedStimulusProducers.push_back(producer);
+	producer->start();
+}
+
+sscl::co::ViralNonPostingInvoker<void> removeManagedStimulusProducerIfUnused(
+	const std::shared_ptr<YuvStimProducer>& producer)
+{
+	if (!producer) {
+		co_return;
+	}
+
+	if (!producer->attachedStimulusBuffers.empty()) {
+		co_return;
+	}
+
+	producer->stop();
+
+	managedStimulusProducers.erase(
+		std::remove_if(
+			managedStimulusProducers.begin(),
+			managedStimulusProducers.end(),
+			[&producer](const std::shared_ptr<YuvStimProducer>& candidate)
+			{
+				return candidate == producer;
+			}),
+		managedStimulusProducers.end());
+
+	const std::shared_ptr<lcamera_dev::CameraSession> deviceSession =
+		producer->deviceSession;
+
+	if (deviceSession) {
+		co_await (*lcameraDevDll.lcameraDev_releaseDeviceCReq)(deviceSession);
+	}
+
+	producer->deviceSession.reset();
+}
 
 void LcameraDevDllState::DlCloser::operator()(void *handle)
 {
@@ -37,7 +77,7 @@ std::shared_ptr<YuvStimProducer> findStimProducerByCameraId(
 	const std::string& resolvedCameraId)
 {
 	for (const std::shared_ptr<YuvStimProducer>& producer :
-		attachedStimulusProducers)
+		managedStimulusProducers)
 	{
 		assert(producer != nullptr);
 		if (producer->resolvedCameraId == resolvedCameraId) {
@@ -83,6 +123,16 @@ bool validateAttachRequest(
 
 namespace {
 
+void stopAllManagedProducersBeforeFinalize()
+{
+	for (const std::shared_ptr<YuvStimProducer>& producer :
+		managedStimulusProducers)
+	{
+		assert(producer != nullptr);
+		producer->stop();
+	}
+}
+
 void loadLcameraDevSymbols()
 {
 	lcameraDevDll.lcameraDev_main = reinterpret_cast<lcameraDev_mainFn *>(
@@ -171,7 +221,7 @@ attachByCreatingProducer(
 		parsedParams,
 		configuredMode);
 
-	attachedStimulusProducers.push_back(producer);
+	addManagedStimulusProducer(producer);
 
 	co_return co_await attachChannelBufferToProducer(desc, producer);
 }
@@ -212,14 +262,8 @@ sscl::co::ViralNonPostingInvoker<int> lcameraBuff_initializeCInd()
 
 sscl::co::ViralNonPostingInvoker<int> lcameraBuff_finalizeCInd()
 {
-	for (const std::shared_ptr<YuvStimProducer>& producer :
+	stopAllManagedProducersBeforeFinalize();
-		attachedStimulusProducers)
+	managedStimulusProducers.clear();
-	{
-		assert(producer != nullptr);
-		producer->deviceSession.reset();
-	}
-
-	attachedStimulusProducers.clear();
 
 	if (lcameraDevDll.lcameraDev_exit) {
 		(*lcameraDevDll.lcameraDev_exit)();
@@ -284,26 +328,11 @@ lcameraBuff_detachDeviceCReq(
 
 	producer->destroyAttachedStimulusBuffer(stimBuffer);
 
-	const std::shared_ptr<lcamera_dev::CameraSession> deviceSession =
-		producer->deviceSession;
-
-	co_await (*lcameraDevDll.lcameraDev_releaseDeviceCReq)(deviceSession);
-
 	if (!producer->attachedStimulusBuffers.empty()) {
 		co_return StimBuffDeviceOpResult{true, desc};
 	}
 
-	attachedStimulusProducers.erase(
+	co_await removeManagedStimulusProducerIfUnused(producer);
-		std::remove_if(
-			attachedStimulusProducers.begin(),
-			attachedStimulusProducers.end(),
-			[&producer](const std::shared_ptr<YuvStimProducer>& candidate)
-			{
-				return candidate == producer;
-			}),
-		attachedStimulusProducers.end());
-
-	producer->deviceSession.reset();
 
 	co_return StimBuffDeviceOpResult{true, desc};
 }

stimBuffApis/lcameraBuff/lcameraBuffInternal.h

@@ -33,7 +33,13 @@ struct LcameraDevDllState
 extern const SmoCallbacks *lcameraBuffSmoHooksPtr;
 extern SmoThreadingModelDesc lcameraBuffThreadingModelDesc;
 extern LcameraDevDllState lcameraDevDll;
-extern std::vector<std::shared_ptr<YuvStimProducer>> attachedStimulusProducers;
+extern std::vector<std::shared_ptr<YuvStimProducer>> managedStimulusProducers;
+
+void addManagedStimulusProducer(
+	const std::shared_ptr<YuvStimProducer>& producer);
+
+sscl::co::ViralNonPostingInvoker<void> removeManagedStimulusProducerIfUnused(
+	const std::shared_ptr<YuvStimProducer>& producer);
 
 std::shared_ptr<YuvStimProducer> findStimProducerByCameraId(
 	const std::string& resolvedCameraId);

stimBuffApis/lcameraBuff/tests/lcameraBuff_configure_hil_tests.cpp

@@ -235,10 +235,10 @@ TEST_F(LcameraBuffConfigureHilTest, AttachThreeYuvChannelsWithOptPlanar480p)
 				++expectedBufferCount;
 
 				ASSERT_EQ(
-					smo::stim_buff::lcamera_buff::attachedStimulusProducers.size(),
+					smo::stim_buff::lcamera_buff::managedStimulusProducers.size(),
 					1u);
 				producer = smo::stim_buff::lcamera_buff
-					::attachedStimulusProducers.front();
+					::managedStimulusProducers.front();
 				ASSERT_TRUE(producer != nullptr);
 				EXPECT_EQ(
 					producer->attachedStimulusBuffers.size(),

stimBuffApis/livoxGen1/livoxGen1.cpp

@@ -20,7 +20,7 @@ const SmoCallbacks *smoHooksPtr = nullptr;
 SmoThreadingModelDesc smoThreadingModelDesc;
 
 // Local collection of stimulus producers
-std::vector<std::shared_ptr<StimulusProducer>> attachedStimulusProducers;
+std::vector<std::shared_ptr<StimulusProducer>> managedStimulusProducers;
 
 LivoxProto1DllState::LivoxProto1DllState()
 : dlopenHandle(nullptr, DlCloser),
@@ -46,7 +46,7 @@ LivoxProto1DllState livoxProto1;
 std::shared_ptr<StimulusProducer> getStimulusProducer(
 	const std::shared_ptr<device::DeviceAttachmentSpec> &spec)
 {
-	for (const auto &stimProducer : attachedStimulusProducers)
+	for (const auto &stimProducer : managedStimulusProducers)
 	{
 		if (livoxProto1::comms::deviceIdentifiersEqual(
 				stimProducer->deviceAttachmentSpec->deviceSelector,
@@ -189,6 +189,8 @@ LivoxProviderParams parseLivoxProviderParams(
 	return params;
 }
 
+constexpr size_t MAX_STIM_PRODUCERS_PER_DEVICE = 2;
+
 // Helper method to ensure StimBuffer is attached
 // Returns true if successful, false on error
 bool ensureStimBufferAttachedWithoutDuplicates(
@@ -224,9 +226,125 @@ bool ensureStimBufferAttachedWithoutDuplicates(
 	return true;
 }
 
+void addManagedStimulusProducer(
+	const std::shared_ptr<PcloudStimulusProducer> &producer,
+	const std::shared_ptr<livoxProto1::Device> &device)
+{
+	if (!producer || !device) {
+		throw std::runtime_error(
+			std::string(__func__) + ": producer and device must be non-null");
+	}
+
+	device->nAttachedStimulusProducers++;
+	if (device->nAttachedStimulusProducers > MAX_STIM_PRODUCERS_PER_DEVICE)
+	{
+		device->nAttachedStimulusProducers--;
+		throw std::runtime_error(
+			std::string(__func__) + ": Each LivoxGen1 device can only have "
+			"at most two StimulusProducers attached to it. Found "
+			+ std::to_string(device->nAttachedStimulusProducers + 1) + ".");
+	}
+
+	managedStimulusProducers.push_back(producer);
+	if (false
+		/*managedStimulusProducers.size() >= 2*nDevicesKnownToGen1Lib */)
+	{
+		/**	TODO:
+		 * It would be nice to add an nDevicesKnownToGen1Lib counter, and
+		 * then add a check here to ensure that
+		 * managedStimulusProducers.size() is always less than or equal to
+		 * 2*nDevicesKnownToGen1Lib.
+		 *
+		 * (2 stim producers per device).
+		 */
+#if 0
+		throw std::runtime_error(
+			std::string(__func__) + ": Number of StimulusProducers attached "
+			"to LivoxGen1 devices known to the library ("
+			+ std::to_string(managedStimulusProducers.size())
+			+ ") is greater than "
+			"expected. Lib knows about "
+			+ std::to_string(nDevicesKnownToGen1Lib) + " devices, "
+			"so there should be at most "
+			+ std::to_string(2*nDevicesKnownToGen1Lib)
+			+ " StimulusProducers attached to devices.");
+#endif
+	}
+
+	producer->start();
+}
+
+sscl::co::ViralNonPostingInvoker<bool> removeManagedStimulusProducerIfUnused(
+	const std::shared_ptr<PcloudStimulusProducer> &producer,
+	const std::shared_ptr<sscl::ComponentThread> &/*componentThread*/)
+{
+	if (!producer) {
+		co_return true;
+	}
+
+	if (!producer->attachedStimulusBuffers.empty()) {
+		co_return true;
+	}
+
+	// No other buffers - stop and remove StimProducer
+	producer->stop();
+
+	auto it = std::find_if(
+		managedStimulusProducers.begin(),
+		managedStimulusProducers.end(),
+		[&producer](const std::shared_ptr<StimulusProducer> &candidate)
+		{
+			/**	FIXME:
+			 * When we implement the ImuStimulusProducer, we need to make
+			 * sure we handle that properly here.
+			 */
+			auto pcloudProducer =
+				std::dynamic_pointer_cast<PcloudStimulusProducer>(candidate);
+			return pcloudProducer && pcloudProducer == producer;
+		});
+	if (it != managedStimulusProducers.end()) {
+		managedStimulusProducers.erase(it);
+	}
+
+	if (!producer->device) {
+		co_return true;
+	}
+
+	if (producer->device->nAttachedStimulusProducers > 0) {
+		producer->device->nAttachedStimulusProducers--;
+	}
+
+	if (producer->device->nAttachedStimulusProducers > 0) {
+		co_return true;
+	}
+
+	const bool destroyed = co_await (*livoxProto1.livoxProto1_destroyDeviceCReq)(
+		producer->device);
+	if (!destroyed)
+	{
+		std::cerr << __func__ << ": Failed to destroy dev "
+			"device " << producer->deviceAttachmentSpec->deviceSelector
+			<< " for stim producer.\n";
+
+		co_return false;
+	}
+
+	co_return true;
+}
+
 namespace {
 
-constexpr size_t MAX_STIM_PRODUCERS_PER_DEVICE = 2;
+void stopAllManagedProducersBeforeFinalize()
+{
+	for (const auto &producer : managedStimulusProducers)
+	{
+		if (!producer) {
+			continue;
+		}
+
+		producer->stop();
+	}
+}
 
 bool validateAttachRequest(
 	const std::shared_ptr<device::DeviceAttachmentSpec> &desc)
@@ -458,44 +576,7 @@ attachByCreatingProducer(
 		desc, deviceResult.device,
 		formatDesc, nDgramsPerFrame);
 
-	deviceResult.device->nAttachedStimulusProducers++;
+	addManagedStimulusProducer(pcloudDataProducer, deviceResult.device);
-	if (deviceResult.device->nAttachedStimulusProducers
-		> MAX_STIM_PRODUCERS_PER_DEVICE)
-	{
-		throw std::runtime_error(
-			std::string(__func__) + ": Each LivoxGen1 device can only have "
-			"at most two StimulusProducers attached to it. Found "
-			+ std::to_string(deviceResult.device->nAttachedStimulusProducers)
-			+ ".");
-	}
-
-	attachedStimulusProducers.push_back(pcloudDataProducer);
-	if (false
-		/*attachedStimulusProducers.size() >= 2*nDevicesKnownToGen1Lib */)
-	{
-		/**	TODO:
-		 * It would be nice to add an nDevicesKnownToGen1Lib counter, and
-		 * then add a check here to ensure that
-		 * attachedStimulusProducers.size() is always less than or equal to
-		 * 2*nDevicesKnownToGen1Lib.
-		 *
-		 * (2 stim producers per device).
-		 */
-#if 0
-		throw std::runtime_error(
-			std::string(__func__) + ": Number of StimulusProducers attached "
-			"to LivoxGen1 devices known to the library ("
-			+ std::to_string(attachedStimulusProducers.size())
-			+ ") is greater than "
-			"expected. Lib knows about "
-			+ std::to_string(nDevicesKnownToGen1Lib) + " devices, "
-			"so there should be at most "
-			+ std::to_string(2*nDevicesKnownToGen1Lib)
-			+ " StimulusProducers attached to devices.");
-#endif
-	}
-
-	pcloudDataProducer->start();
 
 	// Ensure StimBuffer is attached
 	co_return co_await attachBufferAndEnablePcloud(
@@ -592,8 +673,8 @@ livoxGen1_detachDeviceCReq(
 
 	// Add 5ms delay before destroying device
 
-    // Helper method to delay and then call destroyDeviceReq
+	// Helper method to delay and then call destroyDeviceReq
-    // Initialize timer with LivoxGen1 metadata io_context
+	// Initialize timer with LivoxGen1 metadata io_context
 	boost::asio::deadline_timer commandDelayTimer(
 		requestComponentThread->getIoContext());
 	co_await adapters::boostAsio::getDeadlineTimerAReqAwaiter(
@@ -601,40 +682,9 @@ livoxGen1_detachDeviceCReq(
 		commandDelayTimer,
 		boost::posix_time::milliseconds(LIVOX_GEN1_DEVICE_COMMAND_DELAY_MS));
 
-	// No other buffers - stop and remove StimProducer
+	const bool removedOk = co_await removeManagedStimulusProducerIfUnused(
-	stimProducer->stop();
+		stimProducer, requestComponentThread);
-	// Remove stimulus producer from collection before destroying device
+	if (!removedOk) {
-	stimProducer->device->nAttachedStimulusProducers--;
-
-	// Find and remove the producer from the collection by comparing device
-	auto it = std::find_if(
-		attachedStimulusProducers.begin(),
-		attachedStimulusProducers.end(),
-		[&stimProducer](const std::shared_ptr<StimulusProducer> &producer)
-		{
-			/**	FIXME:
-			 * When we implement the ImuStimulusProducer, we need to make
-			 * sure we handle that properly here.
-			 */
-			auto pcloudProducer =
-				std::dynamic_pointer_cast<PcloudStimulusProducer>(producer);
-			return pcloudProducer && pcloudProducer->device == stimProducer->device;
-		});
-	if (it != attachedStimulusProducers.end()) {
-		attachedStimulusProducers.erase(it);
-	}
-
-	const bool destroyed = co_await (*livoxProto1.livoxProto1_destroyDeviceCReq)(
-		stimProducer->device);
-	if (!destroyed) {
-		std::cerr << __func__ << ": Failed to destroy dev "
-			"device " << desc->deviceSelector << " for stim "
-			"producer.\n";
-
-		/**	NOTE:
-		 * There's a decent argument for falling through here and still
-		 * removing the stimulus producer from attachedStimulusProducers.
-		 */
 		co_return StimBuffDeviceOpResult{false, desc};
 	}
 
@@ -723,7 +773,9 @@ sscl::co::ViralNonPostingInvoker<int> livoxGen1_initializeCInd()
 
 sscl::co::ViralNonPostingInvoker<int> livoxGen1_finalizeCInd()
 {
-	attachedStimulusProducers.clear();
+	stopAllManagedProducersBeforeFinalize();
+
+	managedStimulusProducers.clear();
 	if (livoxProto1.livoxProto1_exit) {
 		(*livoxProto1.livoxProto1_exit)();
 	}

stimBuffApis/livoxGen1/livoxGen1Internal.h

@@ -29,7 +29,15 @@ struct LivoxProviderParams
 
 extern const SmoCallbacks *smoHooksPtr;
 extern SmoThreadingModelDesc smoThreadingModelDesc;
-extern std::vector<std::shared_ptr<StimulusProducer>> attachedStimulusProducers;
+extern std::vector<std::shared_ptr<StimulusProducer>> managedStimulusProducers;
+
+void addManagedStimulusProducer(
+	const std::shared_ptr<PcloudStimulusProducer> &producer,
+	const std::shared_ptr<livoxProto1::Device> &device);
+
+sscl::co::ViralNonPostingInvoker<bool> removeManagedStimulusProducerIfUnused(
+	const std::shared_ptr<PcloudStimulusProducer> &producer,
+	const std::shared_ptr<sscl::ComponentThread> &componentThread);
 
 std::shared_ptr<StimulusProducer> getStimulusProducer(
 	const std::shared_ptr<device::DeviceAttachmentSpec> &spec);