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

Pitch estimation pseudo-interpolation bug #15

Closed
alebzk opened this issue Mar 2, 2018 · 1 comment
Closed

Pitch estimation pseudo-interpolation bug #15

alebzk opened this issue Mar 2, 2018 · 1 comment

Comments

@alebzk
Copy link

alebzk commented Mar 2, 2018

I might be wrong, but there could be a bug in the pitch pseudo-interpolation.
Luckily, it's not critical, but better solve if it's an actual bug.

At the end of pitch_search():

   if (best_pitch[0]>0 && best_pitch[0]<(max_pitch>>1)-1)
   {
      opus_val32 a, b, c;
      a = xcorr[best_pitch[0]-1];
      b = xcorr[best_pitch[0]];
      c = xcorr[best_pitch[0]+1];
      if ((c-a) > MULT16_32_Q15(QCONST16(.7f,15),b-a))
         offset = 1;
      else if ((a-c) > MULT16_32_Q15(QCONST16(.7f,15),b-c))
         offset = -1;
      else
         offset = 0;
   } else {
      offset = 0;
   }
   *pitch = 2*best_pitch[0]-offset;

I think that the last line should be *pitch = 2*best_pitch[0]+offset; (plus instead of minus).
For instance, when c-a > .7*(b-a) is true, it means that xcorr[best_pitch[0]+1] is the strongest correlation coefficient (a and b are almost the same, while c is greater than a). Hence, I would return 2*best_pitch[0]+1.

Note that at the end of remove_doubling() the same is done, but in fact *T0_ = 2 * T + offset; is assigned (which has a +). I haven't sent a pull request since this code comes from Opus (and it's used in other projects).

@alebzk alebzk changed the title Ptich estimation pseudo-interpolation bug Pitch estimation pseudo-interpolation bug Mar 5, 2018
@alebzk
Copy link
Author

alebzk commented Mar 5, 2018

Luckily, it's not a bug :) xcorr indices are "inverted lags" due to how indexing operates in the pitch buffer.

@alebzk alebzk closed this as completed Mar 5, 2018
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

No branches or pull requests

1 participant