Bug in LBP [closed]
Hi,
In the last couple of hours, I worked closely with LBP in OpenCV and compared it with the original implementation in Matlab. There was a slight difference in the results I got, so I looked more closely at the code.
I found the following bug. In the lines:
float x = static_cast<float>(-radius * sin(2.0*CV_PI*n/static_cast<float>(neighbors)));
float y = static_cast<float>(radius * cos(2.0*CV_PI*n/static_cast<float>(neighbors)));
the roles of x and y are flipped. It should be :
float y = static_cast<float>(-radius * sin(2.0*CV_PI*n/static_cast<float>(neighbors)));
float x = static_cast<float>(radius * cos(2.0*CV_PI*n/static_cast<float>(neighbors)));
As in Matlab it's:
spoints(i,1) = -radius*sin((i-1)*a); %y
spoints(i,2) = radius*cos((i-1)*a); %x
The reason is obvious - x corresponds to the cos value of the angle and y corresponds to the y value of the angle. After changing the code, the results in Matlab and OpenCV were the same.
I can upload LBP images of "before" and "after" the fix, if you want.
How do I report it?
Thanks,
Gil.
Simply go to "how to contribute page" and make a pull request :) If you don't feel like doing it yourself, make a bug report here. Thanks for digging deeper! However I am wondering if you switch x and y consistently over all calculations, if the result of any detector would be highly influenced. I am guessing it doesn't really matter, tough this is a bug :)
that's a 90° phase shift and an inversion, right ?
but it probably won't matter for the recognition task, since neither the startpoint of the circle matters, nor the direction
maybe talk to bytefish, too
@berak, what you get before the fix are histograms with much more values on the ends (high values and low values). When you plot the LBP images, you see a lot of very bright and very dark pixels. After the fix, there are a lot more shades of gray instead. That's why I believe it does change the results, also in classification.
well, i actually went and tested it - no difference. (att,yale,lfw)
a phase offset or bitwise rotation of the lbp feature will indeed lead to a reordered / shuffled histogram, but the euclidean distance does not care about it ( as long as the heights of the resp. bins are not changed ),
(a*a+b*b+c*c) == (c*c+b*b+a*a)looking at the lbp-feature images might be misleading, imho
Ok but I do want to push towards making a fix, since I think results between different package should be the same. This seems as a small bug that can be easily fixed.
yea, in retrospect, didn't want to sound so negative about gil's findings there.
having the same lbp image output as matlab has is definitely worth it
I do not think you sounded negative, it is good that people try to test different things before finally fixing stuff :D tons has been broken because it wasn't tested correctly
I have a small question - I made the necessary changes to the code. Now I want to "push your branch to your GitHub fork;". How to I do that using GitHub on windows?
Thanks!
Did you follow the windows guide I made? Basically you can do it through TortoiseGIT or you can right click on your folder, open git bash, then type the following: git push -f which will force push all your local changes to your online branch, so that you can create a pull request there.