From d322b7494105a910b4b7de743ea2d84b3f5d3355 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Mon, 5 Dec 2016 23:29:52 +0000 Subject: [PATCH] use drobilla's suggestion to improve Event structure --- libs/ardour/midi_model.cc | 16 +- libs/evoral/evoral/Event.hpp | 584 +++++++++++++++-------------- libs/evoral/evoral/Note.hpp | 4 +- libs/evoral/evoral/PatchChange.hpp | 6 +- libs/evoral/evoral/Sequence.hpp | 4 - libs/evoral/src/Event.cpp | 13 + libs/evoral/src/Note.cpp | 23 +- libs/evoral/src/Sequence.cpp | 26 +- 8 files changed, 364 insertions(+), 312 deletions(-) diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc index 5096779566..c64a3af87d 100644 --- a/libs/ardour/midi_model.cc +++ b/libs/ardour/midi_model.cc @@ -148,6 +148,7 @@ void MidiModel::NoteDiffCommand::add (NotePtr const & note) { _removed_notes.remove(note); + /* copies NotePtr, puts it into an STL (non-invasive) list */ _added_notes.push_back(note); } @@ -155,6 +156,7 @@ void MidiModel::NoteDiffCommand::remove (NotePtr const & note) { _added_notes.remove(note); + /* copies NotePtr, puts it into an STL (non-invasive) list */ _removed_notes.push_back(note); } @@ -242,8 +244,15 @@ MidiModel::NoteDiffCommand::operator() () { MidiModel::WriteLock lock(_model->edit_lock()); - for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); ++i) { - if (!_model->add_note_unlocked(*i)) { + for (NoteList::iterator i = _added_notes.begin(); i != _added_notes.end(); *i) { + + /* locally scoped pointer to note */ + + NotePtr np (*i); + + cerr << "local NP linked? " << np.is_linked() << " iterator version " << (*i).is_linked() << endl; + + if (!_model->add_note_unlocked (np)) { /* failed to add it, so don't leave it in the removed list, to avoid apparent errors on undo. */ @@ -252,7 +261,8 @@ MidiModel::NoteDiffCommand::operator() () } for (NoteList::iterator i = _removed_notes.begin(); i != _removed_notes.end(); ++i) { - _model->remove_note_unlocked(*i); + NotePtr np (*i); + _model->remove_note_unlocked (np); } /* notes we modify in a way that requires remove-then-add to maintain ordering */ diff --git a/libs/evoral/evoral/Event.hpp b/libs/evoral/evoral/Event.hpp index 5f034972a7..ddc4969db7 100644 --- a/libs/evoral/evoral/Event.hpp +++ b/libs/evoral/evoral/Event.hpp @@ -48,227 +48,6 @@ LIBEVORAL_API void init_event_id_counter(event_id_t n); template class Sequence; -#define COMMON_EVENT_DATA \ - EventType _type; /*< Type of event (application relative, NOT MIDI 'type') */ \ - Time _time; /*< Time stamp of event */ \ - uint32_t _size; /*< Size of buffer in bytes */ \ - event_id_t _id; /*< Unique event ID */ \ - uint8_t _buf[0]; /*< Event data. Must be at end, to use C-style variable-sized structure hack */ - -#define COMMON_EVENT_METHODS \ - inline EventType event_type() const { return _type; } \ - inline Time time() const { return _time; } \ - inline uint32_t size() const { return _size; } \ - inline void set_size (uint32_t s) { _size = s; /* CAREFUL !!! */ } \ - inline uint32_t object_size() const { return size() + sizeof (*this); } \ - inline const uint8_t* buffer() const { return _buf; } \ - inline uint8_t* buffer() { return _buf; } \ - \ - inline void set_event_type(EventType t) { _type = t; } \ - \ - inline void set_time(Time t) { _time = t; } \ - \ - inline event_id_t id() const { return _id; } \ - inline void set_id(event_id_t n) { _id = n; } \ - \ - /* The following methods are type specific and only make sense for the \ - correct event type. It is the caller's responsibility to only call \ - methods which make sense for the given event type. Currently this - means \ - they all only make sense for MIDI, but built-in support may be added - for \ - other protocols in the future, or the internal representation may - change \ - to be protocol agnostic. */ \ - \ - uint8_t type() const { return midi_type (_buf); } \ - uint8_t channel() const { return midi_channel (_buf); } \ - bool is_channel_msg() const { return midi_is_channel_msg (_buf); } \ - bool is_note_on() const { return midi_is_note_on (_buf); } \ - bool is_note_off() const { return midi_is_note_off (_buf); } \ - bool is_note() const { return midi_is_note (_buf); } \ - bool is_poly_pressure() const { return midi_is_poly_pressure (_buf); } \ - bool is_channel_pressure() const { return midi_is_channel_pressure (_buf); } \ - bool is_cc() const { return midi_is_cc (_buf); } \ - bool is_pgm_change() const { return midi_is_pgm_change (_buf); } \ - bool is_pitch_bender() const { return midi_is_pitch_bender (_buf); } \ - bool is_channel_event() const { return midi_is_channel_event (_buf); } \ - bool is_smf_meta_event() const { return midi_is_smf_meta_event (_buf); } \ - bool is_sysex() const { return midi_is_sysex (_buf); } \ - bool is_spp() const { return midi_is_spp (_buf, _size); } \ - bool is_mtc_quarter() const { return midi_is_mtc_quarter (_buf, _size); } \ - bool is_mtc_full() const { return midi_is_mtc_full (_buf, _size); } \ - \ - uint8_t note() const { return midi_note (_buf); } \ - uint8_t velocity() const { return midi_velocity (_buf); } \ - uint8_t poly_note() const { return midi_poly_note (_buf); } \ - uint8_t poly_pressure() const { return midi_poly_pressure (_buf); } \ - uint8_t channel_pressure() const { return midi_channel_pressure (_buf); } \ - uint8_t cc_number() const { return midi_cc_number (_buf); } \ - uint8_t cc_value() const { return midi_cc_value (_buf); } \ - uint8_t pgm_number() const { return midi_pgm_number (_buf); } \ - uint8_t pitch_bender_lsb() const { return midi_pitch_bender_lsb (_buf); } \ - uint8_t pitch_bender_msb() const { return midi_pitch_bender_msb (_buf); } \ - uint16_t pitch_bender_value() const { return midi_pitch_bender_value (_buf); } \ - \ - void set_channel(uint8_t channel) { _buf[0] = (0xF0 & _buf[0]) | (0x0F & channel); } \ - void set_type(uint8_t type) { _buf[0] = (0x0F & _buf[0]) | (0xF0 & type); } \ - void set_note(uint8_t num) { _buf[1] = num; } \ - void set_velocity(uint8_t val) { _buf[2] = val; } \ - void set_cc_number(uint8_t num) { _buf[1] = num; } \ - void set_cc_value(uint8_t val) { _buf[2] = val; } \ - void set_pgm_number(uint8_t num) { _buf[1] = num; } \ - \ - uint16_t value() const { \ - switch (type()) { \ - case MIDI_CMD_CONTROL: \ - return cc_value(); \ - case MIDI_CMD_BENDER: \ - return pitch_bender_value(); \ - case MIDI_CMD_NOTE_PRESSURE: \ - return poly_pressure(); \ - case MIDI_CMD_CHANNEL_PRESSURE: \ - return channel_pressure(); \ - default: \ - return 0; \ - } \ - } - -template -bool event_time_order (const E & a, const E & other) -{ - if (a.time() < other.time()) { - return true; - } else if (a.time() > other.time()) { - return false; - } - - if (a.type() != MIDI_EVENT) { - /* times are equal, sort order is arbitrary */ - return false; - } - - /* times are equal. Use MIDI semantics */ - - bool other_first = false; - - /* two events at identical times. we need to determine - the order in which they should occur. - - the rule is: - - Controller messages - Program Change - Note Off - Note On - Note Pressure - Channel Pressure - Pitch Bend - */ - - if (!a.is_channel_msg() || !other.is_channel_msg() || (a.channel() != other.channel())) { - - /* if either message is not a channel message, or if - * the channels are - * different, we don't care about the type. - */ - - other_first = true; - - } else { - - switch (other.type()) { - case MIDI_CMD_CONTROL: - other_first = true; - break; - - case MIDI_CMD_PGM_CHANGE: - switch (a.type()) { - case MIDI_CMD_CONTROL: - break; - case MIDI_CMD_PGM_CHANGE: - case MIDI_CMD_NOTE_OFF: - case MIDI_CMD_NOTE_ON: - case MIDI_CMD_NOTE_PRESSURE: - case MIDI_CMD_CHANNEL_PRESSURE: - case MIDI_CMD_BENDER: - other_first = true; - } - break; - - case MIDI_CMD_NOTE_OFF: - switch (a.type()) { - case MIDI_CMD_CONTROL: - case MIDI_CMD_PGM_CHANGE: - break; - case MIDI_CMD_NOTE_OFF: - case MIDI_CMD_NOTE_ON: - case MIDI_CMD_NOTE_PRESSURE: - case MIDI_CMD_CHANNEL_PRESSURE: - case MIDI_CMD_BENDER: - other_first = true; - } \ - break; - - case MIDI_CMD_NOTE_ON: - switch (a.type()) { - case MIDI_CMD_CONTROL: - case MIDI_CMD_PGM_CHANGE: - case MIDI_CMD_NOTE_OFF: - break; - case MIDI_CMD_NOTE_ON: - case MIDI_CMD_NOTE_PRESSURE: - case MIDI_CMD_CHANNEL_PRESSURE: - case MIDI_CMD_BENDER: - other_first = true; - } - break; - case MIDI_CMD_NOTE_PRESSURE: - switch (a.type()) { - case MIDI_CMD_CONTROL: - case MIDI_CMD_PGM_CHANGE: - case MIDI_CMD_NOTE_OFF: - case MIDI_CMD_NOTE_ON: - break; - case MIDI_CMD_NOTE_PRESSURE: - case MIDI_CMD_CHANNEL_PRESSURE: - case MIDI_CMD_BENDER: - other_first = true; - } - break; - - case MIDI_CMD_CHANNEL_PRESSURE: - switch (a.type()) { - case MIDI_CMD_CONTROL: - case MIDI_CMD_PGM_CHANGE: - case MIDI_CMD_NOTE_OFF: - case MIDI_CMD_NOTE_ON: - case MIDI_CMD_NOTE_PRESSURE: - break; - case MIDI_CMD_CHANNEL_PRESSURE: - case MIDI_CMD_BENDER: - other_first = true; - } - break; - case MIDI_CMD_BENDER: - switch (a.type()) { - case MIDI_CMD_CONTROL: - case MIDI_CMD_PGM_CHANGE: - case MIDI_CMD_NOTE_OFF: - case MIDI_CMD_NOTE_ON: - case MIDI_CMD_NOTE_PRESSURE: - case MIDI_CMD_CHANNEL_PRESSURE: - break; - case MIDI_CMD_BENDER: - other_first = true; - } - break; - } - } - - return other_first; -} - /** An event. * * Template parameter Time is the type of the time stamp used for this event. @@ -302,7 +81,10 @@ public: , _size (other.size()) , _id (next_event_id()) { - memcpy (_buf, other.buffer(), _size); + if (other.size()) { + assert (other.buffer()); + memcpy (_buf, other.buffer(), other.size()); + } } ~Event() {} @@ -310,29 +92,248 @@ public: Event& operator= (Event const & other) { /* Does NOT copy event ID */ if (this != &other) { - _type = other._type; - _time = other._time; - _size = other._size; - memcpy (_buf, other._buf, _size); + _type = other.event_type(); + _time = other.time(); + _size = other.size(); + if (other.size()) { + assert (other.buffer()); + memcpy (_buf, other.buffer(), other.size()); + } } return *this; } bool operator== (Event const & other) const { /* Does NOT compare event IDs */ - return _type == other._type && - _time == other._time && - _size == other._size && - !memcmp (_buf, other._buf, _size); + return _type == other.event_type() && + _time == other.time() && + _size == other.size() && + !memcmp (_buf, other.buffer(), other.size()); } bool time_order_before (Event const & other) const { - return event_time_order (*this, other); + if (time() < other.time()) { + return true; + } else if (time() > other.time()) { + return false; + } + + if (event_type() != MIDI_EVENT) { + /* times are equal, sort order is arbitrary */ + return false; + } + + /* times are equal. Use MIDI semantics */ + + bool other_first = false; + + /* two events at identical times. we need to determine + the order in which they should occur. + + the rule is: + + Controller messages + Program Change + Note Off + Note On + Note Pressure + Channel Pressure + Pitch Bend + */ + + if (!is_channel_msg() || !other.is_channel_msg() || (channel() != other.channel())) { + + /* if either message is not a channel message, or if + * the channels are + * different, we don't care about the type. + */ + + other_first = true; + + } else { + + switch (other.type()) { + case MIDI_CMD_CONTROL: + other_first = true; + break; + + case MIDI_CMD_PGM_CHANGE: + switch (type()) { + case MIDI_CMD_CONTROL: + break; + case MIDI_CMD_PGM_CHANGE: + case MIDI_CMD_NOTE_OFF: + case MIDI_CMD_NOTE_ON: + case MIDI_CMD_NOTE_PRESSURE: + case MIDI_CMD_CHANNEL_PRESSURE: + case MIDI_CMD_BENDER: + other_first = true; + } + break; + + case MIDI_CMD_NOTE_OFF: + switch (type()) { + case MIDI_CMD_CONTROL: + case MIDI_CMD_PGM_CHANGE: + break; + case MIDI_CMD_NOTE_OFF: + case MIDI_CMD_NOTE_ON: + case MIDI_CMD_NOTE_PRESSURE: + case MIDI_CMD_CHANNEL_PRESSURE: + case MIDI_CMD_BENDER: + other_first = true; + } \ + break; + + case MIDI_CMD_NOTE_ON: + switch (type()) { + case MIDI_CMD_CONTROL: + case MIDI_CMD_PGM_CHANGE: + case MIDI_CMD_NOTE_OFF: + break; + case MIDI_CMD_NOTE_ON: + case MIDI_CMD_NOTE_PRESSURE: + case MIDI_CMD_CHANNEL_PRESSURE: + case MIDI_CMD_BENDER: + other_first = true; + } + break; + case MIDI_CMD_NOTE_PRESSURE: + switch (type()) { + case MIDI_CMD_CONTROL: + case MIDI_CMD_PGM_CHANGE: + case MIDI_CMD_NOTE_OFF: + case MIDI_CMD_NOTE_ON: + break; + case MIDI_CMD_NOTE_PRESSURE: + case MIDI_CMD_CHANNEL_PRESSURE: + case MIDI_CMD_BENDER: + other_first = true; + } + break; + + case MIDI_CMD_CHANNEL_PRESSURE: + switch (type()) { + case MIDI_CMD_CONTROL: + case MIDI_CMD_PGM_CHANGE: + case MIDI_CMD_NOTE_OFF: + case MIDI_CMD_NOTE_ON: + case MIDI_CMD_NOTE_PRESSURE: + break; + case MIDI_CMD_CHANNEL_PRESSURE: + case MIDI_CMD_BENDER: + other_first = true; + } + break; + case MIDI_CMD_BENDER: + switch (type()) { + case MIDI_CMD_CONTROL: + case MIDI_CMD_PGM_CHANGE: + case MIDI_CMD_NOTE_OFF: + case MIDI_CMD_NOTE_ON: + case MIDI_CMD_NOTE_PRESSURE: + case MIDI_CMD_CHANNEL_PRESSURE: + break; + case MIDI_CMD_BENDER: + other_first = true; + } + break; + } + } + + return other_first; } - COMMON_EVENT_METHODS; + + inline EventType event_type() const { return _type; } + inline Time time() const { return _time; } + inline uint32_t size() const { return _size; } + inline void set_size (uint32_t s) { _size = s; /* CAREFUL !!! */ } + inline uint32_t object_size() const { return size() + sizeof (*this); } + inline const uint8_t* buffer() const { return _buf; } + inline uint8_t* buffer() { return _buf; } + + inline void set_event_type(EventType t) { _type = t; } + + inline void set_time(Time t) { _time = t; } + + inline event_id_t id() const { return _id; } + inline void set_id(event_id_t n) { _id = n; } + + /* The following methods are type specific and only make sense for the + * correct event type. It is the caller's responsibility to only call + * methods which make sense for the given event type. Currently this + * means they all only make sense for MIDI, but built-in support may be + * added for other protocols in the future, or the internal + * representation may change to be protocol agnostic. + */ + + uint8_t type() const { return midi_type (_buf); } + uint8_t channel() const { return midi_channel (_buf); } + bool is_channel_msg() const { return midi_is_channel_msg (_buf); } + bool is_note_on() const { return midi_is_note_on (_buf); } + bool is_note_off() const { return midi_is_note_off (_buf); } + bool is_note() const { return midi_is_note (_buf); } + bool is_poly_pressure() const { return midi_is_poly_pressure (_buf); } + bool is_channel_pressure() const { return midi_is_channel_pressure (_buf); } + bool is_cc() const { return midi_is_cc (_buf); } + bool is_pgm_change() const { return midi_is_pgm_change (_buf); } + bool is_pitch_bender() const { return midi_is_pitch_bender (_buf); } + bool is_channel_event() const { return midi_is_channel_event (_buf); } + bool is_smf_meta_event() const { return midi_is_smf_meta_event (_buf); } + bool is_sysex() const { return midi_is_sysex (_buf); } + bool is_spp() const { return midi_is_spp (_buf, _size); } + bool is_mtc_quarter() const { return midi_is_mtc_quarter (_buf, _size); } + bool is_mtc_full() const { return midi_is_mtc_full (_buf, _size); } + + uint8_t note() const { return midi_note (_buf); } + uint8_t velocity() const { return midi_velocity (_buf); } + uint8_t poly_note() const { return midi_poly_note (_buf); } + uint8_t poly_pressure() const { return midi_poly_pressure (_buf); } + uint8_t channel_pressure() const { return midi_channel_pressure (_buf); } + uint8_t cc_number() const { return midi_cc_number (_buf); } + uint8_t cc_value() const { return midi_cc_value (_buf); } + uint8_t pgm_number() const { return midi_pgm_number (_buf); } + uint8_t pitch_bender_lsb() const { return midi_pitch_bender_lsb (_buf); } + uint8_t pitch_bender_msb() const { return midi_pitch_bender_msb (_buf); } + uint16_t pitch_bender_value() const { return midi_pitch_bender_value (_buf); } + + void set_channel(uint8_t channel) { _buf[0] = (0xF0 & _buf[0]) | (0x0F & channel); } + void set_type(uint8_t type) { _buf[0] = (0x0F & _buf[0]) | (0xF0 & type); } + void set_note(uint8_t num) { _buf[1] = num; } + void set_velocity(uint8_t val) { _buf[2] = val; } + void set_cc_number(uint8_t num) { _buf[1] = num; } + void set_cc_value(uint8_t val) { _buf[2] = val; } + void set_pgm_number(uint8_t num) { _buf[1] = num; } + + uint16_t value() const { + switch (type()) { + case MIDI_CMD_CONTROL: + return cc_value(); + case MIDI_CMD_BENDER: + return pitch_bender_value(); + case MIDI_CMD_NOTE_PRESSURE: + return poly_pressure(); + case MIDI_CMD_CHANNEL_PRESSURE: + return channel_pressure(); + default: + return 0; + } + } + + public: - COMMON_EVENT_DATA + EventType _type; /*< Type of event (application relative, NOT MIDI 'type') */ \ + Time _time; /*< Time stamp of event */ \ + uint32_t _size; /*< Size of buffer in bytes */ \ + event_id_t _id; /*< Unique event ID */ \ + uint8_t _buf[0]; /*< Event data. Must be at end, to use C-style variable-sized structure hack */ + /* C++ standard: "Nonstatic data members of a (non-union) class with + * the same access control (Clause 11) are allocated so that later + * members have higher addresses within a class object." (9.2.13) + * + * NO MORE DATA MEMBERS AFTER THIS POINT + */ private: /* hide these methods since they are illegal. Event can only be @@ -409,29 +410,66 @@ class LIBEVORAL_API ManagedEvent /* INHERITANCE ILLEGAL HERE */ } ManagedEvent& operator= (ManagedEvent