6
\$\begingroup\$

I have written a code to stitch 2 images using SIFT keypoint descriptor and homography matrix for perspective transform. Are there any areas where code an be improved or optimized?

#include <opencv2/opencv.hpp>
#include <opencv2/highgui.hpp>
#include <opencv2/imgproc.hpp>
#include <opencv2/xfeatures2d.hpp>
#include <opencv2/features2d.hpp>
#include <stdio.h>
#include <iostream>
#include <vector>
#include<algorithm>
#include<cmath>
using namespace cv;
using namespace std;
using namespace xfeatures2d;

void get_Images(Mat &img1, Mat &img2){  
    img1 = imread("../1.jpg");
    img2 = imread("../2.jpg");
    }

void get_keyPoints_n_Descriptors(Mat img1, Mat img2, vector<KeyPoint> &kp1, vector<KeyPoint> &kp2, Mat &des1, Mat &des2){
    Ptr<SIFT> detector = SiftFeatureDetector::create();
    detector->detectAndCompute(img1, noArray(), kp1, des1);
    detector->detectAndCompute(img2, noArray(), kp2, des2);
}

void get_matches(vector<DMatch> &matches, Mat des1, Mat des2){
    FlannBasedMatcher matcher;
    matcher.match(des1, des2, matches);

    if(matches.size()<4){
        cout<<"too few matches"<<endl;
    }
    else{
        cout<<"Good matches found in img1 and img2"<<endl;
    }

}

void find_good_matches(vector<DMatch> &good_matches, Mat des1, Mat des2, vector<DMatch> matches){
    double min_dist = 100;
    double max_dist = 0;

    for(int i = 0; i< des1.rows; i++){
        double dist = matches[i].distance;
        if(dist < min_dist) min_dist = dist;
        if(dist>max_dist) max_dist = dist;
    }

    //Use min distance to find good matches
    int minMatch = 8;
    //vector<DMatch> good_matches;
    for(int i = 0; i< des1.rows; i++){
        if(matches[i].distance < 3*min_dist && matches.size()>minMatch){
            good_matches.push_back(matches[i]);
        }
    }
    //Use all keypoints if number of good matches is less than min matches
    if(good_matches.size() < minMatch){
        for(int i = 0; i < des1.rows; i++){
            if(i < good_matches.size()){
                good_matches[i] = matches[i];
            }
            else{
                good_matches.push_back(matches[i]);
            }
        }
    }

}

void get_homography(Mat &Homography, vector<KeyPoint> kp1, vector<KeyPoint> kp2, vector<DMatch> &good_matches){
    //use source and destination to find homography between 2 images
    vector<Point2f> src, dst;
    for(int i = 0; i <good_matches.size(); i++){
        src.push_back(kp1[good_matches[i].queryIdx].pt);
        dst.push_back(kp2[good_matches[i].trainIdx].pt);
    }

    Homography = findHomography(dst, src, RANSAC);
    cout<<Homography.size()<<endl;
    cout<<Homography<<endl;
}

Mat warp_n_stitch(Mat img1, Mat img2, Mat Homography){
    Mat warped;
    int height, width;
    height = img1.rows + img2.rows;
    width = img1.cols + img2.cols;
    warpPerspective(img2, warped, Homography, Size(width,height), INTER_CUBIC);
    Mat final(Size(width, height), CV_8UC3);
    Mat roi1(final, Rect(0, 0,  img1.cols, img1.rows));
    Mat roi2(final, Rect(0, 0, warped.cols, warped.rows));
    warped.copyTo(roi2);
    img1.copyTo(roi1);
    return final;
}


int main(){
    //Read images
    Mat img1, img2;
    get_Images(img1, img2);

    //get keypoints and descriptors
    vector<KeyPoint> kp1, kp2;
    Mat des1, des2;
    get_keyPoints_n_Descriptors(img1, img2, kp1, kp2, des1, des2);

    //get all matches
    vector<DMatch> matches;
    get_matches(matches, des1, des2);

    //get good matches
    vector<DMatch> good_matches;
    find_good_matches(good_matches, des1, des2, matches);

    //Find homography
    Mat Homography;
    get_homography(Homography, kp1, kp2, good_matches);

    //get warped and stitched image
    Mat final;
    final = warp_n_stitch(img1, img2, Homography);
    imshow("Stitched Image", final);
    waitKey();

    return 0;
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

I can't see why we need to include <stdio.h> or <cmath>.


I don't like this:

using namespace cv;
using namespace std;
using namespace xfeatures2d;

You've just thrown away all the benefit of namespaces:

  • There's now no protection against new versions of libraries defining names used by the others.
  • It's harder for readers to see which library each identifier comes from.

The get_Images function that's called once doesn't provide much benefit compared to initialising the two cv::Mat objects directly in main():

int main()
{
    auto img1 = cv::imread("../1.jpg");
    auto img2 = cv::imread("../2.jpg");
    ⋮

We need to be more robust about checking whether these reads were successful. If either image is a null Mat, we should be printing a message to std::cerr and returning EXIT_FAILURE (from <cstdlib>).


It's better to return values from functions rather than requiring users to pass "out" parameters by reference:

    auto const matches = get_matches(des1, des2);
    auto const good_matches = find_good_matches(des1, des2, matches);
    auto const Homography = get_homography(kp1, kp2, good_matches);

Possibly also:

    auto const [kp1, kp2, des1, des2] = get_keyPoints_n_Descriptors(img1, img2);
\$\endgroup\$
3
  • \$\begingroup\$ thanks for the suggestions. Also what do you mean by "You've just thrown away all the benefit of namespaces:". Is the better practice of using namespaces as follows: cv::Mat img1 = cv::imread("../img1.jpg"? Also which libraries do you guys use for reading any data files (..jpg, .txt)? In python there is OS module, is there a similar practice in industry of C++ coders? \$\endgroup\$ May 26, 2023 at 0:20
  • \$\begingroup\$ I thought I explained that sentence with the points that follow - namespaces exist to keep different libraries' identifiers from colliding with each other. Pulling all their names into the global namespace removes that protection, as if they were never namespaced to begin with. Other alternative is to be more specific, e.g. using cv::Mat; etc. \$\endgroup\$ May 26, 2023 at 7:00
  • \$\begingroup\$ For a concrete example: when you see min(), how easy is it to determine whether that's std::min() or cv::min() that's being called? \$\endgroup\$ May 26, 2023 at 13:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.