Add aux sends at the position that the menu was opened, rather than always pre-fader (#4660).

git-svn-id: svn://localhost/ardour2/branches/3.0@11387 d708f5d6-7413-0410-9779-e7cbd77b26cf

5 files changed, 92 insertions, 61 deletions

diff --git a/gtk2_ardour/processor_box.cc b/gtk2_ardour/processor_box.cc
index 9ee94c1793..beea3c8211 100644
--- a/gtk2_ardour/processor_box.cc
+++ b/gtk2_ardour/processor_box.cc
@@ -942,10 +942,8 @@ ProcessorBox::show_processor_menu (int arg)
 	processor_display.get_pointer (x, y);
 	_placement = processor_display.add_placeholder (y);
 
-	if (_visible_prefader_processors == 0) {
-		if (_placement == 1) {
-			_placement = 0;
-		}
+	if (_visible_prefader_processors == 0 && _placement > 0) {
+		--_placement;
 	}
 }
 
@@ -1312,10 +1310,7 @@ ProcessorBox::choose_aux (boost::weak_ptr<Route> wr)
 		return;
 	}
 
-	boost::shared_ptr<RouteList> rlist (new RouteList);
-	rlist->push_back (_route);
-
-	_session->add_internal_sends (target, PreFader, rlist);
+	_session->add_internal_send (target, _placement, _route);
 }
 
 void
diff --git a/libs/ardour/ardour/route.h b/libs/ardour/ardour/route.h
index a0f9573562..ee2fcadd1c 100644
--- a/libs/ardour/ardour/route.h
+++ b/libs/ardour/ardour/route.h
@@ -242,8 +242,10 @@ class Route : public SessionObject, public Automatable, public RouteGroupMember,
 
 	int add_processor (boost::shared_ptr<Processor>, Placement placement, ProcessorStreams* err = 0, bool activation_allowed = true);
 	int add_processor_by_index (boost::shared_ptr<Processor>, int, ProcessorStreams* err = 0, bool activation_allowed = true);
-	int add_processor (boost::shared_ptr<Processor>, ProcessorList::iterator iter, ProcessorStreams* err = 0, bool activation_allowed = true);
-	int add_processors (const ProcessorList&, boost::shared_ptr<Processor> before, ProcessorStreams* err = 0);
+	int add_processor (boost::shared_ptr<Processor>, boost::shared_ptr<Processor>, ProcessorStreams* err = 0, bool activation_allowed = true);
+	int add_processors (const ProcessorList&, boost::shared_ptr<Processor>, ProcessorStreams* err = 0);
+	boost::shared_ptr<Processor> before_processor_for_placement (Placement);
+	boost::shared_ptr<Processor> before_processor_for_index (int);
 	int remove_processor (boost::shared_ptr<Processor>, ProcessorStreams* err = 0, bool need_process_lock = true);
 	int remove_processors (const ProcessorList&, ProcessorStreams* err = 0);
 	int reorder_processors (const ProcessorList& new_order, ProcessorStreams* err = 0);
@@ -305,7 +307,7 @@ class Route : public SessionObject, public Automatable, public RouteGroupMember,
 
 	PBD::Signal1<void,void*> SelectedChanged;
 
-	int add_aux_send (boost::shared_ptr<Route>, Placement p);
+	int add_aux_send (boost::shared_ptr<Route>, boost::shared_ptr<Processor>);
 	void remove_aux_or_listen (boost::shared_ptr<Route>);
 
 	/**
diff --git a/libs/ardour/ardour/session.h b/libs/ardour/ardour/session.h
index e702612338..0bdec6a9af 100644
--- a/libs/ardour/ardour/session.h
+++ b/libs/ardour/ardour/session.h
@@ -643,6 +643,8 @@ class Session : public PBD::StatefulDestructible, public PBD::ScopedConnectionLi
 	void globally_set_send_gains_to_zero (boost::shared_ptr<Route> dest);
 	void globally_set_send_gains_to_unity (boost::shared_ptr<Route> dest);
 	void add_internal_sends (boost::shared_ptr<Route> dest, Placement p, boost::shared_ptr<RouteList> senders);
+	void add_internal_send (boost::shared_ptr<Route>, int, boost::shared_ptr<Route>);
+	void add_internal_send (boost::shared_ptr<Route>, boost::shared_ptr<Processor>, boost::shared_ptr<Route>);
 
 	static void set_disable_all_loaded_plugins (bool yn) {
 		_disable_all_loaded_plugins = yn;
diff --git a/libs/ardour/route.cc b/libs/ardour/route.cc
index acc353f7db..570c07e9f7 100644
--- a/libs/ardour/route.cc
+++ b/libs/ardour/route.cc
@@ -874,16 +874,17 @@ dump_processors(const string& name, const list<boost::shared_ptr<Processor> >& p
 }
 #endif
 
-int
-Route::add_processor (boost::shared_ptr<Processor> processor, Placement placement, ProcessorStreams* err, bool activation_allowed)
+/** Supposing that we want to insert a Processor at a given Placement, return
+ *  the processor to add the new one before (or 0 to add at the end).
+ */
+boost::shared_ptr<Processor>
+Route::before_processor_for_placement (Placement p)
 {
-	ProcessorList::iterator loc;
-
-	/* XXX this is not thread safe - we don't hold the lock across determining the iter
-	   to add before and actually doing the insertion. dammit.
-	*/
+	Glib::RWLock::ReaderLock lm (_processor_lock);
 
-	if (placement == PreFader) {
+	ProcessorList::iterator loc;
+	
+	if (p == PreFader) {
 		/* generic pre-fader: insert immediately before the amp */
 		loc = find (_processors.begin(), _processors.end(), _amp);
 	} else {
@@ -891,24 +892,20 @@ Route::add_processor (boost::shared_ptr<Processor> processor, Placement placemen
 		loc = find (_processors.begin(), _processors.end(), _main_outs);
 	}
 
-	return add_processor (processor, loc, err, activation_allowed);
+	return loc != _processors.end() ? *loc : boost::shared_ptr<Processor> ();
 }
 
-
-/** Add a processor to a route such that it ends up with a given index into the visible processors.
- *  @param index Index to add the processor at, or -1 to add at the end of the list.
+/** Supposing that we want to insert a Processor at a given index, return
+ *  the processor to add the new one before (or 0 to add at the end).
  */
-
-int
-Route::add_processor_by_index (boost::shared_ptr<Processor> processor, int index, ProcessorStreams* err, bool activation_allowed)
+boost::shared_ptr<Processor>
+Route::before_processor_for_index (int index)
 {
-	/* XXX this is not thread safe - we don't hold the lock across determining the iter
-	   to add before and actually doing the insertion. dammit.
-	*/
-
 	if (index == -1) {
-		return add_processor (processor, _processors.end(), err, activation_allowed);
+		return boost::shared_ptr<Processor> ();
 	}
+
+	Glib::RWLock::ReaderLock lm (_processor_lock);
 	
 	ProcessorList::iterator i = _processors.begin ();
 	int j = 0;
@@ -919,15 +916,36 @@ Route::add_processor_by_index (boost::shared_ptr<Processor> processor, int index
 		
 		++i;
 	}
-	
-	return add_processor (processor, i, err, activation_allowed);
+
+	return (i != _processors.end() ? *i : boost::shared_ptr<Processor> ());
+}
+
+/** Add a processor either pre- or post-fader
+ *  @return 0 on success, non-0 on failure.
+ */
+int
+Route::add_processor (boost::shared_ptr<Processor> processor, Placement placement, ProcessorStreams* err, bool activation_allowed)
+{
+	return add_processor (processor, before_processor_for_placement (placement), err, activation_allowed);
+}
+
+
+/** Add a processor to a route such that it ends up with a given index into the visible processors.
+ *  @param index Index to add the processor at, or -1 to add at the end of the list.
+ *  @return 0 on success, non-0 on failure.
+ */
+int
+Route::add_processor_by_index (boost::shared_ptr<Processor> processor, int index, ProcessorStreams* err, bool activation_allowed)
+{
+	return add_processor (processor, before_processor_for_index (index), err, activation_allowed);
 }
 
 /** Add a processor to the route.
- *  @param iter an iterator in _processors; the new processor will be inserted immediately before this location.
+ *  @param before An existing processor in the list, or 0; the new processor will be inserted immediately before it (or at the end).
+ *  @return 0 on success, non-0 on failure.
  */
 int
-Route::add_processor (boost::shared_ptr<Processor> processor, ProcessorList::iterator iter, ProcessorStreams* err, bool activation_allowed)
+Route::add_processor (boost::shared_ptr<Processor> processor, boost::shared_ptr<Processor> before, ProcessorStreams* err, bool activation_allowed)
 {
 	assert (processor != _meter);
 	assert (processor != _main_outs);
@@ -946,25 +964,32 @@ Route::add_processor (boost::shared_ptr<Processor> processor, ProcessorList::ite
 		boost::shared_ptr<PluginInsert> pi;
 		boost::shared_ptr<PortInsert> porti;
 
-		ProcessorList::iterator loc = find(_processors.begin(), _processors.end(), processor);
-
 		if (processor == _amp) {
-			// Ensure only one amp is in the list at any time
-			if (loc != _processors.end()) {
-				if (iter == loc) { // Already in place, do nothing
+			/* Ensure that only one amp is in the list at any time */
+			ProcessorList::iterator check = find (_processors.begin(), _processors.end(), processor);
+			if (check != _processors.end()) {
+				if (before == _amp) {
+					/* Already in position; all is well */
 					return 0;
-				} else { // New position given, relocate
-					_processors.erase (loc);
+				} else {
+					_processors.erase (check);
 				}
 			}
+		}
 
-		} else {
-			if (loc != _processors.end()) {
-				cerr << "ERROR: Processor added to route twice!" << endl;
+		assert (find (_processors.begin(), _processors.end(), processor) == _processors.end ());
+
+		ProcessorList::iterator loc;
+		if (before) {
+			/* inserting before a processor; find it */
+			loc = find (_processors.begin(), _processors.end(), before);
+			if (loc == _processors.end ()) {
+				/* Not found */
 				return 1;
 			}
-
-			loc = iter;
+		} else {
+			/* inserting at end */
+			loc = _processors.end ();
 		}
 
 		_processors.insert (loc, processor);
@@ -2619,10 +2644,10 @@ Route::enable_monitor_send ()
 
 /** Add an aux send to a route.
  *  @param route route to send to.
- *  @param placement placement for the send.
+ *  @param before Processor to insert before, or 0 to insert at the end.
  */
 int
-Route::add_aux_send (boost::shared_ptr<Route> route, Placement placement)
+Route::add_aux_send (boost::shared_ptr<Route> route, boost::shared_ptr<Processor> before)
 {
 	assert (route != _session.monitor_out ());
 
@@ -2649,7 +2674,7 @@ Route::add_aux_send (boost::shared_ptr<Route> route, Placement placement)
 			listener.reset (new InternalSend (_session, _pannable, _mute_master, route, Delivery::Aux));
 		}
 
-		add_processor (listener, placement);
+		add_processor (listener, before);
 
 	} catch (failed_constructor& err) {
 		return -1;
diff --git a/libs/ardour/session.cc b/libs/ardour/session.cc
index 30e80b2627..fd8e72e22f 100644
--- a/libs/ardour/session.cc
+++ b/libs/ardour/session.cc
@@ -2247,23 +2247,30 @@ Session::globally_add_internal_sends (boost::shared_ptr<Route> dest, Placement p
 void
 Session::add_internal_sends (boost::shared_ptr<Route> dest, Placement p, boost::shared_ptr<RouteList> senders)
 {
-	if (dest->is_monitor() || dest->is_master()) {
+	for (RouteList::iterator i = senders->begin(); i != senders->end(); ++i) {
+		add_internal_send (dest, (*i)->before_processor_for_placement (p), *i);
+	}
+}
+
+void
+Session::add_internal_send (boost::shared_ptr<Route> dest, int index, boost::shared_ptr<Route> sender)
+{
+	add_internal_send (dest, sender->before_processor_for_index (index), sender);
+}
+
+void
+Session::add_internal_send (boost::shared_ptr<Route> dest, boost::shared_ptr<Processor> before, boost::shared_ptr<Route> sender)
+{
+	if (sender->is_monitor() || sender->is_master() || sender == dest || dest->is_monitor() || dest->is_master()) {
 		return;
 	}
 
 	if (!dest->internal_return()) {
-		dest->add_internal_return();
-	}
-	
-	for (RouteList::iterator i = senders->begin(); i != senders->end(); ++i) {
-		
-		if ((*i)->is_monitor() || (*i)->is_master() || (*i) == dest) {
-			continue;
-		}
-		
-		(*i)->add_aux_send (dest, p);
+		dest->add_internal_return ();
 	}
 
+	sender->add_aux_send (dest, before);
+
 	graph_reordered ();
 }