FREAK : better rounding off for weighted dx and dy of orientation pairs #2440
Conversation
|
Thank you for contribution! As a "bugfix" this patch should go into 3.4 branch first. We will merge changes from 3.4 into master regularly (weekly/bi-weekly). So, please:
Note: no needs to re-open PR, apply changes "inplace". |
rindranil
force-pushed
the
freak_rounding_bug
branch
from
8aed68e to
cebb49d
Compare
modules/xfeatures2d/src/freak.cpp
Outdated
| orientationPairs[m].weight_dx = int((dx/(norm_sq))*4096.0+0.5); | ||
| orientationPairs[m].weight_dy = int((dy/(norm_sq))*4096.0+0.5); | ||
| orientationPairs[m].weight_dx = int((dx/(norm_sq))*4096.0+0.5*((dx>0)-(dx<0))); | ||
| orientationPairs[m].weight_dy = int((dy/(norm_sq))*4096.0+0.5*((dy>0)-(dy<0))); |
There was a problem hiding this comment.
Please use cvRound() instead of 0.5 hack and "int" cast.
Another question is it allowed to weight_dx/dy become zero or negative?
Usually "weight" should not be negative. May be there is lost abs() call?
There was a problem hiding this comment.
weight_dx/dy can be negative to preserve the sense of direction
direction0 += delta*(orientationPairs[m].weight_dx)/2048;
direction1 += delta*(orientationPairs[m].weight_dy)/2048;
where delta is the difference in mean intensity between two pairs.
There was a problem hiding this comment.
But after using cvRound() tests are getting failed due to bad accuracy.
There was a problem hiding this comment.
Regression test may fail on such updates - check is usually very strong.
Primary tests are quality tests, like RotationInvariance or checking keypoints.
Testdata for regression test should be updated too. Please clone opencv_extra and open PR with updated file in testdata/cv/features2d/descriptor_extractors (use the same branch name freak_rounding_bug).
Usually it is enough to remove corresponding file and run test (it would generate new data).
| const int x_left = cvRound(xf-radius); | ||
| const int y_top = cvRound(yf-radius); | ||
| const int x_right = cvRound(xf+radius+1);//integral image is 1px wider | ||
| const int y_bottom = cvRound(yf+radius+1);//integral image is 1px higher |
There was a problem hiding this comment.
Why not just cvFloor() / cvCeil()?
There was a problem hiding this comment.
Suppose when xf-radius is 2.9 and 2.1, it will be more sensible to assign 3 and 2 respectively in x_left.
|
I have sent a PR(opencv/opencv_extra#721) for updating test data. |
Merge with extra: opencv/opencv_extra#721
resolves #2430, when value of dx/(norm_sq) is -ve, adding -0.5 will lead to better rounding off.