NDI cleanup to avoid calling NDI while holding the output mutex

src/video/ndi/NdiOutput.cpp

@@ -134,7 +134,7 @@ bool NdiOutput::StartScheduledPlayback(std::string& error)
 
 bool NdiOutput::ScheduleFrame(const VideoIOOutputFrame& frame)
 {
-	if (!mRunning.load(std::memory_order_acquire) || frame.bytes == nullptr || frame.pixelFormat != VideoIOPixelFormat::Bgra8)
+	if (frame.bytes == nullptr || frame.pixelFormat != VideoIOPixelFormat::Bgra8)
 	{
 		++mScheduleFailures;
 		return false;
@@ -143,44 +143,79 @@ bool NdiOutput::ScheduleFrame(const VideoIOOutputFrame& frame)
 	NDIlib_video_frame_v2_t ndiFrame;
 	SetCommonVideoFrameFields(ndiFrame, frame, mConfig);
 
-	std::lock_guard<std::mutex> lock(mMutex);
+	void* completedBuffer = nullptr;
-	if (mSender == nullptr)
 	{
-		++mScheduleFailures;
+		std::lock_guard<std::mutex> sdkLock(mSdkMutex);
-		return false;
+		void* sender = nullptr;
+		{
+			std::lock_guard<std::mutex> stateLock(mMutex);
+			if (!mRunning.load(std::memory_order_acquire) || mSender == nullptr)
+			{
+				++mScheduleFailures;
+				return false;
+			}
+			sender = mSender;
+		}
+
+		const auto scheduleStart = std::chrono::steady_clock::now();
+		NDIlib_send_send_video_async_v2(static_cast<NDIlib_send_instance_t>(sender), &ndiFrame);
+		mScheduleCallMilliseconds.store(
+			std::chrono::duration_cast<std::chrono::duration<double, std::milli>>(std::chrono::steady_clock::now() - scheduleStart).count(),
+			std::memory_order_relaxed);
+
+		std::lock_guard<std::mutex> stateLock(mMutex);
+		completedBuffer = mPendingBuffer;
+		mPendingBuffer = frame.nativeBuffer != nullptr ? frame.nativeBuffer : frame.bytes;
 	}
 
-	const auto scheduleStart = std::chrono::steady_clock::now();
+	CompleteBuffer(completedBuffer, VideoIOCompletionResult::Completed);
-	NDIlib_send_send_video_async_v2(static_cast<NDIlib_send_instance_t>(mSender), &ndiFrame);
-	mScheduleCallMilliseconds.store(
-		std::chrono::duration_cast<std::chrono::duration<double, std::milli>>(std::chrono::steady_clock::now() - scheduleStart).count(),
-		std::memory_order_relaxed);
-
-	CompletePendingLocked(VideoIOCompletionResult::Completed);
-	mPendingBuffer = frame.nativeBuffer != nullptr ? frame.nativeBuffer : frame.bytes;
 	return true;
 }
 
 void NdiOutput::Stop()
 {
-	std::lock_guard<std::mutex> lock(mMutex);
+	void* flushedBuffer = nullptr;
-	if (mSender != nullptr)
+	{
-		FlushPendingLocked();
+		std::lock_guard<std::mutex> sdkLock(mSdkMutex);
-	mRunning.store(false, std::memory_order_release);
+		void* sender = nullptr;
+		{
+			std::lock_guard<std::mutex> stateLock(mMutex);
+			sender = mSender;
+		}
+		if (sender != nullptr)
+			NDIlib_send_send_video_async_v2(static_cast<NDIlib_send_instance_t>(sender), nullptr);
+
+		std::lock_guard<std::mutex> stateLock(mMutex);
+		flushedBuffer = mPendingBuffer;
+		mPendingBuffer = nullptr;
+		mRunning.store(false, std::memory_order_release);
+	}
+
+	CompleteBuffer(flushedBuffer, VideoIOCompletionResult::Flushed);
 }
 
 void NdiOutput::ReleaseResources()
 {
+	void* sender = nullptr;
+	void* flushedBuffer = nullptr;
 	{
-		std::lock_guard<std::mutex> lock(mMutex);
+		std::lock_guard<std::mutex> sdkLock(mSdkMutex);
-		if (mSender != nullptr)
 		{
-			FlushPendingLocked();
+			std::lock_guard<std::mutex> stateLock(mMutex);
-			NDIlib_send_destroy(static_cast<NDIlib_send_instance_t>(mSender));
+			sender = mSender;
 			mSender = nullptr;
-			ReleaseNdiRuntime();
+			flushedBuffer = mPendingBuffer;
+			mPendingBuffer = nullptr;
+		}
+		if (sender != nullptr)
+		{
+			NDIlib_send_send_video_async_v2(static_cast<NDIlib_send_instance_t>(sender), nullptr);
+			NDIlib_send_destroy(static_cast<NDIlib_send_instance_t>(sender));
 		}
 	}
+	CompleteBuffer(flushedBuffer, VideoIOCompletionResult::Flushed);
+	if (sender != nullptr)
+		ReleaseNdiRuntime();
 	mInitialized.store(false, std::memory_order_release);
 	mRunning.store(false, std::memory_order_release);
 }
@@ -201,15 +236,14 @@ NdiOutputMetrics NdiOutput::Metrics() const
 	return metrics;
 }
 
-void NdiOutput::CompletePendingLocked(VideoIOCompletionResult result)
+void NdiOutput::CompleteBuffer(void* buffer, VideoIOCompletionResult result)
 {
-	if (mPendingBuffer == nullptr)
+	if (buffer == nullptr)
 		return;
 
 	VideoIOCompletion completion;
 	completion.result = result;
-	completion.outputFrameBuffer = mPendingBuffer;
+	completion.outputFrameBuffer = buffer;
-	mPendingBuffer = nullptr;
 	++mCompletions;
 	if (result == VideoIOCompletionResult::Dropped)
 		++mDropped;
@@ -220,13 +254,6 @@ void NdiOutput::CompletePendingLocked(VideoIOCompletionResult result)
 		mCompletionCallback(completion);
 }
 
-void NdiOutput::FlushPendingLocked()
-{
-	if (mSender != nullptr)
-		NDIlib_send_send_video_async_v2(static_cast<NDIlib_send_instance_t>(mSender), nullptr);
-	CompletePendingLocked(VideoIOCompletionResult::Flushed);
-}
-
 void NdiOutput::PopulateState(VideoIOState& state, const VideoOutputEdgeConfig& config, const std::string& senderName)
 {
 	state = VideoIOState();

src/video/ndi/NdiOutput.h

@@ -30,13 +30,13 @@ public:
 	NdiOutputMetrics Metrics() const override;
 
 private:
-	void CompletePendingLocked(VideoIOCompletionResult result);
+	void CompleteBuffer(void* buffer, VideoIOCompletionResult result);
-	void FlushPendingLocked();
 	static void PopulateState(VideoIOState& state, const VideoOutputEdgeConfig& config, const std::string& senderName);
 
 	std::string mSenderName;
 	CompletionCallback mCompletionCallback;
 	mutable std::mutex mMutex;
+	mutable std::mutex mSdkMutex;
 	void* mSender = nullptr;
 	void* mPendingBuffer = nullptr;
 	VideoOutputEdgeConfig mConfig;