change the type of reference held by a MidiModel to its MidiSource
This also requires a change in the type of reference held by a MidiAutomationListBinder. Both the MidiSource and MidiModel have a reference to each other, and it is important that we avoid circular references to avoid problems with object destruction. We had been accomplishing this by having the Model hold a weak_ptr<MidiSource>. However, the lifetime of a MidiSource and its MidiModel are coincident and there's really no need to use a smart ptr at all. A normal reference is just fine. However, due to constructors that accept a serialized state, we cannot use an actual reference (we cannot set the constructor in the initializer list), so we use a bare ptr instead. This forces a similar change in MidiAutomationListBinder, which also maintains a reference to the Source. However, the only purpose of this object is to ensure that if the Source is destroyed, relevant commands will be removed from the undo/redo history, and so all that matters here is that the binder connects to the Destroyed signal of the source, and arranges for its own destruction when received. Note that the previous construction of the binder, actually holding a shared_ptr<MidiSource> would appear have prevented the Destroyed signal from ever being emitted (from ~Destructible), and so this may also be a bug fix that allows MidiSources to actually be deleted (the memory object, not the file).
This commit is contained in:
parent
3454353aa3
commit
54597bd803
|
|
@ -48,7 +48,11 @@ MidiAutomationLine::MidiAutomationLine (
|
|||
MementoCommandBinder<ARDOUR::AutomationList>*
|
||||
MidiAutomationLine::memento_command_binder ()
|
||||
{
|
||||
return new ARDOUR::MidiAutomationListBinder (_region->midi_source(), _parameter);
|
||||
/* some weirdness here since _region->midi_source() returns a
|
||||
* shared_ptr<> but the binder accepts a reference.
|
||||
*/
|
||||
|
||||
return new ARDOUR::MidiAutomationListBinder (*(_region->midi_source().get()), _parameter);
|
||||
}
|
||||
|
||||
Temporal::timepos_t
|
||||
|
|
|
|||
|
|
@ -59,10 +59,7 @@ class LIBARDOUR_API MidiModel : public AutomatableSequence<Temporal::Beats> {
|
|||
public:
|
||||
typedef Temporal::Beats TimeType;
|
||||
|
||||
MidiModel (boost::shared_ptr<MidiSource>);
|
||||
|
||||
NoteMode note_mode() const { return (percussive() ? Percussive : Sustained); }
|
||||
void set_note_mode(NoteMode mode) { set_percussive(mode == Percussive); };
|
||||
MidiModel (MidiSource&);
|
||||
|
||||
class LIBARDOUR_API DiffCommand : public Command {
|
||||
public:
|
||||
|
|
@ -302,9 +299,6 @@ public:
|
|||
PBD::Signal0<void> ContentsChanged;
|
||||
PBD::Signal1<void, Temporal::timecnt_t> ContentsShifted;
|
||||
|
||||
boost::shared_ptr<const MidiSource> midi_source ();
|
||||
void set_midi_source (boost::shared_ptr<MidiSource>);
|
||||
|
||||
boost::shared_ptr<Evoral::Note<TimeType> > find_note (NotePtr);
|
||||
PatchChangePtr find_patch_change (Evoral::event_id_t);
|
||||
boost::shared_ptr<Evoral::Note<TimeType> > find_note (Evoral::event_id_t);
|
||||
|
|
@ -318,12 +312,19 @@ public:
|
|||
void insert_silence_at_start (TimeType);
|
||||
void transpose (NoteDiffCommand *, const NotePtr, int);
|
||||
|
||||
protected:
|
||||
protected:
|
||||
int resolve_overlaps_unlocked (const NotePtr, void* arg = 0);
|
||||
|
||||
private:
|
||||
protected:
|
||||
friend class NoteDiffCommand;
|
||||
friend class SysExDiffCommand;
|
||||
friend class PatchChangeDiffCommand;
|
||||
|
||||
MidiSource& midi_source() const { return _midi_source; }
|
||||
|
||||
private:
|
||||
struct WriteLockImpl : public AutomatableSequence<TimeType>::WriteLockImpl {
|
||||
WriteLockImpl(Glib::Threads::Mutex::Lock* slock, Glib::Threads::RWLock& s, Glib::Threads::Mutex& c)
|
||||
WriteLockImpl(Source::WriterLock* slock, Glib::Threads::RWLock& s, Glib::Threads::Mutex& c)
|
||||
: AutomatableSequence<TimeType>::WriteLockImpl(s, c)
|
||||
, source_lock (slock)
|
||||
{}
|
||||
|
|
@ -348,8 +349,7 @@ private:
|
|||
|
||||
PBD::ScopedConnectionList _midi_source_connections;
|
||||
|
||||
// We cannot use a boost::shared_ptr here to avoid a retain cycle
|
||||
boost::weak_ptr<MidiSource> _midi_source;
|
||||
MidiSource& _midi_source;
|
||||
InsertMergePolicy _insert_merge_policy;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -25,11 +25,11 @@
|
|||
|
||||
using namespace ARDOUR;
|
||||
|
||||
MidiAutomationListBinder::MidiAutomationListBinder (boost::shared_ptr<MidiSource> s, Evoral::Parameter p)
|
||||
: _source (s)
|
||||
MidiAutomationListBinder::MidiAutomationListBinder (MidiSource& s, Evoral::Parameter p)
|
||||
: _source (&s)
|
||||
, _parameter (p)
|
||||
{
|
||||
|
||||
_source->Destroyed.connect_same_thread (source_death_connection, boost::bind (&MidiAutomationListBinder::source_died, this));
|
||||
}
|
||||
|
||||
MidiAutomationListBinder::MidiAutomationListBinder (XMLNode* node, Session::SourceMap const & sources)
|
||||
|
|
@ -44,7 +44,7 @@ MidiAutomationListBinder::MidiAutomationListBinder (XMLNode* node, Session::Sour
|
|||
|
||||
Session::SourceMap::const_iterator i = sources.find (PBD::ID (id_str));
|
||||
assert (i != sources.end());
|
||||
_source = boost::dynamic_pointer_cast<MidiSource> (i->second);
|
||||
_source = (boost::dynamic_pointer_cast<MidiSource> (i->second)).get();
|
||||
|
||||
_parameter = EventTypeMap::instance().from_symbol (parameter_str);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -58,37 +58,36 @@ using namespace std;
|
|||
using namespace ARDOUR;
|
||||
using namespace PBD;
|
||||
|
||||
MidiModel::MidiModel (boost::shared_ptr<MidiSource> s)
|
||||
: AutomatableSequence<TimeType> (s->session(), Temporal::BeatTime)
|
||||
MidiModel::MidiModel (MidiSource& s)
|
||||
: AutomatableSequence<TimeType> (s.session(), Temporal::BeatTime)
|
||||
, _midi_source (s)
|
||||
{
|
||||
set_midi_source (s);
|
||||
_midi_source.InterpolationChanged.connect_same_thread (_midi_source_connections, boost::bind (&MidiModel::source_interpolation_changed, this, _1, _2));
|
||||
_midi_source.AutomationStateChanged.connect_same_thread (_midi_source_connections, boost::bind (&MidiModel::source_automation_state_changed, this, _1, _2));
|
||||
}
|
||||
|
||||
MidiModel::NoteDiffCommand*
|
||||
MidiModel::new_note_diff_command (const string& name)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
return new NoteDiffCommand (ms->model(), name);
|
||||
/* return via the MidiSource to get a shared_ptr to
|
||||
* ourselves. Probably faster than shared_from_this()
|
||||
*/
|
||||
return new NoteDiffCommand (_midi_source.model(), name);
|
||||
}
|
||||
|
||||
MidiModel::SysExDiffCommand*
|
||||
MidiModel::new_sysex_diff_command (const string& name)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
return new SysExDiffCommand (ms->model(), name);
|
||||
/* return via the MidiSource to get a shared_ptr to
|
||||
* ourselves. Probably faster than shared_from_this()
|
||||
*/
|
||||
return new SysExDiffCommand (_midi_source.model(), name);
|
||||
}
|
||||
|
||||
MidiModel::PatchChangeDiffCommand*
|
||||
MidiModel::new_patch_change_diff_command (const string& name)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
return new PatchChangeDiffCommand (ms->model(), name);
|
||||
return new PatchChangeDiffCommand (_midi_source.model(), name);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -639,7 +638,7 @@ XMLNode&
|
|||
MidiModel::NoteDiffCommand::get_state ()
|
||||
{
|
||||
XMLNode* diff_command = new XMLNode (NOTE_DIFF_COMMAND_ELEMENT);
|
||||
diff_command->set_property("midi-source", _model->midi_source()->id().to_s());
|
||||
diff_command->set_property("midi-source", _model->midi_source().id().to_s());
|
||||
|
||||
XMLNode* changes = diff_command->add_child(DIFF_NOTES_ELEMENT);
|
||||
for_each(_changes.begin(), _changes.end(),
|
||||
|
|
@ -839,7 +838,7 @@ XMLNode&
|
|||
MidiModel::SysExDiffCommand::get_state ()
|
||||
{
|
||||
XMLNode* diff_command = new XMLNode (SYSEX_DIFF_COMMAND_ELEMENT);
|
||||
diff_command->set_property ("midi-source", _model->midi_source()->id().to_s());
|
||||
diff_command->set_property ("midi-source", _model->midi_source().id().to_s());
|
||||
|
||||
XMLNode* changes = diff_command->add_child(DIFF_SYSEXES_ELEMENT);
|
||||
for_each (_changes.begin(), _changes.end(),
|
||||
|
|
@ -1184,7 +1183,7 @@ XMLNode &
|
|||
MidiModel::PatchChangeDiffCommand::get_state ()
|
||||
{
|
||||
XMLNode* diff_command = new XMLNode (PATCH_CHANGE_DIFF_COMMAND_ELEMENT);
|
||||
diff_command->set_property("midi-source", _model->midi_source()->id().to_s());
|
||||
diff_command->set_property("midi-source", _model->midi_source().id().to_s());
|
||||
|
||||
XMLNode* added = diff_command->add_child (ADDED_PATCH_CHANGES_ELEMENT);
|
||||
for_each (_added.begin(), _added.end(),
|
||||
|
|
@ -1255,24 +1254,18 @@ MidiModel::sync_to_source (const Glib::Threads::Mutex::Lock& source_lock)
|
|||
{
|
||||
ReadLock lock(read_lock());
|
||||
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
if (!ms) {
|
||||
error << "MIDI model has no source to sync to" << endmsg;
|
||||
return false;
|
||||
}
|
||||
|
||||
/* Invalidate and store active notes, which will be picked up by the iterator
|
||||
on the next roll if time progresses linearly. */
|
||||
ms->invalidate(source_lock);
|
||||
_midi_source.invalidate(source_lock);
|
||||
|
||||
/* as of March 2022 or long before , the note mode argument does nothing */
|
||||
_midi_source.mark_streaming_midi_write_started (source_lock, Sustained);
|
||||
|
||||
for (Evoral::Sequence<TimeType>::const_iterator i = begin(TimeType(), true); i != end(); ++i) {
|
||||
ms->append_event_beats(source_lock, *i);
|
||||
_midi_source.append_event_beats(source_lock, *i);
|
||||
}
|
||||
|
||||
ms->mark_streaming_write_completed (source_lock);
|
||||
_midi_source.mark_streaming_write_completed (source_lock);
|
||||
|
||||
set_edited (false);
|
||||
|
||||
|
|
@ -1421,7 +1414,6 @@ MidiModel::find_sysex (Evoral::event_id_t sysex_id)
|
|||
MidiModel::WriteLock
|
||||
MidiModel::edit_lock()
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock();
|
||||
Glib::Threads::Mutex::Lock* source_lock = 0;
|
||||
|
||||
if (ms) {
|
||||
|
|
@ -1631,33 +1623,7 @@ InsertMergePolicy
|
|||
MidiModel::insert_merge_policy () const
|
||||
{
|
||||
/* XXX ultimately this should be a per-track or even per-model policy */
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
return ms->session().config.get_insert_merge_policy ();
|
||||
}
|
||||
|
||||
void
|
||||
MidiModel::set_midi_source (boost::shared_ptr<MidiSource> s)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> old = _midi_source.lock ();
|
||||
|
||||
if (old) {
|
||||
Source::Lock lm(old->mutex());
|
||||
old->invalidate (lm);
|
||||
}
|
||||
|
||||
_midi_source_connections.drop_connections ();
|
||||
|
||||
_midi_source = s;
|
||||
|
||||
s->InterpolationChanged.connect_same_thread (
|
||||
_midi_source_connections, boost::bind (&MidiModel::source_interpolation_changed, this, _1, _2)
|
||||
);
|
||||
|
||||
s->AutomationStateChanged.connect_same_thread (
|
||||
_midi_source_connections, boost::bind (&MidiModel::source_automation_state_changed, this, _1, _2)
|
||||
);
|
||||
return _midi_source.session().config.get_insert_merge_policy ();
|
||||
}
|
||||
|
||||
/** The source has signalled that the interpolation style for a parameter has changed. In order to
|
||||
|
|
@ -1684,10 +1650,7 @@ MidiModel::source_interpolation_changed (Evoral::Parameter p, Evoral::ControlLis
|
|||
void
|
||||
MidiModel::control_list_interpolation_changed (Evoral::Parameter p, Evoral::ControlList::InterpolationStyle s)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
ms->set_interpolation_of (p, s);
|
||||
_midi_source.set_interpolation_of (p, s);
|
||||
}
|
||||
|
||||
void
|
||||
|
|
@ -1705,9 +1668,7 @@ MidiModel::source_automation_state_changed (Evoral::Parameter p, AutoState s)
|
|||
void
|
||||
MidiModel::automation_list_automation_state_changed (Evoral::Parameter p, AutoState s)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
ms->set_automation_state_of (p, s);
|
||||
_midi_source.set_automation_state_of (p, s);
|
||||
}
|
||||
|
||||
boost::shared_ptr<Evoral::Control>
|
||||
|
|
@ -1719,34 +1680,22 @@ MidiModel::control_factory (Evoral::Parameter const & p)
|
|||
automation state from our source.
|
||||
*/
|
||||
|
||||
boost::shared_ptr<MidiSource> ms = _midi_source.lock ();
|
||||
assert (ms);
|
||||
|
||||
c->list()->set_interpolation (ms->interpolation_of (p));
|
||||
c->list()->set_interpolation (_midi_source.interpolation_of (p));
|
||||
|
||||
boost::shared_ptr<AutomationList> al = boost::dynamic_pointer_cast<AutomationList> (c->list ());
|
||||
assert (al);
|
||||
|
||||
al->set_automation_state (ms->automation_state_of (p));
|
||||
al->set_automation_state (_midi_source.automation_state_of (p));
|
||||
|
||||
return c;
|
||||
}
|
||||
|
||||
boost::shared_ptr<const MidiSource>
|
||||
MidiModel::midi_source ()
|
||||
{
|
||||
return _midi_source.lock ();
|
||||
}
|
||||
|
||||
/** Moves notes, patch changes, controllers and sys-ex to insert silence at the start of the model.
|
||||
* Adds commands to the session's current undo stack to reflect the movements.
|
||||
*/
|
||||
void
|
||||
MidiModel::insert_silence_at_start (TimeType t)
|
||||
{
|
||||
boost::shared_ptr<MidiSource> s = _midi_source.lock ();
|
||||
assert (s);
|
||||
|
||||
/* Notes */
|
||||
|
||||
if (!notes().empty ()) {
|
||||
|
|
@ -1756,7 +1705,7 @@ MidiModel::insert_silence_at_start (TimeType t)
|
|||
c->change (*i, NoteDiffCommand::StartTime, (*i)->time() + t);
|
||||
}
|
||||
|
||||
apply_command_as_subcommand (s->session(), c);
|
||||
apply_command_as_subcommand (_midi_source.session(), c);
|
||||
}
|
||||
|
||||
/* Patch changes */
|
||||
|
|
@ -1768,7 +1717,7 @@ MidiModel::insert_silence_at_start (TimeType t)
|
|||
c->change_time (*i, (*i)->time() + t);
|
||||
}
|
||||
|
||||
apply_command_as_subcommand (s->session(), c);
|
||||
apply_command_as_subcommand (_midi_source.session(), c);
|
||||
}
|
||||
|
||||
/* Controllers */
|
||||
|
|
@ -1778,7 +1727,7 @@ MidiModel::insert_silence_at_start (TimeType t)
|
|||
XMLNode& before = ac->alist()->get_state ();
|
||||
i->second->list()->shift (timepos_t::zero (i->second->list()->time_domain()), timecnt_t (t));
|
||||
XMLNode& after = ac->alist()->get_state ();
|
||||
s->session().add_command (new MementoCommand<AutomationList> (new MidiAutomationListBinder (s, i->first), &before, &after));
|
||||
_midi_source.session().add_command (new MementoCommand<AutomationList> (new MidiAutomationListBinder (_midi_source, i->first), &before, &after));
|
||||
}
|
||||
|
||||
/* Sys-ex */
|
||||
|
|
@ -1790,7 +1739,7 @@ MidiModel::insert_silence_at_start (TimeType t)
|
|||
c->change (*i, (*i)->time() + t);
|
||||
}
|
||||
|
||||
apply_command_as_subcommand (s->session(), c);
|
||||
apply_command_as_subcommand (_midi_source.session(), c);
|
||||
}
|
||||
|
||||
ContentsShifted (timecnt_t (t));
|
||||
|
|
|
|||
Loading…
Reference in New Issue