Bugs with code?

[cy-goh]

Hello,

in Track.cpp,

1) for line 361, Shouldn't bool c5 = fdOdo.theta >= 0.0349f be bool c5 = fabs(dOdo.theta) >= 0.0349f?

2) for line 364, after converting to camera frame CTC, shouldnt it be cv::Mat xy = cTc.rowRange(0,3).col(3)?

        cv::Mat xy = cTc.rowRange(0,2).col(3);
        bool c6 = cv::norm(xy) >= ( 0.0523f * Config::UPPER_DEPTH * 0.1f ); // 3 degree = 0.0523 rad

3) The code for checking parallax when triangulating new points, shouldnt the higher the parallax, the better?

bool checkParallax(const Point3f &o1, const Point3f &o2, const Point3f &pt3, int minDegree){
    float minCos[4] = {0.9998, 0.9994, 0.9986, 0.9976};
    Point3f p1 = pt3 - o1;
    Point3f p2 = pt3 - o2;
    float cosParallax = cv::norm(p1.dot(p2)) / ( cv::norm(p1) * cv::norm(p2) );
    return cosParallax >= minCos[minDegree-1];
    // return cosParallax < minCos[minDegree-1];
}

[CrisGao]

When the code for checking parallax is changed to return cosParallax >= minCos[minDegree-1];,then there are few feature points that can be successfully triangulated, and the calculation results basically rely on the data of odom.In theory, the parallax angle should be larger, so it would be better to improve it. @izhengfan

[cy-goh]

@CrisGao , yes you are right. I made a mistake on the cosine.

[izhengfan]

@cy-goh Good catch.

@CrisGao Thanks for the comment. According to your comment, I guess the current checkParallax() does not need revision?

[CrisGao]

@izhengfan ,yes checkParallax()is not wrong.If I understand correctly, I think cv::Mat xy = cTc.rowRange(0,2).col(3); is not wrong, cv::norm(xy) is the Euclidean distance of the plane

[izhengfan]

@izhengfan ,yes checkParallax()is not wrong.If I understand correctly, I think cv::Mat xy = cTc.rowRange(0,2).col(3); is not wrong, cv::norm(xy) is the Euclidean distance of the plane

Thanks for the reminding, let me rethink it.

[cy-goh]

@izhengfan ,yes checkParallax()is not wrong.If I understand correctly, I think cv::Mat xy = cTc.rowRange(0,2).col(3); is not wrong, cv::norm(xy) is the Euclidean distance of the plane

I think cTc is in the camera coordinate frame, with y axis pointing down.

[izhengfan]

@izhengfan ,yes checkParallax()is not wrong.If I understand correctly, I think cv::Mat xy = cTc.rowRange(0,2).col(3); is not wrong, cv::norm(xy) is the Euclidean distance of the plane

I think cTc is in the camera coordinate frame, with y axis pointing down.

Yes, the physical meaning of c6 is whether the distance between the camera center in two moments is large enough to get enough parallax, so it should be like xy = cTc.rowRange(0,3).col(3).

[CrisGao]

I’m sorry that I misunderstood it. Only when the camera is facing up, cTc.rowRange(0,2).col(3) is the translation of the xy plane.

[izhengfan]

I’m sorry that I misunderstood it. Only when the camera is facing up, cTc.rowRange(0,2).col(3) is the translation of the xy plane.

Yes, when the camera is facing forward or to the side, the z component in cTc can affect the parallax.

[cy-goh]

@izhengfan i found quite a fair bit of other bugs. Do you want me to open a new issue, or continue here?

[izhengfan]

@izhengfan i found quite a fair bit of other bugs. Do you want me to open a new issue, or continue here?

Actually you can fork and open a PR. Thus your contributions will be counted.