Fix upvote in voteForKeyframe

src/processor_tracker.cpp

@@ -150,6 +150,13 @@ void ProcessorTracker::process(CaptureBasePtr const _incoming_ptr)
         {
             processKnown();
 
+            // eventually add more features
+            if (last_ptr_->getFeatureList().size() < params_tracker_->min_features_for_keyframe)
+            {
+                WOLF_TRACE("Adding more features...");
+                processNew(params_tracker_->max_new_features);
+            }
+
             if (voteForKeyFrame() && permittedKeyFrame())
             {
                 // We create a KF
@@ -164,9 +171,9 @@ void ProcessorTracker::process(CaptureBasePtr const _incoming_ptr)
                 // process
                 processNew(params_tracker_->max_new_features);
 
-                // Set key
-                last_ptr_->getFramePtr()->setKey();
-
+                //                // Set key
+                //                last_ptr_->getFramePtr()->setKey();
+                //
                 // Set state to the keyframe
                 last_ptr_->getFramePtr()->setState(getProblem()->getState(last_ptr_->getTimeStamp()));
 
@@ -187,10 +194,6 @@ void ProcessorTracker::process(CaptureBasePtr const _incoming_ptr)
             {
                 // We do not create a KF
 
-                // eventually add more features
-                if (last_ptr_->getFeatureList().size() < params_tracker_->min_features_for_keyframe)
-                    processNew(params_tracker_->max_new_features);
-
                 // Advance this
                 last_ptr_->getFramePtr()->addCapture(incoming_ptr_); // Add incoming Capture to the tracker's last Frame
                 last_ptr_->remove();
@@ -207,6 +210,7 @@ void ProcessorTracker::process(CaptureBasePtr const _incoming_ptr)
             break;
     }
 
+    number_of_tracks_ = last_ptr_->getFeatureList().size();
     postProcess();
 }
 

src/processor_tracker.h

@@ -89,6 +89,8 @@ class ProcessorTracker : public ProcessorBase
         FeatureBaseList new_features_last_;     ///< List of new features in \b last for landmark initialization and new key-frame creation.
         FeatureBaseList new_features_incoming_; ///< list of the new features of \b last successfully tracked in \b incoming
 
+        size_t number_of_tracks_;
+
     public:
         ProcessorTracker(const std::string& _type,
                          ProcessorParamsTrackerPtr _params_tracker);
@@ -194,6 +196,16 @@ class ProcessorTracker : public ProcessorBase
 
         FeatureBaseList& getNewFeaturesListLast();
 
+        const size_t& previousNumberOfTracks() const
+        {
+            return number_of_tracks_;
+        }
+
+        size_t& previousNumberOfTracks()
+        {
+            return number_of_tracks_;
+        }
+
     protected:
 
         void computeProcessingStep();

src/processors/processor_tracker_feature_trifocal.cpp

@@ -266,41 +266,24 @@ bool ProcessorTrackerFeatureTrifocal::correctFeatureDrift(const FeatureBasePtr _
 
 bool ProcessorTrackerFeatureTrifocal::voteForKeyFrame()
 {
-    // List of conditions
-
-//    // A. crossing voting threshold with ascending number of features
-//    bool vote_up = true;
-//    // 1. vote if we did not have enough features before
-//    vote_up = vote_up && (last_ptr_->getFeatureList().size() < params_tracker_feature_trifocal_->min_features_for_keyframe);
-//    // 2. vote if we have enough features now
-//    vote_up = vote_up && (incoming_ptr_->getFeatureList().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
-//
-//    // B. crossing voting threshold with descending number of features
-//    bool vote_down = true;
-//    // 1. vote if we had enough features before
-//    vote_down = vote_down && (last_ptr_->getFeatureList().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
-//    // 2. vote if we have not enough features now
-//    vote_down = vote_down && (incoming_ptr_->getFeatureList().size() < params_tracker_feature_trifocal_->min_features_for_keyframe);
-//
-
-//    FIX: This LIST are ALWAYS empty.
-//    WOLF_TRACE("getNewFeaturesListLast().size() ", getNewFeaturesListLast().size());
-//    WOLF_TRACE("getNewFeaturesListIncoming().size() ", getNewFeaturesListIncoming().size());
-
-
     // A. crossing voting threshold with ascending number of features
     bool vote_up = true;
     // 1. vote if we did not have enough features before
-    vote_up = vote_up && (getNewFeaturesListLast().size() < params_tracker_feature_trifocal_->min_features_for_keyframe);
+    vote_up = vote_up && (previousNumberOfTracks() < params_tracker_feature_trifocal_->min_features_for_keyframe);
     // 2. vote if we have enough features now
-    vote_up = vote_up && (getNewFeaturesListIncoming().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
+    vote_up = vote_up && (incoming_ptr_->getFeatureList().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
 
     // B. crossing voting threshold with descending number of features
     bool vote_down = true;
     // 1. vote if we had enough features before
-    vote_down = vote_down && (getNewFeaturesListLast().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
+    vote_down = vote_down && (last_ptr_->getFeatureList().size() >= params_tracker_feature_trifocal_->min_features_for_keyframe);
     // 2. vote if we have not enough features now
-    vote_down = vote_down && (getNewFeaturesListIncoming().size() < params_tracker_feature_trifocal_->min_features_for_keyframe);
+    vote_down = vote_down && (incoming_ptr_->getFeatureList().size() < params_tracker_feature_trifocal_->min_features_for_keyframe);
+
+    if (vote_up)
+        WOLF_TRACE("VOTE UP");
+    if (vote_down)
+        WOLF_TRACE("VOTE DOWN");
 
     return vote_up || vote_down;
 }

[Joan Vallvé Navarro]

Why are we doing processNew() if we haven't decided yet if a new KF is made?

[Joan Solà Ortega]

This is here to add more features in case there are less than required.

We made it so that we can ramp up the number of features across several captures. It's like a workaround. I would like to not have to resort to these kind of things, but it may happen that we are low on features for a while, and a condition featureList()->size() < min_ftr_for_kf in voteForKf() would fire one new keyframe per capture while the scene is not showing enough features.

[Joan Vallvé Navarro]

But it has no impact if a KF is not created since no factors are added and these tracks are starting in a non-key frame...

[Joan Solà Ortega]

Maybe...

So how to tacke this situation where there are insufficient features, less than the necessary for a KF? The think is that we need to avoid the processor creating KFs stupidly

[Joan Vallvé Navarro]

Auxiliary frames are the solution (temporary KF), but you need to add the frame to the solver in order to make these features work.

Here, everything is based on the assumption of deciding KF (or auxiliary) in last. Maybe, this assumption is not fully applicable to the trifocal approach..

[Joan Solà Ortega]

No no, these frames do not go into solving... if there's no features the only thing is wait, but we need to keep detecting things to see if there are indeed features!!!

[Joan Solà Ortega]

Otherwise a processor that runs out of features is dead forever ! ??

[Joan Vallvé Navarro]

If you run out of features, your problem gets disconnected. Apart from this drawback, the processor is not dead, whenever you (or other processor) create a KF, processNew() is called (and so detectNewFeatures()).

I think that to handle the case in which no new features were detected in processNew(), we should add a check after calling it inside the if(voteForKeyFrame()...). Then, abort the KF creation if no new features were detected.

But, now, this call to processNew() is dangerous because is not following the assumptions made in this function: (see comment from processor_tracker_feature.cpp)

/* Rationale: A keyFrame will be created using the last Capture.
     * First, we work on the last Capture to detect new Features,
     * When done, we need to track these new Features to the incoming Capture.
     * At the end, all new Features are appended to the lists of known Features in
     * the last and incoming Captures.
     */

[Joan Solà Ortega]

Yes I agree it's not so good...

The check inside voteForKf() is insufficient, since it is only done once. If not passed, next time you dont vote and you never test again.

Unless you always vote when there are not enough features, but this destroys the logic of voting.

And waiting for external KFs is not good either because there might not be any other sensor.

Maybe this can be left to any one of the individual processors? I dont know how to tackle everything in the general case.

[Joan Vallvé Navarro]

I do not understand why "If not passed, next time you dont vote and you never test again.", it depends on how the voteForKeyFrame() is implemented.. why "always vote when there are not enough features" destroys the logic of voting?