# HG changeset patch # User Chris Cannam # Date 1562336887 -3600 # Node ID 6d09d68165a47e7649dbb96f44213710f5dec5df # Parent 77543124651bbbf92fd3b97cdd8597ce74a9fa3b Further review of ById: make IDs only available when adding a model to the ById store, not by querying the item directly. This means any id encountered in the wild must have been added to the store at some point (even if later released), which simplifies reasoning about lifecycles diff -r 77543124651b -r 6d09d68165a4 base/ById.cpp --- a/base/ById.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/base/ById.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -59,9 +59,10 @@ } } - void add(int id, std::shared_ptr item) { + int add(std::shared_ptr item) { + int id = item->getUntypedId(); if (id == IdAlloc::NO_ID) { - throw std::logic_error("cannot add item with id of NO_ID"); + throw std::logic_error("item id should never be NO_ID"); } QMutexLocker locker(&m_mutex); if (m_items.find(id) != m_items.end()) { @@ -73,12 +74,14 @@ throw std::logic_error("item id is already recorded in add"); } m_items[id] = item; + return id; } void release(int id) { if (id == IdAlloc::NO_ID) { return; } + SVCERR << "ById::release(" << id << ")" << endl; QMutexLocker locker(&m_mutex); if (m_items.find(id) == m_items.end()) { SVCERR << "ById::release: unknown item id " << id << endl; @@ -105,10 +108,10 @@ std::unordered_map> m_items; }; -void -AnyById::add(int id, std::shared_ptr item) +int +AnyById::add(std::shared_ptr item) { - impl().add(id, item); + return impl().add(item); } void diff -r 77543124651b -r 6d09d68165a4 base/ById.h --- a/base/ById.h Thu Jul 04 18:04:21 2019 +0100 +++ b/base/ById.h Fri Jul 05 15:28:07 2019 +0100 @@ -95,6 +95,9 @@ virtual ~WithId() { } +protected: + friend class AnyById; + /** * Return an id for this object. The id is a unique number for * this object among all objects that implement WithId within this @@ -116,6 +119,10 @@ WithTypedId() : WithId() { } +protected: + template + friend class TypedById; + /** * Return an id for this object. The id is a unique value for this * object among all objects that implement WithTypedId within this @@ -131,7 +138,7 @@ class AnyById { public: - static void add(int, std::shared_ptr); + static int add(std::shared_ptr); static void release(int); static std::shared_ptr get(int); @@ -157,11 +164,8 @@ { public: static Id add(std::shared_ptr item) { - auto id = item->getId(); - if (id.isNone()) { - throw std::logic_error("item id should never be None"); - } - AnyById::add(id.untyped, item); + Id id; + id.untyped = AnyById::add(item); return id; } @@ -185,7 +189,7 @@ static std::shared_ptr get(Id id) { return getAs(id); } - + /** * If the Item type is an XmlExportable, return the export ID of * the given item ID. A call to this function will fail to compile diff -r 77543124651b -r 6d09d68165a4 base/test/TestById.h --- a/base/test/TestById.h Thu Jul 04 18:04:21 2019 +0100 +++ b/base/test/TestById.h Fri Jul 05 15:28:07 2019 +0100 @@ -23,7 +23,10 @@ struct WithoutId {}; -struct A : public WithTypedId {}; +// We'll need to change access levels for getId() and getUntypedId() +// to test the raw calls + +struct A : public WithTypedId { public: using WithTypedId::getId; }; struct B1 : public A {}; struct B2 : public A {}; @@ -31,7 +34,7 @@ typedef TypedById AById; -struct X : virtual public WithId {}; +struct X : virtual public WithId { public: using WithId::getUntypedId; }; struct Y : public X, public B2, public M {}; class TestById : public QObject @@ -77,13 +80,14 @@ void anySimple() { auto a = std::make_shared(); - AnyById::add(a->getId().untyped, a); + int id = AnyById::add(a); + QCOMPARE(id, a->getId().untyped); - auto aa = AnyById::getAs(a->getId().untyped); + auto aa = AnyById::getAs(id); QVERIFY(!!aa); QCOMPARE(aa->getId(), a->getId()); QCOMPARE(aa.get(), a.get()); // same object, not just same id! - AnyById::release(a->getId().untyped); + AnyById::release(id); } void typedEmpty() { @@ -102,15 +106,27 @@ AById::release(a); } - void typedRelease() { + void typedReleaseById() { auto a = std::make_shared(); - AById::add(a); + auto aid = AById::add(a); - auto aa = AById::get(a->getId()); + auto aa = AById::get(aid); + QVERIFY(!!aa); + AById::release(aid); + + aa = AById::get(aid); + QVERIFY(!aa); + } + + void typedReleaseByItem() { + auto a = std::make_shared(); + auto aid = AById::add(a); + + auto aa = AById::get(aid); QVERIFY(!!aa); AById::release(a); - aa = AById::get(a->getId()); + aa = AById::get(aid); QVERIFY(!aa); } diff -r 77543124651b -r 6d09d68165a4 data/model/AlignmentModel.cpp --- a/data/model/AlignmentModel.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/AlignmentModel.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -194,7 +194,7 @@ } void -AlignmentModel::pathSourceChangedWithin(sv_frame_t, sv_frame_t) +AlignmentModel::pathSourceChangedWithin(ModelId, sv_frame_t, sv_frame_t) { if (!m_pathComplete) return; constructPath(); @@ -202,7 +202,7 @@ } void -AlignmentModel::pathSourceCompletionChanged() +AlignmentModel::pathSourceCompletionChanged(ModelId) { auto pathSourceModel = ModelById::getAs(m_pathSource); @@ -233,7 +233,7 @@ } } - emit completionChanged(); + emit completionChanged(getId()); } void @@ -392,17 +392,17 @@ if (pathSourceModel) { connect(pathSourceModel.get(), - SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)), - this, SLOT(pathSourceChangedWithin(sv_frame_t, sv_frame_t))); + SIGNAL(modelChangedWithin(ModelId, sv_frame_t, sv_frame_t)), + this, SLOT(pathSourceChangedWithin(ModelId, sv_frame_t, sv_frame_t))); - connect(pathSourceModel.get(), SIGNAL(completionChanged()), - this, SLOT(pathSourceCompletionChanged())); + connect(pathSourceModel.get(), SIGNAL(completionChanged(ModelId)), + this, SLOT(pathSourceCompletionChanged(ModelId))); constructPath(); constructReversePath(); if (pathSourceModel->isReady()) { - pathSourceCompletionChanged(); + pathSourceCompletionChanged(m_pathSource); } } } diff -r 77543124651b -r 6d09d68165a4 data/model/AlignmentModel.h --- a/data/model/AlignmentModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/AlignmentModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -63,8 +63,8 @@ void setPath(const Path &path); void toXml(QTextStream &stream, - QString indent = "", - QString extraAttributes = "") const override; + QString indent = "", + QString extraAttributes = "") const override; QString toDelimitedDataString(QString, DataExportOptions, sv_frame_t, sv_frame_t) const override { @@ -72,11 +72,11 @@ } signals: - void completionChanged(); + void completionChanged(ModelId); protected slots: - void pathSourceChangedWithin(sv_frame_t startFrame, sv_frame_t endFrame); - void pathSourceCompletionChanged(); + void pathSourceChangedWithin(ModelId, sv_frame_t startFrame, sv_frame_t endFrame); + void pathSourceCompletionChanged(ModelId); protected: ModelId m_reference; diff -r 77543124651b -r 6d09d68165a4 data/model/DeferredNotifier.h --- a/data/model/DeferredNotifier.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/DeferredNotifier.h Fri Jul 05 15:28:07 2019 +0100 @@ -30,7 +30,8 @@ NOTIFY_DEFERRED }; - DeferredNotifier(Model *m, Mode mode) : m_model(m), m_mode(mode) { } + DeferredNotifier(Model *m, ModelId id, Mode mode) : + m_model(m), m_modelId(id), m_mode(mode) { } Mode getMode() const { return m_mode; @@ -41,7 +42,7 @@ void update(sv_frame_t frame, sv_frame_t duration) { if (m_mode == NOTIFY_ALWAYS) { - m_model->modelChangedWithin(frame, frame + duration); + m_model->modelChangedWithin(m_modelId, frame, frame + duration); } else { QMutexLocker locker(&m_mutex); m_extents.sample(frame); @@ -60,7 +61,7 @@ } } if (shouldEmit) { - m_model->modelChangedWithin(from, to); + m_model->modelChangedWithin(m_modelId, from, to); QMutexLocker locker(&m_mutex); m_extents.reset(); } @@ -68,6 +69,7 @@ private: Model *m_model; + ModelId m_modelId; Mode m_mode; QMutex m_mutex; Extents m_extents; diff -r 77543124651b -r 6d09d68165a4 data/model/Dense3DModelPeakCache.cpp --- a/data/model/Dense3DModelPeakCache.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/Dense3DModelPeakCache.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -38,8 +38,8 @@ EditableDenseThreeDimensionalModel::NoCompression, false)); - connect(source.get(), SIGNAL(modelChanged()), - this, SLOT(sourceModelChanged())); + connect(source.get(), SIGNAL(modelChanged(ModelId)), + this, SLOT(sourceModelChanged(ModelId))); } Dense3DModelPeakCache::~Dense3DModelPeakCache() @@ -61,7 +61,7 @@ } void -Dense3DModelPeakCache::sourceModelChanged() +Dense3DModelPeakCache::sourceModelChanged(ModelId) { if (m_coverage.size() > 0) { // The last peak may have come from an incomplete read, which diff -r 77543124651b -r 6d09d68165a4 data/model/Dense3DModelPeakCache.h --- a/data/model/Dense3DModelPeakCache.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/Dense3DModelPeakCache.h Fri Jul 05 15:28:07 2019 +0100 @@ -116,7 +116,7 @@ } protected slots: - void sourceModelChanged(); + void sourceModelChanged(ModelId); private: ModelId m_source; diff -r 77543124651b -r 6d09d68165a4 data/model/EditableDenseThreeDimensionalModel.cpp --- a/data/model/EditableDenseThreeDimensionalModel.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/EditableDenseThreeDimensionalModel.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -359,15 +359,16 @@ if (m_notifyOnAdd) { if (allChange) { - emit modelChanged(); + emit modelChanged(getId()); } else { - emit modelChangedWithin(windowStart, windowStart + m_resolution); + emit modelChangedWithin(getId(), + windowStart, windowStart + m_resolution); } } else { if (allChange) { m_sinceLastNotifyMin = -1; m_sinceLastNotifyMax = -1; - emit modelChanged(); + emit modelChanged(getId()); } else { if (m_sinceLastNotifyMin == -1 || windowStart < m_sinceLastNotifyMin) { @@ -393,14 +394,14 @@ { while ((int)m_binNames.size() <= n) m_binNames.push_back(""); m_binNames[n] = name; - emit modelChanged(); + emit modelChanged(getId()); } void EditableDenseThreeDimensionalModel::setBinNames(std::vector names) { m_binNames = names; - emit modelChanged(); + emit modelChanged(getId()); } bool @@ -474,21 +475,22 @@ if (completion == 100) { m_notifyOnAdd = true; // henceforth - emit modelChanged(); + emit modelChanged(getId()); } else if (!m_notifyOnAdd) { if (update && m_sinceLastNotifyMin >= 0 && m_sinceLastNotifyMax >= 0) { - emit modelChangedWithin(m_sinceLastNotifyMin, + emit modelChangedWithin(getId(), + m_sinceLastNotifyMin, m_sinceLastNotifyMax + m_resolution); m_sinceLastNotifyMin = m_sinceLastNotifyMax = -1; } else { - emit completionChanged(); + emit completionChanged(getId()); } } else { - emit completionChanged(); + emit completionChanged(getId()); } } } diff -r 77543124651b -r 6d09d68165a4 data/model/FFTModel.cpp --- a/data/model/FFTModel.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/FFTModel.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -63,10 +63,10 @@ auto model = ModelById::getAs(m_model); if (model) { - connect(model.get(), SIGNAL(modelChanged()), - this, SIGNAL(modelChanged())); - connect(model.get(), SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t)), - this, SIGNAL(modelChangedWithin(sv_frame_t, sv_frame_t))); + connect(model.get(), SIGNAL(modelChanged(ModelId)), + this, SIGNAL(modelChanged(ModelId))); + connect(model.get(), SIGNAL(modelChangedWithin(ModelId, sv_frame_t, sv_frame_t)), + this, SIGNAL(modelChangedWithin(ModelId, sv_frame_t, sv_frame_t))); } } diff -r 77543124651b -r 6d09d68165a4 data/model/ImageModel.h --- a/data/model/ImageModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/ImageModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -48,6 +48,7 @@ m_sampleRate(sampleRate), m_resolution(resolution), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -84,12 +85,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -152,7 +153,8 @@ { QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), e.getFrame() + m_resolution); + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + m_resolution); } /** diff -r 77543124651b -r 6d09d68165a4 data/model/Model.cpp --- a/data/model/Model.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/Model.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -56,8 +56,8 @@ auto model = ModelById::get(m_sourceModel); if (model) { - connect(model.get(), SIGNAL(alignmentCompletionChanged()), - this, SIGNAL(alignmentCompletionChanged())); + connect(model.get(), SIGNAL(alignmentCompletionChanged(ModelId)), + this, SIGNAL(alignmentCompletionChanged(ModelId))); } diff -r 77543124651b -r 6d09d68165a4 data/model/Model.h --- a/data/model/Model.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/Model.h Fri Jul 05 15:28:07 2019 +0100 @@ -294,24 +294,18 @@ sv_frame_t startFrame, sv_frame_t duration) const = 0; - /*!!! -public slots: - void aboutToDelete(); - void sourceModelAboutToBeDeleted(); - */ - signals: /** * Emitted when a model has been edited (or more data retrieved * from cache, in the case of a cached model that generates slowly) */ - void modelChanged(); + void modelChanged(ModelId myId); /** * Emitted when a model has been edited (or more data retrieved * from cache, in the case of a cached model that generates slowly) */ - void modelChangedWithin(sv_frame_t startFrame, sv_frame_t endFrame); + void modelChangedWithin(ModelId myId, sv_frame_t startFrame, sv_frame_t endFrame); /** * Emitted when some internal processing has advanced a stage, but @@ -319,35 +313,23 @@ * updating any progress meters or other monitoring, but not * refreshing the actual view. */ - void completionChanged(); + void completionChanged(ModelId myId); /** * Emitted when internal processing is complete (i.e. when * isReady() would return true, with completion at 100). */ - void ready(); + void ready(ModelId myId); /** * Emitted when the completion percentage changes for the * calculation of this model's alignment model. */ - void alignmentCompletionChanged(); - - /** - * Emitted when something notifies this model (through calling - * aboutToDelete() that it is about to delete it. Note that this - * depends on an external agent such as a Document object or - * owning model telling the model that it is about to delete it; - * there is nothing in the model to guarantee that this signal - * will be emitted before the actual deletion. - */ - //!!! our goal is to get rid of (the need for) this -//!!! void aboutToBeDeleted(); + void alignmentCompletionChanged(ModelId myId); protected: Model() : //!!! m_abandoning(false), -//!!! m_aboutToDelete(false), m_extendTo(0) { } // Not provided. @@ -358,7 +340,6 @@ ModelId m_alignmentModel; QString m_typeUri; //!!! bool m_abandoning; -//!!! bool m_aboutToDelete; sv_frame_t m_extendTo; }; diff -r 77543124651b -r 6d09d68165a4 data/model/NoteModel.h --- a/data/model/NoteModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/NoteModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -58,6 +58,7 @@ m_units(""), m_extendTo(0), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -84,6 +85,7 @@ m_units(""), m_extendTo(0), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -145,12 +147,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -219,7 +221,7 @@ m_notifier.update(e.getFrame(), e.getDuration() + m_resolution); if (allChange) { - emit modelChanged(); + emit modelChanged(getId()); } } @@ -228,7 +230,8 @@ QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + e.getDuration() + m_resolution); } diff -r 77543124651b -r 6d09d68165a4 data/model/ReadOnlyWaveFileModel.cpp --- a/data/model/ReadOnlyWaveFileModel.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/ReadOnlyWaveFileModel.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -590,14 +590,14 @@ SVDEBUG << "ReadOnlyWaveFileModel(" << objectName() << ")::fillTimerTimedOut: extent = " << fillExtent << endl; #endif if (fillExtent > m_lastFillExtent) { - emit modelChangedWithin(m_lastFillExtent, fillExtent); + emit modelChangedWithin(getId(), m_lastFillExtent, fillExtent); m_lastFillExtent = fillExtent; } } else { #ifdef DEBUG_WAVE_FILE_MODEL SVDEBUG << "ReadOnlyWaveFileModel(" << objectName() << ")::fillTimerTimedOut: no thread" << endl; #endif - emit modelChanged(); + emit modelChanged(getId()); } } @@ -616,10 +616,10 @@ SVDEBUG << "ReadOnlyWaveFileModel(" << objectName() << ")::cacheFilled, about to emit things" << endl; #endif if (getEndFrame() > prevFillExtent) { - emit modelChangedWithin(prevFillExtent, getEndFrame()); + emit modelChangedWithin(getId(), prevFillExtent, getEndFrame()); } - emit modelChanged(); - emit ready(); + emit modelChanged(getId()); + emit ready(getId()); } void diff -r 77543124651b -r 6d09d68165a4 data/model/RegionModel.h --- a/data/model/RegionModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/RegionModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -50,6 +50,7 @@ m_valueQuantization(0), m_haveDistinctValues(false), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -67,6 +68,7 @@ m_valueQuantization(0), m_haveDistinctValues(false), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -120,12 +122,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -197,7 +199,7 @@ m_notifier.update(e.getFrame(), e.getDuration() + m_resolution); if (allChange) { - emit modelChanged(); + emit modelChanged(getId()); } } @@ -206,7 +208,8 @@ QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + e.getDuration() + m_resolution); } diff -r 77543124651b -r 6d09d68165a4 data/model/SparseOneDimensionalModel.h --- a/data/model/SparseOneDimensionalModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/SparseOneDimensionalModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -49,6 +49,7 @@ m_resolution(resolution), m_haveTextLabels(false), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -97,12 +98,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -166,7 +167,8 @@ { QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), e.getFrame() + m_resolution); + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + m_resolution); } /** diff -r 77543124651b -r 6d09d68165a4 data/model/SparseTimeValueModel.h --- a/data/model/SparseTimeValueModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/SparseTimeValueModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -48,6 +48,7 @@ m_haveExtents(false), m_haveTextLabels(false), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -68,6 +69,7 @@ m_haveExtents(true), m_haveTextLabels(false), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -127,12 +129,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -205,7 +207,7 @@ m_notifier.update(e.getFrame(), m_resolution); if (allChange) { - emit modelChanged(); + emit modelChanged(getId()); } } @@ -214,7 +216,8 @@ QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), e.getFrame() + m_resolution); + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + m_resolution); } /** diff -r 77543124651b -r 6d09d68165a4 data/model/TextModel.h --- a/data/model/TextModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/TextModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -46,6 +46,7 @@ m_sampleRate(sampleRate), m_resolution(resolution), m_notifier(this, + getId(), notifyOnAdd ? DeferredNotifier::NOTIFY_ALWAYS : DeferredNotifier::NOTIFY_DEFERRED), @@ -82,12 +83,12 @@ m_notifier.makeDeferredNotifications(); } - emit completionChanged(); + emit completionChanged(getId()); if (completion == 100) { // henceforth: m_notifier.switchMode(DeferredNotifier::NOTIFY_ALWAYS); - emit modelChanged(); + emit modelChanged(getId()); } } @@ -147,7 +148,8 @@ { QMutexLocker locker(&m_mutex); m_events.remove(e); } - emit modelChangedWithin(e.getFrame(), e.getFrame() + m_resolution); + emit modelChangedWithin(getId(), + e.getFrame(), e.getFrame() + m_resolution); } /** diff -r 77543124651b -r 6d09d68165a4 data/model/WritableWaveFileModel.cpp --- a/data/model/WritableWaveFileModel.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/WritableWaveFileModel.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -256,8 +256,8 @@ m_reader->updateDone(); m_proportion = 100; - emit modelChanged(); - emit writeCompleted(); + emit modelChanged(getId()); + emit writeCompleted(getId()); } void diff -r 77543124651b -r 6d09d68165a4 data/model/WritableWaveFileModel.h --- a/data/model/WritableWaveFileModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/WritableWaveFileModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -197,7 +197,7 @@ QString extraAttributes = "") const override; signals: - void writeCompleted(); + void writeCompleted(ModelId); protected: ReadOnlyWaveFileModel *m_model; diff -r 77543124651b -r 6d09d68165a4 data/model/test/TestFFTModel.h --- a/data/model/test/TestFFTModel.h Thu Jul 04 18:04:21 2019 +0100 +++ b/data/model/test/TestFFTModel.h Fri Jul 05 15:28:07 2019 +0100 @@ -90,8 +90,7 @@ ModelId makeMock(std::vector sorts, int length, int pad) { auto mwm = std::make_shared(sorts, length, pad); - ModelById::add(mwm); - return mwm->getId(); + return ModelById::add(mwm); } void releaseMock(ModelId id) { diff -r 77543124651b -r 6d09d68165a4 rdf/RDFImporter.cpp --- a/rdf/RDFImporter.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/rdf/RDFImporter.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -58,7 +58,7 @@ bool isOK(); QString getErrorString() const; - std::vector getDataModels(ProgressReporter *); + std::vector getDataModels(ProgressReporter *); protected: BasicStore *m_store; @@ -66,22 +66,22 @@ QString m_uristring; QString m_errorString; - std::map m_audioModelMap; + std::map m_audioModelMap; sv_samplerate_t m_sampleRate; - std::map > m_labelValueMap; + std::map > m_labelValueMap; - void getDataModelsAudio(std::vector &, ProgressReporter *); - void getDataModelsSparse(std::vector &, ProgressReporter *); - void getDataModelsDense(std::vector &, ProgressReporter *); + void getDataModelsAudio(std::vector &, ProgressReporter *); + void getDataModelsSparse(std::vector &, ProgressReporter *); + void getDataModelsDense(std::vector &, ProgressReporter *); - void getDenseModelTitle(Model *, QString, QString); + QString getDenseModelTitle(QString featureUri, QString featureTypeUri); void getDenseFeatureProperties(QString featureUri, sv_samplerate_t &sampleRate, int &windowLength, int &hopSize, int &width, int &height); - void fillModel(Model *, sv_frame_t, sv_frame_t, + void fillModel(ModelId, sv_frame_t, sv_frame_t, bool, std::vector &, QString); }; @@ -119,7 +119,7 @@ return m_d->getErrorString(); } -std::vector +std::vector RDFImporter::getDataModels(ProgressReporter *r) { return m_d->getDataModels(r); @@ -169,10 +169,10 @@ return m_errorString; } -std::vector +std::vector RDFImporterImpl::getDataModels(ProgressReporter *reporter) { - std::vector models; + std::vector models; getDataModelsAudio(models, reporter); @@ -206,7 +206,7 @@ } void -RDFImporterImpl::getDataModelsAudio(std::vector &models, +RDFImporterImpl::getDataModelsAudio(std::vector &models, ProgressReporter *reporter) { Nodes sigs = m_store->match @@ -270,24 +270,25 @@ reporter->setMessage(RDFImporter::tr("Importing audio referenced in RDF...")); } fs->waitForData(); - ReadOnlyWaveFileModel *newModel = new ReadOnlyWaveFileModel(*fs, m_sampleRate); + auto newModel = std::make_shared + (*fs, m_sampleRate); if (newModel->isOK()) { cerr << "Successfully created wave file model from source at \"" << source << "\"" << endl; - models.push_back(newModel); - m_audioModelMap[signal] = newModel; + auto modelId = ModelById::add(newModel); + models.push_back(modelId); + m_audioModelMap[signal] = modelId; if (m_sampleRate == 0) { m_sampleRate = newModel->getSampleRate(); } } else { m_errorString = QString("Failed to create wave file model from source at \"%1\"").arg(source); - delete newModel; } delete fs; } } void -RDFImporterImpl::getDataModelsDense(std::vector &models, +RDFImporterImpl::getDataModelsDense(std::vector &models, ProgressReporter *reporter) { if (reporter) { @@ -342,7 +343,7 @@ if (height == 1) { - SparseTimeValueModel *m = new SparseTimeValueModel + auto m = std::make_shared (sampleRate, hopSize, false); for (int j = 0; j < values.size(); ++j) { @@ -351,16 +352,13 @@ m->add(e); } - getDenseModelTitle(m, feature, type); - + m->setObjectName(getDenseModelTitle(feature, type)); m->setRDFTypeURI(type); - - models.push_back(m); + models.push_back(ModelById::add(m)); } else { - EditableDenseThreeDimensionalModel *m = - new EditableDenseThreeDimensionalModel + auto m = std::make_shared (sampleRate, hopSize, height, EditableDenseThreeDimensionalModel::NoCompression, false); @@ -380,18 +378,15 @@ m->setColumn(x++, column); } - getDenseModelTitle(m, feature, type); - + m->setObjectName(getDenseModelTitle(feature, type)); m->setRDFTypeURI(type); - - models.push_back(m); + models.push_back(ModelById::add(m)); } } } -void -RDFImporterImpl::getDenseModelTitle(Model *m, - QString featureUri, +QString +RDFImporterImpl::getDenseModelTitle(QString featureUri, QString featureTypeUri) { Node n = m_store->complete @@ -399,8 +394,7 @@ if (n.type == Node::Literal && n.value != "") { SVDEBUG << "RDFImporterImpl::getDenseModelTitle: Title (from signal) \"" << n.value << "\"" << endl; - m->setObjectName(n.value); - return; + return n.value; } n = m_store->complete @@ -408,11 +402,11 @@ if (n.type == Node::Literal && n.value != "") { SVDEBUG << "RDFImporterImpl::getDenseModelTitle: Title (from signal type) \"" << n.value << "\"" << endl; - m->setObjectName(n.value); - return; + return n.value; } SVDEBUG << "RDFImporterImpl::getDenseModelTitle: No title available for feature <" << featureUri << ">" << endl; + return {}; } void @@ -481,7 +475,7 @@ } void -RDFImporterImpl::getDataModelsSparse(std::vector &models, +RDFImporterImpl::getDataModelsSparse(std::vector &models, ProgressReporter *reporter) { if (reporter) { @@ -510,8 +504,8 @@ (Triple(Node(), expand("a"), expand("mo:Signal"))).subjects(); // Map from timeline uri to event type to dimensionality to - // presence of duration to model ptr. Whee! - std::map > > > + // presence of duration to model id. Whee! + std::map > > > modelMap; foreach (Node sig, sigs) { @@ -617,7 +611,7 @@ if (values.size() == 1) dimensions = 2; else if (values.size() > 1) dimensions = 3; - Model *model = nullptr; + ModelId modelId; if (modelMap[timeline][type][dimensions].find(haveDuration) == modelMap[timeline][type][dimensions].end()) { @@ -628,7 +622,9 @@ << ", time = " << time << ", duration = " << duration << endl; */ - + + Model *model = nullptr; + if (!haveDuration) { if (dimensions == 1) { @@ -668,7 +664,7 @@ if (m_audioModelMap.find(source) != m_audioModelMap.end()) { cerr << "source model for " << model << " is " << m_audioModelMap[source] << endl; - model->setSourceModel(m_audioModelMap[source]->getId()); + model->setSourceModel(m_audioModelMap[source]); } QString title = m_store->complete @@ -680,16 +676,20 @@ } model->setObjectName(title); - modelMap[timeline][type][dimensions][haveDuration] = model; - models.push_back(model); + modelId = ModelById::add(std::shared_ptr(model)); + modelMap[timeline][type][dimensions][haveDuration] = modelId; + models.push_back(modelId); } - model = modelMap[timeline][type][dimensions][haveDuration]; + modelId = modelMap[timeline][type][dimensions][haveDuration]; - if (model) { - sv_frame_t ftime = RealTime::realTime2Frame(time, m_sampleRate); - sv_frame_t fduration = RealTime::realTime2Frame(duration, m_sampleRate); - fillModel(model, ftime, fduration, haveDuration, values, label); + if (!modelId.isNone()) { + sv_frame_t ftime = + RealTime::realTime2Frame(time, m_sampleRate); + sv_frame_t fduration = + RealTime::realTime2Frame(duration, m_sampleRate); + fillModel(modelId, ftime, fduration, + haveDuration, values, label); } } } @@ -697,7 +697,7 @@ } void -RDFImporterImpl::fillModel(Model *model, +RDFImporterImpl::fillModel(ModelId modelId, sv_frame_t ftime, sv_frame_t fduration, bool haveDuration, @@ -706,17 +706,13 @@ { // SVDEBUG << "RDFImporterImpl::fillModel: adding point at frame " << ftime << endl; - SparseOneDimensionalModel *sodm = - dynamic_cast(model); - if (sodm) { + if (auto sodm = ModelById::getAs(modelId)) { Event point(ftime, label); sodm->add(point); return; } - TextModel *tm = - dynamic_cast(model); - if (tm) { + if (auto tm = ModelById::getAs(modelId)) { Event e (ftime, values.empty() ? 0.5f : values[0] < 0.f ? 0.f : values[0] > 1.f ? 1.f : values[0], // I was young and feckless once too @@ -725,17 +721,13 @@ return; } - SparseTimeValueModel *stvm = - dynamic_cast(model); - if (stvm) { + if (auto stvm = ModelById::getAs(modelId)) { Event e(ftime, values.empty() ? 0.f : values[0], label); stvm->add(e); return; } - NoteModel *nm = - dynamic_cast(model); - if (nm) { + if (auto nm = ModelById::getAs(modelId)) { if (haveDuration) { float value = 0.f, level = 1.f; if (!values.empty()) { @@ -764,16 +756,14 @@ return; } - RegionModel *rm = - dynamic_cast(model); - if (rm) { + if (auto rm = ModelById::getAs(modelId)) { float value = 0.f; if (values.empty()) { // no values? map each unique label to a distinct value - if (m_labelValueMap[model].find(label) == m_labelValueMap[model].end()) { - m_labelValueMap[model][label] = rm->getValueMaximum() + 1.f; + if (m_labelValueMap[modelId].find(label) == m_labelValueMap[modelId].end()) { + m_labelValueMap[modelId][label] = rm->getValueMaximum() + 1.f; } - value = m_labelValueMap[model][label]; + value = m_labelValueMap[modelId][label]; } else { value = values[0]; } diff -r 77543124651b -r 6d09d68165a4 rdf/RDFImporter.h --- a/rdf/RDFImporter.h Thu Jul 04 18:04:21 2019 +0100 +++ b/rdf/RDFImporter.h Fri Jul 05 15:28:07 2019 +0100 @@ -22,8 +22,8 @@ #include #include "base/BaseTypes.h" +#include "data/model/Model.h" -class Model; class RDFImporterImpl; class ProgressReporter; @@ -48,14 +48,11 @@ QString getErrorString() const; /** - * Return a list of models imported from the RDF source. The - * models were heap-allocated by this class and are not registered - * with any other owner; the caller takes ownership and must - * arrange for them to be deleted manually or managed by a smart - * pointer. + * Return a list of models imported from the RDF source. The + * models have been newly created and registered with ById; the + * caller must arrange to release them. */ - //!!! todo: ModelId-ise - std::vector getDataModels(ProgressReporter *reporter); + std::vector getDataModels(ProgressReporter *reporter); enum RDFDocumentType { AudioRefAndAnnotations, diff -r 77543124651b -r 6d09d68165a4 transform/FeatureExtractionModelTransformer.cpp --- a/transform/FeatureExtractionModelTransformer.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/transform/FeatureExtractionModelTransformer.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -517,8 +517,7 @@ if (out) { out->setSourceModel(getInputModel()); - ModelById::add(out); - m_outputs.push_back(out->getId()); + m_outputs.push_back(ModelById::add(out)); } } @@ -604,8 +603,8 @@ additional->setScaleUnits(baseModel->getScaleUnits()); additional->setRDFTypeURI(baseModel->getRDFTypeURI()); - ModelId additionalId = additional->getId(); - ModelById::add(std::shared_ptr(additional)); + ModelId additionalId = ModelById::add + (std::shared_ptr(additional)); m_additionalModels[n][binNo] = additionalId; return additionalId; } @@ -648,7 +647,8 @@ } if (m_abandoned) return; - auto input = ModelById::getAs(getInputModel()); + ModelId inputId = getInputModel(); + auto input = ModelById::getAs(inputId); if (!input) { abandon(); return; @@ -677,7 +677,7 @@ if (frequencyDomain) { for (int ch = 0; ch < channelCount; ++ch) { FFTModel *model = new FFTModel - (input->getId(), + (inputId, channelCount == 1 ? m_input.getChannel() : ch, primaryTransform.getWindowType(), blockSize, diff -r 77543124651b -r 6d09d68165a4 transform/RealTimeEffectModelTransformer.cpp --- a/transform/RealTimeEffectModelTransformer.cpp Thu Jul 04 18:04:21 2019 +0100 +++ b/transform/RealTimeEffectModelTransformer.cpp Fri Jul 05 15:28:07 2019 +0100 @@ -93,8 +93,7 @@ auto model = std::make_shared (input->getSampleRate(), outputChannels); - ModelById::add(model); - m_outputs.push_back(model->getId()); + m_outputs.push_back(ModelById::add(model)); } else { @@ -103,8 +102,7 @@ 0.0, 0.0, false); if (m_units != "") model->setScaleUnits(m_units); - ModelById::add(model); - m_outputs.push_back(model->getId()); + m_outputs.push_back(ModelById::add(model)); } }