changeset 1752:6d09d68165a4 by-id

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
author Chris Cannam
date Fri, 05 Jul 2019 15:28:07 +0100
parents 77543124651b
children d954dfccd922 043e05c956fb
files base/ById.cpp base/ById.h base/test/TestById.h data/model/AlignmentModel.cpp data/model/AlignmentModel.h data/model/DeferredNotifier.h data/model/Dense3DModelPeakCache.cpp data/model/Dense3DModelPeakCache.h data/model/EditableDenseThreeDimensionalModel.cpp data/model/FFTModel.cpp data/model/ImageModel.h data/model/Model.cpp data/model/Model.h data/model/NoteModel.h data/model/ReadOnlyWaveFileModel.cpp data/model/RegionModel.h data/model/SparseOneDimensionalModel.h data/model/SparseTimeValueModel.h data/model/TextModel.h data/model/WritableWaveFileModel.cpp data/model/WritableWaveFileModel.h data/model/test/TestFFTModel.h rdf/RDFImporter.cpp rdf/RDFImporter.h transform/FeatureExtractionModelTransformer.cpp transform/RealTimeEffectModelTransformer.cpp
diffstat 26 files changed, 203 insertions(+), 196 deletions(-) [+]
line wrap: on
line diff
--- 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<WithId> item) {
+    int add(std::shared_ptr<WithId> 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<int, std::shared_ptr<WithId>> m_items;
 };
 
-void
-AnyById::add(int id, std::shared_ptr<WithId> item)
+int
+AnyById::add(std::shared_ptr<WithId> item)
 {
-    impl().add(id, item);
+    return impl().add(item);
 }
 
 void
--- 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 <typename Item, typename Id>
+    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<WithId>);
+    static int add(std::shared_ptr<WithId>);
     static void release(int);
     static std::shared_ptr<WithId> get(int); 
 
@@ -157,11 +164,8 @@
 {
 public:
     static Id add(std::shared_ptr<Item> 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<Item> get(Id id) {
         return getAs<Item>(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
--- 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<A> {};
+// We'll need to change access levels for getId() and getUntypedId()
+// to test the raw calls
+
+struct A : public WithTypedId<A> { public: using WithTypedId<A>::getId; };
 struct B1 : public A {};
 struct B2 : public A {};
 
@@ -31,7 +34,7 @@
 
 typedef TypedById<A, A::Id> 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<A>();
-        AnyById::add(a->getId().untyped, a);
+        int id = AnyById::add(a);
+        QCOMPARE(id, a->getId().untyped);
 
-        auto aa = AnyById::getAs<A>(a->getId().untyped);
+        auto aa = AnyById::getAs<A>(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<A>();
-        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<A>();
+        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);
     }
 
--- 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<SparseTimeValueModel>(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);
         }
     }
 }
--- 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;
--- 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<sv_frame_t> m_extents;
--- 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
--- 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;
--- 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<QString> 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());
         }            
     }
 }
--- 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<DenseTimeValueModel>(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)));
     }
 }
 
--- 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);
     }
 
     /**
--- 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)));
     }
         
     
--- 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;
 };
 
--- 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);
     }
 
--- 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
--- 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);
     }
     
--- 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);
     }
     
     /**
--- 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);
     }
     
     /**
--- 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);
     }
 
     /**
--- 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
--- 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;
--- 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<Sort> sorts, int length, int pad) {
         auto mwm = std::make_shared<MockWaveModel>(sorts, length, pad);
-        ModelById::add(mwm);
-        return mwm->getId();
+        return ModelById::add(mwm);
     }
 
     void releaseMock(ModelId id) {
--- 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<Model *> getDataModels(ProgressReporter *);
+    std::vector<ModelId> getDataModels(ProgressReporter *);
 
 protected:
     BasicStore *m_store;
@@ -66,22 +66,22 @@
 
     QString m_uristring;
     QString m_errorString;
-    std::map<QString, Model *> m_audioModelMap;
+    std::map<QString, ModelId> m_audioModelMap;
     sv_samplerate_t m_sampleRate;
 
-    std::map<Model *, std::map<QString, float> > m_labelValueMap;
+    std::map<ModelId, std::map<QString, float> > m_labelValueMap;
 
-    void getDataModelsAudio(std::vector<Model *> &, ProgressReporter *);
-    void getDataModelsSparse(std::vector<Model *> &, ProgressReporter *);
-    void getDataModelsDense(std::vector<Model *> &, ProgressReporter *);
+    void getDataModelsAudio(std::vector<ModelId> &, ProgressReporter *);
+    void getDataModelsSparse(std::vector<ModelId> &, ProgressReporter *);
+    void getDataModelsDense(std::vector<ModelId> &, 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<float> &, QString);
 };
 
@@ -119,7 +119,7 @@
     return m_d->getErrorString();
 }
 
-std::vector<Model *>
+std::vector<ModelId>
 RDFImporter::getDataModels(ProgressReporter *r)
 {
     return m_d->getDataModels(r);
@@ -169,10 +169,10 @@
     return m_errorString;
 }
 
-std::vector<Model *>
+std::vector<ModelId>
 RDFImporterImpl::getDataModels(ProgressReporter *reporter)
 {
-    std::vector<Model *> models;
+    std::vector<ModelId> models;
 
     getDataModelsAudio(models, reporter);
 
@@ -206,7 +206,7 @@
 }
 
 void
-RDFImporterImpl::getDataModelsAudio(std::vector<Model *> &models,
+RDFImporterImpl::getDataModelsAudio(std::vector<ModelId> &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<ReadOnlyWaveFileModel>
+            (*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<Model *> &models,
+RDFImporterImpl::getDataModelsDense(std::vector<ModelId> &models,
                                     ProgressReporter *reporter)
 {
     if (reporter) {
@@ -342,7 +343,7 @@
 
         if (height == 1) {
 
-            SparseTimeValueModel *m = new SparseTimeValueModel
+            auto m = std::make_shared<SparseTimeValueModel>
                 (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<EditableDenseThreeDimensionalModel>
                 (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<Model *> &models,
+RDFImporterImpl::getDataModelsSparse(std::vector<ModelId> &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<QString, std::map<QString, std::map<int, std::map<bool, Model *> > > >
+    // presence of duration to model id.  Whee!
+    std::map<QString, std::map<QString, std::map<int, std::map<bool, ModelId> > > >
         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>(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<SparseOneDimensionalModel *>(model);
-    if (sodm) {
+    if (auto sodm = ModelById::getAs<SparseOneDimensionalModel>(modelId)) {
         Event point(ftime, label);
         sodm->add(point);
         return;
     }
 
-    TextModel *tm =
-        dynamic_cast<TextModel *>(model);
-    if (tm) {
+    if (auto tm = ModelById::getAs<TextModel>(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<SparseTimeValueModel *>(model);
-    if (stvm) {
+    if (auto stvm = ModelById::getAs<SparseTimeValueModel>(modelId)) {
         Event e(ftime, values.empty() ? 0.f : values[0], label);
         stvm->add(e);
         return;
     }
 
-    NoteModel *nm =
-        dynamic_cast<NoteModel *>(model);
-    if (nm) {
+    if (auto nm = ModelById::getAs<NoteModel>(modelId)) {
         if (haveDuration) {
             float value = 0.f, level = 1.f;
             if (!values.empty()) {
@@ -764,16 +756,14 @@
         return;
     }
 
-    RegionModel *rm = 
-        dynamic_cast<RegionModel *>(model);
-    if (rm) {
+    if (auto rm = ModelById::getAs<RegionModel>(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];
         }
--- 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 <vector>
 
 #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<Model *> getDataModels(ProgressReporter *reporter);
+    std::vector<ModelId> getDataModels(ProgressReporter *reporter);
 
     enum RDFDocumentType {
         AudioRefAndAnnotations,
--- 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<SparseTimeValueModel>(additional));
+    ModelId additionalId = ModelById::add
+        (std::shared_ptr<SparseTimeValueModel>(additional));
     m_additionalModels[n][binNo] = additionalId;
     return additionalId;
 }
@@ -648,7 +647,8 @@
     }
     if (m_abandoned) return;
 
-    auto input = ModelById::getAs<DenseTimeValueModel>(getInputModel());
+    ModelId inputId = getInputModel();
+    auto input = ModelById::getAs<DenseTimeValueModel>(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,
--- 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<WritableWaveFileModel>
             (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));
     }
 }