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
54597bd803
commit
1686db8b0c
@ -79,14 +79,6 @@ AutomationStreamView::add_region_view_internal (boost::shared_ptr<Region> region
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (wait_for_data) {
|
||||
boost::shared_ptr<MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(region);
|
||||
if (mr) {
|
||||
Source::Lock lock(mr->midi_source()->mutex());
|
||||
mr->midi_source()->load_model(lock);
|
||||
}
|
||||
}
|
||||
|
||||
const boost::shared_ptr<AutomationControl> control = boost::dynamic_pointer_cast<AutomationControl> (
|
||||
region->control (_automation_view.parameter(), true)
|
||||
);
|
||||
|
@ -257,11 +257,6 @@ MidiRegionView::init (bool wfd)
|
||||
{
|
||||
PublicEditor::DropDownKeys.connect (sigc::mem_fun (*this, &MidiRegionView::drop_down_keys));
|
||||
|
||||
if (wfd) {
|
||||
Glib::Threads::Mutex::Lock lm(midi_region()->midi_source(0)->mutex());
|
||||
midi_region()->midi_source(0)->load_model(lm);
|
||||
}
|
||||
|
||||
_model = midi_region()->midi_source(0)->model();
|
||||
_enable_display = false;
|
||||
fill_color_name = "midi frame base";
|
||||
|
@ -193,11 +193,6 @@ MidiStreamView::display_region(MidiRegionView* region_view, bool load_model)
|
||||
return;
|
||||
}
|
||||
|
||||
if (load_model) {
|
||||
Glib::Threads::Mutex::Lock lm(source->mutex());
|
||||
source->load_model(lm);
|
||||
}
|
||||
|
||||
if (!source->model()) {
|
||||
error << _("attempt to display MIDI region with no model") << endmsg;
|
||||
return;
|
||||
@ -225,8 +220,6 @@ MidiStreamView::update_contents_metrics(boost::shared_ptr<Region> r)
|
||||
{
|
||||
boost::shared_ptr<MidiRegion> mr = boost::dynamic_pointer_cast<MidiRegion>(r);
|
||||
if (mr) {
|
||||
Glib::Threads::Mutex::Lock lm(mr->midi_source(0)->mutex());
|
||||
mr->midi_source(0)->load_model(lm);
|
||||
_range_dirty = update_data_note_range(
|
||||
mr->model()->lowest_note(),
|
||||
mr->model()->highest_note());
|
||||
|
@ -67,8 +67,8 @@ public:
|
||||
XMLNode& get_state ();
|
||||
int set_state (const XMLNode&, int version);
|
||||
|
||||
void load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload=false);
|
||||
void destroy_model (const Glib::Threads::Mutex::Lock& lock);
|
||||
void load_model (const WriterLock& lock, bool force_reload=false);
|
||||
void destroy_model (const WriterLock& lock);
|
||||
|
||||
static bool safe_midi_file_extension (const std::string& path);
|
||||
static bool valid_midi_file (const std::string& path);
|
||||
@ -110,6 +110,8 @@ public:
|
||||
timepos_t const & position,
|
||||
timecnt_t const & cnt);
|
||||
|
||||
void load_model_unlocked (bool force_reload=false);
|
||||
|
||||
};
|
||||
|
||||
}; /* namespace ARDOUR */
|
||||
|
@ -474,6 +474,14 @@ write_midi_data_to_new_files (Evoral::SMF* source, ImportStatus& status,
|
||||
smfs->update_length (timepos_t (length_beats.round_up_to_multiple(Temporal::Beats(pulses_per_bar,0))));
|
||||
|
||||
smfs->mark_streaming_write_completed (source_lock);
|
||||
|
||||
/* the streaming write that we've just finished
|
||||
* only wrote data to the SMF object, which is
|
||||
* ultimately an on-disk data structure. So now
|
||||
* we pull the data back from disk to build our
|
||||
* in-memory MidiModel version.
|
||||
*/
|
||||
|
||||
smfs->load_model (source_lock, true);
|
||||
|
||||
if (status.cancel) {
|
||||
|
@ -143,11 +143,6 @@ MidiRegion::clone (boost::shared_ptr<MidiSource> newsrc, ThawList* tl) const
|
||||
|
||||
{
|
||||
boost::shared_ptr<MidiSource> ms = midi_source(0);
|
||||
Source::Lock lm (ms->mutex());
|
||||
|
||||
if (!ms->model()) {
|
||||
ms->load_model (lm);
|
||||
}
|
||||
|
||||
/* Lock our source since we'll be reading from it. write_to() will
|
||||
take a lock on newsrc.
|
||||
|
@ -94,6 +94,9 @@ SMFSource::SMFSource (Session& s, const string& path, Source::Flag flags)
|
||||
_open = true;
|
||||
}
|
||||
|
||||
/* there's no data to load into the model but create it anyway */
|
||||
|
||||
_model = boost::shared_ptr<MidiModel> (new MidiModel (*this));
|
||||
}
|
||||
|
||||
/** Constructor used for external-to-session files. File must exist. */
|
||||
@ -119,6 +122,9 @@ SMFSource::SMFSource (Session& s, const string& path)
|
||||
}
|
||||
|
||||
_open = true;
|
||||
|
||||
/* no lock required since we do not actually exist yet */
|
||||
load_model_unlocked (true);
|
||||
}
|
||||
|
||||
/** Constructor used for existing internal-to-session files. */
|
||||
@ -180,6 +186,8 @@ SMFSource::SMFSource (Session& s, const XMLNode& node, bool must_exist)
|
||||
/* no fd left open here */
|
||||
}
|
||||
|
||||
/* no lock required since we do not actually exist yet */
|
||||
load_model_unlocked (true);
|
||||
}
|
||||
|
||||
SMFSource::~SMFSource ()
|
||||
@ -634,27 +642,27 @@ static bool compare_eventlist (
|
||||
void
|
||||
SMFSource::load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload)
|
||||
{
|
||||
if (_writing) {
|
||||
return;
|
||||
invalidate (lock);
|
||||
load_model_unlocked (force_reload);
|
||||
invalidate (lock);
|
||||
}
|
||||
|
||||
if (_model && !force_reload) {
|
||||
return;
|
||||
}
|
||||
void
|
||||
SMFSource::load_model_unlocked (bool force_reload)
|
||||
{
|
||||
std::cerr << "\n\n\n\n\n";
|
||||
PBD::stacktrace (std::cerr, 8);
|
||||
|
||||
assert (!_writing);
|
||||
|
||||
std::cerr << "\n***** " << path() << " ACTUALLY LOADING, existing model: " << _model << std::endl;
|
||||
|
||||
if (!_model) {
|
||||
boost::shared_ptr<SMFSource> smf = boost::dynamic_pointer_cast<SMFSource> ( shared_from_this () );
|
||||
_model = boost::shared_ptr<MidiModel> (new MidiModel (smf));
|
||||
_model = boost::shared_ptr<MidiModel> (new MidiModel (*this));
|
||||
} else {
|
||||
_model->clear();
|
||||
}
|
||||
|
||||
invalidate(lock);
|
||||
|
||||
if (writable() && !_open) {
|
||||
return;
|
||||
}
|
||||
|
||||
_model->start_write();
|
||||
Evoral::SMF::seek_to_start();
|
||||
|
||||
@ -768,7 +776,6 @@ SMFSource::load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload
|
||||
|
||||
_model->end_write (Evoral::Sequence<Temporal::Beats>::ResolveStuckNotes, _length.beats());
|
||||
_model->set_edited (false);
|
||||
invalidate(lock);
|
||||
|
||||
free (buf);
|
||||
}
|
||||
|
@ -234,8 +234,6 @@ SourceFactory::create (Session& s, const XMLNode& node, bool defer_peaks)
|
||||
} else if (type == DataType::MIDI) {
|
||||
try {
|
||||
boost::shared_ptr<SMFSource> src (new SMFSource (s, node));
|
||||
Source::Lock lock (src->mutex ());
|
||||
src->load_model (lock, true);
|
||||
BOOST_MARK_SOURCE (src);
|
||||
src->check_for_analysis_data_on_disk ();
|
||||
SourceCreated (src);
|
||||
@ -308,8 +306,6 @@ SourceFactory::createExternal (DataType type, Session& s, const string& path,
|
||||
} else if (type == DataType::MIDI) {
|
||||
try {
|
||||
boost::shared_ptr<SMFSource> src (new SMFSource (s, path));
|
||||
Source::Lock lock (src->mutex ());
|
||||
src->load_model (lock, true);
|
||||
BOOST_MARK_SOURCE (src);
|
||||
|
||||
if (announce) {
|
||||
@ -357,8 +353,6 @@ SourceFactory::createWritable (DataType type, Session& s, const std::string& pat
|
||||
|
||||
assert (src->writable ());
|
||||
|
||||
Source::Lock lock (src->mutex ());
|
||||
src->load_model (lock, true);
|
||||
BOOST_MARK_SOURCE (src);
|
||||
|
||||
// no analysis data - this is a new file
|
||||
|
Loading…
Reference in New Issue
Block a user