Add Harris-Laplace affine detector #933
Conversation
The code structure had to be changed because of the features2d/xfeatures2d split and to avoid changing multiple modules (gaussian_pyramid doesn't seem ready for inclusion in the (x)imgproc module). Except for changes related to this, the patches remain unchanged, including the outdated copyright notice at the top of the files.
…HarrisLaplace and AffineFeature2D
|
The failed checks taught me a lesson about the difficulty of cross-platform C++ development. Hopefully I didn't miss anything. As a note: the warning about overloaded virtuals that clang helpfully shows can also be enabled in GCC using -Woverloaded-virtual, but then I get a number of additional warnings across the OpenCV code base. I'm not sure whether that is indicative of low precision by GCC or whether those warnings ought to be fixed. |
alalek
left a comment
alalek
left a comment
There was a problem hiding this comment.
Great job! Thank you.
Please take a look on notes below.
| License Agreement | ||
| For Open Source Computer Vision Library |
There was a problem hiding this comment.
Could you please use "unmodified" license header or just use the shorter variant:
// This file is part of OpenCV project.
// It is subject to the license terms in the LICENSE file found in the top-level directory
// of this distribution and at http://opencv.org/license.html
| Size_<float> axes; //!< the lengths of the major and minor ellipse axes | ||
| double phi; //!< the rotation angle | ||
| float size; | ||
| float si; //!< the integration scale at which the parameters were estimated |
There was a problem hiding this comment.
KeyPoint already have similar fields:
- centre -> pt
- phi -> angle
- size -> size
- si (float) -> octave (int)
| Mat col0 = U.col(0); | ||
| transf.col(0).copyTo(col0); | ||
| Mat col1 = U.col(1); | ||
| transf.col(1).copyTo(col1); |
There was a problem hiding this comment.
For fixed size matrices it is better to use Matx: Matx22f and similar.
|
@alalek, I hope my changes address your concerns. However, the build bot seems to have hit a bug: It has been "building" for 3 hours now and the build IDs link to builds for an unrelated pull request. |
|
👍 |
| float ratio = 1 - q; | ||
|
|
||
| //if ratio == 1 means q == 0 and one axes equals to 0 | ||
| if (!isnan(ratio) && ratio != 1) |
There was a problem hiding this comment.
Should this be using std::isnan() similar to other places in the codebase? I ran into an error locally until I switched to that, though I'm still hitting other issues with the master branch, so I can't confirm that it solved everything.
|
@Yorwba I'm a little bit confused about how to use this, could you please give an example code? |
|
@lovaj Say you have an Ptr<AffineFeature2D> detector = AffineFeature2D::create(
HarrisLaplaceFeatureDetector::create(/* adjust parameters as necessary */),
yourFeatureDescriptor
);
vector<Elliptic_KeyPoint> keypoints;
Mat descriptors;
detector->detectAndCompute(image, noArray() /* no mask */, keypoints, descriptors);and then you can use the |
|
Ok only to make it clear, this PR added Harris-Laplace and Harris-Affine ( If i am correct harris-affine can be achieved by using Harris-Laplace detector with |
ghost
mentioned this pull request
|
I would like to add support for elliptic keypoints to the mser detector. How would I do that? The problem is, that mser is in the features2d module, and elliptic keypoints in the xfeatures2d module. |
6 participants
This pullrequest changes
I took the old patches found at http://code.opencv.org/issues/1053 and rewrote the interface to (hopefully) conform to the OpenCV coding style. While doing this I noticed that the code for affine adaptation was actually mostly independent from the key-point detector, so I pulled it out into a separate class AffineFeature2D that can wrap any feature detector and descriptor.
What I did not get to work were the Python/Java wrappers, simply putting CV_EXPORTS_W on the functions lead to issues with the generated code.