Skip to content

Bump dependency versions #13

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

galactic-src
Copy link

Hi, I'm interested in learning some Rust and some voice processing, so this was a natural repo to look at!

I've had a first pass at bringing dependencies up to date. The biggest change is sample 0.10 -> dasp 0.11

A couple of things I wanted to specifically check with you:

  • in dasp (previously "sample") Window<S, W>::new now requires S of type f64, so pitch() is now limited to f64 samples. This works fine for your data, but presumably the whole point of having this generic library is that these functions can be applied to any Sample data type. https://docs.rs/dasp_signal/0.11.0/src/dasp_signal/window/mod.rs.html#79
  • you had a number of Windows with Sample type [f64; 1], rather than a straight f64. Switching to f64 was one way to resolve type issues after updating to dasp - is there a need for the single-value array type?
  • I was confused as to why you implemented a Lag type for the Hanning Window, rather than pass the new lag type directly to pitch. Perhaps you had some other functionality in mind? Rather than the previous Type, there are now two Window types, one of which functions similarly to Type (they even import it as WindowType internally https://docs.rs/dasp_signal/0.11.0/src/dasp_signal/window/mod.rs.html#7), which is what I've done with HanningLag. I might be able to reinstate the previous arrangement though - would there be benefit to that? I'm guessing you had some direction you were considering exploring.

I'm happy to go through and try to sort any warnings too, but thought perhaps a separate PR after this might be appropriate.
Also happy to switch the examples around to a more standard setup so they can be run as cargo run --example example-name if that would appeal.

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

Successfully merging this pull request may close these issues.

1 participant