From 333e20a3a60cccbab57957b8266ea043bb7bfb8a Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Mon, 12 Jun 2017 02:25:20 +0200 Subject: VCA/SlavableAutomationCtrl re-design: * remember master-ctrl value on assignment & save with session * Control/AutomationCtrl only stores ctrl's own value (w/o master) * virtual AutomationControl::get_value () -> use SlavableAC method * MasterRecord uses weak-ptr (fixes recursive ~Controllable() deadlock) --- libs/ardour/ardour/automation_control.h | 7 +- libs/ardour/ardour/slavable_automation_control.h | 25 +++- libs/ardour/ardour/solo_control.h | 2 +- libs/ardour/ardour/solo_isolate_control.h | 2 +- libs/ardour/automation_control.cc | 7 + libs/ardour/slavable_automation_control.cc | 177 +++++++++++------------ libs/ardour/solo_control.cc | 4 +- libs/ardour/solo_isolate_control.cc | 2 +- 8 files changed, 120 insertions(+), 106 deletions(-) (limited to 'libs/ardour') diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h index a16929afa5..9879767807 100644 --- a/libs/ardour/ardour/automation_control.h +++ b/libs/ardour/ardour/automation_control.h @@ -91,9 +91,10 @@ public: void start_touch(double when); void stop_touch(bool mark, double when); - /* inherited from PBD::Controllable. - */ - double get_value () const; + /* inherited from PBD::Controllable. */ + virtual double get_value () const; + virtual double get_save_value () const; + /* inherited from PBD::Controllable. * Derived classes MUST call ::writable() to verify * that writing to the parameter is legal at that time. diff --git a/libs/ardour/ardour/slavable_automation_control.h b/libs/ardour/ardour/slavable_automation_control.h index ceafc79193..29ab2e1055 100644 --- a/libs/ardour/ardour/slavable_automation_control.h +++ b/libs/ardour/ardour/slavable_automation_control.h @@ -84,12 +84,21 @@ protected: class MasterRecord { public: - MasterRecord (boost::shared_ptr gc, double r) + MasterRecord (boost::weak_ptr gc, double vc, double vm) : _master (gc) , _yn (false) + , _val_ctrl (vc) + , _val_master (vm) {} - boost::shared_ptr master() const { return _master; } + boost::shared_ptr master() const { assert(_master.lock()); return _master.lock(); } + + double val_ctrl () const { return _val_ctrl; } + double val_master () const { return _val_master; } + + double master_ratio () const { return _val_master == 0 ? master()->get_value() : master()->get_value() / _val_master; } + + int set_state (XMLNode const&, int); /* for boolean/toggled controls, we store a boolean value to * indicate if this master returned true/false (1.0/0.0) from @@ -99,18 +108,22 @@ protected: bool yn() const { return _yn; } void set_yn (bool yn) { _yn = yn; } - PBD::ScopedConnection connection; + PBD::ScopedConnection changed_connection; + PBD::ScopedConnection dropped_connection; private: - boost::shared_ptr _master; + boost::weak_ptr _master; /* holds most recently seen master value for boolean/toggle controls */ bool _yn; + + /* values at time of assignment */ + double _val_ctrl; + double _val_master; }; mutable Glib::Threads::RWLock master_lock; typedef std::map Masters; Masters _masters; - std::map, PBD::ScopedConnection> masters_connections; void master_going_away (boost::weak_ptr); double get_value_locked() const; @@ -121,7 +134,7 @@ protected: virtual bool boolean_automation_run_locked (framepos_t start, pframes_t len); bool boolean_automation_run (framepos_t start, pframes_t len); - virtual void master_changed (bool from_self, GroupControlDisposition gcd, boost::shared_ptr); + virtual void master_changed (bool from_self, GroupControlDisposition gcd, boost::weak_ptr); virtual double get_masters_value_locked () const; virtual void pre_remove_master (boost::shared_ptr) {} virtual void post_add_master (boost::shared_ptr) {} diff --git a/libs/ardour/ardour/solo_control.h b/libs/ardour/ardour/solo_control.h index cc86983582..8026e5ea26 100644 --- a/libs/ardour/ardour/solo_control.h +++ b/libs/ardour/ardour/solo_control.h @@ -98,7 +98,7 @@ class LIBARDOUR_API SoloControl : public SlavableAutomationControl protected: void actually_set_value (double, PBD::Controllable::GroupControlDisposition group_override); - void master_changed (bool from_self, GroupControlDisposition, boost::shared_ptr m); + void master_changed (bool from_self, GroupControlDisposition, boost::weak_ptr m); void pre_remove_master (boost::shared_ptr); void post_add_master (boost::shared_ptr); diff --git a/libs/ardour/ardour/solo_isolate_control.h b/libs/ardour/ardour/solo_isolate_control.h index 1e49469067..190e0cdda3 100644 --- a/libs/ardour/ardour/solo_isolate_control.h +++ b/libs/ardour/ardour/solo_isolate_control.h @@ -73,7 +73,7 @@ class LIBARDOUR_API SoloIsolateControl : public SlavableAutomationControl XMLNode& get_state (); protected: - void master_changed (bool from_self, PBD::Controllable::GroupControlDisposition gcd, boost::shared_ptr); + void master_changed (bool from_self, PBD::Controllable::GroupControlDisposition gcd, boost::weak_ptr); void actually_set_value (double, PBD::Controllable::GroupControlDisposition group_override); private: diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc index 4e81de8aaf..055c000bc9 100644 --- a/libs/ardour/automation_control.cc +++ b/libs/ardour/automation_control.cc @@ -92,6 +92,13 @@ AutomationControl::get_value() const return Control::get_double (from_list, _session.transport_frame()); } +double +AutomationControl::get_save_value() const +{ + /* save user-value, not incl masters */ + return Control::get_double (); +} + void AutomationControl::pre_realtime_queue_stuff (double val, PBD::Controllable::GroupControlDisposition gcd) { diff --git a/libs/ardour/slavable_automation_control.cc b/libs/ardour/slavable_automation_control.cc index 5ab968f69c..adde7cdac3 100644 --- a/libs/ardour/slavable_automation_control.cc +++ b/libs/ardour/slavable_automation_control.cc @@ -57,7 +57,6 @@ SlavableAutomationControl::~SlavableAutomationControl () double SlavableAutomationControl::get_masters_value_locked () const { - if (_desc.toggled) { for (Masters::const_iterator mr = _masters.begin(); mr != _masters.end(); ++mr) { if (mr->second.master()->get_value()) { @@ -70,7 +69,7 @@ SlavableAutomationControl::get_masters_value_locked () const double v = 1.0; /* the masters function as a scaling factor */ for (Masters::const_iterator mr = _masters.begin(); mr != _masters.end(); ++mr) { - v *= mr->second.master()->get_value (); + v *= mr->second.master_ratio (); } return v; @@ -146,6 +145,7 @@ SlavableAutomationControl::masters_curve_multiply (framepos_t start, framepos_t = boost::dynamic_pointer_cast(mr->second.master()); assert (sc); rv |= sc->masters_curve_multiply (start, end, vec, veclen); + apply_gain_to_buffer (vec, veclen, mr->second.master_ratio ()); } return rv; } @@ -179,38 +179,19 @@ SlavableAutomationControl::add_master (boost::shared_ptr m, b std::pair res; { + const double master_value = m->get_value(); Glib::Threads::RWLock::WriterLock lm (master_lock); - pair newpair (m->id(), MasterRecord (m, 1.0)); + pair newpair (m->id(), MasterRecord (boost::weak_ptr (m), get_value_locked(), master_value)); res = _masters.insert (newpair); if (res.second) { - if (!loading) { - - if (!_desc.toggled) { - const double master_value = m->get_value(); - - if (master_value == 0.0) { - AutomationControl::set_double (0.0, Controllable::NoGroup); - } else { - /* scale control's own value by - amount that the master will - contribute. - */ - AutomationControl::set_double ((Control::get_double() / master_value), Controllable::NoGroup); - } - } - } - /* note that we bind @param m as a weak_ptr, thus avoiding holding a reference to the control in the binding itself. */ - assert (masters_connections.find (boost::weak_ptr(m)) == masters_connections.end()); - PBD::ScopedConnection con; - m->DropReferences.connect_same_thread (con, boost::bind (&SlavableAutomationControl::master_going_away, this, boost::weak_ptr(m))); - masters_connections[boost::weak_ptr(m)] = con; + m->DropReferences.connect_same_thread (res.first->second.dropped_connection, boost::bind (&SlavableAutomationControl::master_going_away, this, boost::weak_ptr(m))); /* Store the connection inside the MasterRecord, so that when we destroy it, the connection is destroyed @@ -228,7 +209,7 @@ SlavableAutomationControl::add_master (boost::shared_ptr m, b because the change came from the master. */ - m->Changed.connect_same_thread (res.first->second.connection, boost::bind (&SlavableAutomationControl::master_changed, this, _1, _2, m)); + m->Changed.connect_same_thread (res.first->second.changed_connection, boost::bind (&SlavableAutomationControl::master_changed, this, _1, _2, boost::weak_ptr(m))); } } @@ -295,14 +276,18 @@ SlavableAutomationControl::update_boolean_masters_records (boost::shared_ptr m) +SlavableAutomationControl::master_changed (bool /*from_self*/, GroupControlDisposition gcd, boost::weak_ptr wm) { + boost::shared_ptr m = wm.lock (); + assert (m); Glib::Threads::RWLock::ReaderLock lm (master_lock, Glib::Threads::TRY_LOCK); if (!lm.locked ()) { /* boolean_automation_run_locked () special case */ return; } bool send_signal = handle_master_change (m); + lm.release (); // update_boolean_masters_records() takes lock + update_boolean_masters_records (m); if (send_signal) { Changed (false, Controllable::NoGroup); /* EMIT SIGNAL */ @@ -321,38 +306,32 @@ SlavableAutomationControl::master_going_away (boost::weak_ptr void SlavableAutomationControl::remove_master (boost::shared_ptr m) { + if (_session.deletion_in_progress()) { + /* no reason to care about new values or sending signals */ + return; + } + pre_remove_master (m); + double new_val = AutomationControl::get_double(); + const double old_val = new_val; { Glib::Threads::RWLock::WriterLock lm (master_lock); - masters_connections.erase (boost::weak_ptr(m)); - if (!_masters.erase (m->id())) { - return; - } - - if (!_session.deletion_in_progress()) { + Masters::const_iterator mi = _masters.find (m->id ()); - const double master_value = m->get_value (); + /* when un-assigning we apply the master-value permanently */ + if (mi != _masters.end()) { + new_val *= mi->second.master_ratio (); + } - if (master_value == 0.0) { - /* slave would have been set to 0.0 as well, - so just leave it there, and the user can - bring it back up. this fits with the - "removing a VCA does not change the level" rule. - */ - } else { - /* bump up the control's own value by the level - of the master that is being removed. - */ - AutomationControl::set_double (AutomationControl::get_double() * master_value, Controllable::NoGroup); - } + if (!_masters.erase (m->id())) { + return; } } - if (_session.deletion_in_progress()) { - /* no reason to care about new values or sending signals */ - return; + if (old_val != new_val) { + AutomationControl::set_double (new_val, Controllable::NoGroup); } MasterStatusChange (); /* EMIT SIGNAL */ @@ -365,31 +344,32 @@ SlavableAutomationControl::remove_master (boost::shared_ptr m void SlavableAutomationControl::clear_masters () { - double current_value; - double new_value; - bool had_masters = false; + if (_session.deletion_in_progress()) { + /* no reason to care about new values or sending signals */ + return; + } + + double new_val = AutomationControl::get_double(); + const double old_val = new_val; /* null ptr means "all masters */ pre_remove_master (boost::shared_ptr()); { Glib::Threads::RWLock::WriterLock lm (master_lock); - current_value = get_value_locked (); - if (!_masters.empty()) { - had_masters = true; + if (_masters.empty()) { + return; } - _masters.clear (); - masters_connections.clear (); - new_value = get_value_locked (); - } + /* permanently apply masters value */ + new_val *= get_masters_value_locked (); - if (had_masters) { - MasterStatusChange (); /* EMIT SIGNAL */ + _masters.clear (); } - if (new_value != current_value) { - actually_set_value (current_value, Controllable::UseGroup); + if (old_val != new_val) { + AutomationControl::set_double (new_val, Controllable::NoGroup); } + MasterStatusChange (); /* EMIT SIGNAL */ /* no need to update boolean masters records, since all MRs will have * been removed already. @@ -446,6 +426,11 @@ SlavableAutomationControl::find_next_event_locked (double now, double end, Evora bool SlavableAutomationControl::handle_master_change (boost::shared_ptr) { + /* Derived classes can implement this for special cases (e.g. mute). + * This method is called with a ReaderLock (master_lock) held. + * + * return true if the changed master value resulted + * in a change of the control itself. */ return true; // emit Changed } @@ -517,6 +502,23 @@ SlavableAutomationControl::slaved () const return !_masters.empty(); } +int +SlavableAutomationControl::MasterRecord::set_state (XMLNode const& n, int) +{ + bool yn; + double v; + if (n.get_property (X_("yn"), yn)) { + _yn = yn; + } + if (n.get_property (X_("val-ctrl"), v)) { + _val_ctrl = v; + } + if (n.get_property (X_("val-master"), v)) { + _val_master = v; + } + return 0; +} + void SlavableAutomationControl::use_saved_master_ratios () { @@ -526,28 +528,19 @@ SlavableAutomationControl::use_saved_master_ratios () Glib::Threads::RWLock::ReaderLock lm (master_lock); - /* use stored state, do not recompute */ - - if (_desc.toggled) { - - XMLNodeList nlist = _masters_node->children(); - XMLNodeIterator niter; - - for (niter = nlist.begin(); niter != nlist.end(); ++niter) { - ID id_val; - bool yn; - if (!(*niter)->get_property (X_("id"), id_val) || !(*niter)->get_property (X_("yn"), yn)) { - continue; - } + XMLNodeList nlist = _masters_node->children(); + XMLNodeIterator niter; - Masters::iterator mi = _masters.find (id_val); - if (mi != _masters.end()) { - mi->second.set_yn (yn); - } + for (niter = nlist.begin(); niter != nlist.end(); ++niter) { + ID id_val; + if (!(*niter)->get_property (X_("id"), id_val)) { + continue; } - - } else { - + Masters::iterator mi = _masters.find (id_val); + if (mi == _masters.end()) { + continue; + } + mi->second.set_state (**niter, Stateful::loading_state_version); } delete _masters_node; @@ -566,23 +559,21 @@ SlavableAutomationControl::get_state () { Glib::Threads::RWLock::ReaderLock lm (master_lock); - if (!_masters.empty()) { - XMLNode* masters_node = new XMLNode (X_("masters")); + for (Masters::iterator mr = _masters.begin(); mr != _masters.end(); ++mr) { + XMLNode* mnode = new XMLNode (X_("master")); + mnode->set_property (X_("id"), mr->second.master()->id()); - if (_desc.toggled) { - for (Masters::iterator mr = _masters.begin(); mr != _masters.end(); ++mr) { - XMLNode* mnode = new XMLNode (X_("master")); - mnode->set_property (X_("id"), mr->second.master()->id()); + if (_desc.toggled) { mnode->set_property (X_("yn"), mr->second.yn()); - masters_node->add_child_nocopy (*mnode); + } else { + mnode->set_property (X_("val-ctrl"), mr->second.val_ctrl()); + mnode->set_property (X_("val-master"), mr->second.val_master()); } - } else { - + masters_node->add_child_nocopy (*mnode); + node.add_child_nocopy (*masters_node); } - - node.add_child_nocopy (*masters_node); } } diff --git a/libs/ardour/solo_control.cc b/libs/ardour/solo_control.cc index 0b07f88c6b..9d898493dc 100644 --- a/libs/ardour/solo_control.cc +++ b/libs/ardour/solo_control.cc @@ -259,8 +259,10 @@ SoloControl::get_state () } void -SoloControl::master_changed (bool /*from self*/, GroupControlDisposition, boost::shared_ptr m) +SoloControl::master_changed (bool /*from self*/, GroupControlDisposition, boost::weak_ptr wm) { + boost::shared_ptr m = wm.lock (); + assert (m); bool send_signal = false; _transition_into_solo = 0; diff --git a/libs/ardour/solo_isolate_control.cc b/libs/ardour/solo_isolate_control.cc index 90132a951c..9be6e1a7d5 100644 --- a/libs/ardour/solo_isolate_control.cc +++ b/libs/ardour/solo_isolate_control.cc @@ -42,7 +42,7 @@ SoloIsolateControl::SoloIsolateControl (Session& session, std::string const & na } void -SoloIsolateControl::master_changed (bool from_self, PBD::Controllable::GroupControlDisposition gcd, boost::shared_ptr) +SoloIsolateControl::master_changed (bool from_self, PBD::Controllable::GroupControlDisposition gcd, boost::weak_ptr) { if (!_soloable.can_solo()) { return; -- cgit v1.2.3