diff options
-rw-r--r-- | libs/ardour/ardour/midi_model.h | 4 | ||||
-rw-r--r-- | libs/ardour/ardour/midi_track.h | 11 | ||||
-rw-r--r-- | libs/ardour/midi_model.cc | 131 | ||||
-rw-r--r-- | libs/ardour/midi_track.cc | 2 | ||||
-rw-r--r-- | libs/evoral/evoral/ControlSet.hpp | 2 | ||||
-rw-r--r-- | libs/evoral/evoral/PatchChange.hpp | 10 | ||||
-rw-r--r-- | libs/evoral/evoral/Sequence.hpp | 14 | ||||
-rw-r--r-- | libs/evoral/evoral/types.hpp | 27 | ||||
-rw-r--r-- | libs/evoral/src/Sequence.cpp | 129 |
9 files changed, 254 insertions, 76 deletions
diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h index 0d11f940b9..3ecfca7d1c 100644 --- a/libs/ardour/ardour/midi_model.h +++ b/libs/ardour/ardour/midi_model.h @@ -114,6 +114,8 @@ public: struct NoteChange { NoteDiffCommand::Property property; NotePtr note; + uint32_t note_id; + union { uint8_t old_value; TimeType old_time; @@ -161,6 +163,7 @@ public: private: struct Change { boost::shared_ptr<Evoral::Event<TimeType> > sysex; + gint sysex_id; SysExDiffCommand::Property property; TimeType old_time; TimeType new_time; @@ -204,6 +207,7 @@ public: struct Change { PatchChangePtr patch; Property property; + gint patch_id; union { TimeType old_time; uint8_t old_channel; diff --git a/libs/ardour/ardour/midi_track.h b/libs/ardour/ardour/midi_track.h index be209bc0f6..3b75c0a51b 100644 --- a/libs/ardour/ardour/midi_track.h +++ b/libs/ardour/ardour/midi_track.h @@ -180,18 +180,29 @@ private: void filter_channels (BufferSet& bufs, ChannelMode mode, uint32_t mask); +/* if mode is ForceChannel, force mask to the lowest set channel or 1 if no + * channels are set. + */ +#define force_mask(mode,mask) (((mode) == ForceChannel) ? (((mask) ? (1<<(ffs((mask))-1)) : 1)) : mask) + void _set_playback_channel_mode(ChannelMode mode, uint16_t mask) { + mask = force_mask (mode, mask); g_atomic_int_set(&_playback_channel_mask, (uint32_t(mode) << 16) | uint32_t(mask)); } void _set_playback_channel_mask (uint16_t mask) { + mask = force_mask (get_playback_channel_mode(), mask); g_atomic_int_set(&_playback_channel_mask, (uint32_t(get_playback_channel_mode()) << 16) | uint32_t(mask)); } void _set_capture_channel_mode(ChannelMode mode, uint16_t mask) { + mask = force_mask (mode, mask); g_atomic_int_set(&_capture_channel_mask, (uint32_t(mode) << 16) | uint32_t(mask)); } void _set_capture_channel_mask (uint16_t mask) { + mask = force_mask (get_capture_channel_mode(), mask); g_atomic_int_set(&_capture_channel_mask, (uint32_t(get_capture_channel_mode()) << 16) | uint32_t(mask)); } + +#undef force_mask }; } /* namespace ARDOUR*/ diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc index 4c6f6633d5..5c1f65d96b 100644 --- a/libs/ardour/midi_model.cc +++ b/libs/ardour/midi_model.cc @@ -289,6 +289,15 @@ MidiModel::NoteDiffCommand::operator() () for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { Property prop = i->property; + + if (!i->note) { + /* note found during deserialization, so try + again now that the model state is different. + */ + i->note = _model->find_note (i->note_id); + assert (i->note); + } + switch (prop) { case NoteNumber: if (temporary_removals.find (i->note) == temporary_removals.end()) { @@ -341,7 +350,7 @@ MidiModel::NoteDiffCommand::operator() () _removed_notes.push_back (*i); } } - + if (!side_effect_removals.empty()) { cerr << "SER: \n"; for (set<NotePtr>::iterator i = side_effect_removals.begin(); i != side_effect_removals.end(); ++i) { @@ -373,15 +382,25 @@ MidiModel::NoteDiffCommand::undo () /* notes we modify in a way that requires remove-then-add to maintain ordering */ set<NotePtr> temporary_removals; + + /* lazily discover any affected notes that were not discovered when + * loading the history because of deletions, etc. + */ + + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { + if (!i->note) { + i->note = _model->find_note (i->note_id); + assert (i->note); + } + } + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { Property prop = i->property; - switch (prop) { + switch (prop) { case NoteNumber: - if ( - temporary_removals.find (i->note) == temporary_removals.end() && - find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end() - ) { + if (temporary_removals.find (i->note) == temporary_removals.end() && + find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end()) { /* We only need to mark this note for re-add if (a) we haven't already marked it and (b) it isn't on the _removed_notes @@ -396,10 +415,8 @@ MidiModel::NoteDiffCommand::undo () break; case StartTime: - if ( - temporary_removals.find (i->note) == temporary_removals.end() && - find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end() - ) { + if (temporary_removals.find (i->note) == temporary_removals.end() && + find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end()) { /* See above ... */ @@ -410,10 +427,8 @@ MidiModel::NoteDiffCommand::undo () break; case Channel: - if ( - temporary_removals.find (i->note) == temporary_removals.end() && - find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end() - ) { + if (temporary_removals.find (i->note) == temporary_removals.end() && + find (_removed_notes.begin(), _removed_notes.end(), i->note) == _removed_notes.end()) { /* See above ... */ @@ -453,7 +468,7 @@ MidiModel::NoteDiffCommand::undo () _model->add_note_unlocked (*i); } } - + _model->ContentsChanged(); /* EMIT SIGNAL */ } @@ -651,15 +666,13 @@ MidiModel::NoteDiffCommand::unmarshal_change (XMLNode *xml_change) } /* we must point at the instance of the note that is actually in the model. - so go look for it ... + so go look for it ... it may not be there (it could have been + deleted in a later operation, so store the note id so that we can + look it up again later). */ change.note = _model->find_note (note_id); - - if (!change.note) { - warning << "MIDI note #" << note_id << " not found in model - programmers should investigate this" << endmsg; - return change; - } + change.note_id = note_id; return change; } @@ -790,6 +803,15 @@ MidiModel::SysExDiffCommand::operator() () _model->remove_sysex_unlocked (*i); } + /* find any sysex events that were missing when unmarshalling */ + + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { + if (!i->sysex) { + i->sysex = _model->find_sysex (i->sysex_id); + assert (i->sysex); + } + } + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { switch (i->property) { case Time: @@ -811,6 +833,15 @@ MidiModel::SysExDiffCommand::undo () _model->add_sysex_unlocked (*i); } + /* find any sysex events that were missing when unmarshalling */ + + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { + if (!i->sysex) { + i->sysex = _model->find_sysex (i->sysex_id); + assert (i->sysex); + } + } + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { switch (i->property) { case Time: @@ -899,11 +930,7 @@ MidiModel::SysExDiffCommand::unmarshal_change (XMLNode *xml_change) */ change.sysex = _model->find_sysex (sysex_id); - - if (!change.sysex) { - warning << "Sys-ex #" << sysex_id << " not found in model - programmers should investigate this" << endmsg; - return change; - } + change.sysex_id = sysex_id; return change; } @@ -1033,6 +1060,15 @@ MidiModel::PatchChangeDiffCommand::operator() () _model->remove_patch_change_unlocked (*i); } + /* find any patch change events that were missing when unmarshalling */ + + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { + if (!i->patch) { + i->patch = _model->find_patch_change (i->patch_id); + assert (i->patch); + } + } + set<PatchChangePtr> temporary_removals; for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { @@ -1081,6 +1117,15 @@ MidiModel::PatchChangeDiffCommand::undo () _model->add_patch_change_unlocked (*i); } + /* find any patch change events that were missing when unmarshalling */ + + for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { + if (!i->patch) { + i->patch = _model->find_patch_change (i->patch_id); + assert (i->patch); + } + } + set<PatchChangePtr> temporary_removals; for (ChangeList::iterator i = _changes.begin(); i != _changes.end(); ++i) { @@ -1207,10 +1252,10 @@ MidiModel::PatchChangeDiffCommand::unmarshal_patch_change (XMLNode* n) XMLProperty* prop; Evoral::event_id_t id; Evoral::MusicalTime time = 0; - uint8_t channel = 0; - uint8_t program = 0; + int channel = 0; + int program = 0; int bank = 0; - + if ((prop = n->property ("id")) != 0) { istringstream s (prop->value()); s >> id; @@ -1246,6 +1291,7 @@ MidiModel::PatchChangeDiffCommand::unmarshal_change (XMLNode* n) { XMLProperty* prop; Change c; + int an_int; prop = n->property ("property"); assert (prop); @@ -1255,6 +1301,10 @@ MidiModel::PatchChangeDiffCommand::unmarshal_change (XMLNode* n) assert (prop); Evoral::event_id_t const id = atoi (prop->value().c_str()); + /* we need to load via an int intermediate for all properties that are + actually uint8_t (char/byte). + */ + prop = n->property ("old"); assert (prop); { @@ -1262,11 +1312,14 @@ MidiModel::PatchChangeDiffCommand::unmarshal_change (XMLNode* n) if (c.property == Time) { s >> c.old_time; } else if (c.property == Channel) { - s >> c.old_channel; + s >> an_int; + c.old_channel = an_int; } else if (c.property == Program) { - s >> c.old_program; + s >> an_int; + c.old_program = an_int; } else if (c.property == Bank) { - s >> c.old_bank; + s >> an_int; + c.old_bank = an_int; } } @@ -1274,19 +1327,23 @@ MidiModel::PatchChangeDiffCommand::unmarshal_change (XMLNode* n) assert (prop); { istringstream s (prop->value ()); + if (c.property == Time) { s >> c.new_time; } else if (c.property == Channel) { - s >> c.new_channel; + s >> an_int; + c.new_channel = an_int; } else if (c.property == Program) { - s >> c.new_program; + s >> an_int; + c.new_program = an_int; } else if (c.property == Bank) { - s >> c.new_bank; + s >> an_int; + c.new_bank = an_int; } } c.patch = _model->find_patch_change (id); - assert (c.patch); + c.patch_id = id; return c; } @@ -1584,7 +1641,7 @@ MidiModel::write_lock() assert (ms); assert (!ms->mutex().trylock ()); - return WriteLock(new WriteLockImpl(NULL, _lock, _control_lock)); + return WriteLock(new WriteLockImpl(0, _lock, _control_lock)); } int diff --git a/libs/ardour/midi_track.cc b/libs/ardour/midi_track.cc index 0f1b2b52af..1a618a01dd 100644 --- a/libs/ardour/midi_track.cc +++ b/libs/ardour/midi_track.cc @@ -169,7 +169,7 @@ MidiTrack::set_state (const XMLNode& node, int version) playback_channel_mode = ChannelMode (string_2_enum(prop->value(), playback_channel_mode)); } if ((prop = node.property ("capture-channel-mode")) != 0) { - playback_channel_mode = ChannelMode (string_2_enum(prop->value(), capture_channel_mode)); + capture_channel_mode = ChannelMode (string_2_enum(prop->value(), capture_channel_mode)); } if ((prop = node.property ("channel-mode")) != 0) { /* 3.0 behaviour where capture and playback modes were not separated */ diff --git a/libs/evoral/evoral/ControlSet.hpp b/libs/evoral/evoral/ControlSet.hpp index 431885112b..716d199fec 100644 --- a/libs/evoral/evoral/ControlSet.hpp +++ b/libs/evoral/evoral/ControlSet.hpp @@ -65,7 +65,7 @@ public: void what_has_data(std::set<Parameter>&) const; - Glib::Threads::Mutex& control_lock() const { return _control_lock; } + Glib::Threads::Mutex& control_lock() const { return _control_lock; } protected: virtual void control_list_marked_dirty () {} diff --git a/libs/evoral/evoral/PatchChange.hpp b/libs/evoral/evoral/PatchChange.hpp index 82e2023941..b1a42c6f2e 100644 --- a/libs/evoral/evoral/PatchChange.hpp +++ b/libs/evoral/evoral/PatchChange.hpp @@ -119,7 +119,7 @@ public: uint8_t channel () const { return _program_change.buffer()[0] & 0xf; } inline bool operator< (const PatchChange<Time>& o) const { - if (time() != o.time()) { + if (!musical_time_equal (time(), o.time())) { return time() < o.time(); } @@ -131,7 +131,7 @@ public: } inline bool operator== (const PatchChange<Time>& o) const { - return (time() == o.time() && program() == o.program() && bank() == o.bank()); + return (musical_time_equal (time(), o.time()) && program() == o.program() && bank() == o.bank()); } /** The PatchChange is made up of messages() MIDI messages; this method returns them by index. @@ -165,4 +165,10 @@ private: } +template<typename Time> +std::ostream& operator<< (std::ostream& o, const Evoral::PatchChange<Time>& p) { + o << "Patch Change " << p.id() << " @ " << p.time() << " bank " << (int) p.bank() << " program " << (int) p.program(); + return o; +} + #endif diff --git a/libs/evoral/evoral/Sequence.hpp b/libs/evoral/evoral/Sequence.hpp index 815d02f980..26bef20232 100644 --- a/libs/evoral/evoral/Sequence.hpp +++ b/libs/evoral/evoral/Sequence.hpp @@ -69,8 +69,7 @@ protected: struct WriteLockImpl { WriteLockImpl(Glib::Threads::RWLock& s, Glib::Threads::Mutex& c) : sequence_lock(new Glib::Threads::RWLock::WriterLock(s)) - , control_lock(new Glib::Threads::Mutex::Lock(c)) - { } + , control_lock(new Glib::Threads::Mutex::Lock(c)) { } ~WriteLockImpl() { delete sequence_lock; delete control_lock; @@ -88,7 +87,7 @@ public: typedef boost::shared_ptr<WriteLockImpl> WriteLock; virtual ReadLock read_lock() const { return ReadLock(new Glib::Threads::RWLock::ReaderLock(_lock)); } - virtual WriteLock write_lock() { return WriteLock(new WriteLockImpl(_lock, _control_lock)); } + virtual WriteLock write_lock() { return WriteLock(new WriteLockImpl(_lock, _control_lock)); } void clear(); @@ -126,7 +125,7 @@ public: struct EarlierNoteComparator { inline bool operator()(const boost::shared_ptr< const Note<Time> > a, const boost::shared_ptr< const Note<Time> > b) const { - return a->time() < b->time(); + return musical_time_less_than (a->time(), b->time()); } }; @@ -134,6 +133,7 @@ public: typedef const Note<Time>* value_type; inline bool operator()(const boost::shared_ptr< const Note<Time> > a, const boost::shared_ptr< const Note<Time> > b) const { + return musical_time_greater_than (a->time(), b->time()); return a->time() > b->time(); } }; @@ -142,7 +142,7 @@ public: typedef const Note<Time>* value_type; inline bool operator()(const boost::shared_ptr< const Note<Time> > a, const boost::shared_ptr< const Note<Time> > b) const { - return a->end_time() > b->end_time(); + return musical_time_less_than (a->end_time(), b->end_time()); } }; @@ -186,7 +186,7 @@ public: struct EarlierSysExComparator { inline bool operator() (constSysExPtr a, constSysExPtr b) const { - return a->time() < b->time(); + return musical_time_less_than (a->time(), b->time()); } }; @@ -199,7 +199,7 @@ public: struct EarlierPatchChangeComparator { inline bool operator() (constPatchChangePtr a, constPatchChangePtr b) const { - return a->time() < b->time(); + return musical_time_less_than (a->time(), b->time()); } }; diff --git a/libs/evoral/evoral/types.hpp b/libs/evoral/evoral/types.hpp index 7bdbdc7a2e..000b79bb94 100644 --- a/libs/evoral/evoral/types.hpp +++ b/libs/evoral/evoral/types.hpp @@ -43,6 +43,33 @@ static inline bool musical_time_equal (MusicalTime a, MusicalTime b) { return fabs (a - b) <= (1.0/1920.0); } +static inline bool musical_time_less_than (MusicalTime a, MusicalTime b) { + /* acceptable tolerance is 1 tick. Nice if there was no magic number here */ + if (fabs (a - b) <= (1.0/1920.0)) { + return false; /* effectively identical */ + } else { + return a < b; + } +} + +static inline bool musical_time_greater_than (MusicalTime a, MusicalTime b) { + /* acceptable tolerance is 1 tick. Nice if there was no magic number here */ + if (fabs (a - b) <= (1.0/1920.0)) { + return false; /* effectively identical */ + } else { + return a > b; + } +} + +static inline bool musical_time_greater_or_equal_to (MusicalTime a, MusicalTime b) { + /* acceptable tolerance is 1 tick. Nice if there was no magic number here */ + if (fabs (a - b) <= (1.0/1920.0)) { + return true; /* effectively identical, note the "or_equal_to" */ + } else { + return a >= b; + } +} + /** Type of an event (opaque, mapped by application) */ typedef uint32_t EventType; diff --git a/libs/evoral/src/Sequence.cpp b/libs/evoral/src/Sequence.cpp index 71bdcb7c03..738d5a19c9 100644 --- a/libs/evoral/src/Sequence.cpp +++ b/libs/evoral/src/Sequence.cpp @@ -720,25 +720,27 @@ void Sequence<Time>::remove_note_unlocked(const constNotePtr note) { bool erased = false; + bool id_matched = false; - _edited = true; - - DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1 remove note %2 @ %3\n", this, (int)note->note(), note->time())); + DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1 remove note #%2 %3 @ %4\n", this, note->id(), (int)note->note(), note->time())); - typename Sequence<Time>::Notes::iterator i = note_lower_bound(note->time()); - while (i != _notes.end() && (*i)->time() == note->time()) { + /* first try searching for the note using the time index, which is + * faster since the container is "indexed" by time. (technically, this + * means that lower_bound() can do a binary search rather than linear) + * + * this may not work, for reasons explained below. + */ - typename Sequence<Time>::Notes::iterator tmp = i; - ++tmp; + typename Sequence<Time>::Notes::iterator i; + + for (i = note_lower_bound(note->time()); i != _notes.end() && musical_time_equal ((*i)->time(), note->time()); ++i) { if (*i == note) { - NotePtr n = *i; - - DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\terasing note %2 @ %3\n", this, (int)(*i)->note(), (*i)->time())); + DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\terasing note #%2 %3 @ %4\n", this, (*i)->id(), (int)(*i)->note(), (*i)->time())); _notes.erase (i); - if (n->note() == _lowest_note || n->note() == _highest_note) { + if (note->note() == _lowest_note || note->note() == _highest_note) { _lowest_note = 127; _highest_note = 0; @@ -752,30 +754,100 @@ Sequence<Time>::remove_note_unlocked(const constNotePtr note) } erased = true; + break; } + } - i = tmp; + DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\ttime-based lookup did not find note #%2 %3 @ %4\n", this, note->id(), (int)note->note(), note->time())); + + if (!erased) { + + /* if the note's time property was changed in tandem with some + * other property as the next operation after it was added to + * the sequence, then at the point where we call this to undo + * the add, the note we are targetting currently has a + * different time property than the one we we passed via + * the argument. + * + * in this scenario, we have no choice other than to linear + * search the list of notes and find the note by ID. + */ + + for (i = _notes.begin(); i != _notes.end(); ++i) { + + if ((*i)->id() == note->id()) { + + DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\tID-based pass, erasing note #%2 %3 @ %4\n", this, (*i)->id(), (int)(*i)->note(), (*i)->time())); + _notes.erase (i); + + if (note->note() == _lowest_note || note->note() == _highest_note) { + + _lowest_note = 127; + _highest_note = 0; + + for (typename Sequence<Time>::Notes::iterator ii = _notes.begin(); ii != _notes.end(); ++ii) { + if ((*ii)->note() < _lowest_note) + _lowest_note = (*ii)->note(); + if ((*ii)->note() > _highest_note) + _highest_note = (*ii)->note(); + } + } + + erased = true; + id_matched = true; + break; + } + } } + + if (erased) { - Pitches& p (pitches (note->channel())); + Pitches& p (pitches (note->channel())); + + typename Pitches::iterator j; - NotePtr search_note(new Note<Time>(0, 0, 0, note->note(), 0)); + /* if we had to ID-match above, we can't expect to find it in + * pitches via note comparison either. so do another linear + * search to locate it. otherwise, we can use the note index + * to potentially speed things up. + */ - typename Pitches::iterator j = p.lower_bound (search_note); - while (j != p.end() && (*j)->note() == note->note()) { - typename Pitches::iterator tmp = j; - ++tmp; + if (id_matched) { + + for (j = p.begin(); j != p.end(); ++j) { + if ((*j)->id() == note->id()) { + p.erase (j); + break; + } + } + + } else { - if (*j == note) { - DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\terasing pitch %2 @ %3\n", this, (int)(*j)->note(), (*j)->time())); - p.erase (j); + /* Now find the same note in the "pitches" list (which indexes + * notes by channel+time. We care only about its note number + * so the search_note has all other properties unset. + */ + + NotePtr search_note (new Note<Time>(0, 0, 0, note->note(), 0)); + + for (j = p.lower_bound (search_note); j != p.end() && (*j)->note() == note->note(); ++j) { + + if ((*j) == note) { + DEBUG_TRACE (DEBUG::Sequence, string_compose ("%1\terasing pitch %2 @ %3\n", this, (int)(*j)->note(), (*j)->time())); + p.erase (j); + break; + } + } } - j = tmp; - } + if (j == p.end()) { + warning << string_compose ("erased note %1 not found in pitches for channel %2", *note, (int) note->channel()) << endmsg; + } - if (!erased) { - cerr << "Unable to find note to erase matching " << *note.get() << endl; + _edited = true; + + } else { + cerr << "Unable to find note to erase matching " << *note.get() << endmsg; } } @@ -784,12 +856,13 @@ void Sequence<Time>::remove_patch_change_unlocked (const constPatchChangePtr p) { typename Sequence<Time>::PatchChanges::iterator i = patch_change_lower_bound (p->time ()); - while (i != _patch_changes.end() && (*i)->time() == p->time()) { + + while (i != _patch_changes.end() && (musical_time_equal ((*i)->time(), p->time()))) { typename Sequence<Time>::PatchChanges::iterator tmp = i; ++tmp; - if (*i == p) { + if (**i == *p) { _patch_changes.erase (i); } @@ -1148,7 +1221,7 @@ Sequence<Time>::patch_change_lower_bound (Time t) const { PatchChangePtr search (new PatchChange<Time> (t, 0, 0, 0)); typename Sequence<Time>::PatchChanges::const_iterator i = _patch_changes.lower_bound (search); - assert (i == _patch_changes.end() || (*i)->time() >= t); + assert (i == _patch_changes.end() || musical_time_greater_or_equal_to ((*i)->time(), t)); return i; } |