From a7067557107fc2f01586a88bb8b0a097914798ea Mon Sep 17 00:00:00 2001 From: David Robillard Date: Wed, 17 Dec 2014 16:05:27 -0500 Subject: Fix various MIDI locking issues. Attempt to make mistakes much less likely in the future by statically requiring caller to pass scoped locks where necessary. --- libs/ardour/midi_source.cc | 61 ++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 29 deletions(-) (limited to 'libs/ardour/midi_source.cc') diff --git a/libs/ardour/midi_source.cc b/libs/ardour/midi_source.cc index adf28cff09..1b1cf20c68 100644 --- a/libs/ardour/midi_source.cc +++ b/libs/ardour/midi_source.cc @@ -177,22 +177,21 @@ MidiSource::update_length (framecnt_t) } void -MidiSource::invalidate () +MidiSource::invalidate (const Lock& lock) { _model_iter_valid = false; _model_iter.invalidate(); } framecnt_t -MidiSource::midi_read (Evoral::EventSink& dst, +MidiSource::midi_read (const Lock& lm, + Evoral::EventSink& dst, framepos_t source_start, framepos_t start, framecnt_t cnt, MidiStateTracker* tracker, const std::set& filtered) const { - Glib::Threads::Mutex::Lock lm (_lock); - BeatsFramesConverter converter(_session.tempo_map(), source_start); DEBUG_TRACE (DEBUG::MidiSourceIO, @@ -233,22 +232,21 @@ MidiSource::midi_read (Evoral::EventSink& dst, } return cnt; } else { - return read_unlocked (dst, source_start, start, cnt, tracker); + return read_unlocked (lm, dst, source_start, start, cnt, tracker); } } framecnt_t -MidiSource::midi_write (MidiRingBuffer& source, +MidiSource::midi_write (const Lock& lm, + MidiRingBuffer& source, framepos_t source_start, framecnt_t cnt) { - Glib::Threads::Mutex::Lock lm (_lock); - - const framecnt_t ret = write_unlocked (source, source_start, cnt); + const framecnt_t ret = write_unlocked (lm, source, source_start, cnt); if (cnt == max_framecnt) { _last_read_end = 0; - invalidate(); + invalidate(lm); } else { _capture_length += cnt; } @@ -257,7 +255,7 @@ MidiSource::midi_write (MidiRingBuffer& source, } void -MidiSource::mark_streaming_midi_write_started (NoteMode mode) +MidiSource::mark_streaming_midi_write_started (const Lock& lock, NoteMode mode) { if (_model) { _model->set_note_mode (mode); @@ -292,14 +290,15 @@ MidiSource::mark_write_starting_now (framecnt_t position, } void -MidiSource::mark_streaming_write_started () +MidiSource::mark_streaming_write_started (const Lock& lock) { NoteMode note_mode = _model ? _model->note_mode() : Sustained; - mark_streaming_midi_write_started (note_mode); + mark_streaming_midi_write_started (lock, note_mode); } void -MidiSource::mark_midi_streaming_write_completed (Evoral::Sequence::StuckNoteOption option, +MidiSource::mark_midi_streaming_write_completed (const Lock& lock, + Evoral::Sequence::StuckNoteOption option, Evoral::MusicalTime end) { if (_model) { @@ -318,37 +317,39 @@ MidiSource::mark_midi_streaming_write_completed (Evoral::Sequence::DeleteStuckNotes); + mark_midi_streaming_write_completed (lock, Evoral::Sequence::DeleteStuckNotes); } int -MidiSource::write_to (boost::shared_ptr newsrc, Evoral::MusicalTime begin, Evoral::MusicalTime end) +MidiSource::write_to (const Lock& lock, boost::shared_ptr newsrc, Evoral::MusicalTime begin, Evoral::MusicalTime end) { + Lock newsrc_lock (newsrc->mutex ()); + newsrc->set_timeline_position (_timeline_position); newsrc->copy_interpolation_from (this); newsrc->copy_automation_state_from (this); if (_model) { if (begin == Evoral::MinMusicalTime && end == Evoral::MaxMusicalTime) { - _model->write_to (newsrc); + _model->write_to (newsrc, newsrc_lock); } else { - _model->write_section_to (newsrc, begin, end); + _model->write_section_to (newsrc, newsrc_lock, begin, end); } } else { error << string_compose (_("programming error: %1"), X_("no model for MidiSource during ::clone()")); return -1; } - newsrc->flush_midi(); + newsrc->flush_midi(newsrc_lock); /* force a reload of the model if the range is partial */ if (begin != Evoral::MinMusicalTime || end != Evoral::MaxMusicalTime) { - newsrc->load_model (true, true); + newsrc->load_model (newsrc_lock, true); } else { - newsrc->set_model (_model); + newsrc->set_model (newsrc_lock, _model); } /* this file is not removable (but since it is MIDI, it is mutable) */ @@ -361,6 +362,8 @@ MidiSource::write_to (boost::shared_ptr newsrc, Evoral::MusicalTime void MidiSource::session_saved() { + Lock lm (_lock); + /* this writes a copy of the data to disk. XXX do we need to do this every time? */ @@ -375,18 +378,18 @@ MidiSource::session_saved() _model.reset (); /* Flush model contents to disk. */ - mm->sync_to_source (); + mm->sync_to_source (lm); /* Reacquire model. */ _model = mm; } else { - flush_midi(); + flush_midi(lm); } } void -MidiSource::set_note_mode(NoteMode mode) +MidiSource::set_note_mode(const Lock& lock, NoteMode mode) { if (_model) { _model->set_note_mode(mode); @@ -394,18 +397,18 @@ MidiSource::set_note_mode(NoteMode mode) } void -MidiSource::drop_model () +MidiSource::drop_model (const Lock& lock) { _model.reset(); - invalidate(); + invalidate(lock); ModelChanged (); /* EMIT SIGNAL */ } void -MidiSource::set_model (boost::shared_ptr m) +MidiSource::set_model (const Lock& lock, boost::shared_ptr m) { _model = m; - invalidate(); + invalidate(lock); ModelChanged (); /* EMIT SIGNAL */ } -- cgit v1.2.3