HKUST-Aerial-Robotics / VINS-Mobile

Monocular Visual-Inertial State Estimator on Mobile Phones
GNU General Public License v3.0
1.28k stars 525 forks source link

bug report #80

Closed vonzy closed 7 years ago

vonzy commented 7 years ago

In vins.cpp, the following code segment seems trying to release pre_integrations between all_image_frame.begin() and it_0.

but actually 'all_image_frame.erase(all_image_frame.begin(), it_0);' will remove all ImageFrame from all_image_frame.begin() to it_0, with it_0 not being removed. Then 'delete it_0->second.pre_integration;' will make the first ImageFrame of all_image_frame have a pre_integration member which have been released.

So the right code might be

        if (solver_flag == INITIAL)
        {
            double t_0 = Headers[0];
            map<double, ImageFrame>::iterator it_0;
            map<double, ImageFrame>::iterator it;
            it_0 = all_image_frame.find(t_0);
            // here should delete integration between  (all_image_frame.begin(), it_0)
            for (it = all_image_frame.begin(); it != it_0; ++it) {
                 delete it->second.pre_integration;
                 it->second.pre_integration = NULL;
            }
            all_image_frame.erase(all_image_frame.begin(), it_0);
        }
PeiliangLi commented 7 years ago

In our code, pre_integration connects the current frame and the previous frame. So the first frame doesn't need to have pre_integration term.

vonzy commented 7 years ago

holyshit.... when I debug the program, I found a member points to released resource, I thought it was a bug... okay, you are right!