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

Adds Dense SIFT feature type. http://www.vlfeat.org/overview/dsift.html #236

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sashichandra
Copy link

No description provided.

@sashichandra
Copy link
Author

sashichandra commented Apr 24, 2018 via email

@russkel
Copy link
Contributor

russkel commented Apr 24, 2018

Hi @sashichandra, sorry I deleted my comment after I found the file.

@russkel
Copy link
Contributor

russkel commented May 13, 2018

Is there a chance you could resolve the conflict in the features.py file @sashichandra ?

@paulinus are you willing to merge this? Otherwise I'll pull this into my own working copy.

@paulinus
Copy link
Member

@russkel, I've resolved the conflicts on this branch and put the result in this branch https://github.com/paulinus/OpenSfM/tree/sashichandra-dsift

About merging it to master, the main concern is that Dense SIFT is not a suitable feature for SfM in general. The reason for that is that it extracts features on a fixed grid that is not adapted to the actual image texture. As a result, corresponding features in different images are not centered around the same real-world point. This adds substantial error to triangulation and resection algorithms to the point where they no longer work.

Testing this branch on berlin and lund dataset, I was unable to get good reconstructions. Only berlin yield a reasonable reconstruction after setting dsift_step to a small number so that the discretization error was reduced. That produced a lot more features which were then slow to match.

If there is a trick to make things work better, please let me know.

@facebook-github-bot
Copy link
Contributor

Hi @sashichandra!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

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.

4 participants