From b196cef2c4443ca2a6e7d26b03b3691d0ad706c5 Mon Sep 17 00:00:00 2001
From: Robin Gareus <robin@gareus.org>
Date: Sat, 25 Apr 2020 17:52:54 +0200
Subject: [PATCH] Prevent delay-line reconfiguration while processing

Various backends have different strategies for updating latencies,
and different thread semantics for latency update callbacks.

With jack2 it is possible that processing happens concurrently
while port latencies are being changed.

Ardour internal backends emit the latency callback from the
main process thread, serializing latency changes with processing.

Various user actions can also trigger latency changes. e.g.
with a stopped engine it is possible to re-order plugins.
---
 libs/ardour/route.cc   | 13 ++++++++-----
 libs/ardour/session.cc | 43 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/libs/ardour/route.cc b/libs/ardour/route.cc
index c50b4aab25..e9e2e4604b 100644
--- a/libs/ardour/route.cc
+++ b/libs/ardour/route.cc
@@ -2202,6 +2202,8 @@ Route::reorder_processors (const ProcessorList& new_order, ProcessorStreams* err
 			_pending_processor_order.clear ();
 			setup_invisible_processors ();
 
+			update_signal_latency (true);
+
 			processors_changed (RouteProcessorChange ()); /* EMIT SIGNAL */
 			set_processor_positions ();
 		} else {
@@ -2226,6 +2228,12 @@ Route::reorder_processors (const ProcessorList& new_order, ProcessorStreams* err
 		}
 
 		lm.release();
+
+		/* update processor input/output latency (total signal_latency does not change).
+		 * delaylines may changes, so the Engine Lock is required.
+		 */
+		update_signal_latency (true);
+
 		lx.release();
 
 		processors_changed (RouteProcessorChange ()); /* EMIT SIGNAL */
@@ -2240,11 +2248,6 @@ Route::reorder_processors (const ProcessorList& new_order, ProcessorStreams* err
 		g_atomic_int_set (&_pending_process_reorder, 1);
 	}
 
-	/* update processor input/output latency
-	 * (total signal_latency does not change)
-	 */
-	update_signal_latency (true);
-
 	return 0;
 }
 
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 4c343bfba2..35e27ea123 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -6450,13 +6450,10 @@ Session::missing_filesources (DataType dt) const
 void
 Session::initialize_latencies ()
 {
-        {
-                Glib::Threads::Mutex::Lock lm (_engine.process_lock());
-                update_latency (false);
-                update_latency (true);
-        }
+	update_latency (false);
+	update_latency (true);
 
-        set_worst_io_latencies ();
+	set_worst_io_latencies ();
 }
 
 void
@@ -6486,6 +6483,15 @@ Session::send_latency_compensation_change ()
 bool
 Session::update_route_latency (bool playback, bool apply_to_delayline)
 {
+#ifndef NDEBUG
+	if (apply_to_delayline) {
+		/* apply_to_delayline can no be called concurrently with processing */
+		Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock (), Glib::Threads::TRY_LOCK);
+		/* Caller must hold process lock already */
+		assert (!lm.locked ());
+	}
+#endif
+
 	/* Note: RouteList is process-graph sorted */
 	boost::shared_ptr<RouteList> r = routes.reader ();
 
@@ -6561,7 +6567,17 @@ Session::update_latency (bool playback)
 	if (playback) {
 		Glib::Threads::Mutex::Lock lx (_update_latency_lock);
 		set_worst_output_latency ();
-		update_route_latency (true, true);
+
+		/* With internal backends, AudioEngine::latency_callback () -> this method
+		 * is called from the main_process_thread.
+		 *
+		 * However jack2 can concurrently process and reconfigure port latencies.
+		 * Processing needs to be blocked while re-configuring delaylines.
+		 */
+		Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
+
+		update_route_latency (true, /*apply_to_delayline*/ true);
+
 	} else {
 		Glib::Threads::Mutex::Lock lx (_update_latency_lock);
 		set_worst_input_latency ();
@@ -6638,6 +6654,10 @@ Session::update_latency_compensation (bool force_whole_graph, bool called_from_b
 		return;
 	}
 
+	if (_engine.in_process_thread ()) {
+		called_from_backend = true;
+	}
+
 	/* this lock is not usually contended, but under certain conditions,
 	 * update_latency_compensation may be called concurrently.
 	 * e.g. drag/drop copy a latent plugin while rolling.
@@ -6686,6 +6706,14 @@ Session::update_latency_compensation (bool force_whole_graph, bool called_from_b
 			DEBUG_TRACE (DEBUG::LatencyCompensation, "update_latency_compensation called from engine, don't call back into engine\n");
 		}
 	} else {
+
+#ifndef MIXBUS /* mixbus already has the process-locked */
+		Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock (), Glib::Threads::NOT_LOCK);
+		if (!called_from_backend) {
+			lm.acquire ();
+		}
+#endif
+
 		DEBUG_TRACE (DEBUG::LatencyCompensation, "update_latency_compensation: directly apply to routes\n");
 		boost::shared_ptr<RouteList> r = routes.reader ();
 		for (RouteList::iterator i = r->begin(); i != r->end(); ++i) {
@@ -6972,7 +7000,6 @@ Session::auto_connect_thread_run ()
 			 * modifies the capture-offset, which can be a problem.
 			 */
 			while (g_atomic_int_and (&_latency_recompute_pending, 0)) {
-				Glib::Threads::Mutex::Lock lm (AudioEngine::instance()->process_lock ());
 				update_latency_compensation (false, false);
 			}
 		}