Make project on timeVaryingTransforms safer

Add a TimeInvariantWrapperTransform member of TimeVaryingTransform, and use that in TimeVaryingTransform::project for thread safety. This is the strategy previously used in Distribute, but putting the alias on TimeVaryingTransform gives us more safety if Distribute is not in fact the top level transform. Note: copies are only made off of calls to project, so having the alias on every TimeVaryingTransform is not too wasteful.

Make project on timeVaryingTransforms safer
Add a TimeInvariantWrapperTransform member of TimeVaryingTransform, and use that in TimeVaryingTransform::project for thread safety. This is the strategy previously used in Distribute, but putting the alias on TimeVaryingTransform gives us more safety if Distribute is not in fact the top level transform. Note: copies are only made off of calls to project, so having the alias on every TimeVaryingTransform is not too wasteful.
Charles Otto
1 parent 5bab078d
Showing 3 changed files with 91 additions and 68 deletions
openbr/plugins/meta.cpp
openbr/plugins/openbr_internal.h
openbr/plugins/stream.cpp
@@ -264,7 +264,6 @@ class ContractTransform : public UntrainableMetaTransform
     virtual void project(const TemplateList &src, TemplateList &dst) const
     {
-        //dst = Expanded(src);
         if (src.empty()) return;
         Template out;
@@ -682,8 +681,10 @@ public:
     void init()
     {
-        if (transform && transform->timeVarying())
-            transform = new br::TimeInvariantWrapperTransform(transform);
+        if (!transform)
+            return;
+
+        trainable = transform->trainable;
     }
 };
@@ -24,81 +24,27 @@ private:
 };
 /*!
- * \brief A br::MetaTransform that does not require training data.
+ * \brief A br::Transform expecting multiple matrices per template.
  */
-class BR_EXPORT UntrainableMetaTransform : public UntrainableTransform
+class BR_EXPORT MetaTransform : public Transform
 {
     Q_OBJECT
 protected:
-    UntrainableMetaTransform() : UntrainableTransform(false) {}
-};
-
-/*!
- * \brief A br::Transform for which the results of project may change due to prior calls to project
- */
-class BR_EXPORT TimeVaryingTransform : public Transform
-{
-    Q_OBJECT
-
-public:
-    virtual bool timeVarying() const { return true; }
-
-    virtual void project(const Template &src, Template &dst) const
-    {
-        qFatal("No const project defined for time-varying transform");
-        (void) dst; (void) src;
-    }
-
-    virtual void project(const TemplateList &src, TemplateList &dst) const
-    {
-        qFatal("No const project defined for time-varying transform");
-        (void) dst; (void) src;
-    }
-
-    // Get a compile failure if this isn't here to go along with the other
-    // projectUpdate, no idea why
-    virtual void projectUpdate(const Template & src, Template & dst)
-    {
-        (void) src; (void) dst;
-        qFatal("do something useful");
-    }
-
-    virtual void projectUpdate(const TemplateList &src, TemplateList &dst)
-    {
-        foreach (const Template & src_part, src) {
-            Template out;
-            projectUpdate(src_part, out);
-            dst.append(out);
-        }
-    }
-
-    /*!
-     *\brief For transforms that don't do any training, this default implementation
-     * which creates a new copy of the Transform from its description string is sufficient.
-     */
-    virtual Transform * smartCopy()
-    {
-        return this->clone();
-    }
-
-
-protected:
-    TimeVaryingTransform(bool independent = true, bool trainable = true) : Transform(independent, trainable) {}
+    MetaTransform() : Transform(false) {}
 };
 /*!
- * \brief A br::Transform expecting multiple matrices per template.
+ * \brief A br::MetaTransform that does not require training data.
  */
-class BR_EXPORT MetaTransform : public Transform
+class BR_EXPORT UntrainableMetaTransform : public UntrainableTransform
 {
     Q_OBJECT
 protected:
-    MetaTransform() : Transform(false) {}
+    UntrainableMetaTransform() : UntrainableTransform(false) {}
 };
-
 class TransformCopier : public ResourceMaker<Transform>
 {
 public:
@@ -151,6 +97,71 @@ private:
     Transform * baseTransform;
 };
+/*!
+ * \brief A br::Transform for which the results of project may change due to prior calls to project
+ */
+class BR_EXPORT TimeVaryingTransform : public Transform
+{
+    Q_OBJECT
+
+public:
+
+    virtual bool timeVarying() const { return true; }
+
+    virtual void project(const Template &src, Template &dst) const
+    {
+        timeInvariantAlias->project(src,dst);
+    }
+
+    virtual void project(const TemplateList &src, TemplateList &dst) const
+    {
+        timeInvariantAlias->project(src,dst);
+    }
+
+    // Get a compile failure if this isn't here to go along with the other
+    // projectUpdate, no idea why
+    virtual void projectUpdate(const Template & src, Template & dst)
+    {
+        (void) src; (void) dst;
+        qFatal("do something useful");
+    }
+
+    virtual void projectUpdate(const TemplateList &src, TemplateList &dst)
+    {
+        foreach (const Template & src_part, src) {
+            Template out;
+            projectUpdate(src_part, out);
+            dst.append(out);
+        }
+    }
+
+    /*!
+     *\brief For transforms that don't do any training, this default implementation
+     * which creates a new copy of the Transform from its description string is sufficient.
+     */
+    virtual Transform * smartCopy()
+    {
+        return this->clone();
+    }
+
+    void init()
+    {
+        delete timeInvariantAlias;
+        timeInvariantAlias = new TimeInvariantWrapperTransform(this);
+    }
+
+protected:
+    Transform * timeInvariantAlias;
+    TimeVaryingTransform(bool independent = true, bool trainable = true) : Transform(independent, trainable)
+    {
+        timeInvariantAlias = NULL;
+    }
+    ~TimeVaryingTransform()
+    {
+        delete timeInvariantAlias;
+    }
+};
+
 /*!
  * \brief A MetaTransform that aggregates some sub-transforms
@@ -165,15 +176,17 @@ public:
     virtual void project(const Template &src, Template &dst) const
     {
-        if (timeVarying()) qFatal("No const project defined for time-varying transform");
+        if (timeVarying()) {
+            timeInvariantAlias->project(src,dst);
+            return;
+        }
         _project(src, dst);
     }
     virtual void project(const TemplateList &src, TemplateList &dst) const
     {
         if (timeVarying()) {
-            CompositeTransform * non_const = const_cast<CompositeTransform *>(this);
-            non_const->projectUpdate(src,dst);
+            timeInvariantAlias->project(src,dst);
             return;
         }
         _project(src, dst);
@@ -190,6 +203,10 @@ public:
             isTimeVarying = isTimeVarying || transform->timeVarying();
             trainable = trainable || transform->trainable;
         }
+
+        // If we are time varying, set up timeInvariantAlias
+        if (this->timeVarying())
+            TimeVaryingTransform::init();
     }
     /*!
@@ -707,8 +707,7 @@ public:
         if (input == NULL) {
             qFatal("null input to multi-thread stage");
         }
-        // Project the input we got
-        transform->projectUpdate(input->data);
+        input->data >> *transform;
         should_continue = nextStage->tryAcquireNextStage(input);
@@ -1300,6 +1299,12 @@ public:
     {
         if (!transform)
             return;
+
+        // Set up timeInvariantAlias
+        // this is only safe because copies are actually made in project
+        // calls, not during init.
+        TimeVaryingTransform::init();
+
         trainable = transform->trainable;
         basis.setParent(this->parent());