port changes to ARDOUR::Location and ARDOUR::Locations APIs from Tracks to Ardour.

Fixes deadlocks caused by mutex on Locations list, and clarifies the purposes and uses of the class-level and
object-level change-related signals.

gtk2_ardour/editor.cc

@@ -1356,7 +1356,6 @@ Editor::set_session (Session *t)
 	_session->locations()->added.connect (_session_connections, invalidator (*this), boost::bind (&Editor::add_new_location, this, _1), gui_context());
 	_session->locations()->removed.connect (_session_connections, invalidator (*this), boost::bind (&Editor::location_gone, this, _1), gui_context());
 	_session->locations()->changed.connect (_session_connections, invalidator (*this), boost::bind (&Editor::refresh_location_display, this), gui_context());
-	_session->locations()->StateChanged.connect (_session_connections, invalidator (*this), boost::bind (&Editor::refresh_location_display, this), gui_context());
 	_session->history().Changed.connect (_session_connections, invalidator (*this), boost::bind (&Editor::history_changed, this), gui_context());
 
 	playhead_cursor->show ();

gtk2_ardour/editor.h

@@ -572,7 +572,7 @@ class Editor : public PublicEditor, public PBD::ScopedConnectionList, public ARD
 	void location_changed (ARDOUR::Location *);
 	void location_flags_changed (ARDOUR::Location *);
 	void refresh_location_display ();
-	void refresh_location_display_internal (ARDOUR::Locations::LocationList&);
+	void refresh_location_display_internal (const ARDOUR::Locations::LocationList&);
 	void add_new_location (ARDOUR::Location *);
 	ArdourCanvas::Container* add_new_location_internal (ARDOUR::Location *);
 	void location_gone (ARDOUR::Location *);

gtk2_ardour/editor_markers.cc

@@ -486,7 +486,7 @@ Editor::find_location_from_marker (Marker *marker, bool& is_start) const
 }
 
 void
-Editor::refresh_location_display_internal (Locations::LocationList& locations)
+Editor::refresh_location_display_internal (const Locations::LocationList& locations)
 {
 	/* invalidate all */
 
@@ -496,7 +496,7 @@ Editor::refresh_location_display_internal (Locations::LocationList& locations)
 
 	/* add new ones */
 
-	for (Locations::LocationList::iterator i = locations.begin(); i != locations.end(); ++i) {
+	for (Locations::LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
 
 		LocationMarkerMap::iterator x;
 

gtk2_ardour/location_ui.cc

@@ -959,7 +959,7 @@ LocationUI::location_removed (Location* location)
 }
 
 void
-LocationUI::map_locations (Locations::LocationList& locations)
+LocationUI::map_locations (const Locations::LocationList& locations)
 {
 	Locations::LocationList::iterator i;
 	gint n;
@@ -968,9 +968,8 @@ LocationUI::map_locations (Locations::LocationList& locations)
 	LocationSortByStart cmp;
 
 	temp.sort (cmp);
-	locations = temp;
 
-	for (n = 0, i = locations.begin(); i != locations.end(); ++n, ++i) {
+	for (n = 0, i = temp.begin(); i != temp.end(); ++n, ++i) {
 
 		Location* location = *i;
 
@@ -1074,10 +1073,10 @@ LocationUI::set_session(ARDOUR::Session* s)
 	SessionHandlePtr::set_session (s);
 
 	if (_session) {
-		_session->locations()->changed.connect (_session_connections, invalidator (*this), boost::bind (&LocationUI::locations_changed, this, _1), gui_context());
-		_session->locations()->StateChanged.connect (_session_connections, invalidator (*this), boost::bind (&LocationUI::refresh_location_list, this), gui_context());
 		_session->locations()->added.connect (_session_connections, invalidator (*this), boost::bind (&LocationUI::location_added, this, _1), gui_context());
 		_session->locations()->removed.connect (_session_connections, invalidator (*this), boost::bind (&LocationUI::location_removed, this, _1), gui_context());
+		_session->locations()->changed.connect (_session_connections, invalidator (*this), boost::bind (&LocationUI::refresh_location_list, this), gui_context());
+
 		_clock_group->set_clock_mode (clock_mode_from_session_instant_xml ());
 	}
 
@@ -1087,17 +1086,6 @@ LocationUI::set_session(ARDOUR::Session* s)
 	refresh_location_list ();
 }
 
-void
-LocationUI::locations_changed (Locations::Change c)
-{
-	/* removal is signalled by both a removed and a changed signal emission from Locations,
-	   so we don't need to refresh the list on a removal
-	*/
-	if (c != Locations::REMOVAL) {
-		refresh_location_list ();
-	}
-}
-
 void
 LocationUI::session_going_away()
 {

gtk2_ardour/location_ui.h

@@ -202,8 +202,7 @@ class LocationUI : public Gtk::HBox, public ARDOUR::SessionHandlePtr
 
 	void location_removed (ARDOUR::Location *);
 	void location_added (ARDOUR::Location *);
-	void locations_changed (ARDOUR::Locations::Change);
+	void map_locations (const ARDOUR::Locations::LocationList&);
-	void map_locations (ARDOUR::Locations::LocationList&);
 
         ClockGroup* _clock_group;
 	AudioClock::Mode clock_mode_from_session_instant_xml () const;

libs/ardour/ardour/location.h

@@ -53,6 +53,7 @@ class LIBARDOUR_API Location : public SessionHandleRef, public PBD::StatefulDest
 		IsRangeMarker = 0x20,
 		IsSessionRange = 0x40,
 		IsSkip = 0x80,
+		IsSkipping = 0x100, /* skipping is active (or not) */
 	};
 
 	Location (Session &);
@@ -78,7 +79,7 @@ class LIBARDOUR_API Location : public SessionHandleRef, public PBD::StatefulDest
 	int move_to (framepos_t pos);
 
 	const std::string& name() const { return _name; }
-	void set_name (const std::string &str) { _name = str; name_changed(this); }
+	void set_name (const std::string &str);
 
 	void set_auto_punch (bool yn, void *src);
 	void set_auto_loop (bool yn, void *src);
@@ -86,6 +87,7 @@ class LIBARDOUR_API Location : public SessionHandleRef, public PBD::StatefulDest
 	void set_cd (bool yn, void *src);
 	void set_is_range_marker (bool yn, void* src);
         void set_skip (bool yn);
+        void set_skipping (bool yn);
 
 	bool is_auto_punch () const { return _flags & IsAutoPunch; }
 	bool is_auto_loop () const { return _flags & IsAutoLoop; }
@@ -95,6 +97,7 @@ class LIBARDOUR_API Location : public SessionHandleRef, public PBD::StatefulDest
 	bool is_session_range () const { return _flags & IsSessionRange; }
 	bool is_range_marker() const { return _flags & IsRangeMarker; }
 	bool is_skip() const { return _flags & IsSkip; }
+	bool is_skipping() const { return (_flags & IsSkip) && (_flags & IsSkipping); }
 	bool matches (Flags f) const { return _flags & f; }
 
 	Flags flags () const { return _flags; }
@@ -197,28 +200,26 @@ class LIBARDOUR_API Locations : public SessionHandleRef, public PBD::StatefulDes
 
 	void find_all_between (framepos_t start, framepos_t, LocationList&, Location::Flags);
 
-	enum Change {
-		ADDITION, ///< a location was added, but nothing else changed
-		REMOVAL, ///< a location was removed, but nothing else changed
-		OTHER ///< something more complicated happened
-	};
-
 	PBD::Signal1<void,Location*> current_changed;
-	/** something changed about the location list; the parameter gives some idea as to what */
+
-	PBD::Signal1<void,Change>    changed;
+        /* Objects that care about individual addition and removal of Locations should connect to added/removed.
-	/** a location has been added to the end of the list */
+           If an object additionally cares about potential mass clearance of Locations, they should connect to changed.
+        */
+
 	PBD::Signal1<void,Location*> added;
 	PBD::Signal1<void,Location*> removed;
-	PBD::Signal1<void,const PBD::PropertyChange&>    StateChanged;
+	PBD::Signal0<void> changed; /* emitted when any action that could have added/removed more than 1 location actually removed 1 or more */
 
-	template<class T> void apply (T& obj, void (T::*method)(LocationList&)) {
+	template<class T> void apply (T& obj, void (T::*method)(const LocationList&)) const {
-		Glib::Threads::Mutex::Lock lm (lock);
+                /* We don't want to hold the lock while the given method runs, so take a copy
-		(obj.*method)(locations);
+                   of the list and pass that instead.
-	}
+                */
-
+                Locations::LocationList copy;
-	template<class T1, class T2> void apply (T1& obj, void (T1::*method)(LocationList&, T2& arg), T2& arg) {
+                {
-		Glib::Threads::Mutex::Lock lm (lock);
+                        Glib::Threads::Mutex::Lock lm (lock);
-		(obj.*method)(locations, arg);
+                        copy = locations;
+                }
+		(obj.*method)(copy);
 	}
 
   private:

libs/ardour/ardour/midi_scene_changer.h

@@ -65,7 +65,7 @@ class MIDISceneChanger : public SceneChanger
 
 	void bank_change_input (MIDI::Parser&, unsigned short, int channel);
 	void program_change_input (MIDI::Parser&, MIDI::byte, int channel);
-	void locations_changed (Locations::Change);
+	void locations_changed ();
 
 	PBD::ScopedConnectionList incoming_connections;
 };

libs/ardour/ardour/session.h

@@ -1169,10 +1169,15 @@ class LIBARDOUR_API Session : public PBD::StatefulDestructible, public PBD::Scop
 	void reset_rf_scale (framecnt_t frames_moved);
 
 	Locations*       _locations;
-	void              locations_changed ();
+	void              location_added (Location*);
-	void              locations_added (Location*);
+	void              location_removed (Location*);
-	void              handle_locations_changed (Locations::LocationList&);
+        void              locations_changed ();
-	void              sync_locations_to_skips (Locations::LocationList&);
+        void             _locations_changed (const Locations::LocationList&);
+
+        void              update_skips (Location*, bool consolidate);
+        Locations::LocationList consolidate_skips (Location*);
+	void              sync_locations_to_skips (const Locations::LocationList&);
+        PBD::ScopedConnectionList skip_connections;
 
 	PBD::ScopedConnectionList punch_connections;
 	void             auto_punch_start_changed (Location *);
@@ -1634,7 +1639,7 @@ class LIBARDOUR_API Session : public PBD::StatefulDestructible, public PBD::Scop
 	/** true if timecode transmission by the transport is suspended, otherwise false */
 	mutable gint _suspend_timecode_transmission;
 
-	void update_locations_after_tempo_map_change (Locations::LocationList &);
+	void update_locations_after_tempo_map_change (const Locations::LocationList &);
 
 	void start_time_changed (framepos_t);
 	void end_time_changed (framepos_t);

libs/ardour/location.cc

@@ -161,6 +161,18 @@ Location::operator= (const Location& other)
 	return this;
 }
 
+/** Set location name
+ */
+
+void
+Location::set_name (const std::string& str)
+{ 
+        _name = str; 
+
+        name_changed (this); /* EMIT SIGNAL */
+        NameChanged  (); /* EMIT SIGNAL */
+}
+
 /** Set start position.
  *  @param s New start.
  *  @param force true to force setting, even if the given new start is after the current end.
@@ -282,6 +294,7 @@ Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute)
 		if (allow_bbt_recompute) {
 			recompute_bbt_from_frames ();
 		}
+
 		end_changed(this); /* EMIT SIGNAL */
 		EndChanged(); /* EMIT SIGNAL */
 
@@ -296,22 +309,92 @@ Location::set_end (framepos_t e, bool force, bool allow_bbt_recompute)
 }
 
 int
-Location::set (framepos_t start, framepos_t end, bool allow_bbt_recompute)
+Location::set (framepos_t s, framepos_t e, bool allow_bbt_recompute)
 {
-	if (start < 0 || end < 0) {
+	if (s < 0 || e < 0) {
 		return -1;
 	}
 
 	/* check validity */
-	if (((is_auto_punch() || is_auto_loop()) && start >= end) || (!is_mark() && start > end)) {
+	if (((is_auto_punch() || is_auto_loop()) && s >= e) || (!is_mark() && s > e)) {
 		return -1;
 	}
 
-	/* now we know these values are ok, so force-set them */
+        bool start_change = false;
-	int const s = set_start (start, true, allow_bbt_recompute);
+        bool end_change = false;
-	int const e = set_end (end, true, allow_bbt_recompute);
 
-	return (s == 0 && e == 0) ? 0 : -1;
+	if (is_mark()) {
+
+		if (_start != s) {
+			_start = s;
+			_end = s;
+
+			if (allow_bbt_recompute) {
+				recompute_bbt_from_frames ();
+			}
+
+                        start_change = true;
+                        end_change = true;
+		}
+
+		assert (_start >= 0);
+		assert (_end >= 0);
+
+	} else {
+                
+                if (s != _start) {
+
+                        framepos_t const old = _start;
+                        _start = s;
+
+                        if (allow_bbt_recompute) {
+                                recompute_bbt_from_frames ();
+                        }
+
+                        start_change = true;
+                        
+                        if (is_session_range ()) {
+                                Session::StartTimeChanged (old); /* EMIT SIGNAL */
+                                AudioFileSource::set_header_position_offset (s);
+                        }
+                }
+                        
+                 
+                if (e != _end) {
+                        
+                        framepos_t const old = _end;
+                        _end = e;
+
+                        if (allow_bbt_recompute) {
+                                recompute_bbt_from_frames ();
+                        }
+                        
+                        end_change = true;
+
+                        if (is_session_range()) {
+                                Session::EndTimeChanged (old); /* EMIT SIGNAL */
+                        }
+                }
+
+                assert (_end >= 0);
+        }
+
+        if (start_change) {
+                start_changed(this); /* EMIT SIGNAL */
+                StartChanged(); /* EMIT SIGNAL */
+        }
+
+        if (end_change) {
+                end_changed(this); /* EMIT SIGNAL */
+                EndChanged(); /* EMIT SIGNAL */
+        }
+
+        if (start_change && end_change) {
+                changed (this);
+                Changed ();
+        }
+
+        return 0;
 }
 
 int
@@ -386,6 +469,17 @@ Location::set_skip (bool yn)
         }
 }
 
+void
+Location::set_skipping (bool yn)
+{
+        if (is_range_marker() && is_skip() && length() > 0) {
+                if (set_flag_internal (yn, IsSkipping)) {
+                        flags_changed (this);
+                        FlagsChanged ();
+                }
+        }
+}
+
 void
 Location::set_auto_punch (bool yn, void*)
 {
@@ -655,11 +749,6 @@ Locations::Locations (Session& s)
 	: SessionHandleRef (s)
 {
 	current_location = 0;
-
-	Location::changed.connect_same_thread (*this, boost::bind (&Locations::location_changed, this, _1));
-	Location::start_changed.connect_same_thread (*this, boost::bind (&Locations::location_changed, this, _1));
-	Location::end_changed.connect_same_thread (*this, boost::bind (&Locations::location_changed, this, _1));
-	Location::flags_changed.connect_same_thread (*this, boost::bind (&Locations::location_changed, this, _1));
 }
 
 Locations::~Locations ()
@@ -758,7 +847,7 @@ Locations::clear ()
 		current_location = 0;
 	}
 
-	changed (OTHER); /* EMIT SIGNAL */
+	changed (); /* EMIT SIGNAL */
 	current_changed (0); /* EMIT SIGNAL */
 }
 
@@ -781,8 +870,8 @@ Locations::clear_markers ()
 			i = tmp;
 		}
 	}
-
+        
-	changed (OTHER); /* EMIT SIGNAL */
+	changed (); /* EMIT SIGNAL */
 }
 
 void
@@ -820,7 +909,7 @@ Locations::clear_ranges ()
 		current_location = 0;
 	}
 
-	changed (OTHER); /* EMIT SIGNAL */
+        changed ();
 	current_changed (0); /* EMIT SIGNAL */
 }
 
@@ -881,21 +970,13 @@ Locations::remove (Location *loc)
 	if (was_removed) {
 
 		removed (loc); /* EMIT SIGNAL */
-
+                
 		if (was_current) {
 			 current_changed (0); /* EMIT SIGNAL */
 		}
-
-		changed (REMOVAL); /* EMIT_SIGNAL */
 	}
 }
 
-void
-Locations::location_changed (Location* /*loc*/)
-{
-	changed (OTHER); /* EMIT SIGNAL */
-}
-
 XMLNode&
 Locations::get_state ()
 {
@@ -1005,7 +1086,7 @@ Locations::set_state (const XMLNode& node, int version)
 		}
 	}
 
-	changed (OTHER); /* EMIT SIGNAL */
+	changed (); /* EMIT SIGNAL */
 
 	return 0;
 }

libs/ardour/midi_scene_changer.cc

@@ -42,7 +42,7 @@ MIDISceneChanger::MIDISceneChanger (Session& s)
 	, last_delivered_bank (-1)
 	  
 {
-	_session.locations()->changed.connect_same_thread (*this, boost::bind (&MIDISceneChanger::locations_changed, this, _1));
+	_session.locations()->changed.connect_same_thread (*this, boost::bind (&MIDISceneChanger::locations_changed, this));
 	Location::scene_changed.connect_same_thread (*this, boost::bind (&MIDISceneChanger::gather, this));
 }
 
@@ -51,7 +51,7 @@ MIDISceneChanger::~MIDISceneChanger ()
 }
 
 void
-MIDISceneChanger::locations_changed (Locations::Change)
+MIDISceneChanger::locations_changed ()
 {
 	gather ();
 }

libs/ardour/session.cc

@@ -1235,9 +1235,9 @@ Session::set_auto_punch_location (Location* location)
 
 	punch_connections.drop_connections ();
 
-	location->start_changed.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_start_changed, this, _1));
+	location->StartChanged.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_start_changed, this, location));
-	location->end_changed.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_end_changed, this, _1));
+	location->EndChanged.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_end_changed, this, location));
-	location->changed.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_changed, this, _1));
+	location->Changed.connect_same_thread (punch_connections, boost::bind (&Session::auto_punch_changed, this, location));
 
 	location->set_auto_punch (true, this);
 
@@ -1277,9 +1277,9 @@ Session::set_auto_loop_location (Location* location)
 
 	loop_connections.drop_connections ();
 
-	location->start_changed.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, _1));
+	location->StartChanged.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, location));
-	location->end_changed.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, _1));
+	location->EndChanged.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, location));
-	location->changed.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, _1));
+	location->Changed.connect_same_thread (loop_connections, boost::bind (&Session::auto_loop_changed, this, location));
 
 	location->set_auto_loop (true, this);
 
@@ -1293,73 +1293,165 @@ Session::set_auto_loop_location (Location* location)
 }
 
 void
-Session::locations_added (Location *)
+Session::update_skips (Location* loc, bool consolidate)
 {
-	_locations->apply (*this, &Session::sync_locations_to_skips);
+        Locations::LocationList skips;
+
+        if (consolidate) {
+
+                skips = consolidate_skips (loc);
+
+        } else {
+                Locations::LocationList all_locations = _locations->list ();
+                
+                for (Locations::LocationList::iterator l = all_locations.begin(); l != all_locations.end(); ++l) {
+                        if ((*l)->is_skip ()) {
+                                skips.push_back (*l);
+                        }
+                }
+        }
+
+        sync_locations_to_skips (skips);
+}
+
+Locations::LocationList
+Session::consolidate_skips (Location* loc)
+{
+        Locations::LocationList all_locations = _locations->list ();
+        Locations::LocationList skips;
+
+        for (Locations::LocationList::iterator l = all_locations.begin(); l != all_locations.end(); ) {
+
+                if (!(*l)->is_skip ()) {
+                        ++l;
+                        continue;
+                }
+
+                /* don't test against self */
+
+                if (*l == loc) {
+                        ++l;
+                        continue;
+                }
+                        
+                switch (Evoral::coverage ((*l)->start(), (*l)->end(), loc->start(), loc->end())) {
+                case Evoral::OverlapInternal:
+                case Evoral::OverlapExternal:
+                case Evoral::OverlapStart:
+                case Evoral::OverlapEnd:
+                        /* adjust new location to cover existing one */
+                        loc->set_start (min (loc->start(), (*l)->start()));
+                        loc->set_end (max (loc->end(), (*l)->end()));
+                        /* we don't need this one any more */
+                        _locations->remove (*l);
+                        /* the location has been deleted, so remove reference to it in our local list */
+                        l = all_locations.erase (l);
+                        break;
+
+                case Evoral::OverlapNone:
+                        skips.push_back (*l);
+                        ++l;
+                        break;
+                }
+        }
+
+        /* add the new one, which now covers the maximal appropriate range based on overlaps with existing skips */
+
+        skips.push_back (loc);
+
+        return skips;
+}
+
+void
+Session::sync_locations_to_skips (const Locations::LocationList& locations)
+{
+	clear_events (SessionEvent::Skip);
+
+	for (Locations::LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
+                
+		Location* location = *i;
+                
+		if (location->is_skipping()) {
+			SessionEvent* ev = new SessionEvent (SessionEvent::Skip, SessionEvent::Add, location->start(), location->end(), 1.0);
+			queue_event (ev);
+		}
+	}
+}
+
+void
+Session::location_added (Location *location)
+{
+        if (location->is_auto_punch()) {
+                set_auto_punch_location (location);
+        }
+
+        if (location->is_auto_loop()) {
+                set_auto_loop_location (location);
+        }
+        
+        if (location->is_session_range()) {
+                /* no need for any signal handling or event setting with the session range,
+                   because we keep a direct reference to it and use its start/end directly.
+                */
+                _session_range_location = location;
+        }
+
+        if (location->is_skip()) {
+                /* listen for per-location signals that require us to update skip-locate events */
+
+                location->StartChanged.connect_same_thread (skip_connections, boost::bind (&Session::update_skips, this, location, true));
+                location->EndChanged.connect_same_thread (skip_connections, boost::bind (&Session::update_skips, this, location, true));
+                location->Changed.connect_same_thread (skip_connections, boost::bind (&Session::update_skips, this, location, true));
+                location->FlagsChanged.connect_same_thread (skip_connections, boost::bind (&Session::update_skips, this, location, false));
+
+                update_skips (location, true);
+        }
+
+	set_dirty ();
+}
+
+void
+Session::location_removed (Location *location)
+{
+        if (location->is_auto_loop()) {
+                set_auto_loop_location (0);
+        }
+        
+        if (location->is_auto_punch()) {
+                set_auto_punch_location (0);
+        }
+
+        if (location->is_session_range()) {
+                /* this is never supposed to happen */
+                error << _("programming error: session range removed!") << endl;
+        }
+
+        if (location->is_skip()) {
+                
+                update_skips (location, false);
+        }
+
 	set_dirty ();
 }
 
 void
 Session::locations_changed ()
 {
-	_locations->apply (*this, &Session::handle_locations_changed);
+        _locations->apply (*this, &Session::_locations_changed);
 }
 
 void
-Session::handle_locations_changed (Locations::LocationList& locations)
+Session::_locations_changed (const Locations::LocationList& locations)
 {
-	Locations::LocationList::iterator i;
+        /* There was some mass-change in the Locations object. 
-	Location* location;
-	bool set_loop = false;
-	bool set_punch = false;
 
-	for (i = locations.begin(); i != locations.end(); ++i) {
+           We might be re-adding a location here but it doesn't actually matter
+           for all the locations that the Session takes an interest in.
+        */
 
-		location =* i;
+	for (Locations::LocationList::const_iterator i = locations.begin(); i != locations.end(); ++i) {
-
+                location_added (*i);
-		if (location->is_auto_punch()) {
+        }
-			set_auto_punch_location (location);
-			set_punch = true;
-		}
-		if (location->is_auto_loop()) {
-			set_auto_loop_location (location);
-			set_loop = true;
-		}
-
-		if (location->is_session_range()) {
-			_session_range_location = location;
-		}
-	}
-
-	sync_locations_to_skips (locations);
-
-	if (!set_loop) {
-		set_auto_loop_location (0);
-	}
-	if (!set_punch) {
-		set_auto_punch_location (0);
-	}
-
-	set_dirty();
-}
-
-void
-Session::sync_locations_to_skips (Locations::LocationList& locations)
-{
-	Locations::LocationList::iterator i;
-	Location* location;
-
-	clear_events (SessionEvent::Skip);
-
-	for (i = locations.begin(); i != locations.end(); ++i) {
-
-		location = *i;
-
-		if (location->is_skip()) {
-			SessionEvent* ev = new SessionEvent (SessionEvent::Skip, SessionEvent::Add, location->start(), location->end(), 1.0);
-			queue_event (ev);
-		}
-	}
 }
 
 void
@@ -4025,9 +4117,9 @@ Session::tempo_map_changed (const PropertyChange&)
 }
 
 void
-Session::update_locations_after_tempo_map_change (Locations::LocationList& loc)
+Session::update_locations_after_tempo_map_change (const Locations::LocationList& loc)
 {
-	for (Locations::LocationList::iterator i = loc.begin(); i != loc.end(); ++i) {
+	for (Locations::LocationList::const_iterator i = loc.begin(); i != loc.end(); ++i) {
 		(*i)->recompute_frames_from_bbt ();
 	}
 }

libs/ardour/session_state.cc

@@ -316,10 +316,9 @@ Session::post_engine_init ()
 		
 		initialize_latencies ();
 		
+		_locations->added.connect_same_thread (*this, boost::bind (&Session::location_added, this, _1));
+		_locations->removed.connect_same_thread (*this, boost::bind (&Session::location_removed, this, _1));
 		_locations->changed.connect_same_thread (*this, boost::bind (&Session::locations_changed, this));
-		_locations->added.connect_same_thread (*this, boost::bind (&Session::locations_added, this, _1));
-		
-		
 		
 	} catch (AudioEngine::PortRegistrationFailure& err) {
 		/* handle this one in a different way than all others, so that its clear what happened */