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

Algorithm Flaws #1

Open
mysteryx93 opened this issue Jun 26, 2022 · 4 comments
Open

Algorithm Flaws #1

mysteryx93 opened this issue Jun 26, 2022 · 4 comments

Comments

@mysteryx93
Copy link

I've taken your pitch-detection idea to convert it to C#, and am getting wildly inconsistent results. So I started to look at the code and saw a few problems.

  1. You're scanning from the whole range of notes, valuing extreme lows/highs as much as mid-tones. It makes more sense to analyze mid-tones between around 80hz and 1400hz. Or, apply a curve to value mid-tones more in a smoother way, but haven't worked out the right math yet.

  2. Even with FFT32768, there are only 2-3 FFT band differences between notes in the lower tones, and 6-7 FFT band differences between notes in the higher tones. That's not quite enough for the kind of measurements we're trying to do with 0.1hz precision... That alone can explain the flaky results.

  3. In lower tones, FFT bands are larger (band = 1/3 of a note) than in higher tones (band = 1/4 of a note). As a result, lower-pitch analysis like 424hz and 330hz get a higher score than higher pitch like 440hz+.

I've also found a series of algorithms here, but I'm not sure whether it's doing the same thing as what we're doing here.
https://github.com/sevagh/pitch-detection

@mysteryx93
Copy link
Author

After adding weighting to compensate for band variable width, I get pretty good results! Ideally would add a weighting curve to favor middle-tones on top of that, but the math can be delicate to get right.

var factor = (freq - lastFreq) / freqStep;

https://github.com/mysteryx93/NaturalGroundingPlayer/blob/master/BassAudio/PitchDetector.cs

@mysteryx93
Copy link
Author

I refreshed my parabolic math skills and created a nice curve for 80 tones.

var curve = (float)-Math.Pow(j - 1 - 40, 2) / 2000 + 1;

Unfortunately, I get better results with 60 flat values than with 80 parabolic values. It was worth a try.

I also tried doing proper rounding instead of rounding down to find the FFT band, but rounding down gave better results somehow.

// var index = (int)Math.Round(tones[j] / freqStep, 0);
var index = (int)(tones[j] / freqStep);

I'm pretty satisfied with the results. Source code here.

@CardLin
Copy link
Owner

CardLin commented Nov 1, 2022

wow!!
Add weighting to middle tone is awesome.
I think we can set a profile to focus on different curve just like a equalizer.

@mysteryx93
Copy link
Author

mysteryx93 commented Nov 2, 2022

Actually, according to my tests, weighting middle tones more somehow gave worse results than flat weighting; but also discarding extreme highs and lows.

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

2 participants