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

SURF integration / opencv-contrib #13

Merged
merged 13 commits into from
Jul 24, 2022
Merged

SURF integration / opencv-contrib #13

merged 13 commits into from
Jul 24, 2022

Conversation

zerofivefiveseven
Copy link
Collaborator

xfeatures2d which contains SURF appears in whitelist but don't in the library summary while build

(surf pointer don't work correct)
add orb class declaration to feature_lib.h
add surf class definition to feature_lib.cpp
add orb class definition to feature_lib.cpp
…OAD_DIR}/opencv-${OPENCV_VERSION}.zip. And also change the NOT EXISTS check. And also change that path in the archive_extract command
@zerofivefiveseven
Copy link
Collaborator Author

yesterday it builds with xfeatures2d
working on it

download opencv-contrib download link edited
opencv-extra DOWNLOAD parameter(url) changed
extracting opencv-extra DESTINATION parameter changed
-DOPENCV_EXTRA_MODULES_PATH parameter changed
@zerofivefiveseven
Copy link
Collaborator Author

It builds, but xfeatures2d can't get in touch with project
[ 50%] Linking CXX shared library libfeatureDetect.so
/usr/bin/ld: cannot find -lopencv_xfeatures2d: No such file or directory

@zerofivefiveseven zerofivefiveseven added the help wanted Extra attention is needed label Jul 23, 2022
new instances of surf and orb in detection_example.cpp
new print func in detection_example.cpp
@zerofivefiveseven zerofivefiveseven added ready to view and removed help wanted Extra attention is needed work in progress labels Jul 23, 2022
Copy link
Owner

@TimPushkin TimPushkin left a comment

Choose a reason for hiding this comment

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

Comments on CMake style.

C++ code will be reviewed by @anastasiia-kornilova later, I guess, but in general I'd say that:

  1. You probably can move a lot of duplicated code from the detectors in their common abstract parent.
  2. In the example it would be better to accept a command line argument with the algorithm name and run only the selected one because running all three is pretty messy.
  3. Maybe we don't even need to print descriptors as it looks like a mess in the output -- just put descriptor size for each keypoint (in some algorithms the size may be dynamic, though, not in ours).

BuildOpenCV.cmake Outdated Show resolved Hide resolved
BuildOpenCV.cmake Outdated Show resolved Hide resolved
BuildOpenCV.cmake Outdated Show resolved Hide resolved
Copy link
Collaborator

@anastasiia-kornilova anastasiia-kornilova left a comment

Choose a reason for hiding this comment

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

I absolutely agree with @TimPushkin comments, but for now it looks like they are not crucial for MVP. I suggest to move all suggestions into separate issues and after the full ready working pipeline we will beautify those things.

BuildOpenCV.cmake Outdated Show resolved Hide resolved
remove space between -D and OPENCV_ENABLE_NONFRE
now we use that func instead prev version
Copy link
Owner

@TimPushkin TimPushkin left a comment

Choose a reason for hiding this comment

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

Small style fixes required

BuildOpenCV.cmake Outdated Show resolved Hide resolved
BuildOpenCV.cmake Outdated Show resolved Hide resolved
BuildOpenCV.cmake Outdated Show resolved Hide resolved
@zerofivefiveseven zerofivefiveseven merged commit 9e71df5 into cpp/main Jul 24, 2022
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.

None yet

3 participants