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/ardour/audiofilesource.h | 2 +- libs/ardour/ardour/audiosource.h | 2 +- libs/ardour/ardour/midi_model.h | 13 +++++-- libs/ardour/ardour/midi_playlist_source.h | 16 ++++---- libs/ardour/ardour/midi_source.h | 56 +++++++++++++++++----------- libs/ardour/ardour/midi_state_tracker.h | 4 +- libs/ardour/ardour/smf_source.h | 29 ++++++++------- libs/ardour/ardour/source.h | 6 ++- libs/ardour/audio_diskstream.cc | 9 +++-- libs/ardour/audiofilesource.cc | 4 +- libs/ardour/audiosource.cc | 2 +- libs/ardour/import.cc | 11 ++++-- libs/ardour/midi_diskstream.cc | 10 +++-- libs/ardour/midi_model.cc | 54 ++++++++++++++------------- libs/ardour/midi_playlist_source.cc | 20 +++++----- libs/ardour/midi_region.cc | 15 ++++++-- libs/ardour/midi_source.cc | 61 +++++++++++++++--------------- libs/ardour/midi_state_tracker.cc | 4 +- libs/ardour/midi_stretch.cc | 4 +- libs/ardour/smf_source.cc | 62 +++++++++++++++---------------- libs/ardour/source_factory.cc | 6 +-- 21 files changed, 220 insertions(+), 170 deletions(-) (limited to 'libs') diff --git a/libs/ardour/ardour/audiofilesource.h b/libs/ardour/ardour/audiofilesource.h index 4831eb2081..9e63f4c81d 100644 --- a/libs/ardour/ardour/audiofilesource.h +++ b/libs/ardour/ardour/audiofilesource.h @@ -64,7 +64,7 @@ public: virtual int update_header (framepos_t when, struct tm&, time_t) = 0; virtual int flush_header () = 0; - void mark_streaming_write_completed (); + void mark_streaming_write_completed (const Lock& lock); int setup_peakfile (); diff --git a/libs/ardour/ardour/audiosource.h b/libs/ardour/ardour/audiosource.h index 7d1878f286..622bc6052a 100644 --- a/libs/ardour/ardour/audiosource.h +++ b/libs/ardour/ardour/audiosource.h @@ -60,7 +60,7 @@ class LIBARDOUR_API AudioSource : virtual public Source, virtual float sample_rate () const = 0; - virtual void mark_streaming_write_completed (); + virtual void mark_streaming_write_completed (const Lock& lock); virtual bool can_truncate_peaks() const { return true; } diff --git a/libs/ardour/ardour/midi_model.h b/libs/ardour/ardour/midi_model.h index 4d12839625..1988c1a2d1 100644 --- a/libs/ardour/ardour/midi_model.h +++ b/libs/ardour/ardour/midi_model.h @@ -238,10 +238,15 @@ public: void apply_command (Session& session, Command* cmd); void apply_command_as_subcommand (Session& session, Command* cmd); - bool sync_to_source (); - bool write_to(boost::shared_ptr source); - bool write_section_to (boost::shared_ptr source, Evoral::MusicalTime begin = Evoral::MinMusicalTime, - Evoral::MusicalTime end = Evoral::MaxMusicalTime); + bool sync_to_source (const Glib::Threads::Mutex::Lock& source_lock); + + bool write_to(boost::shared_ptr source, + const Glib::Threads::Mutex::Lock& source_lock); + + bool write_section_to(boost::shared_ptr source, + const Glib::Threads::Mutex::Lock& source_lock, + Evoral::MusicalTime begin = Evoral::MinMusicalTime, + Evoral::MusicalTime end = Evoral::MaxMusicalTime); // MidiModel doesn't use the normal AutomationList serialisation code // since controller data is stored in the .mid diff --git a/libs/ardour/ardour/midi_playlist_source.h b/libs/ardour/ardour/midi_playlist_source.h index 7a61f5aa02..f064553342 100644 --- a/libs/ardour/ardour/midi_playlist_source.h +++ b/libs/ardour/ardour/midi_playlist_source.h @@ -45,10 +45,10 @@ public: XMLNode& get_state (); int set_state (const XMLNode&, int version); - void append_event_unlocked_beats(const Evoral::Event& ev); - void append_event_unlocked_frames(const Evoral::Event& ev, framepos_t source_start); - void load_model(bool lock=true, bool force_reload=false); - void destroy_model(); + void append_event_beats(const Glib::Threads::Mutex::Lock& lock, const Evoral::Event& ev); + void append_event_frames(const Glib::Threads::Mutex::Lock& lock, const Evoral::Event& ev, framepos_t source_start); + void load_model(const Glib::Threads::Mutex::Lock& lock, bool force_reload=false); + void destroy_model(const Glib::Threads::Mutex::Lock& lock); protected: friend class SourceFactory; @@ -58,15 +58,17 @@ protected: MidiPlaylistSource (Session&, const XMLNode&); - void flush_midi(); + void flush_midi(const Lock& lock); - framecnt_t read_unlocked (Evoral::EventSink& dst, + framecnt_t read_unlocked (const Lock& lock, + Evoral::EventSink& dst, framepos_t position, framepos_t start, framecnt_t cnt, MidiStateTracker* tracker) const; - framecnt_t write_unlocked (MidiRingBuffer& dst, + framecnt_t write_unlocked (const Lock& lock, + MidiRingBuffer& dst, framepos_t position, framecnt_t cnt); diff --git a/libs/ardour/ardour/midi_source.h b/libs/ardour/ardour/midi_source.h index 2ce92ba3cf..2b78230a00 100644 --- a/libs/ardour/ardour/midi_source.h +++ b/libs/ardour/ardour/midi_source.h @@ -57,7 +57,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha * \param end time of latest event that can be written. * \return zero on success, non-zero if the write failed for any reason. */ - int write_to (boost::shared_ptr newsrc, + int write_to (const Lock& lock, + boost::shared_ptr newsrc, Evoral::MusicalTime begin = Evoral::MinMusicalTime, Evoral::MusicalTime end = Evoral::MaxMusicalTime); @@ -70,7 +71,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha * \param tracker an optional pointer to MidiStateTracker object, for note on/off tracking. * \param filtered Parameters whose MIDI messages will not be returned. */ - virtual framecnt_t midi_read (Evoral::EventSink& dst, + virtual framecnt_t midi_read (const Lock& lock, + Evoral::EventSink& dst, framepos_t source_start, framepos_t start, framecnt_t cnt, @@ -82,22 +84,33 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha * @param source_start This source's start position in session frames. * @param cnt The length of time to write. */ - virtual framecnt_t midi_write (MidiRingBuffer& src, + virtual framecnt_t midi_write (const Lock& lock, + MidiRingBuffer& src, framepos_t source_start, framecnt_t cnt); - virtual void append_event_unlocked_beats(const Evoral::Event& ev) = 0; + /** Append a single event with a timestamp in beats. + * + * Caller must ensure that the event is later than the last written event. + */ + virtual void append_event_beats(const Lock& lock, + const Evoral::Event& ev) = 0; - virtual void append_event_unlocked_frames(const Evoral::Event& ev, - framepos_t source_start) = 0; + /** Append a single event with a timestamp in frames. + * + * Caller must ensure that the event is later than the last written event. + */ + virtual void append_event_frames(const Lock& lock, + const Evoral::Event& ev, + framepos_t source_start) = 0; virtual bool empty () const; virtual framecnt_t length (framepos_t pos) const; virtual void update_length (framecnt_t); - virtual void mark_streaming_midi_write_started (NoteMode mode); - virtual void mark_streaming_write_started (); - virtual void mark_streaming_write_completed (); + virtual void mark_streaming_midi_write_started (const Lock& lock, NoteMode mode); + virtual void mark_streaming_write_started (const Lock& lock); + virtual void mark_streaming_write_completed (const Lock& lock); /** Mark write starting with the given time parameters. * @@ -119,6 +132,7 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha * etc. */ virtual void mark_midi_streaming_write_completed ( + const Lock& lock, Evoral::Sequence::StuckNoteOption stuck_option, Evoral::MusicalTime when = Evoral::MusicalTime()); @@ -137,19 +151,17 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha void set_length_beats(TimeType l) { _length_beats = l; } TimeType length_beats() const { return _length_beats; } - virtual void load_model(bool lock=true, bool force_reload=false) = 0; - virtual void destroy_model() = 0; + virtual void load_model(const Glib::Threads::Mutex::Lock& lock, bool force_reload=false) = 0; + virtual void destroy_model(const Glib::Threads::Mutex::Lock& lock) = 0; - /** This must be called with the source lock held whenever the - * source/model contents have been changed (reset iterators/cache/etc). - */ - void invalidate(); + /** Reset cached information (like iterators) when things have changed. */ + void invalidate(const Glib::Threads::Mutex::Lock& lock); - void set_note_mode(NoteMode mode); + void set_note_mode(const Glib::Threads::Mutex::Lock& lock, NoteMode mode); boost::shared_ptr model() { return _model; } - void set_model (boost::shared_ptr); - void drop_model(); + void set_model(const Glib::Threads::Mutex::Lock& lock, boost::shared_ptr); + void drop_model(const Glib::Threads::Mutex::Lock& lock); Evoral::ControlList::InterpolationStyle interpolation_of (Evoral::Parameter) const; void set_interpolation_of (Evoral::Parameter, Evoral::ControlList::InterpolationStyle); @@ -169,9 +181,10 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha PBD::Signal2 AutomationStateChanged; protected: - virtual void flush_midi() = 0; + virtual void flush_midi(const Lock& lock) = 0; - virtual framecnt_t read_unlocked (Evoral::EventSink& dst, + virtual framecnt_t read_unlocked (const Lock& lock, + Evoral::EventSink& dst, framepos_t position, framepos_t start, framecnt_t cnt, @@ -182,7 +195,8 @@ class LIBARDOUR_API MidiSource : virtual public Source, public boost::enable_sha * @param position This source's start position in session frames. * @param cnt The duration of this block to write for. */ - virtual framecnt_t write_unlocked (MidiRingBuffer& source, + virtual framecnt_t write_unlocked (const Lock& lock, + MidiRingBuffer& source, framepos_t position, framecnt_t cnt) = 0; diff --git a/libs/ardour/ardour/midi_state_tracker.h b/libs/ardour/ardour/midi_state_tracker.h index d8a31c10aa..98611bc722 100644 --- a/libs/ardour/ardour/midi_state_tracker.h +++ b/libs/ardour/ardour/midi_state_tracker.h @@ -20,6 +20,8 @@ #ifndef __ardour_midi_state_tracker_h__ #define __ardour_midi_state_tracker_h__ +#include + #include "ardour/midi_buffer.h" namespace Evoral { @@ -44,7 +46,7 @@ public: void remove (uint8_t note, uint8_t chn); void resolve_notes (MidiBuffer& buffer, framepos_t time); void resolve_notes (Evoral::EventSink& buffer, framepos_t time); - void resolve_notes (MidiSource& src, Evoral::MusicalTime time); + void resolve_notes (MidiSource& src, const Glib::Threads::Mutex::Lock& lock, Evoral::MusicalTime time); void dump (std::ostream&); void reset (); bool empty() const { return _on == 0; } diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h index 56b70b15bd..d93fdb8506 100644 --- a/libs/ardour/ardour/smf_source.h +++ b/libs/ardour/ardour/smf_source.h @@ -47,26 +47,24 @@ public: virtual ~SMFSource (); - bool safe_file_extension (const std::string& path) const { + bool safe_file_extension (const std::string& path) const { return safe_midi_file_extension(path); } - void append_event_unlocked_beats (const Evoral::Event& ev); - void append_event_unlocked_frames (const Evoral::Event& ev, framepos_t source_start); + void append_event_beats (const Lock& lock, const Evoral::Event& ev); + void append_event_frames (const Lock& lock, const Evoral::Event& ev, framepos_t source_start); - void mark_streaming_midi_write_started (NoteMode mode); - void mark_streaming_write_completed (); - void mark_midi_streaming_write_completed (Evoral::Sequence::StuckNoteOption, + void mark_streaming_midi_write_started (const Lock& lock, NoteMode mode); + void mark_streaming_write_completed (const Lock& lock); + void mark_midi_streaming_write_completed (const Lock& lock, + Evoral::Sequence::StuckNoteOption, Evoral::MusicalTime when = Evoral::MusicalTime()); XMLNode& get_state (); int set_state (const XMLNode&, int version); - void load_model (bool lock=true, bool force_reload=false); - void destroy_model (); - - void flush_midi (); - void ensure_disk_file (); + void load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload=false); + void destroy_model (const Glib::Threads::Mutex::Lock& lock); static bool safe_midi_file_extension (const std::string& path); static bool valid_midi_file (const std::string& path); @@ -75,6 +73,7 @@ public: protected: void set_path (const std::string& newpath); + void flush_midi (const Lock& lock); private: bool _open; @@ -87,13 +86,17 @@ public: int open_for_write (); - framecnt_t read_unlocked (Evoral::EventSink& dst, + void ensure_disk_file (const Lock& lock); + + framecnt_t read_unlocked (const Lock& lock, + Evoral::EventSink& dst, framepos_t position, framepos_t start, framecnt_t cnt, MidiStateTracker* tracker) const; - framecnt_t write_unlocked (MidiRingBuffer& src, + framecnt_t write_unlocked (const Lock& lock, + MidiRingBuffer& src, framepos_t position, framecnt_t cnt); diff --git a/libs/ardour/ardour/source.h b/libs/ardour/ardour/source.h index e009e0ef51..18de40771c 100644 --- a/libs/ardour/ardour/source.h +++ b/libs/ardour/ardour/source.h @@ -51,6 +51,8 @@ class LIBARDOUR_API Source : public SessionObject Empty = 0x100, /* used for MIDI only */ }; + typedef Glib::Threads::Mutex::Lock Lock; + Source (Session&, DataType type, const std::string& name, Flag flags=Flag(0)); Source (Session&, const XMLNode&); @@ -69,8 +71,8 @@ class LIBARDOUR_API Source : public SessionObject void mark_for_remove(); - virtual void mark_streaming_write_started () {} - virtual void mark_streaming_write_completed () = 0; + virtual void mark_streaming_write_started (const Lock& lock) {} + virtual void mark_streaming_write_completed (const Lock& lock) = 0; virtual void session_saved() {} diff --git a/libs/ardour/audio_diskstream.cc b/libs/ardour/audio_diskstream.cc index 4c3f7e6437..eb85538811 100644 --- a/libs/ardour/audio_diskstream.cc +++ b/libs/ardour/audio_diskstream.cc @@ -1771,13 +1771,15 @@ AudioDiskstream::prep_record_enable () for (ChannelList::iterator chan = c->begin(); chan != c->end(); ++chan) { (*chan)->source.request_input_monitoring (!(_session.config.get_auto_input() && rolling)); capturing_sources.push_back ((*chan)->write_source); - (*chan)->write_source->mark_streaming_write_started (); + (*chan)->write_source->mark_streaming_write_started ( + Source::Lock((*chan)->write_source->mutex())); } } else { for (ChannelList::iterator chan = c->begin(); chan != c->end(); ++chan) { capturing_sources.push_back ((*chan)->write_source); - (*chan)->write_source->mark_streaming_write_started (); + (*chan)->write_source->mark_streaming_write_started ( + Source::Lock((*chan)->write_source->mutex())); } } @@ -1963,7 +1965,8 @@ AudioDiskstream::reset_write_sources (bool mark_write_complete, bool /*force*/) if ((*chan)->write_source) { if (mark_write_complete) { - (*chan)->write_source->mark_streaming_write_completed (); + (*chan)->write_source->mark_streaming_write_completed ( + Source::Lock((*chan)->write_source->mutex())); (*chan)->write_source->done_with_peakfile_writes (); } diff --git a/libs/ardour/audiofilesource.cc b/libs/ardour/audiofilesource.cc index 40c3d2f012..6d9414a5cb 100644 --- a/libs/ardour/audiofilesource.cc +++ b/libs/ardour/audiofilesource.cc @@ -302,13 +302,13 @@ AudioFileSource::set_state (const XMLNode& node, int version) } void -AudioFileSource::mark_streaming_write_completed () +AudioFileSource::mark_streaming_write_completed (const Lock& lock) { if (!writable()) { return; } - AudioSource::mark_streaming_write_completed (); + AudioSource::mark_streaming_write_completed (lock); } int diff --git a/libs/ardour/audiosource.cc b/libs/ardour/audiosource.cc index b0b229a57a..93a0ca4fee 100644 --- a/libs/ardour/audiosource.cc +++ b/libs/ardour/audiosource.cc @@ -923,7 +923,7 @@ AudioSource::available_peaks (double zoom_factor) const } void -AudioSource::mark_streaming_write_completed () +AudioSource::mark_streaming_write_completed (const Lock& lock) { Glib::Threads::Mutex::Lock lm (_peaks_ready_lock); diff --git a/libs/ardour/import.cc b/libs/ardour/import.cc index 38a3da2fdc..2b3f105879 100644 --- a/libs/ardour/import.cc +++ b/libs/ardour/import.cc @@ -354,7 +354,9 @@ write_midi_data_to_new_files (Evoral::SMF* source, ImportStatus& status, boost::shared_ptr smfs = boost::dynamic_pointer_cast (*s); - smfs->drop_model (); + Glib::Threads::Mutex::Lock source_lock(smfs->mutex()); + + smfs->drop_model (source_lock); source->seek_to_track (i); uint64_t t = 0; @@ -384,11 +386,12 @@ write_midi_data_to_new_files (Evoral::SMF* source, ImportStatus& status, } if (first) { - smfs->mark_streaming_write_started (); + smfs->mark_streaming_write_started (source_lock); first = false; } - smfs->append_event_unlocked_beats( + smfs->append_event_beats( + source_lock, Evoral::Event( 0, Evoral::MusicalTime::ticks_at_rate(t, source->ppqn()), @@ -408,7 +411,7 @@ write_midi_data_to_new_files (Evoral::SMF* source, ImportStatus& status, const Evoral::MusicalTime length_beats = Evoral::MusicalTime::ticks_at_rate(t, source->ppqn()); BeatsFramesConverter converter(smfs->session().tempo_map(), pos); smfs->update_length(pos + converter.to(length_beats.round_up_to_beat())); - smfs->mark_streaming_write_completed (); + smfs->mark_streaming_write_completed (source_lock); if (status.cancel) { break; diff --git a/libs/ardour/midi_diskstream.cc b/libs/ardour/midi_diskstream.cc index 27ec409b15..e2fd6d1681 100644 --- a/libs/ardour/midi_diskstream.cc +++ b/libs/ardour/midi_diskstream.cc @@ -851,7 +851,8 @@ MidiDiskstream::do_flush (RunContext /*context*/, bool force_flush) } if (record_enabled() && ((total > disk_io_chunk_frames) || force_flush)) { - if (_write_source->midi_write (*_capture_buf, get_capture_start_frame (0), to_write) != to_write) { + Source::Lock lm(_write_source->mutex()); + if (_write_source->midi_write (lm, *_capture_buf, get_capture_start_frame (0), to_write) != to_write) { error << string_compose(_("MidiDiskstream %1: cannot write to disk"), id()) << endmsg; return -1; } @@ -919,6 +920,8 @@ MidiDiskstream::transport_stopped_wallclock (struct tm& /*when*/, time_t /*twhen /* phew, we have data */ + Source::Lock source_lock(_write_source->mutex()); + /* figure out the name for this take */ srcs.push_back (_write_source); @@ -936,7 +939,7 @@ MidiDiskstream::transport_stopped_wallclock (struct tm& /*when*/, time_t /*twhen where all the data is already on disk. */ - _write_source->mark_midi_streaming_write_completed (Evoral::Sequence::ResolveStuckNotes, total_capture_beats); + _write_source->mark_midi_streaming_write_completed (source_lock, Evoral::Sequence::ResolveStuckNotes, total_capture_beats); /* we will want to be able to keep (over)writing the source but we don't want it to be removable. this also differs @@ -1280,7 +1283,8 @@ MidiDiskstream::reset_write_sources (bool mark_write_complete, bool /*force*/) } if (_write_source && mark_write_complete) { - _write_source->mark_streaming_write_completed (); + Source::Lock lm(_write_source->mutex()); + _write_source->mark_streaming_write_completed (lm); } use_new_write_source (0); } diff --git a/libs/ardour/midi_model.cc b/libs/ardour/midi_model.cc index 6c2132562a..f64a5f6d0c 100644 --- a/libs/ardour/midi_model.cc +++ b/libs/ardour/midi_model.cc @@ -1426,25 +1426,23 @@ MidiModel::PatchChangeDiffCommand::get_state () * `Discrete' mode). */ bool -MidiModel::write_to (boost::shared_ptr source) +MidiModel::write_to (boost::shared_ptr source, + const Glib::Threads::Mutex::Lock& source_lock) { ReadLock lock(read_lock()); const bool old_percussive = percussive(); set_percussive(false); - boost::shared_ptr ms = _midi_source.lock (); - assert (ms); - - source->drop_model(); - source->mark_streaming_midi_write_started (note_mode()); + source->drop_model(source_lock); + source->mark_streaming_midi_write_started (source_lock, note_mode()); for (Evoral::Sequence::const_iterator i = begin(TimeType(), true); i != end(); ++i) { - source->append_event_unlocked_beats(*i); + source->append_event_beats(source_lock, *i); } set_percussive(old_percussive); - source->mark_streaming_write_completed(); + source->mark_streaming_write_completed(source_lock); set_edited(false); @@ -1457,7 +1455,7 @@ MidiModel::write_to (boost::shared_ptr source) of the model. */ bool -MidiModel::sync_to_source () +MidiModel::sync_to_source (const Glib::Threads::Mutex::Lock& source_lock) { ReadLock lock(read_lock()); @@ -1465,16 +1463,19 @@ MidiModel::sync_to_source () set_percussive(false); boost::shared_ptr ms = _midi_source.lock (); - assert (ms); + if (!ms) { + error << "MIDI model has no source to sync to" << endmsg; + return false; + } - ms->mark_streaming_midi_write_started (note_mode()); + ms->mark_streaming_midi_write_started (source_lock, note_mode()); for (Evoral::Sequence::const_iterator i = begin(TimeType(), true); i != end(); ++i) { - ms->append_event_unlocked_beats(*i); + ms->append_event_beats(source_lock, *i); } set_percussive (old_percussive); - ms->mark_streaming_write_completed (); + ms->mark_streaming_write_completed (source_lock); set_edited (false); @@ -1489,7 +1490,10 @@ MidiModel::sync_to_source () * destroying the original note durations. */ bool -MidiModel::write_section_to (boost::shared_ptr source, Evoral::MusicalTime begin_time, Evoral::MusicalTime end_time) +MidiModel::write_section_to (boost::shared_ptr source, + const Glib::Threads::Mutex::Lock& source_lock, + Evoral::MusicalTime begin_time, + Evoral::MusicalTime end_time) { ReadLock lock(read_lock()); MidiStateTracker mst; @@ -1497,11 +1501,8 @@ MidiModel::write_section_to (boost::shared_ptr source, Evoral::Music const bool old_percussive = percussive(); set_percussive(false); - boost::shared_ptr ms = _midi_source.lock (); - assert (ms); - - source->drop_model(); - source->mark_streaming_midi_write_started (note_mode()); + source->drop_model(source_lock); + source->mark_streaming_midi_write_started (source_lock, note_mode()); for (Evoral::Sequence::const_iterator i = begin(TimeType(), true); i != end(); ++i) { const Evoral::Event& ev (*i); @@ -1526,22 +1527,22 @@ MidiModel::write_section_to (boost::shared_ptr source, Evoral::Music continue; } - source->append_event_unlocked_beats (*i); + source->append_event_beats (source_lock, *i); mst.remove (mev->note(), mev->channel()); } else if (mev->is_note_on()) { mst.add (mev->note(), mev->channel()); - source->append_event_unlocked_beats(*i); + source->append_event_beats(source_lock, *i); } else { - source->append_event_unlocked_beats(*i); + source->append_event_beats(source_lock, *i); } } } - mst.resolve_notes (*source, end_time); + mst.resolve_notes (*source, source_lock, end_time); set_percussive(old_percussive); - source->mark_streaming_write_completed(); + source->mark_streaming_write_completed(source_lock); set_edited(false); @@ -1630,7 +1631,7 @@ MidiModel::edit_lock() assert (ms); Glib::Threads::Mutex::Lock* source_lock = new Glib::Threads::Mutex::Lock (ms->mutex()); - ms->invalidate(); // Release cached iterator's read lock on model + ms->invalidate(*source_lock); // Release cached iterator's read lock on model return WriteLock(new WriteLockImpl(source_lock, _lock, _control_lock)); } @@ -1855,7 +1856,8 @@ MidiModel::set_midi_source (boost::shared_ptr s) boost::shared_ptr old = _midi_source.lock (); if (old) { - old->invalidate (); + Source::Lock lm(old->mutex()); + old->invalidate (lm); } _midi_source_connections.drop_connections (); diff --git a/libs/ardour/midi_playlist_source.cc b/libs/ardour/midi_playlist_source.cc index cd5362c3da..587bc7b12f 100644 --- a/libs/ardour/midi_playlist_source.cc +++ b/libs/ardour/midi_playlist_source.cc @@ -122,7 +122,8 @@ MidiPlaylistSource::length (framepos_t) const } framecnt_t -MidiPlaylistSource::read_unlocked (Evoral::EventSink& dst, +MidiPlaylistSource::read_unlocked (const Lock& lock, + Evoral::EventSink& dst, framepos_t /*position*/, framepos_t start, framecnt_t cnt, MidiStateTracker*) const @@ -137,7 +138,8 @@ MidiPlaylistSource::read_unlocked (Evoral::EventSink& dst, } framecnt_t -MidiPlaylistSource::write_unlocked (MidiRingBuffer&, +MidiPlaylistSource::write_unlocked (const Lock&, + MidiRingBuffer&, framepos_t, framecnt_t) { @@ -147,33 +149,33 @@ MidiPlaylistSource::write_unlocked (MidiRingBuffer&, } void -MidiPlaylistSource::append_event_unlocked_beats(const Evoral::Event& /*ev*/) +MidiPlaylistSource::append_event_beats(const Glib::Threads::Mutex::Lock& /*lock*/, const Evoral::Event& /*ev*/) { - fatal << string_compose (_("programming error: %1"), "MidiPlaylistSource::append_event_unlocked_beats() called - should be impossible") << endmsg; + fatal << string_compose (_("programming error: %1"), "MidiPlaylistSource::append_event_beats() called - should be impossible") << endmsg; abort(); /*NOTREACHED*/ } void -MidiPlaylistSource::append_event_unlocked_frames(const Evoral::Event& /* ev */, framepos_t /*source_start*/) +MidiPlaylistSource::append_event_frames(const Glib::Threads::Mutex::Lock& /*lock*/, const Evoral::Event& /* ev */, framepos_t /*source_start*/) { - fatal << string_compose (_("programming error: %1"), "MidiPlaylistSource::append_event_unlocked_frames() called - should be impossible") << endmsg; + fatal << string_compose (_("programming error: %1"), "MidiPlaylistSource::append_event_frames() called - should be impossible") << endmsg; abort(); /*NOTREACHED*/ } void -MidiPlaylistSource::load_model (bool, bool) +MidiPlaylistSource::load_model (const Glib::Threads::Mutex::Lock&, bool) { /* nothing to do */ } void -MidiPlaylistSource::destroy_model () +MidiPlaylistSource::destroy_model (const Glib::Threads::Mutex::Lock&) { /* nothing to do */ } void -MidiPlaylistSource::flush_midi () +MidiPlaylistSource::flush_midi (const Lock& lock) { } diff --git a/libs/ardour/midi_region.cc b/libs/ardour/midi_region.cc index f6631b9de5..f79e5ef203 100644 --- a/libs/ardour/midi_region.cc +++ b/libs/ardour/midi_region.cc @@ -150,8 +150,11 @@ MidiRegion::clone (boost::shared_ptr newsrc) const Evoral::MusicalTime const bbegin = bfc.from (_start); Evoral::MusicalTime const bend = bfc.from (_start + _length); - if (midi_source(0)->write_to (newsrc, bbegin, bend)) { - return boost::shared_ptr (); + { + Source::Lock lm(newsrc->mutex()); + if (midi_source(0)->write_to (lm, newsrc, bbegin, bend)) { + return boost::shared_ptr (); + } } PropertyList plist; @@ -272,7 +275,10 @@ MidiRegion::_read_at (const SourceList& /*srcs*/, Evoral::EventSink& } boost::shared_ptr src = midi_source(chan_n); - src->set_note_mode(mode); + + Glib::Threads::Mutex::Lock lm(src->mutex()); + + src->set_note_mode(lm, mode); /* cerr << "MR " << name () << " read @ " << position << " * " << to_read @@ -285,6 +291,7 @@ MidiRegion::_read_at (const SourceList& /*srcs*/, Evoral::EventSink& /* This call reads events from a source and writes them to `dst' timed in session frames */ if (src->midi_read ( + lm, // source lock dst, // destination buffer _position - _start, // start position of the source in session frames _start + internal_offset, // where to start reading in the source @@ -429,7 +436,7 @@ MidiRegion::model_automation_state_changed (Evoral::Parameter const & p) the iterator. */ Glib::Threads::Mutex::Lock lm (midi_source(0)->mutex()); - midi_source(0)->invalidate (); + midi_source(0)->invalidate (lm); } /** This is called when a trim drag has resulted in a -ve _start time for this region. 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 */ } diff --git a/libs/ardour/midi_state_tracker.cc b/libs/ardour/midi_state_tracker.cc index afe6f07db7..f919a02f35 100644 --- a/libs/ardour/midi_state_tracker.cc +++ b/libs/ardour/midi_state_tracker.cc @@ -168,7 +168,7 @@ MidiStateTracker::resolve_notes (Evoral::EventSink &dst, framepos_t } void -MidiStateTracker::resolve_notes (MidiSource& src, Evoral::MusicalTime time) +MidiStateTracker::resolve_notes (MidiSource& src, const MidiSource::Lock& lock, Evoral::MusicalTime time) { DEBUG_TRACE (PBD::DEBUG::MidiTrackers, string_compose ("%1 MS-resolve notes @ %2 on = %3\n", this, time, _on)); @@ -186,7 +186,7 @@ MidiStateTracker::resolve_notes (MidiSource& src, Evoral::MusicalTime time) ev.set_channel (channel); ev.set_note (note); ev.set_velocity (0); - src.append_event_unlocked_beats (ev); + src.append_event_beats (lock, ev); DEBUG_TRACE (PBD::DEBUG::MidiTrackers, string_compose ("%1: MS-resolved note %2/%3 at %4\n", this, (int) note, (int) channel, time)); _active_notes[note + 128 * channel]--; diff --git a/libs/ardour/midi_stretch.cc b/libs/ardour/midi_stretch.cc index f5b3a47b41..0e75cdca1d 100644 --- a/libs/ardour/midi_stretch.cc +++ b/libs/ardour/midi_stretch.cc @@ -76,7 +76,7 @@ MidiStretch::run (boost::shared_ptr r, Progress*) return -1; boost::shared_ptr src = region->midi_source(0); - src->load_model(); + src->load_model(Glib::Threads::Mutex::Lock(src->mutex())); boost::shared_ptr old_model = src->model(); @@ -88,7 +88,7 @@ MidiStretch::run (boost::shared_ptr r, Progress*) Glib::Threads::Mutex::Lock sl (new_src->mutex ()); - new_src->load_model(false, true); + new_src->load_model(sl, true); boost::shared_ptr new_model = new_src->model(); new_model->start_write(); diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc index b25977fe8d..519d8bbf10 100644 --- a/libs/ardour/smf_source.cc +++ b/libs/ardour/smf_source.cc @@ -203,7 +203,8 @@ SMFSource::open_for_write () /** All stamps in audio frames */ framecnt_t -SMFSource::read_unlocked (Evoral::EventSink& destination, +SMFSource::read_unlocked (const Lock& lock, + Evoral::EventSink& destination, framepos_t const source_start, framepos_t start, framecnt_t duration, @@ -303,12 +304,13 @@ SMFSource::read_unlocked (Evoral::EventSink& destination, } framecnt_t -SMFSource::write_unlocked (MidiRingBuffer& source, +SMFSource::write_unlocked (const Lock& lock, + MidiRingBuffer& source, framepos_t position, framecnt_t cnt) { if (!_writing) { - mark_streaming_write_started (); + mark_streaming_write_started (lock); } framepos_t time; @@ -372,7 +374,7 @@ SMFSource::write_unlocked (MidiRingBuffer& source, continue; } - append_event_unlocked_frames(ev, position); + append_event_frames(lock, ev, position); } Evoral::SMF::flush (); @@ -383,13 +385,14 @@ SMFSource::write_unlocked (MidiRingBuffer& source, /** Append an event with a timestamp in beats */ void -SMFSource::append_event_unlocked_beats (const Evoral::Event& ev) +SMFSource::append_event_beats (const Glib::Threads::Mutex::Lock& lock, + const Evoral::Event& ev) { if (!_writing || ev.size() == 0) { return; } - /*printf("SMFSource: %s - append_event_unlocked_beats ID = %d time = %lf, size = %u, data = ", + /*printf("SMFSource: %s - append_event_beats ID = %d time = %lf, size = %u, data = ", name().c_str(), ev.id(), ev.time(), ev.size()); for (size_t i = 0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n");*/ @@ -435,13 +438,15 @@ SMFSource::append_event_unlocked_beats (const Evoral::Event /** Append an event with a timestamp in frames (framepos_t) */ void -SMFSource::append_event_unlocked_frames (const Evoral::Event& ev, framepos_t position) +SMFSource::append_event_frames (const Glib::Threads::Mutex::Lock& lock, + const Evoral::Event& ev, + framepos_t position) { if (!_writing || ev.size() == 0) { return; } - // printf("SMFSource: %s - append_event_unlocked_frames ID = %d time = %u, size = %u, data = ", + // printf("SMFSource: %s - append_event_frames ID = %d time = %u, size = %u, data = ", // name().c_str(), ev.id(), ev.time(), ev.size()); // for (size_t i=0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n"); @@ -508,33 +513,30 @@ SMFSource::set_state (const XMLNode& node, int version) } void -SMFSource::mark_streaming_midi_write_started (NoteMode mode) +SMFSource::mark_streaming_midi_write_started (const Lock& lock, NoteMode mode) { - /* CALLER MUST HOLD LOCK */ - if (!_open && open_for_write()) { error << string_compose (_("cannot open MIDI file %1 for write"), _path) << endmsg; /* XXX should probably throw or return something */ return; } - MidiSource::mark_streaming_midi_write_started (mode); + MidiSource::mark_streaming_midi_write_started (lock, mode); Evoral::SMF::begin_write (); _last_ev_time_beats = Evoral::MusicalTime(); _last_ev_time_frames = 0; } void -SMFSource::mark_streaming_write_completed () +SMFSource::mark_streaming_write_completed (const Lock& lock) { - mark_midi_streaming_write_completed (Evoral::Sequence::DeleteStuckNotes); + mark_midi_streaming_write_completed (lock, Evoral::Sequence::DeleteStuckNotes); } void -SMFSource::mark_midi_streaming_write_completed (Evoral::Sequence::StuckNoteOption stuck_notes_option, Evoral::MusicalTime when) +SMFSource::mark_midi_streaming_write_completed (const Lock& lm, Evoral::Sequence::StuckNoteOption stuck_notes_option, Evoral::MusicalTime when) { - Glib::Threads::Mutex::Lock lm (_lock); - MidiSource::mark_midi_streaming_write_completed (stuck_notes_option, when); + MidiSource::mark_midi_streaming_write_completed (lm, stuck_notes_option, when); if (!writable()) { warning << string_compose ("attempt to write to unwritable SMF file %1", _path) << endmsg; @@ -596,16 +598,12 @@ static bool compare_eventlist ( } void -SMFSource::load_model (bool lock, bool force_reload) +SMFSource::load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload) { if (_writing) { return; } - boost::shared_ptr lm; - if (lock) - lm = boost::shared_ptr(new Glib::Threads::Mutex::Lock(_lock)); - if (_model && !force_reload) { return; } @@ -616,7 +614,7 @@ SMFSource::load_model (bool lock, bool force_reload) _model->clear(); } - invalidate(); + invalidate(lock); if (writable() && !_open) { return; @@ -707,33 +705,33 @@ SMFSource::load_model (bool lock, bool force_reload) _model->end_write (Evoral::Sequence::ResolveStuckNotes, _length_beats); _model->set_edited (false); - invalidate(); + invalidate(lock); free(buf); } void -SMFSource::destroy_model () +SMFSource::destroy_model (const Glib::Threads::Mutex::Lock& lock) { //cerr << _name << " destroying model " << _model.get() << endl; _model.reset(); - invalidate(); + invalidate(lock); } void -SMFSource::flush_midi () +SMFSource::flush_midi (const Lock& lock) { if (!writable() || _length_beats == 0.0) { return; } - ensure_disk_file (); + ensure_disk_file (lock); Evoral::SMF::end_write (); /* data in the file means its no longer removable */ mark_nonremovable (); - invalidate(); + invalidate(lock); } void @@ -745,7 +743,7 @@ SMFSource::set_path (const string& p) /** Ensure that this source has some file on disk, even if it's just a SMF header */ void -SMFSource::ensure_disk_file () +SMFSource::ensure_disk_file (const Lock& lock) { if (!writable()) { return; @@ -757,9 +755,9 @@ SMFSource::ensure_disk_file () */ boost::shared_ptr mm = _model; _model.reset (); - mm->sync_to_source (); + mm->sync_to_source (lock); _model = mm; - invalidate(); + invalidate(lock); } else { /* No model; if it's not already open, it's an empty source, so create and open it for writing. diff --git a/libs/ardour/source_factory.cc b/libs/ardour/source_factory.cc index 6d2bb80b30..4005148564 100644 --- a/libs/ardour/source_factory.cc +++ b/libs/ardour/source_factory.cc @@ -205,7 +205,7 @@ SourceFactory::create (Session& s, const XMLNode& node, bool defer_peaks) } } else if (type == DataType::MIDI) { boost::shared_ptr src (new SMFSource (s, node)); - src->load_model (true, true); + src->load_model (Glib::Threads::Mutex::Lock(src->mutex()), true); #ifdef BOOST_SP_ENABLE_DEBUG_HOOKS // boost_debug_shared_ptr_mark_interesting (src, "Source"); #endif @@ -273,7 +273,7 @@ SourceFactory::createExternal (DataType type, Session& s, const string& path, } else if (type == DataType::MIDI) { boost::shared_ptr src (new SMFSource (s, path)); - src->load_model (true, true); + src->load_model (Glib::Threads::Mutex::Lock(src->mutex()), true); #ifdef BOOST_SP_ENABLE_DEBUG_HOOKS // boost_debug_shared_ptr_mark_interesting (src, "Source"); #endif @@ -324,7 +324,7 @@ SourceFactory::createWritable (DataType type, Session& s, const std::string& pat boost::shared_ptr src (new SMFSource (s, path, SndFileSource::default_writable_flags)); assert (src->writable ()); - src->load_model (true, true); + src->load_model (Glib::Threads::Mutex::Lock(src->mutex()), true); #ifdef BOOST_SP_ENABLE_DEBUG_HOOKS // boost_debug_shared_ptr_mark_interesting (src, "Source"); #endif -- cgit v1.2.3