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 dublicated SIFT-Dectector instance in feature matching #1053

Closed
wants to merge 1 commit into from

Conversation

kielnino
Copy link
Contributor

It seems to me that the SIFT detector is instantiated, but then not used, because the object is created again in the following while loop. This merge request removes the unnecessary code.
I have also made the missing parameters of the OpenCV-Sift implementation configurable.

* add sift parameters
@facebook-github-bot
Copy link
Contributor

@fabianschenk has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kielnino
Copy link
Contributor Author

In the old version, the detector was created unnecessarily and then overwritten later, but the feature descriptor was not recreated in the loop. It was only a copy of the original and then deleted detector object, but is now also copied again each time in the loop. I wonder if the current behavior might be creating some overhead?

@kielnino
Copy link
Contributor Author

Maybe we should move the line descriptor = detector to run only once after the while-Loop, conditioned on the context.OPENCVXX flags?

@kielnino
Copy link
Contributor Author

@fabianschenk Do I need to adjust something to make the FB-internal tests pass?

And in addition do you have any thoughts about this?

Maybe we should move the line descriptor = detector to run only once after the while-Loop, conditioned on the context.OPENCVXX flags?

@facebook-github-bot
Copy link
Contributor

@fabianschenk merged this pull request in b18e638.

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

Successfully merging this pull request may close these issues.

2 participants