Logical flaw in OpenCV 2.4.8 SIFT implementation (Bug #3585)

Added by Kang Song about 11 years ago. Updated almost 11 years ago.

Status:Cancelled Start date:2014-03-03
Priority:Normal Due date:
Assignee:Vadim Pisarevsky % Done:

0%
Category:nonfree
Target version:-
Affected version:2.4.8 (latest release) Operating System:Linux
Difficulty:Medium HW Platform:x64
Pull request:

Description

In the nonfree model, sift.cpp file, SIFT::buildGaussianPyramid method, the part where the sigma values are calculated by:
sig[0] = sigma;
double k = pow( 2., 1. / nOctaveLayers );
for( int i = 1; i < nOctaveLayers + 3; i++ )
{
    double sig_prev = pow(k, (double)(i-1))*sigma;
    double sig_total = sig_prev*k;
    sig[i] = std::sqrt(sig_total*sig_total - sig_prev*sig_prev);
}

however I tested the logic by using nOctaveLayers = 2 (which is chosen for the ease of calculation), the result of the sigma values are listed below according to the logic of the code:
  • sig0 = sigma
  • sig1 = sigma
  • sig2 = sqrt(2)*sigma
  • sig3 = 2*sigma
  • sig4 = 2* sqrt(2)*sigma
But shouldn't it be:
  • sig0 = sigma
  • sig1 = sqrt(2)*sigma
  • sig2 = 2*sigma
  • sig3 = 2* sqrt(2)*sigma
  • sig4 = 4*sigma

In order to make sure my understanding is correct, I looked up some lecture notes, it turns out my understanding is correct according to this link: http://inst.eecs.berkeley.edu/~ee225b/sp11/lectures/sift_Final.ppt

History

#1
Updated by Dinar Ahmatnurov about 11 years ago

  • Status changed from New to Open
  • Difficulty changed from Easy to Medium

#2
Updated by Chris Garry almost 11 years ago

Could I take this on?

#3
Updated by Chris Garry almost 11 years ago

Those "sig" values are NOT the "sigmas" at each level like you think they are, but are the 'incremental' sigma values needed to attain the sigmas you are thinking of. So, the sigma value at level i is actually sig_total, and the sigma value at level i-1 is sig_prev, such that:

sig_total = sig_prev (*) sig[i] (convolution)

[1] Add the following print statement to the code in the for loop to see what I mean:

printf("Sigma[%d]=%f (sigma at level %d), Sigma[%d]=%f (sigma at level %d), Incremental sigma used to get from %d to %d: sig[%d]=%f\n",i-1,sig_prev,i-1,i,sig_total,i,i-1,i,i,sig[i]);

[2] Run the unit tests and you will see printed (note unit tests are using k=cbrt(2) rather than sqrt(2)):

Sigma0=1.600000 (sigma at level 0), Sigma1=2.015874 (sigma at level 1), Incremental sigma used to get from 0 to 1: sig1=1.226273
Sigma1=2.015874 (sigma at level 1), Sigma2=2.539842 (sigma at level 2), Incremental sigma used to get from 1 to 2: sig2=1.545008
Sigma2=2.539842 (sigma at level 2), Sigma3=3.200000 (sigma at level 3), Incremental sigma used to get from 2 to 3: sig3=1.946588
Sigma3=3.200000 (sigma at level 3), Sigma4=4.031747 (sigma at level 4), Incremental sigma used to get from 3 to 4: sig4=2.452547
Sigma4=4.031747 (sigma at level 4), Sigma5=5.079683 (sigma at level 5), Incremental sigma used to get from 4 to 5: sig5=3.090016

The reason it is like this is to keep the gaussian kernel small by keeping tracking of incremental sigma values needed to get from level i-1 to i rather than the entire sigma. It's for efficiency purposes.

  • Status changed from Open to Cancelled

Also available in: Atom PDF