Bugzilla – Bug 638
Pitch detection in Change Pitch wildly inaccurate at high sample rates
Last modified: 2018-08-20 11:51:29 UTC
Detected note frequencies become increasing inaccurate at higher sample rates. At a sample rate of 44100 Hz, pitch detection is reasonably accurate in the range 200 Hz to 5000 Hz. At a sample rate of 96000 Hz, pitch detection is only reliable above about 500 Hz. At 192000 Hz sample rate, it is only reliable above about 1 kHz. As the name of the effect implies "musical pitch", the detection should be reasonably accurate for "notes" in the 200 Hz to 1000 Hz range at all popular sample rates.
Created attachment 374 [details] Auto size detection window according to sample rate At higher sample rates (which are increasingly common these days) a fixed FFT window size of 1024 samples is inadequate for detecting "note" frequencies. This patch therefore adjusts the window size proportionally for high sample rates. The total analysis period is also adjusted automatically to remain at around 0.2 seconds (as per the original effect) which should be suitable in most cases for detecting the first note of the selection. As there may be non-musical applications where users require reasonable detection at very low sample rates (proposed by Gale), the minimum window size has be capped at 512 samples. This patch has been tested at all preset Project rates from 2,000 Hz to 384,000 Hz and should be good down to around 100 Hz in each case. Middle C (261 Hz) and "A 440" are correctly detected at all these rates. To assist with reviewing the patch, I have retained debug messages, which should be removed before committing.
(In reply to comment #1) > the minimum window size has be capped at 512 samples. Oops. That should say "256 samples".
(In reply to comment #1) > This patch has been tested at all preset Project rates from 2,000 Hz to 384,000 > Hz and should be good down to around 100 Hz in each case. Marked as "patch_ready". It's recently been tested down to 1000 Hz project rate by me. Also at low sample rates, detection down to and including 20 Hz frequency is very reasonable (and far better than now).
(In reply to comment #3) Generally looks good to me. Why the switch from float buffer[analyzeSize]; to float *buffer; buffer = new float[analyzeSize]; , etc?
(In reply to comment #4) > Why the switch from > float buffer[analyzeSize]; > to > float *buffer; > buffer = new float[analyzeSize]; > , etc? That may not be necessary. It is not required on Linux. When working on this patch I has a_lot_of_trouble with Windows build errors. I think that just got put in as part of trying to get it to build on Windows (I don't have a Windows development machine so fixing Windows build errors was a royal pain).
(In reply to comment #5) > --- Comment #5 from Steve Daulton <stevethefiddle@gmail.com> 2013-05-18 06:05:30 EDT --- > (In reply to comment #4) >> Why the switch from >> float buffer[analyzeSize]; >> to >> float *buffer; >> buffer = new float[analyzeSize]; >> , etc? > > That may not be necessary. It is not required on Linux. Oops, I had done only a code review. You're right that it doesn't work in MSVC now that the array sizes are not constants. Committed. > > When working on this patch I has a_lot_of_trouble with Windows build errors. I > think that just got put in as part of trying to get it to build on Windows (I > don't have a Windows development machine so fixing Windows build errors was a > royal pain). > Perhaps you should focus on getting your stuff working only on Linux. It seems that you like submitting a series of incremental patches, but Gale can build on Linux, I think, and most of the incremental responses are from him. When it's agreed ready on Linux, we can do the cross-platform details. Will that work?
As far as I can see this is now fixed on all three platforms.
