Ticket #7 (assigned enhancement)
yinfft can be further optimised
| Reported by: | ciacciax@yahoo.com | Owned by: | piem |
|---|---|---|---|
| Priority: | normal | Milestone: | 0.3.3 |
| Component: | corelib | Version: | 0.3.2 |
| Severity: | normal | Keywords: | |
| Cc: |
Description (last modified by piem) (diff)
Simplifying everything, your YinFFT algorithm can be summarized by five steps:
- windowing
- compute the FFT of the windowed signal
- compute the magnitude of the spectrum of step 2
- compute the FFT of the array obtained in step 3
- computing yinFFT using this formula: yinFFT[tau] = sum(above) - radius[tau] * cos(phase[tau]);
(point 5 is not very explanatory, but you should be able to understand it)
I noticed that in your implementation, you handle complex points using (radius, phase) notation instead of the "standard" (real,imaginary) one, and this slows down the execution of your algorithm. In point 2, FFTW returns an array of (half)complex numbers, where each point has a real and a imaginary part. Then, you convert the outcome of FFTW in the following way:
- radius = sqrt(re * re + im * im)
- phase = tan2(im, re)
In the point 3, when you compute the weighted magnitude of the spectrum, you square the radius of the complex number. This means that in point 2, you use sqrt() to get the square-root of the magnitude and in point 3 you square the radius, operations that could be avoid.
In point 5, you subtract radius[tau] * cos(phase[tau]) to the sum computed above. The expression "radius[tau] * cos(phase[tau])" simplifies to "real part of the complex number at tau". This means that in the point 4 you convert the FFT outcome to polar coordinates, and then in point 5 you try to extract the real part that before the conversion was already available.
What I am trying to say is that if you didn't convert the outcome of FFTW to polar coordinates, you could avoid a lot of unnecessary computations
- in point 2 the sqrt could be skipped
- in point 2 there is no sense in computing the phase (which requires trigonometric functions) because it is not needed
- (same as point 2 in point 4)
- in point 5 "radius[tau] * cos(phase[tau])" could be replaced by "real part of complex at index tau"
I attach to this ticket the way I would rewrite pitchyinfft.c
To see the real changes, diff my file with the one in aubio 0.3.2
Best regards Andrea