Added new unit test for cv::initInverseRectificationMap() by IanMaquignaz · Pull Request #20259 · opencv/opencv

IanMaquignaz · 2021-06-11T00:16:55Z

Added new unit test for cv::initInverseRectificationMap()

Replaces unit test from #20165 and completes the updating of cv::initInverseRectificationMap() started in #20247

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

IanMaquignaz · 2021-06-11T01:38:35Z

Included sample distortion coefficients from a DSLR camera and a Point Grey camera. Current parameters pass using an image of size_w_h(512 + 3, 512), through note the error can exceed the 1e-3 threshold if image size is increased to be larger.

double d[5]={ // Non-zero distortion from a Point Grey Camera
        -3.4134571357400023e-03, 2.9733267766101856e-03, // K1, K2
        3.6653586399031184e-03, -3.1960714017365702e-03, // P1, P2
        0. // K3
    };

alalek · 2021-06-11T09:15:25Z

modules/calib3d/test/test_undistort.cpp

    EXPECT_LE(cvtest::norm(dst, mesh_uv, NORM_INF), 1e-3);
 }

+TEST(Calib3d_initInverseRectificationMap, regression_14467)


regression_14467

Please change this name

fixed. Assumed number is arbitrary and changed it to regression_14468.

alalek · 2021-06-11T09:15:25Z

modules/calib3d/test/test_undistort.cpp

    EXPECT_LE(cvtest::norm(dst, mesh_uv, NORM_INF), 1e-3);
 }

+TEST(Calib3d_initInverseRectificationMap, regression_14467)


regression_14467

Please change this name

Assumed this is a duplicate of the comment above.

alalek · 2021-06-11T09:15:32Z

modules/calib3d/test/test_undistort.cpp


+TEST(Calib3d_initInverseRectificationMap, regression_14467)
+{
+    Size size_w_h(512 + 3, 512);


It makes sense to use a really different width/height. Use some regular resolution like 640x480 or 1280x720 (camera matrix should be updated too)

Updated intrinsics and extrinsics specification to use experimentally collected values.

Size size_w_h(1280, 800);

// Camera Matrix double k[9]={ 1.5393951443032472e+03, 0., 6.7491727003047140e+02, 0., 1.5400748240626747e+03, 5.1226968329123963e+02, 0., 0., 1. };

double d[5]={ // Non-zero distortion -3.4134571357400023e-03, 2.9733267766101856e-03, // K1, K2 3.6653586399031184e-03, -3.1960714017365702e-03, // P1, P2 0. // K3 };

Note, K1 and K2 have been increased from -3.4134571357400023e-01, 2.9733267766101856e-01 to meet error threshold of 2e-1

double R[9]={ // Random transform 9.6625486010428052e-01, 1.6055789378989216e-02, 2.5708706103628531e-01, -8.0300261706161002e-03, 9.9944797497929860e-01, -3.2237617614807819e-02, -2.5746274294459848e-01, 2.9085338870243265e-02, 9.6585039165403186e-01 };

alalek · 2021-06-11T09:27:23Z

modules/calib3d/test/test_undistort.cpp

+
+    // Rotation
+    //double R[9]={1., 0., 0., 0., 1., 0., 0., 0., 1.}; // Identity transform (none)
+    double R[9]={1., 0.05, 0., 0.05, 1.0, 0.005, 0.005, 0., 1.}; // Random transform


valid transforms have some idempotent properties, like det(rotation) = 1 or -1

See above comment

IanMaquignaz · 2021-06-12T15:36:58Z

It seems someone added a shell script which tries curl -s https://api.github.com/repos/IanMaquignaz/opencv_extra/branches

I don't have a fork of opencv_extra...

-- edit --
Fork as been added for opencv_extra and opencv_contrib

alalek · 2021-06-12T20:09:41Z

Ignore CN* builders. They have sporadic network issues.

opencv_extra/contrib forks are optional. This command just checks that.

IanMaquignaz · 2021-06-12T20:39:42Z

Ignore CN* builders. They have sporadic network issues.

opencv_extra/contrib forks are optional. This command just checks that.

@alalek Ok! Then unless you think there is merit to the alternate unit test I wrote today, then I think it's good to go?
See 1780 TEST(Calib3d_initUndistortRectifyMap, regression_14467)

alalek · 2021-06-17T08:33:50Z

modules/calib3d/test/test_undistort.cpp

    EXPECT_LE(cvtest::norm(dst, mesh_uv, NORM_INF), 1e-3);
 }

+TEST(Calib3d_initInverseRectificationMap, regression_14468)


14468

What does it mean?
Add PR/issue number. Use #20165 as reference on original code.

Haha, I incorrectly assumed that was an arbitrary number. Correction has been applied.

alalek · 2021-06-17T08:44:20Z

modules/calib3d/test/test_undistort.cpp

+            double Y = R[3]*x_ + R[4]*y_ + R[5];
+            double Z = R[6]*x_ + R[7]*y_ + R[8];
+            double x__ = X/Z;
+            double y__ = Y/Z;


Perhaps it makes sense to add check that Z value doesn't drops to zero (fabs(Z) > 1e-6)

This undistort function performs a rough estimate and should not be used as an undistort method. There are numerous weaknesses, including lack of regression (undistort should be iterative for best results) and missing handling for distortion coefficients outside of radial distortion.

As the coefficients are hard-coded to the unit test, I don't believe it is necessary to be overly thorough. If that is desired, full replication of cvUndistortPointsInternal() is necessary. This was attempted in the proposed alternate unit test but, aside from increasing the length of the code, did not add significant value.

@alalek

Function is validated. Included an update to DISABLED_Calib3d_InitInverseRectificationMap. Includes updates per input from @alalek and unit test regression # to reflect PR #

alalek

Thank you 👍

IanMaquignaz force-pushed the inverseRectification_newUnitTest branch from 9ec0560 to 698fc4d Compare June 11, 2021 01:48

IanMaquignaz marked this pull request as ready for review June 11, 2021 03:00

alalek reviewed Jun 11, 2021

View reviewed changes

IanMaquignaz force-pushed the inverseRectification_newUnitTest branch 2 times, most recently from c32c028 to c768a7c Compare June 11, 2021 21:07

alalek reviewed Jun 17, 2021

View reviewed changes

Added new unit test for initInverseRectificationMap()

464441d

Function is validated. Included an update to DISABLED_Calib3d_InitInverseRectificationMap. Includes updates per input from @alalek and unit test regression # to reflect PR #

IanMaquignaz force-pushed the inverseRectification_newUnitTest branch from 91f63f8 to 464441d Compare June 17, 2021 16:49

alalek approved these changes Jun 20, 2021

View reviewed changes

opencv-pushbot merged commit 25f908b into opencv:master Jun 20, 2021

alalek mentioned this pull request Oct 15, 2021

(5.x) Merge 4.x #20886

Merged

Uh oh!

Conversation

IanMaquignaz commented Jun 11, 2021

Uh oh!

IanMaquignaz commented Jun 11, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IanMaquignaz Jun 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IanMaquignaz commented Jun 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek commented Jun 12, 2021

Uh oh!

IanMaquignaz commented Jun 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

IanMaquignaz Jun 11, 2021 •

edited

Loading

IanMaquignaz commented Jun 12, 2021 •

edited

Loading

IanMaquignaz commented Jun 12, 2021 •

edited

Loading