From 65cc9264c802e15043c8f59c05c34941f8ff31bf Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Fri, 30 Apr 2021 04:09:25 +0200 Subject: [PATCH] Create regions with property changes suspended This fixes various issues with signal emission(s) when creating regions from withing playlist operations. eg. Playlist::duplicate() takes RegionWriteLock() and then calls RegionFactory::create(). see also 6a82aa392c6dd0f4057f5ae37095cf1fd4478626 --- libs/ardour/ardour/playlist.h | 21 +------------- libs/ardour/ardour/region_factory.h | 11 +++++--- libs/ardour/ardour/thawlist.h | 35 +++++++++++++++++++++++ libs/ardour/playlist.cc | 28 +++++++++---------- libs/ardour/region_factory.cc | 20 ++++++++++++-- libs/ardour/thawlist.cc | 43 +++++++++++++++++++++++++++++ libs/ardour/wscript | 1 + 7 files changed, 118 insertions(+), 41 deletions(-) create mode 100644 libs/ardour/ardour/thawlist.h create mode 100644 libs/ardour/thawlist.cc diff --git a/libs/ardour/ardour/playlist.h b/libs/ardour/ardour/playlist.h index ebc15cb28e..2557bbe919 100644 --- a/libs/ardour/ardour/playlist.h +++ b/libs/ardour/ardour/playlist.h @@ -53,6 +53,7 @@ #include "ardour/data_type.h" #include "ardour/region.h" #include "ardour/session_object.h" +#include "ardour/thawlist.h" namespace ARDOUR { @@ -280,26 +281,6 @@ protected: friend class Session; protected: - class ThawList : public RegionList - { - public: - void add (boost::shared_ptr r) - { - if (std::find (begin (), end (), r) != end ()) { - return; - } - r->suspend_property_changes (); - push_back (r); - } - - void release () - { - for (RegionList::iterator i = begin (); i != end (); ++i) { - (*i)->resume_property_changes (); - } - clear (); - } - }; class RegionReadLock : public Glib::Threads::RWLock::ReaderLock { diff --git a/libs/ardour/ardour/region_factory.h b/libs/ardour/ardour/region_factory.h index a3f61f8819..f0bb5fa1ad 100644 --- a/libs/ardour/ardour/region_factory.h +++ b/libs/ardour/ardour/region_factory.h @@ -33,6 +33,7 @@ #include "pbd/signals.h" #include "ardour/libardour_visibility.h" +#include "ardour/thawlist.h" #include "ardour/types.h" class XMLNode; @@ -63,9 +64,11 @@ public: static PBD::Signal1 > CheckNewRegion; /** create a "pure copy" of Region \p other */ - static boost::shared_ptr create (boost::shared_ptr other, bool announce = false, bool fork = false); + static boost::shared_ptr create (boost::shared_ptr other, bool announce, bool fork = false, ThawList* tl = 0); + + /** Lua binding to create a "pure copy" of Region \p other */ static boost::shared_ptr create (boost::shared_ptr other, bool announce, bool fork) { - return create (boost::shared_ptr(other), announce, fork); + return create (boost::shared_ptr(other), announce, fork, 0); } /** create a region from a single Source */ @@ -77,10 +80,10 @@ public: const PBD::PropertyList&, bool announce = true); /** create a copy of \p other starting at zero within \p other's sources */ static boost::shared_ptr create (boost::shared_ptr other, - const PBD::PropertyList&, bool announce = true); + const PBD::PropertyList&, bool announce = true, ThawList* tl = 0); /** create a copy of \p other starting at \p offset within \p other */ static boost::shared_ptr create (boost::shared_ptr other, ARDOUR::MusicSample offset, - const PBD::PropertyList&, bool announce = true); + const PBD::PropertyList&, bool announce = true, ThawList* tl = 0); /** create a "copy" of \p other but using a different set of sources \p srcs */ static boost::shared_ptr create (boost::shared_ptr other, const SourceList& srcs, const PBD::PropertyList&, bool announce = true); diff --git a/libs/ardour/ardour/thawlist.h b/libs/ardour/ardour/thawlist.h new file mode 100644 index 0000000000..eb551a28ce --- /dev/null +++ b/libs/ardour/ardour/thawlist.h @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2021 Robin Gareus + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#ifndef __ardour_thawlist_h__ +#define __ardour_thawlist_h__ + +#include "ardour/types.h" +#include "ardour/libardour_visibility.h" + +namespace ARDOUR { + +class LIBARDOUR_API ThawList : public RegionList +{ + public: + void add (boost::shared_ptr r); + void release (); +}; + +} +#endif diff --git a/libs/ardour/playlist.cc b/libs/ardour/playlist.cc index 46fb124a98..edb29c5537 100644 --- a/libs/ardour/playlist.cc +++ b/libs/ardour/playlist.cc @@ -284,7 +284,7 @@ Playlist::Playlist (boost::shared_ptr other, samplepos_t start, plist.add (Properties::layer, region->layer ()); plist.add (Properties::layering_index, region->layering_index ()); - new_region = RegionFactory::create (region, plist); + new_region = RegionFactory::create (region, plist, true, &thawlist); add_region_internal (new_region, position, thawlist); } @@ -717,7 +717,7 @@ Playlist::add_region (boost::shared_ptr region, samplepos_t position, fl */ for (int i = 0; i < itimes; ++i) { - boost::shared_ptr copy = RegionFactory::create (region, true); + boost::shared_ptr copy = RegionFactory::create (region, true, false, &rlock.thawlist); add_region_internal (copy, pos, rlock.thawlist, sub_num); set_layer (copy, DBL_MAX); pos += region->length (); @@ -738,7 +738,7 @@ Playlist::add_region (boost::shared_ptr region, samplepos_t position, fl plist.add (Properties::name, name); plist.add (Properties::layer, region->layer ()); - boost::shared_ptr sub = RegionFactory::create (region, plist); + boost::shared_ptr sub = RegionFactory::create (region, plist, true, &rlock.thawlist); add_region_internal (sub, pos, rlock.thawlist, sub_num); set_layer (sub, DBL_MAX); } @@ -1035,7 +1035,7 @@ Playlist::partition_internal (samplepos_t start, samplepos_t end, bool cutting, /* see note in :_split_region() * for MusicSample is needed to offset region-gain */ - region = RegionFactory::create (current, MusicSample (pos2 - pos1, 0), plist); + region = RegionFactory::create (current, MusicSample (pos2 - pos1, 0), plist, true, &thawlist); add_region_internal (region, start, thawlist); new_regions.push_back (region); } @@ -1055,7 +1055,7 @@ Playlist::partition_internal (samplepos_t start, samplepos_t end, bool cutting, plist.add (Properties::right_of_split, true); maybe_add_start_beats (_session.tempo_map (), plist, current, current->start (), current->start () + (pos3 - pos1)); - region = RegionFactory::create (current, MusicSample (pos3 - pos1, 0), plist); + region = RegionFactory::create (current, MusicSample (pos3 - pos1, 0), plist, true, &thawlist); add_region_internal (region, end, thawlist); new_regions.push_back (region); @@ -1093,7 +1093,7 @@ Playlist::partition_internal (samplepos_t start, samplepos_t end, bool cutting, plist.add (Properties::left_of_split, true); maybe_add_start_beats (_session.tempo_map (), plist, current, current->start (), current->start () + (pos2 - pos1)); - region = RegionFactory::create (current, MusicSample (pos2 - pos1, 0), plist); + region = RegionFactory::create (current, MusicSample (pos2 - pos1, 0), plist, true, &thawlist); add_region_internal (region, start, thawlist); new_regions.push_back (region); @@ -1135,7 +1135,7 @@ Playlist::partition_internal (samplepos_t start, samplepos_t end, bool cutting, plist.add (Properties::right_of_split, true); maybe_add_start_beats (_session.tempo_map (), plist, current, current->start (), current->start ()); - region = RegionFactory::create (current, plist); + region = RegionFactory::create (current, plist, true, &thawlist); add_region_internal (region, pos1, thawlist); new_regions.push_back (region); @@ -1279,7 +1279,7 @@ Playlist::paste (boost::shared_ptr other, samplepos_t position, float RegionWriteLock rl1 (this); while (itimes--) { for (RegionList::iterator i = other->regions.begin (); i != other->regions.end (); ++i) { - boost::shared_ptr copy_of_region = RegionFactory::create (*i, true); + boost::shared_ptr copy_of_region = RegionFactory::create (*i, true, false, &rl1.thawlist); /* put these new regions on top of all existing ones, but preserve the ordering they had in the original playlist. @@ -1312,7 +1312,7 @@ Playlist::duplicate (boost::shared_ptr region, samplepos_t position, sam int itimes = (int)floor (times); while (itimes--) { - boost::shared_ptr copy = RegionFactory::create (region, true); + boost::shared_ptr copy = RegionFactory::create (region, true, false, &rl.thawlist); add_region_internal (copy, position, rl.thawlist); set_layer (copy, DBL_MAX); position += gap; @@ -1330,7 +1330,7 @@ Playlist::duplicate (boost::shared_ptr region, samplepos_t position, sam plist.add (Properties::length, length); plist.add (Properties::name, name); - boost::shared_ptr sub = RegionFactory::create (region, plist); + boost::shared_ptr sub = RegionFactory::create (region, plist, true, &rl.thawlist); add_region_internal (sub, position, rl.thawlist); set_layer (sub, DBL_MAX); } @@ -1345,7 +1345,7 @@ Playlist::duplicate_until (boost::shared_ptr region, samplepos_t positio RegionWriteLock rl (this); while (position + region->length () - 1 < end) { - boost::shared_ptr copy = RegionFactory::create (region, true); + boost::shared_ptr copy = RegionFactory::create (region, true, false, &rl.thawlist); add_region_internal (copy, position, rl.thawlist); set_layer (copy, DBL_MAX); position += gap; @@ -1363,7 +1363,7 @@ Playlist::duplicate_until (boost::shared_ptr region, samplepos_t positio plist.add (Properties::length, length); plist.add (Properties::name, name); - boost::shared_ptr sub = RegionFactory::create (region, plist); + boost::shared_ptr sub = RegionFactory::create (region, plist, false, &rl.thawlist); add_region_internal (sub, position, rl.thawlist); set_layer (sub, DBL_MAX); } @@ -1508,7 +1508,7 @@ Playlist::_split_region (boost::shared_ptr region, const MusicSample& pl * since it supplies that offset to the Region constructor, which * is necessary to get audio region gain envelopes right. */ - left = RegionFactory::create (region, MusicSample (0, 0), plist, true); + left = RegionFactory::create (region, MusicSample (0, 0), plist, true, &thawlist); } RegionFactory::region_name (after_name, region->name (), false); @@ -1523,7 +1523,7 @@ Playlist::_split_region (boost::shared_ptr region, const MusicSample& pl plist.add (Properties::layer, region->layer ()); /* same note as above */ - right = RegionFactory::create (region, before, plist, true); + right = RegionFactory::create (region, before, plist, true, &thawlist); } add_region_internal (left, region->position (), thawlist, 0); diff --git a/libs/ardour/region_factory.cc b/libs/ardour/region_factory.cc index 9709467281..371b1d8266 100644 --- a/libs/ardour/region_factory.cc +++ b/libs/ardour/region_factory.cc @@ -51,7 +51,7 @@ std::map RegionFactory::region_name_map; RegionFactory::CompoundAssociations RegionFactory::_compound_associations; boost::shared_ptr -RegionFactory::create (boost::shared_ptr region, bool announce, bool fork) +RegionFactory::create (boost::shared_ptr region, bool announce, bool fork, ThawList* tl) { boost::shared_ptr ret; boost::shared_ptr ar; @@ -86,6 +86,11 @@ RegionFactory::create (boost::shared_ptr region, bool announce, bo } if (ret) { + if (tl) { + ret->suspend_property_changes (); + tl->add (ret); + } + ret->set_name (new_region_name(ret->name())); if (ret->session().config.get_glue_new_regions_to_bars_and_beats() && ret->position_lock_style() != MusicTime) { @@ -104,7 +109,7 @@ RegionFactory::create (boost::shared_ptr region, bool announce, bo } boost::shared_ptr -RegionFactory::create (boost::shared_ptr region, const PropertyList& plist, bool announce) +RegionFactory::create (boost::shared_ptr region, const PropertyList& plist, bool announce, ThawList* tl) { boost::shared_ptr ret; boost::shared_ptr other_a; @@ -126,6 +131,11 @@ RegionFactory::create (boost::shared_ptr region, const PropertyList& pli } if (ret) { + if (tl) { + ret->suspend_property_changes (); + tl->add (ret); + } + ret->apply_changes (plist); if (ret->session().config.get_glue_new_regions_to_bars_and_beats() && ret->position_lock_style() != MusicTime) { @@ -143,7 +153,7 @@ RegionFactory::create (boost::shared_ptr region, const PropertyList& pli } boost::shared_ptr -RegionFactory::create (boost::shared_ptr region, MusicSample offset, const PropertyList& plist, bool announce) +RegionFactory::create (boost::shared_ptr region, MusicSample offset, const PropertyList& plist, bool announce, ThawList* tl) { boost::shared_ptr ret; boost::shared_ptr other_a; @@ -165,6 +175,10 @@ RegionFactory::create (boost::shared_ptr region, MusicSample offset, con } if (ret) { + if (tl) { + ret->suspend_property_changes (); + tl->add (ret); + } ret->apply_changes (plist); if (ret->session().config.get_glue_new_regions_to_bars_and_beats() && ret->position_lock_style() != MusicTime) { diff --git a/libs/ardour/thawlist.cc b/libs/ardour/thawlist.cc new file mode 100644 index 0000000000..5e7c94c4c2 --- /dev/null +++ b/libs/ardour/thawlist.cc @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2021 Robin Gareus + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include + +#include "ardour/region.h" +#include "ardour/thawlist.h" + +using namespace ARDOUR; + +void +ThawList::add (boost::shared_ptr r) +{ + if (std::find (begin (), end (), r) != end ()) { + return; + } + r->suspend_property_changes (); + push_back (r); +} + +void +ThawList::release () +{ + for (RegionList::iterator i = begin (); i != end (); ++i) { + (*i)->resume_property_changes (); + } + clear (); +} diff --git a/libs/ardour/wscript b/libs/ardour/wscript index 03f21b7881..7dffae14dc 100644 --- a/libs/ardour/wscript +++ b/libs/ardour/wscript @@ -252,6 +252,7 @@ libardour_sources = [ 'template_utils.cc', 'tempo.cc', 'tempo_map_importer.cc', + 'thawlist.cc', 'thread_buffers.cc', 'ticker.cc', 'track.cc',