From: Robin Gareus Date: Sun, 23 Jul 2017 21:52:20 +0000 (+0200) Subject: Fix ctrl-list guard-points and concurrency issues X-Git-Tag: 5.11~189 X-Git-Url: https://main.carlh.net/gitweb/?a=commitdiff_plain;h=2006701f7337a7e5a2eeac17a7374d59d1f98579;p=ardour.git Fix ctrl-list guard-points and concurrency issues * lock list when editing (prevent concurrent modification of insert iterator * don't add a guard-point if an event is already present between the target and guard-point-position * remove existing automation-events (old guard points) when touching automation w/o change * don't unset "new write pass" when not rolling (fixes issues when not rolling but locating with write-enabled) --- diff --git a/libs/evoral/evoral/ControlList.hpp b/libs/evoral/evoral/ControlList.hpp index 8df5004b39..4aececf40d 100644 --- a/libs/evoral/evoral/ControlList.hpp +++ b/libs/evoral/evoral/ControlList.hpp @@ -358,7 +358,7 @@ private: void unlocked_remove_duplicates (); void unlocked_invalidate_insert_iterator (); - void add_guard_point (double when); + void add_guard_point (double when, double offset); }; } // namespace Evoral diff --git a/libs/evoral/src/ControlList.cpp b/libs/evoral/src/ControlList.cpp index a0660ee000..0bba1dba11 100644 --- a/libs/evoral/src/ControlList.cpp +++ b/libs/evoral/src/ControlList.cpp @@ -27,6 +27,8 @@ #define isnan_local std::isnan #endif +#define GUARD_POINT_DELTA 64 + #include #include #include @@ -488,13 +490,37 @@ ControlList::set_in_write_pass (bool yn, bool add_point, double when) _in_write_pass = yn; if (yn && add_point) { - add_guard_point (when); + Glib::Threads::RWLock::WriterLock lm (_lock); + add_guard_point (when, 0); } } void -ControlList::add_guard_point (double when) +ControlList::add_guard_point (double when, double offset) { + // caller needs to hold writer-lock + if (offset < 0 && when < offset) { + return; + } + assert (offset <= 0); + + if (offset != 0) { + /* check if there are points between when + offset .. when */ + ControlEvent cp (when + offset, 0.0); + iterator s; + iterator e; + if ((s = lower_bound (_events.begin(), _events.end(), &cp, time_comparator)) != _events.end()) { + cp.when = when; + e = lower_bound (_events.begin(), _events.end(), &cp, time_comparator); + if (s != e) { + DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 add_guard_point, none added, found event between %2 and %3\n", this, when - offset, when)); + return; + } + } + } + + when += offset; + ControlEvent cp (when, 0.0); most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator); @@ -524,8 +550,7 @@ ControlList::add_guard_point (double when) ++most_recent_insert_iterator; } else { - /* insert a new control event at the right spot - */ + /* insert a new control event at the right spot */ DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert eval-value %2 just before iterator @ %3\n", this, eval_value, (*most_recent_insert_iterator)->when)); @@ -540,9 +565,12 @@ ControlList::add_guard_point (double when) ++most_recent_insert_iterator; } - /* don't do this again till the next write pass */ - - new_write_pass = false; + /* don't do this again till the next write pass, + * unless we're not in a write-pass (transport stopped) + */ + if (_in_write_pass) { + new_write_pass = false; + } } bool @@ -554,48 +582,49 @@ ControlList::in_write_pass () const bool ControlList::editor_add (double when, double value, bool with_guard) { - /* this is for making changes from a graphical line editor - */ + /* this is for making changes from a graphical line editor */ + { + Glib::Threads::RWLock::WriterLock lm (_lock); - ControlEvent cp (when, 0.0f); - iterator i = lower_bound (_events.begin(), _events.end(), &cp, time_comparator); + ControlEvent cp (when, 0.0f); + iterator i = lower_bound (_events.begin(), _events.end(), &cp, time_comparator); - if (i != _events.end () && (*i)->when == when) { - return false; - } + if (i != _events.end () && (*i)->when == when) { + return false; + } - /* clamp new value to allowed range */ - value = std::min ((double)_desc.upper, std::max ((double)_desc.lower, value)); + /* clamp new value to allowed range */ + value = std::min ((double)_desc.upper, std::max ((double)_desc.lower, value)); - if (_events.empty()) { + if (_events.empty()) { - /* as long as the point we're adding is not at zero, - * add an "anchor" point there. - */ + /* as long as the point we're adding is not at zero, + * add an "anchor" point there. + */ - if (when >= 1) { - _events.insert (_events.end(), new ControlEvent (0, value)); - DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 added value %2 at zero\n", this, value)); + if (when >= 1) { + _events.insert (_events.end(), new ControlEvent (0, value)); + DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 added value %2 at zero\n", this, value)); + } } - } - insert_position = when; - if (with_guard) { - if (when > 64) { - add_guard_point (when - 64); + insert_position = when; + if (with_guard) { + add_guard_point (when, -GUARD_POINT_DELTA); + maybe_add_insert_guard (when); + i = lower_bound (_events.begin(), _events.end(), &cp, time_comparator); } - maybe_add_insert_guard (when); - } - iterator result; - DEBUG_TRACE (DEBUG::ControlList, string_compose ("editor_add: actually add when= %1 value= %2\n", when, value)); - result = _events.insert (i, new ControlEvent (when, value)); + iterator result; + DEBUG_TRACE (DEBUG::ControlList, string_compose ("editor_add: actually add when= %1 value= %2\n", when, value)); + result = _events.insert (i, new ControlEvent (when, value)); - if (i == result) { - return false; - } + if (i == result) { + return false; + } - mark_dirty (); + mark_dirty (); + } maybe_signal_changed (); return true; @@ -604,17 +633,18 @@ ControlList::editor_add (double when, double value, bool with_guard) void ControlList::maybe_add_insert_guard (double when) { + // caller needs to hold writer-lock if (most_recent_insert_iterator != _events.end()) { - if ((*most_recent_insert_iterator)->when - when > 64) { + if ((*most_recent_insert_iterator)->when - when > GUARD_POINT_DELTA) { /* Next control point is some distance from where our new point is going to go, so add a new point to avoid changing the shape of the line too much. The insert iterator needs to point to the new control point so that our insert will happen correctly. */ - most_recent_insert_iterator = _events.insert ( - most_recent_insert_iterator, - new ControlEvent (when + 64, (*most_recent_insert_iterator)->value)); + most_recent_insert_iterator = _events.insert ( most_recent_insert_iterator, + new ControlEvent (when + GUARD_POINT_DELTA, (*most_recent_insert_iterator)->value)); + DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 added insert guard point @ %2 = %3\n", - this, when + 64, + this, when + GUARD_POINT_DELTA, (*most_recent_insert_iterator)->value)); } } @@ -624,6 +654,7 @@ ControlList::maybe_add_insert_guard (double when) bool ControlList::maybe_insert_straight_line (double when, double value) { + // caller needs to hold writer-lock if (_events.empty()) { return false; } @@ -652,6 +683,7 @@ ControlList::maybe_insert_straight_line (double when, double value) ControlList::iterator ControlList::erase_from_iterator_to (iterator iter, double when) { + // caller needs to hold writer-lock while (iter != _events.end()) { if ((*iter)->when < when) { DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase existing @ %2\n", this, (*iter)->when)); @@ -706,7 +738,7 @@ ControlList::add (double when, double value, bool with_guards, bool with_initial /* first write in a write pass: add guard point if requested */ if (with_guards) { - add_guard_point (insert_position); + add_guard_point (insert_position, 0); did_write_during_pass = true; } else { /* not adding a guard, but we need to set iterator appropriately */ @@ -725,9 +757,11 @@ ControlList::add (double when, double value, bool with_guards, bool with_initial ++most_recent_insert_iterator; } - most_recent_insert_iterator = erase_from_iterator_to(most_recent_insert_iterator, when); if (with_guards) { + most_recent_insert_iterator = erase_from_iterator_to (most_recent_insert_iterator, when + GUARD_POINT_DELTA); maybe_add_insert_guard (when); + } else { + most_recent_insert_iterator = erase_from_iterator_to(most_recent_insert_iterator, when); } } else if (!_in_write_pass) { @@ -793,20 +827,19 @@ ControlList::add (double when, double value, bool with_guards, bool with_initial } if (have_point1 && have_point2) { + DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 no change: move existing at %3 to %2\n", this, when, (*most_recent_insert_iterator)->when)); (*most_recent_insert_iterator)->when = when; done = true; } else { ++most_recent_insert_iterator; } } - //done = maybe_insert_straight_line (when, value) || done; - /* if the transport is stopped, add guard points (?) */ - if (!done && !_in_write_pass && when > 64) { - add_guard_point (when - 64); - maybe_add_insert_guard (when); - } - if (with_guards) { + /* if the transport is stopped, add guard points */ + if (!done && !_in_write_pass) { + add_guard_point (when, -GUARD_POINT_DELTA); + maybe_add_insert_guard (when); + } else if (with_guards) { maybe_add_insert_guard (when); }