alesgenova / pitch-detection

A collection of algorithms to determine the pitch of a sound sample.
MIT License
231 stars 27 forks source link

Consider switching to `easyfft` #28

Open WalterSmuts opened 1 year ago

WalterSmuts commented 1 year ago

This makes the logic simpler and manages the planner and scratch buffers behind the scenes resulting in a ~80% benchmark improvement across the board:

McLeod get_pitch        time:   [11.114 µs 11.118 µs 11.121 µs]
                        change: [-81.978% -81.964% -81.951%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) low severe
  1 (1.00%) high mild

Autocorrelation get_pitch
                        time:   [7.9310 µs 7.9348 µs 7.9388 µs]
                        change: [-86.719% -86.711% -86.703%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) low mild
  1 (1.00%) high mild

YIN get_pitch           time:   [8.8091 µs 8.8150 µs 8.8222 µs]
                        change: [-78.127% -78.106% -78.085%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 13 outliers among 100 measurements (13.00%)
  3 (3.00%) high mild
  10 (10.00%) high severe

detect_peaks            time:   [523.75 ns 524.12 ns 524.48 ns]
                        change: [+0.0519% +0.3183% +0.5176%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) high mild
  1 (1.00%) high severe

Unfortunately it requires an extra trait bound on Default for the fft elements. This is not neccesary but requires changes to rustfft tracked in https://github.com/ejmahler/RustFFT/issues/105.

alesgenova commented 1 year ago

Good morning,

Thank you for the contribution!

I don't see any problem with this PR and I will get it merged, just want to make sure that I can still build the library for wasm first.

You seem to know your way around the various fft libraries, so I have an unrelated question for you.

I was trying to get this library built for a no_std environment, but I didn't get far as rustfft brings in a few dependencies (mainly the num traits) that are no_std compatible but only when specifying some flags, which rustfft doesn't expose to its consumers.

Do you know if there is a workaround for this?

WalterSmuts commented 1 year ago

I don't have much experience with no_std but here goes some info. Take it with a grain of salt however because I've not ever really been involved:

let mut planner = FftPlanner::new(); let fft = planner.plan_fft_forward(1234); // <-- This returns an Arc<dyn Fft>


so you'd atleast need an allocator which AFAIK can be achieved without std.
* Rustfft does some feature detection at runtime that I think needs an OS:
https://doc.rust-lang.org/std/macro.is_x86_feature_detected.html?search=is_x86_feature_detected
* There may be a `no_std` version: https://doc.rust-lang.org/std/macro.is_x86_feature_detected.html?search=is_x86_feature_detected
* There seems to be a `no_std` fft crate that's quite popular: https://crates.io/crates/microfft
* All in all I think `rustfft` is just for `std` compatible targets. Perhaps open an issue to be sure? I think it's definitely something others will be interested in too.