Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove cv.sfm dependency #42

Merged
merged 3 commits into from
May 22, 2024
Merged

Remove cv.sfm dependency #42

merged 3 commits into from
May 22, 2024

Conversation

philipqueen
Copy link

Closes #4.

I decided to take this on to better understand the math going on in the backend here. I haven't built the sfm version of opencv myself, and don't have the pseye cameras or drones to test my replacement of it myself, so thorough real use testing should be done before merging this.

I did look at the opencv_sfm test suite, but they only test by making raw data, doing one set of conversions, converting back, and testing that the results match. Since I didn't implement the reverse of these functions I didn't implement that, and it did not give me examples to test these functions with.

The file that contained the functions I adapted from included a BSD license that I have included here out of caution.

@jyjblrd jyjblrd changed the base branch from main to no-cv-sfm May 22, 2024 10:34
@jyjblrd
Copy link
Owner

jyjblrd commented May 22, 2024

Hey Philip,

Thanks for the PR! This could be very helpful. Unfortunately I don't have the time/ability to properly test this right now, so I'll pull it into a new branch and update the readme to let people know that the no opencv-sfm code is experimentally available.

The best way to probably test this would be to generate random data and compare the opencv-sfm results to your code, which unfortunately would also require setting up some testing infrastructure which doesn't currently exist in the codebase.

@jyjblrd jyjblrd merged commit e2a0a40 into jyjblrd:no-cv-sfm May 22, 2024
@philipqueen
Copy link
Author

Great that sounds good to me!

If I end up figuring out the testing I’ll make another PR into that branch.

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.

Remove cv.sfm dependency
2 participants