changeset 109:78a7c77ba432

A more general solution (I hope) to the problem of making sure transforms are always run in a consistent order

--- a/runner/FeatureExtractionManager.cpp	Thu Oct 02 14:31:47 2014 +0100
+++ b/runner/FeatureExtractionManager.cpp	Thu Oct 02 14:54:09 2014 +0100
@@ -350,6 +350,10 @@
         plugin = m_transformPluginMap[transform];
     }
 
+    if (m_plugins.find(plugin) == m_plugins.end()) {
+        m_orderedPlugins.push_back(plugin);
+    }
+
     m_plugins[plugin][transform] = writers;
 
     return true;
@@ -613,12 +617,11 @@
     int latestEndFrame = frameCount;
     bool haveExtents = false;
 
-    for (PluginMap::iterator pi = m_plugins.begin();
-         pi != m_plugins.end(); ++pi) {
+    foreach (Plugin *plugin, m_orderedPlugins) {
 
-        Plugin *plugin = pi->first;
+        PluginMap::iterator pi = m_plugins.find(plugin);
 
-//        std::cerr << "Calling reset on " << plugin << std::endl;
+        std::cerr << "Calling reset on " << plugin << std::endl;
         plugin->reset();
 
         for (TransformWriterMap::iterator ti = pi->second.begin();
@@ -641,6 +644,11 @@
                 latestEndFrame = startFrame + duration;
             }
 
+            cerr << "startFrame for transform " << startFrame << endl;
+            cerr << "duration for transform " << duration << endl;
+            cerr << "earliestStartFrame becomes " << earliestStartFrame << endl;
+            cerr << "latestEndFrame becomes " << latestEndFrame << endl;
+
             haveExtents = true;
 
             string outputId = transform.getOutput().toStdString();
@@ -672,8 +680,9 @@
     int startFrame = earliestStartFrame;
     int endFrame = latestEndFrame;
     
-    for (PluginMap::iterator pi = m_plugins.begin();
-         pi != m_plugins.end(); ++pi) { 
+    foreach (Plugin *plugin, m_orderedPlugins) {
+
+        PluginMap::iterator pi = m_plugins.find(plugin);
 
         for (TransformWriterMap::const_iterator ti = pi->second.begin();
              ti != pi->second.end(); ++ti) {
@@ -745,10 +754,9 @@
         Vamp::RealTime timestamp = Vamp::RealTime::frame2RealTime
             (i, m_sampleRate);
         
-        for (PluginMap::iterator pi = m_plugins.begin();
-             pi != m_plugins.end(); ++pi) {
+        foreach (Plugin *plugin, m_orderedPlugins) {
 
-            Plugin *plugin = pi->first;
+            PluginMap::iterator pi = m_plugins.find(plugin);
             Plugin::FeatureSet featureSet = plugin->process(data, timestamp);
 
             if (!m_summariesOnly) {
@@ -765,41 +773,9 @@
 
     lifemgr.destroy(); // deletes reader, data
         
-    // In order to ensure our results are written to the output in a
-    // fixed order (and not one that depends on the pointer value of
-    // each plugin on the heap in any given run of the program) we
-    // take the plugins' entries from the plugin map and sort them
-    // into a new, temporary map that is indexed by the first
-    // transform for each plugin. We then iterate over than instead of
-    // over m_plugins in order to get the right ordering.
+    foreach (Plugin *plugin, m_orderedPlugins) {
 
-    // This is not the most elegant way to do this -- it would be more
-    // elegant to impose an ordering directly on the plugins that are
-    // used as keys to m_plugins. But the plugin type comes from the
-    // Vamp SDK, so this change is more localised.
-
-    // Thanks to Matthias for this.
-
-    typedef map<Transform, PluginMap::value_type> OrderedPluginMap;
-    OrderedPluginMap orderedPlugins;
-
-    for (PluginMap::iterator pi = m_plugins.begin();
-         pi != m_plugins.end(); ++pi) { 
-        Transform firstForPlugin = (pi->second).begin()->first;
-        orderedPlugins.insert(OrderedPluginMap::value_type(firstForPlugin, *pi));
-    }
-
-    for (OrderedPluginMap::iterator superPi = orderedPlugins.begin();
-         superPi != orderedPlugins.end(); ++superPi) {
-
-        // The value we extract from this map is just the same as the
-        // value_type we get from iterating over our PluginMap
-        // directly -- but we happen to get them in the right order
-        // now because the map iterator is ordered by the Transform
-        // key type ordering
-        PluginMap::value_type pi = superPi->second;
-
-        Plugin *plugin = pi.first;
+        PluginMap::iterator pi = m_plugins.find(plugin);
         Plugin::FeatureSet featureSet = plugin->getRemainingFeatures();
 
         if (!m_summariesOnly) {
@@ -942,8 +918,9 @@
 
 void FeatureExtractionManager::finish()
 {
-    for (PluginMap::iterator pi = m_plugins.begin();
-         pi != m_plugins.end(); ++pi) {
+    foreach (Plugin *plugin, m_orderedPlugins) {
+
+        PluginMap::iterator pi = m_plugins.find(plugin);
 
         for (TransformWriterMap::iterator ti = pi->second.begin();
              ti != pi->second.end(); ++ti) {

--- a/runner/FeatureExtractionManager.h	Thu Oct 02 14:31:47 2014 +0100
+++ b/runner/FeatureExtractionManager.h	Thu Oct 02 14:54:09 2014 +0100
@@ -79,6 +79,16 @@
     typedef map<Vamp::Plugin *, TransformWriterMap> PluginMap;
     PluginMap m_plugins;
 
+    // When we run plugins, we want to run them in a known order so as
+    // to get the same results on each run of Sonic Annotator with the
+    // same transforms. But if we just iterate through our PluginMap,
+    // we get them in an arbitrary order based on pointer
+    // address. This vector provides an underlying order for us. Note
+    // that the TransformWriterMap is consistently ordered (because
+    // the key is a Transform which has a proper ordering) so using
+    // this gives us a consistent order across the whole PluginMap
+    vector<Vamp::Plugin *> m_orderedPlugins;
+
     // And a map back from transforms to their plugins.  Note that
     // this is keyed by transform, not transform ID -- two differently
     // configured transforms with the same ID must use different

--- a/tests/expected/transforms-basic-percussiononsets-multiple-outputs-start-and-duration.csv	Thu Oct 02 14:31:47 2014 +0100
+++ b/tests/expected/transforms-basic-percussiononsets-multiple-outputs-start-and-duration.csv	Thu Oct 02 14:54:09 2014 +0100
@@ -29,6 +29,7 @@
 ,1.325079365,162
 ,1.336689342,176
 ,1.348299319,169
+,1.650158730
 ,1.359909297,186
 ,1.371519274,153
 ,1.383129251,166
@@ -61,7 +62,6 @@
 ,1.696598639,160
 ,1.708208616,179
 ,1.719818594,164
-,1.650158730
 ,1.731428571,157
 ,1.743038548,157
 ,1.754648526,194
@@ -94,6 +94,7 @@
 ,2.068117913,179
 ,2.079727891,162
 ,2.091337868,168
+,2.416417233
 ,2.102947845,177
 ,2.114557823,179
 ,2.126167800,169
@@ -126,7 +127,6 @@
 ,2.439637188,161
 ,2.451247165,166
 ,2.462857142,189
-,2.416417233
 ,2.474467120,167
 ,2.486077097,166
 ,2.497687074,150