From a2b3e31e109cb89583986ad31cc2af1f8a435f09 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Wed, 2 Jun 2021 16:33:27 -0600 Subject: [PATCH] libardour: remove ripple callback API, and fix deadlock issues with ripple/ripple_locked/ripple_unlocked/remove_gaps --- libs/ardour/ardour/playlist.h | 22 ++++---- libs/ardour/playlist.cc | 99 +++++++++++++++++++---------------- 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h index 45f2c5917a..babbd84e23 100644 --- a/libs/ardour/ardour/playlist.h +++ b/libs/ardour/ardour/playlist.h @@ -191,16 +191,14 @@ public: void shuffle (boost::shared_ptr, int dir); - typedef boost::function RippleCallback; - - void ripple (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback ripple_callback); - void ripple (samplepos_t at, samplecnt_t distance, boost::shared_ptr exclude, RippleCallback ripple_callback) + void ripple (samplepos_t at, samplecnt_t distance, RegionList* exclude); + void ripple (samplepos_t at, samplecnt_t distance, boost::shared_ptr exclude) { RegionList el; if (exclude) { el.push_back (exclude); } - ripple (at, distance, &el, ripple_callback); + ripple (at, distance, &el); } void update_after_tempo_map_change (); @@ -305,24 +303,24 @@ protected: friend class Session; protected: - class RegionReadLock : public Glib::Threads::RWLock::ReaderLock { public: RegionReadLock (Playlist* pl) - : Glib::Threads::RWLock::ReaderLock (pl->region_lock) + : Glib::Threads::RWLock::ReaderLock (pl->region_lock) { } ~RegionReadLock () {} + }; class RegionWriteLock : public Glib::Threads::RWLock::WriterLock { public: RegionWriteLock (Playlist* pl, bool do_block_notify = true) - : Glib::Threads::RWLock::WriterLock (pl->region_lock) - , playlist (pl) - , block_notify (do_block_notify) + : Glib::Threads::RWLock::WriterLock (pl->region_lock) + , playlist (pl) + , block_notify (do_block_notify) { if (block_notify) { playlist->delay_notifications (); @@ -429,8 +427,8 @@ protected: void splice_locked (samplepos_t at, samplecnt_t distance, boost::shared_ptr exclude); void splice_unlocked (samplepos_t at, samplecnt_t distance, boost::shared_ptr exclude, ThawList& thawlist); - void ripple_locked (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback); - void ripple_unlocked (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback, ThawList& thawlist, bool notify = true); + void ripple_locked (samplepos_t at, samplecnt_t distance, RegionList* exclude); + bool ripple_unlocked (samplepos_t at, samplecnt_t distance, RegionList* exclude, ThawList& thawlist); virtual void remove_dependents (boost::shared_ptr /*region*/) {} virtual void region_going_away (boost::weak_ptr /*region*/) {} diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc index 6c6e89a3dd..b1811fd115 100644 --- a/libs/ardour/playlist.cc +++ b/libs/ardour/playlist.cc @@ -874,44 +874,46 @@ Playlist::remove_region_internal (boost::shared_ptr region, ThawList& th void Playlist::remove_gaps (samplepos_t gap_threshold, samplepos_t leave_gap, boost::function gap_callback) { - RegionWriteLock rlock (this); - RegionList::iterator i; - RegionList::iterator nxt (regions.end()); bool closed = false; - RippleCallback null_ripple_callback; - if (regions.size() < 2) { - return; - } + { + RegionWriteLock rlock (this); + RegionList::iterator i; + RegionList::iterator nxt (regions.end()); - for (i = regions.begin(); i != regions.end(); ++i) { - - nxt = i; - ++nxt; - - if (nxt == regions.end()) { - break; + if (regions.size() < 2) { + return; } - samplepos_t end_of_this_region = (*i)->position() + (*i)->length(); + for (i = regions.begin(); i != regions.end(); ++i) { - if (end_of_this_region >= (*nxt)->position()) { - continue; + nxt = i; + ++nxt; + + if (nxt == regions.end()) { + break; + } + + samplepos_t end_of_this_region = (*i)->position() + (*i)->length(); + + if (end_of_this_region >= (*nxt)->position()) { + continue; + } + + const samplepos_t gap = (*nxt)->position() - end_of_this_region; + + if (gap < gap_threshold) { + continue; + } + + const samplepos_t shift = gap - leave_gap; + + (void) ripple_unlocked ((*nxt)->position(), -shift, 0, rlock.thawlist); + + gap_callback ((*nxt)->position(), shift); + + closed = true; } - - const samplepos_t gap = (*nxt)->position() - end_of_this_region; - - if (gap < gap_threshold) { - continue; - } - - const samplepos_t shift = gap - leave_gap; - - ripple_unlocked ((*nxt)->position(), -shift, 0, null_ripple_callback, rlock.thawlist, false); - - gap_callback ((*nxt)->position(), shift); - - closed = true; } if (closed) { @@ -1654,23 +1656,33 @@ Playlist::splice_unlocked (samplepos_t at, samplecnt_t distance, boost::shared_p } void -Playlist::ripple (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback ripple_callback) +Playlist::ripple (samplepos_t at, samplecnt_t distance, RegionList* exclude) { - ripple_locked (at, distance, exclude, ripple_callback); + ripple_locked (at, distance, exclude); } void -Playlist::ripple_locked (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback ripple_callback) +Playlist::ripple_locked (samplepos_t at, samplecnt_t distance, RegionList* exclude) { - RegionWriteLock rl (this); - ripple_unlocked (at, distance, exclude, ripple_callback, rl.thawlist); + bool changed = false; + { + RegionWriteLock rl (this); + changed = ripple_unlocked (at, distance, exclude, rl.thawlist); + } + + if (changed) { + notify_contents_changed (); + } } -void -Playlist::ripple_unlocked (samplepos_t at, samplecnt_t distance, RegionList* exclude, RippleCallback ripple_callback, ThawList& thawlist, bool notify) + +bool +Playlist::ripple_unlocked (samplepos_t at, samplecnt_t distance, RegionList* exclude, ThawList& thawlist) { - if (distance == 0) { - return; + bool changed = false; + + if (distance == 0 || regions.empty()) { + return false; } _rippling = true; @@ -1695,16 +1707,13 @@ Playlist::ripple_unlocked (samplepos_t at, samplecnt_t distance, RegionList* exc thawlist.add (*i); (*i)->set_position (new_pos); + changed = true; } } _rippling = false; - ripple_callback (*this, at, distance); - - if (notify) { - notify_contents_changed (); - } + return changed; } void