fix logic for removing an AutomationControl from a ControlGroup

diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h
index d84f0a091fc8573af4922fc99c06faee074b5290..a2eff17d9aea5aaa8dc048b851c917dbd71589d3 100644 (file)
--- a/libs/ardour/ardour/automation_control.h
+++ b/libs/ardour/ardour/automation_control.h
@@ -33,10 +33,12 @@
 #include "evoral/types.hpp"
 #include "evoral/Control.hpp"
 
-#include "ardour/libardour_visibility.h"
 #include "ardour/automation_list.h"
+#include "ardour/control_group_member.h"
 #include "ardour/parameter_descriptor.h"
 
+#include "ardour/libardour_visibility.h"
+
 namespace ARDOUR {
 
 class Session;
@@ -49,6 +51,7 @@ class LIBARDOUR_API AutomationControl
        : public PBD::Controllable
        , public Evoral::Control
        , public boost::enable_shared_from_this<AutomationControl>
+       , public ControlGroupMember
 {
     public:
        AutomationControl(ARDOUR::Session&,
@@ -116,8 +119,6 @@ class LIBARDOUR_API AutomationControl
        const ARDOUR::Session& session() const { return _session; }
        void commit_transaction (bool did_write);
 
-       void set_group (boost::shared_ptr<ControlGroup>);
-
   protected:
        ARDOUR::Session& _session;
        boost::shared_ptr<ControlGroup> _group;
@@ -132,6 +133,14 @@ class LIBARDOUR_API AutomationControl
        */
 
        virtual void actually_set_value (double value, PBD::Controllable::GroupControlDisposition);
+
+  private:
+       /* I am unclear on why we have to make ControlGroup a friend in order
+          to get access to the ::set_group() method when it is already
+          declared to be a friend in ControlGroupMember. Oh well.
+       */
+       friend class ControlGroup;
+       void set_group (boost::shared_ptr<ControlGroup>);
 };
 
 class SlavableAutomationControl : public AutomationControl

diff --git a/libs/ardour/ardour/control_group_member.h b/libs/ardour/ardour/control_group_member.h
new file mode 100644 (file)
index 0000000..385de9c
--- /dev/null
+++ b/libs/ardour/ardour/control_group_member.h
@@ -0,0 +1,40 @@
+/*
+  Copyright (C) 2016 Paul Davis
+
+  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.,
+  675 Mass Ave, Cambridge, MA 02139, USA.
+*/
+
+#ifndef __libardour_control_group_member_h__
+#define __libardour_control_group_member_h__
+
+namespace ARDOUR {
+
+class ControlGroup;
+
+class LIBARDOUR_API ControlGroupMember
+{
+  public:
+       virtual ~ControlGroupMember () {};
+  private:
+       friend class ControlGroup;
+       /* Only a ControlGroup can call this; all membership changes must be
+          mediated by the ControlGroup, not by operating on the member.
+       */
+       virtual void set_group (boost::shared_ptr<ControlGroup>) = 0;
+};
+
+} /* namespace */
+
+#endif /* __libardour_control_group_member_h__ */

diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc
index 7efaa07f233158c345af4bdd718abb9dd86315a8..99e2f5416541eba6620c92f903c2df2c88eaf3be 100644 (file)
--- a/libs/ardour/automation_control.cc
+++ b/libs/ardour/automation_control.cc
@@ -268,9 +268,11 @@ AutomationControl::interface_to_internal (double val) const
 void
 AutomationControl::set_group (boost::shared_ptr<ControlGroup> cg)
 {
-       if (_group) {
-               _group->remove_control (shared_from_this());
-       }
+       /* this method can only be called by a ControlGroup. We do not need
+          to ensure consistency by calling ControlGroup::remove_control(),
+          since we are guaranteed that the ControlGroup will take care of that
+          for us.
+       */
 
        _group = cg;
 }

diff --git a/libs/ardour/control_group.cc b/libs/ardour/control_group.cc
index 6752940d27c421b502f3c155fe4cc56866ebc0fe..2fe6af26ef4133d81bbd56cb70fb734560dff772 100644 (file)
--- a/libs/ardour/control_group.cc
+++ b/libs/ardour/control_group.cc
@@ -104,9 +104,19 @@ ControlGroup::control_going_away (boost::weak_ptr<AutomationControl> wac)
 int
 ControlGroup::remove_control (boost::shared_ptr<AutomationControl> ac)
 {
-       Glib::Threads::RWLock::WriterLock lm (controls_lock);
+       int erased;
+
+       {
+               Glib::Threads::RWLock::WriterLock lm (controls_lock);
+               erased = _controls.erase (ac->id());
+       }
+
+       if (erased) {
+               ac->set_group (boost::shared_ptr<ControlGroup>());
+       }
+
        /* return zero if erased, non-zero otherwise */
-       return !(_controls.erase (ac->id()) > 0);
+       return !erased;
 }
 
 int