Skip to content

Conversation

take1014
Copy link
Contributor

resolves #4270

@hrnr
Copy link
Contributor

hrnr commented May 26, 2018

there is already estimateAffinePartial2D and friends https://docs.opencv.org/master/d9/d0c/group__calib3d.html#gad767faff73e9cbd8b9d92b955b50062d

I think we should really get rid of estimateRigidTransform in OpenCV 4, or reimplement using estimateAffinePartial2D to avoid duplicate code.

Mat aff_est2 = estimateRigidTransform(img, rotated, true, 500, 3, 0.5f);
norm = cvtest::norm(aff_est1, aff_est2, NORM_INF);
EXPECT_DOUBLE_EQ(norm, 0.);
Mat aff_est3 = estimateRigidTransform(img, rotated, true, 100, 8, 0.1f);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the test checks the result doesn't depend on RANSAC parameters rather than its correctness. Wouldn't it be better to check the result against aff matrix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the follow.
Certainly, I need to fix this code as you pointed out.
However, as alalek and hrnr say, I do not modify the interface of estimateRigidTransform, because we need to unify the "posture estimation" algorithm to calib3d's function instead of video module's.

@alalek
Copy link
Member

alalek commented May 28, 2018

I agree with @hrnr to keep "pose estimation" algorithms in a single place - calib3d module.
"Video" module should reuse existed functionality instead of re-implementing similar algorithms.

@take1014
Copy link
Contributor Author

take1014 commented May 28, 2018

@alalek, @hrnr, @terfendail
Thanks for the comments.
I agree with you.
I think "estimateAffinePartial2D" should be reused instead of "estimateRigidTransform".

@take1014 take1014 closed this May 28, 2018
@take1014 take1014 deleted the estimateRigidTransform_4270 branch May 28, 2018 13:42
@hrnr
Copy link
Contributor

hrnr commented May 28, 2018

maybe we should deprecate estimateRigidTransform in OpenCV 4. The RANSAC part could be reimplemented using calib3d right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RANSAC parameters for estimateRigidTransform
4 participants