From 83555ec290ea0556c9a0af3ad56fdd399577d028 Mon Sep 17 00:00:00 2001
From: Robin Gareus <robin@gareus.org>
Date: Sun, 14 May 2023 20:38:31 +0200
Subject: [PATCH] Consolidate playlist block & ignore notifications

Despite the names suggesting otherwise
`block_notifications` and `ignore_state_changes` are used
for the same purpose.

The only difference is that ::freeze ::thaw explicitly
modified `ignore_state_changes` **in addition** to
`block_notifications`.
---
 libs/ardour/ardour/playlist.h | 11 +++-----
 libs/ardour/playlist.cc       | 48 ++++++++++-------------------------
 2 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h
index cd92ed2bc1..4e912bd83f 100644
--- a/libs/ardour/ardour/playlist.h
+++ b/libs/ardour/ardour/playlist.h
@@ -351,10 +351,9 @@ protected:
 	PBD::ScopedConnectionList            region_drop_references_connections;
 	DataType                             _type;
 	uint32_t                             _sort_id;
-	mutable std::atomic<int>            block_notifications;
-	mutable std::atomic<int>            ignore_state_changes;
-	std::set<std::shared_ptr<Region> > pending_adds;
-	std::set<std::shared_ptr<Region> > pending_removes;
+	mutable std::atomic<int>             block_notifications;
+	std::set<std::shared_ptr<Region> >   pending_adds;
+	std::set<std::shared_ptr<Region> >   pending_removes;
 	RegionList                           pending_bounds;
 	bool                                 pending_contents_change;
 	bool                                 pending_layering;
@@ -393,8 +392,7 @@ protected:
 
 	bool holding_state () const
 	{
-		return block_notifications.load () != 0 ||
-		       ignore_state_changes.load () != 0;
+		return block_notifications.load () != 0;
 	}
 
 	void         delay_notifications ();
@@ -472,7 +470,6 @@ private:
 	mutable Glib::Threads::RWLock region_lock;
 
 private:
-	void freeze_locked ();
 	void setup_layering_indices (RegionList const &);
 	void coalesce_and_check_crossfades (std::list<Temporal::TimeRange>);
 	std::shared_ptr<RegionList> find_regions_at (timepos_t const &);
diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc
index c0b229121a..ebed4a77f6 100644
--- a/libs/ardour/playlist.cc
+++ b/libs/ardour/playlist.cc
@@ -322,7 +322,6 @@ Playlist::init (bool hide)
 	_xml_node_name = X_("Playlist");
 
 	block_notifications.store (0);
-	ignore_state_changes.store (0);
 	pending_contents_change     = false;
 	pending_layering            = false;
 	first_set_state             = true;
@@ -422,22 +421,14 @@ Playlist::freeze ()
 	/* flush any ongoing reads, paricularly AudioPlaylist::read(),
 	 * before beginning to modify the playlist.
 	 */
-	RegionWriteLock rlock (this);
-	freeze_locked ();
-}
-
-void
-Playlist::freeze_locked ()
-{
+	RegionWriteLock rlock (this, false);
 	delay_notifications ();
-	ignore_state_changes.fetch_add (1);
 }
 
 /** @param from_undo true if this thaw is triggered by the end of an undo on this playlist */
 void
 Playlist::thaw (bool from_undo)
 {
-	PBD::atomic_dec_and_test (ignore_state_changes);
 	release_notifications (from_undo);
 }
 
@@ -2217,20 +2208,15 @@ Playlist::update (const RegionListProperty::ChangeRecord& change)
 	DEBUG_TRACE (DEBUG::Properties, string_compose ("Playlist %1 updates from a change record with %2 adds %3 removes\n",
 	                                                name (), change.added.size (), change.removed.size ()));
 
-	{
-		RegionWriteLock rlock (this);
-		freeze_locked ();
-		/* add the added regions */
-		for (RegionListProperty::ChangeContainer::const_iterator i = change.added.begin(); i != change.added.end(); ++i) {
-			add_region_internal ((*i), (*i)->position(), rlock.thawlist);
-		}
-		/* remove the removed regions */
-		for (RegionListProperty::ChangeContainer::const_iterator i = change.removed.begin (); i != change.removed.end (); ++i) {
-			remove_region_internal (*i, rlock.thawlist);
-		}
+	RegionWriteLock rlock (this);
+	/* add the added regions */
+	for (RegionListProperty::ChangeContainer::const_iterator i = change.added.begin(); i != change.added.end(); ++i) {
+		add_region_internal ((*i), (*i)->position(), rlock.thawlist);
+	}
+	/* remove the removed regions */
+	for (RegionListProperty::ChangeContainer::const_iterator i = change.removed.begin (); i != change.removed.end (); ++i) {
+		remove_region_internal (*i, rlock.thawlist);
 	}
-
-	thaw ();
 }
 
 int
@@ -3002,19 +2988,13 @@ Playlist::ripple (timepos_t const & at, timecnt_t const & distance, RegionList *
 void
 Playlist::update_after_tempo_map_change ()
 {
-	{
-		RegionWriteLock rlock (const_cast<Playlist*> (this));
-		RegionList      copy (regions.rlist ());
+	RegionWriteLock rlock (const_cast<Playlist*> (this));
+	RegionList      copy (regions.rlist ());
 
-		freeze_locked ();
-
-		for (auto & r : copy) {
-			rlock.thawlist.add (r);
-			r->update_after_tempo_map_change ();
-		}
+	for (auto & r : copy) {
+		rlock.thawlist.add (r);
+		r->update_after_tempo_map_change ();
 	}
-	/* possibly causes a contents changed notification (flush_notifications()) */
-	thaw ();
 }
 
 void