Bug in tracksMerger

Author: Fabien Servant

src/aliceVision/track/TracksMerger.cpp

@@ -5,6 +5,7 @@
 // You can obtain one at https://mozilla.org/MPL/2.0/.
 
 #include <aliceVision/track/TracksMerger.hpp>
+#include <aliceVision/system/Logger.hpp>
 
 namespace aliceVision
 {
@@ -13,24 +14,32 @@ namespace track
 
 bool TracksMerger::addTrackMap(const track::TracksMap & inputTracks)
 {
+    // Loop over the tracks to add 
     for (const auto & [idTrack, track]: inputTracks)
     {
         IndexT foundTrack = UndefinedIndexT;
-        size_t newSize = track.featPerView.size();
-        
+
+        // Find if one of the feature is already existing in a track
+        // In this case we would like to merge both tracks instead of adding a track
         for (const auto & [idView, feat]: track.featPerView)
         {
             TuplePoint tp = std::make_tuple(track.descType, idView, feat.featureId);
-            auto it = _existingTracks.find(tp);
-            if (it != _existingTracks.end())
+            if (_existingTracks.find(tp) == _existingTracks.end())
+            {
+                continue;
+            }
+
+            if (foundTrack != UndefinedIndexT)
             {
-                foundTrack = it->second;
-                break;
+                ALICEVISION_LOG_INFO("Multiple tracks possible. Keeping the first one.");
+                continue;
             }
+
+            foundTrack = _existingTracks[tp];
         }
-        
+
         if (foundTrack == UndefinedIndexT)
-        {   
+        {
             //Simply add track
             foundTrack = _lastIndex;
             _lastIndex++;
@@ -40,25 +49,20 @@ bool TracksMerger::addTrackMap(const track::TracksMap & inputTracks)
         auto & outputTrack = _tracks[foundTrack];
         outputTrack.descType = track.descType;
 
-        //Previous Size is either 0 if new track, or the size of the matching track
-        size_t oldSize = outputTrack.featPerView.size();
-        
-        //Append all features from existing track
+        // Find if one of the feature is already existing in a track
+        // In this case we would like to merge both tracks instead of adding a track
         for (const auto & [idView, feat]: track.featPerView)
         {
-            TuplePoint tp = std::make_tuple(track.descType, idView, feat.featureId);
-            _existingTracks[tp] = foundTrack;
-
-            // Replace only if the new tracks is longer than the old one.
-            if (outputTrack.featPerView.find(idView) != outputTrack.featPerView.end())
+            if (track.featPerView.find(idView) != track.featPerView.end())
             {
-                if (newSize < oldSize)
-                {
-                    continue;
-                }
-            } 
-            
+                //Keep the accumulated one
+                continue;
+            }
+
             outputTrack.featPerView[idView] = feat;
+
+            TuplePoint tp = std::make_tuple(track.descType, idView, feat.featureId);
+            _existingTracks[tp] = foundTrack;
         }
     }
 

src/aliceVision/track/TracksMerger.hpp

@@ -20,6 +20,7 @@ class TracksMerger
 public:
     //viewId, featureId is a unique identifier for an observation
     using TuplePoint = std::tuple<feature::EImageDescriberType, IndexT, std::size_t>;
+
 public:
 
     /**