Mercurial > hg > svapp
changeset 691:c8ba09756eff by-id
Work on management of alignment-related models
| author | Chris Cannam |
|---|---|
| date | Fri, 12 Jul 2019 13:58:02 +0100 |
| parents | 827a522a5da4 |
| children | ad5917362158 |
| files | framework/Align.cpp framework/Align.h framework/Document.cpp framework/Document.h framework/MainWindowBase.cpp framework/SVFileReader.cpp |
| diffstat | 6 files changed, 179 insertions(+), 129 deletions(-) [+] |
line wrap: on
line diff
--- a/framework/Align.cpp Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/Align.cpp Fri Jul 12 13:58:02 2019 +0100 @@ -103,40 +103,47 @@ if (!reference || !other) return false; - // This involves creating either three or four new models: + // This involves creating a number of new models: // // 1. an AggregateWaveModel to provide the mixdowns of the main // model and the new model in its two channels, as input to the - // MATCH plugin + // MATCH plugin. We just call this one aggregateModel // // 2a. a SparseTimeValueModel which will be automatically created // by FeatureExtractionModelTransformer when running the // TuningDifference plugin to receive the relative tuning of the // second model (if pitch-aware alignment is enabled in the - // preferences) + // preferences). We call this tuningDiffOutputModel. // // 2b. a SparseTimeValueModel which will be automatically created // by FeatureExtractionPluginTransformer when running the MATCH - // plugin to perform alignment (so containing the alignment path) + // plugin to perform alignment (so containing the alignment path). + // We call this one pathOutputModel. // - // 3. an AlignmentModel, which stores the path model and carries - // out alignment lookups on it. + // 2c. a SparseTimeValueModel used solely to provide faked + // completion information to the AlignmentModel while a + // TuningDifference calculation is going on. We call this + // preparatoryModel. // - // The AggregateWaveModel [1] is registered with the document, - // which deletes it when it is invalidated (when one of its - // components is deleted). The SparseTimeValueModel [2a] is reused - // by us when starting the alignment process proper, and is then - // deleted by us. The SparseTimeValueModel [2b] is passed to the - // AlignmentModel, which takes ownership of it. The AlignmentModel - // is attached to the new model we are aligning, which also takes - // ownership of it. The only one of these models that we need to - // delete here is the SparseTimeValueModel [2a]. - //!!! todo: review the above, especially management of AlignmentModel + // 3. an AlignmentModel, which stores the path and carries out + // alignment lookups on it. We just call this one alignmentModel. // - // (We also create a sneaky additional SparseTimeValueModel - // temporarily so we can attach completion information to it - - // this is quite unnecessary from the perspective of simply - // producing the results.) + // Models 1 and 3 are registered with the document, which will + // eventually release them. We don't release them here except in + // the case where an activity fails before the point where we + // would otherwise have registered them with the document. + // + // Models 2a (tuningDiffOutputModel), 2b (pathOutputModel) and 2c + // (preparatoryModel) are not registered with the document. Model + // 2b (pathOutputModel) is not registered because we do not have a + // stable reference to the document at the point where it is + // created. Model 2c (preparatoryModel) is not registered because + // it is a bodge that we are embarrassed about, so we try to + // manage it ourselves without anyone else noticing. Model 2a is + // not registered for symmetry with the other two. These have to + // be released by us when finished with, but their lifespans do + // not extend beyond the end of the alignment procedure, so this + // should be ok. AggregateWaveModel::ChannelSpecList components; @@ -148,7 +155,7 @@ auto aggregateModel = std::make_shared<AggregateWaveModel>(components); auto aggregateModelId = ModelById::add(aggregateModel); - doc->addAggregateModel(aggregateModelId); + doc->addNonDerivedModel(aggregateModelId); auto alignmentModel = std::make_shared<AlignmentModel> (referenceId, otherId, ModelId()); @@ -161,6 +168,7 @@ if (beginTransformDrivenAlignment(aggregateModelId, alignmentModelId)) { other->setAlignment(alignmentModelId); + doc->addNonDerivedModel(alignmentModelId); } else { error = alignmentModel->getError(); ModelById::release(alignmentModel); @@ -186,20 +194,24 @@ ModelTransformerFactory *mtf = ModelTransformerFactory::getInstance(); QString message; - ModelId transformOutput = mtf->transform(transform, - aggregateModelId, - message); + ModelId tuningDiffOutputModelId = mtf->transform(transform, + aggregateModelId, + message); - auto tdout = ModelById::getAs<SparseTimeValueModel>(transformOutput); - if (!tdout) { + auto tuningDiffOutputModel = + ModelById::getAs<SparseTimeValueModel>(tuningDiffOutputModelId); + if (!tuningDiffOutputModel) { SVCERR << "Align::alignModel: ERROR: Failed to create tuning-difference output model (no Tuning Difference plugin?)" << endl; error = message; + ModelById::release(alignmentModel); return false; } other->setAlignment(alignmentModelId); + doc->addNonDerivedModel(alignmentModelId); - connect(tdout.get(), SIGNAL(completionChanged(ModelId)), + connect(tuningDiffOutputModel.get(), + SIGNAL(completionChanged(ModelId)), this, SLOT(tuningDifferenceCompletionChanged(ModelId))); TuningDiffRec rec; @@ -216,43 +228,45 @@ rec.preparatory = preparatoryModelId; alignmentModel->setPathFrom(rec.preparatory); - m_pendingTuningDiffs[transformOutput] = rec; + m_pendingTuningDiffs[tuningDiffOutputModelId] = rec; } return true; } void -Align::tuningDifferenceCompletionChanged(ModelId tdId) +Align::tuningDifferenceCompletionChanged(ModelId tuningDiffOutputModelId) { QMutexLocker locker(&m_mutex); - if (m_pendingTuningDiffs.find(tdId) == m_pendingTuningDiffs.end()) { + if (m_pendingTuningDiffs.find(tuningDiffOutputModelId) == + m_pendingTuningDiffs.end()) { SVCERR << "ERROR: Align::tuningDifferenceCompletionChanged: Model " - << tdId << " not found in pending tuning diff map!" << endl; + << tuningDiffOutputModelId + << " not found in pending tuning diff map!" << endl; return; } - auto td = ModelById::getAs<SparseTimeValueModel>(tdId); - if (!td) { + auto tuningDiffOutputModel = + ModelById::getAs<SparseTimeValueModel>(tuningDiffOutputModelId); + if (!tuningDiffOutputModel) { SVCERR << "WARNING: Align::tuningDifferenceCompletionChanged: Model " - << tdId << " not known as SparseTimeValueModel" << endl; + << tuningDiffOutputModelId + << " not known as SparseTimeValueModel" << endl; return; } - TuningDiffRec rec = m_pendingTuningDiffs[tdId]; + TuningDiffRec rec = m_pendingTuningDiffs[tuningDiffOutputModelId]; - auto alignment = ModelById::getAs<AlignmentModel>(rec.alignment); - if (!alignment) { + auto alignmentModel = ModelById::getAs<AlignmentModel>(rec.alignment); + if (!alignmentModel) { SVCERR << "WARNING: Align::tuningDifferenceCompletionChanged:" << "alignment model has disappeared" << endl; return; } int completion = 0; - bool done = td->isReady(&completion); - -// SVCERR << "Align::tuningDifferenceCompletionChanged: done = " << done << ", completion = " << completion << endl; + bool done = tuningDiffOutputModel->isReady(&completion); if (!done) { // This will be the completion the alignment model reports, @@ -260,28 +274,30 @@ // 99 (not 100!) and then back to 0 again when we start // calculating the actual path in the following phase int clamped = (completion == 100 ? 99 : completion); -// SVCERR << "Align::tuningDifferenceCompletionChanged: setting rec.preparatory completion to " << clamped << endl; - auto preparatory = ModelById::getAs<SparseTimeValueModel> - (rec.preparatory); - if (preparatory) { - preparatory->setCompletion(clamped); + auto preparatoryModel = + ModelById::getAs<SparseTimeValueModel>(rec.preparatory); + if (preparatoryModel) { + preparatoryModel->setCompletion(clamped); } return; } float tuningFrequency = 440.f; - if (!td->isEmpty()) { - tuningFrequency = td->getAllEvents()[0].getValue(); + if (!tuningDiffOutputModel->isEmpty()) { + tuningFrequency = tuningDiffOutputModel->getAllEvents()[0].getValue(); SVCERR << "Align::tuningDifferenceCompletionChanged: Reported tuning frequency = " << tuningFrequency << endl; } else { SVCERR << "Align::tuningDifferenceCompletionChanged: No tuning frequency reported" << endl; } - m_pendingTuningDiffs.erase(tdId); - ModelById::release(tdId); + ModelById::release(tuningDiffOutputModel); - alignment->setPathFrom({}); + alignmentModel->setPathFrom({}); // replace preparatoryModel + ModelById::release(rec.preparatory); + rec.preparatory = {}; + + m_pendingTuningDiffs.erase(tuningDiffOutputModelId); beginTransformDrivenAlignment (rec.input, rec.alignment, tuningFrequency); @@ -296,8 +312,10 @@ TransformFactory *tf = TransformFactory::getInstance(); - auto aggregateModel = ModelById::getAs<AggregateWaveModel>(aggregateModelId); - auto alignmentModel = ModelById::getAs<AlignmentModel>(alignmentModelId); + auto aggregateModel = + ModelById::getAs<AggregateWaveModel>(aggregateModelId); + auto alignmentModel = + ModelById::getAs<AlignmentModel>(alignmentModelId); if (!aggregateModel || !alignmentModel) { SVCERR << "Align::alignModel: ERROR: One or other of the aggregate & alignment models has disappeared" << endl; @@ -320,29 +338,31 @@ ModelTransformerFactory *mtf = ModelTransformerFactory::getInstance(); QString message; - ModelId transformOutput = mtf->transform + ModelId pathOutputModelId = mtf->transform (transform, aggregateModelId, message); - if (transformOutput.isNone()) { + if (pathOutputModelId.isNone()) { transform.setStepSize(0); - transformOutput = mtf->transform + pathOutputModelId = mtf->transform (transform, aggregateModelId, message); } - auto path = ModelById::getAs<SparseTimeValueModel>(transformOutput); + auto pathOutputModel = + ModelById::getAs<SparseTimeValueModel>(pathOutputModelId); //!!! callers will need to be updated to get error from //!!! alignment model after initial call - if (!path) { + if (!pathOutputModel) { SVCERR << "Align::alignModel: ERROR: Failed to create alignment path (no MATCH plugin?)" << endl; - ModelById::release(transformOutput); alignmentModel->setError(message); return false; } - path->setCompletion(0); - alignmentModel->setPathFrom(transformOutput); //!!! who releases transformOutput? + pathOutputModel->setCompletion(0); + alignmentModel->setPathFrom(pathOutputModelId); + + m_pendingAlignments[alignmentModelId] = pathOutputModelId; connect(alignmentModel.get(), SIGNAL(completionChanged(ModelId)), this, SLOT(alignmentCompletionChanged(ModelId))); @@ -351,20 +371,30 @@ } void -Align::alignmentCompletionChanged(ModelId modelId) +Align::alignmentCompletionChanged(ModelId alignmentModelId) { QMutexLocker locker (&m_mutex); - auto am = ModelById::getAs<AlignmentModel>(modelId); - if (am && am->isReady()) { - disconnect(am.get(), SIGNAL(completionChanged(ModelId)), + auto alignmentModel = ModelById::getAs<AlignmentModel>(alignmentModelId); + + if (alignmentModel && alignmentModel->isReady()) { + + if (m_pendingAlignments.find(alignmentModelId) != + m_pendingAlignments.end()) { + ModelId pathOutputModelId = m_pendingAlignments[alignmentModelId]; + ModelById::release(pathOutputModelId); + m_pendingAlignments.erase(alignmentModelId); + } + + disconnect(alignmentModel.get(), + SIGNAL(completionChanged(ModelId)), this, SLOT(alignmentCompletionChanged(ModelId))); - emit alignmentComplete(modelId); + emit alignmentComplete(alignmentModelId); } } bool -Align::alignModelViaProgram(Document *, +Align::alignModelViaProgram(Document *doc, ModelId referenceId, ModelId otherId, QString program, @@ -417,11 +447,12 @@ << endl; error = "Alignment program could not be started"; m_pendingProcesses.erase(process); - //!!! who releases alignmentModel? does this? review other->setAlignment({}); + ModelById::release(alignmentModelId); delete process; } + doc->addNonDerivedModel(alignmentModelId); return success; }
--- a/framework/Align.h Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/Align.h Fri Jul 12 13:58:02 2019 +0100 @@ -117,6 +117,9 @@ // needed for subsequent alignment std::map<ModelId, TuningDiffRec> m_pendingTuningDiffs; + // alignment model id -> path output model id + std::map<ModelId, ModelId> m_pendingAlignments; + // external alignment subprocess -> model into which to stuff the // results (an AlignmentModel) std::map<QProcess *, ModelId> m_pendingProcesses;
--- a/framework/Document.cpp Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/Document.cpp Fri Jul 12 13:58:02 2019 +0100 @@ -73,22 +73,40 @@ CommandHistory::getInstance()->clear(); #ifdef DEBUG_DOCUMENT - SVDEBUG << "Document::~Document: about to delete layers" << endl; + SVCERR << "Document::~Document: about to delete layers" << endl; #endif while (!m_layers.empty()) { deleteLayer(*m_layers.begin(), true); } +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::~Document: about to release normal models" << endl; +#endif for (auto mr: m_models) { ModelById::release(mr.first); } + +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::~Document: about to release aggregate models" << endl; +#endif for (auto m: m_aggregateModels) { ModelById::release(m); } +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::~Document: about to release alignment models" << endl; +#endif + for (auto m: m_alignmentModels) { + ModelById::release(m); + } + +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::~Document: about to release main model" << endl; +#endif if (!m_mainModel.isNone()) { ModelById::release(m_mainModel); } + m_mainModel = {}; emit mainModelChanged({}); } @@ -141,7 +159,7 @@ newLayer->setObjectName(getUniqueLayerName(newLayer->objectName())); - addImportedModel(modelId); + addNonDerivedModel(modelId); setModel(newLayer, modelId); //!!! and all channels @@ -173,7 +191,7 @@ } auto newModelId = ModelById::add(newModel); - addImportedModel(newModelId); + addNonDerivedModel(newModelId); setModel(newLayer, newModelId); return newLayer; @@ -607,10 +625,25 @@ } void -Document::addImportedModel(ModelId modelId) +Document::addNonDerivedModel(ModelId modelId) { + if (ModelById::isa<AggregateWaveModel>(modelId)) { +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::addNonDerivedModel: Model " << modelId << " is an aggregate model, adding it to aggregates" << endl; +#endif + m_aggregateModels.insert(modelId); + return; + } + if (ModelById::isa<AlignmentModel>(modelId)) { +#ifdef DEBUG_DOCUMENT + SVCERR << "Document::addNonDerivedModel: Model " << modelId << " is an alignment model, adding it to alignments" << endl; +#endif + m_alignmentModels.insert(modelId); + return; + } + if (m_models.find(modelId) != m_models.end()) { - SVCERR << "WARNING: Document::addImportedModel: Model already added" + SVCERR << "WARNING: Document::addNonDerivedModel: Model already added" << endl; return; } @@ -623,19 +656,19 @@ m_models[modelId] = rec; #ifdef DEBUG_DOCUMENT - SVDEBUG << "Document::addImportedModel: Added model " << modelId << endl; - SVDEBUG << "Models now: "; + SVCERR << "Document::addNonDerivedModel: Added model " << modelId << endl; + SVCERR << "Models now: "; for (const auto &rec : m_models) { - SVDEBUG << rec.first << " "; + SVCERR << rec.first << " "; } - SVDEBUG << endl; + SVCERR << endl; #endif if (m_autoAlignment) { - SVDEBUG << "Document::addImportedModel: auto-alignment is on, aligning model if possible" << endl; + SVDEBUG << "Document::addNonDerivedModel: auto-alignment is on, aligning model if possible" << endl; alignModel(modelId); } else { - SVDEBUG << "Document(" << this << "): addImportedModel: auto-alignment is off" << endl; + SVDEBUG << "Document(" << this << "): addNonDerivedModel: auto-alignment is off" << endl; } emit modelAdded(modelId); @@ -666,21 +699,15 @@ SVDEBUG << endl; #endif - if (m_autoAlignment) { - SVDEBUG << "Document::addAdditionalModel: auto-alignment is on, aligning model if possible" << endl; + if (m_autoAlignment && + ModelById::isa<RangeSummarisableTimeValueModel>(modelId)) { + SVDEBUG << "Document::addAdditionalModel: auto-alignment is on and model is an alignable type, aligning it if possible" << endl; alignModel(modelId); } emit modelAdded(modelId); } -void -Document::addAggregateModel(ModelId modelId) -{ - m_aggregateModels.insert(modelId); - SVDEBUG << "Document::addAggregateModel(" << modelId << ")" << endl; -} - ModelId Document::addDerivedModel(const Transform &transform, const ModelTransformer::Input &input, @@ -758,7 +785,13 @@ // borrowed-pointer mechanism will at least prevent memory errors, // although the other code will have to stop whatever it's doing. - SVCERR << "Document::releaseModel(" << modelId << ")" << endl; + if (auto model = ModelById::get(modelId)) { + SVCERR << "Document::releaseModel(" << modelId << "), name " + << model->objectName() << ", type " + << typeid(*model.get()).name() << endl; + } else { + SVCERR << "Document::releaseModel(" << modelId << ")" << endl; + } if (modelId.isNone()) { return; @@ -777,7 +810,7 @@ } if (m_models.find(modelId) == m_models.end()) { - // No point in releasing aggregate models and the like, + // No point in releasing aggregate and alignment models, // they're not large #ifdef DEBUG_DOCUMENT SVCERR << "Document::releaseModel: It's not a regular layer model, ignoring" << endl; @@ -902,14 +935,7 @@ << modelId << endl; return; } -/*!!! - if (model && model != m_mainModel) { - ModelList::iterator mitr = findModelInList(model); - if (mitr != m_models.end()) { - mitr->refcount ++; - } - } -*/ + if (!modelId.isNone() && !previousModel.isNone()) { PlayParameterRepository::getInstance()->copyParameters (previousModel.untyped, modelId.untyped); @@ -1100,9 +1126,10 @@ << "): is main model, setting alignment to itself" << endl; auto alignment = std::make_shared<AlignmentModel>(modelId, modelId, ModelId()); - - //!!! hang on, who tracks alignment models? - rm->setAlignment(ModelById::add(alignment)); + + ModelId alignmentModelId = ModelById::add(alignment); + rm->setAlignment(alignmentModelId); + m_alignmentModels.insert(alignmentModelId); return; }
--- a/framework/Document.h Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/Document.h Fri Jul 12 13:58:02 2019 +0100 @@ -223,38 +223,25 @@ * Add a derived model associated with the given transform. This * is necessary to register any derived model that was not created * by the document using createDerivedModel or - * createDerivedLayer. The model must have been added to ModelById - * already, and Document will take responsibility for releasing it - * later. + * createDerivedLayer. Document will take responsibility for + * releasing the model later. */ void addAlreadyDerivedModel(const Transform &transform, const ModelTransformer::Input &input, ModelId outputModelToAdd); /** - * Add an imported (non-derived, non-main) model. This is - * necessary to register any imported model that is associated - * with a layer. The model must have been added to ModelById - * already, and Document will take responsibility for releasing it - * later. + * Add an imported model, i.e. any model (other than the main + * model) that has been created by any means other than as the + * output of a transform. This is necessary to register any + * imported model that is to be associated with a layer, and also + * to make sure that the model is released by the Document + * later. Aggregate models, alignment models, and miscellaneous + * temporary models should also be added in this way, unless the + * temporary models are large enough to need managing in a way + * that guarantees the shortest possible lifespan. */ - void addImportedModel(ModelId); - - /** - *!!! todo: do we still need this to be separate? - * - * Add an aggregate model (one which represents a set of component - * wave models as one model per channel in a single wave - * model). Aggregate models are unusual in that they are created - * for a single transform each and have no refcount. (This - * probably isn't ideal!) They are recorded separately from other - * models, and will be deleted if any of their component models - * are removed. - * - * The model must have been added to ModelById already, and - * Document will take responsibility for releasing it later. - */ - void addAggregateModel(ModelId); // an AggregateWaveModel + void addNonDerivedModel(ModelId); /** * Associate the given model with the given layer. The model must @@ -393,7 +380,9 @@ // These must be stored in increasing order of id (as in the // ordered std::map), to ensure repeatability for automated tests std::map<ModelId, ModelRecord> m_models; + std::set<ModelId> m_aggregateModels; + std::set<ModelId> m_alignmentModels; /** * Add an extra derived model (returned at the end of processing a
--- a/framework/MainWindowBase.cpp Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/MainWindowBase.cpp Fri Jul 12 13:58:02 2019 +0100 @@ -1727,7 +1727,7 @@ CommandHistory::getInstance()->startCompoundOperation (tr("Import \"%1\"").arg(source.getBasename()), true); - m_document->addImportedModel(newModel); + m_document->addNonDerivedModel(newModel); AddPaneCommand *command = new AddPaneCommand(this); CommandHistory::getInstance()->addCommand(command); @@ -1771,7 +1771,7 @@ CommandHistory::getInstance()->startCompoundOperation (tr("Import \"%1\"").arg(source.getBasename()), true); - m_document->addImportedModel(newModel); + m_document->addNonDerivedModel(newModel); if (replace) { m_document->removeLayerFromView(pane, replace); @@ -3321,7 +3321,7 @@ CommandHistory::getInstance()->startCompoundOperation (tr("Import Recorded Audio"), true); - m_document->addImportedModel(modelId); + m_document->addNonDerivedModel(modelId); AddPaneCommand *command = new AddPaneCommand(this); CommandHistory::getInstance()->addCommand(command);
--- a/framework/SVFileReader.cpp Fri Jul 12 09:40:56 2019 +0100 +++ b/framework/SVFileReader.cpp Fri Jul 12 13:58:02 2019 +0100 @@ -484,7 +484,7 @@ // the .sv file. (pity we don't have a nicer way to handle // this) if (!ModelById::isa<AlignmentModel>(modelId)) { - m_document->addImportedModel(modelId); + m_document->addNonDerivedModel(modelId); } // but we add all models including alignment ones to the added