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

Update to rustfft 6.0 and Rust 2018 edition #6

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

Conversation

sunsided
Copy link

@sunsided sunsided commented Oct 13, 2021

Heya! When I tried to use stft from crates.io it failed with an error related to nalgebra-0.5. I poked around in the forks and found YoshieraHuang/stft which adds some modernizations on which this PR is based:

The latter commit allows cargo bench and cargo criterion to be run with stable Rust.

From there, I updated the Cargo.toml to use edition = "2018" and updated the dependencies to rustfft = "6.0", num = "0.4" and apodize = "1.0". I made sure to sanitize the code using rustfmt and Clippy.

The newer rustfft version appears to do FFT in-place now, but has support for an externally allocated scratch buffer. I added this to the STFT struct - the benchmarks indicate a slight performance increase with that change.

The existing integration tests were lacking an assertion to actually ensure the correct results are calculated. I added a dev dependency on approx = "0.5" to do perform said comparisons; the calculations are now also performed twice to ensure that a call to compute_column() does not
corrupt the internal state.

@sunsided sunsided force-pushed the feature/integrate-YoshieraHuang branch from 2542bda to bc09103 Compare October 20, 2021 17:31
@rohansatapathy
Copy link

It seems like the author has abandoned this project, @sunsided would you be able to release your version under a new name on crates.io to ensure that it has a working build? Maybe it could be called ruststft to match the naming convention of rustfft.

@sunsided
Copy link
Author

@rohansatapathy Will do. I've reached out to @snd by mail first to see if he can add me as a maintainer, too. In that case we don't require the parallel crates.

@sunsided
Copy link
Author

Okay, I published it as ruststft version 0.3.0. If @snd gets back to me regarding maintainership on crates.io and/or merges the changes himself, I'll see to yank my version to not create confusion.

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.

3 participants