-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add FFT buffer size of 32768 #59
Add FFT buffer size of 32768 #59
Conversation
For reference this is the sample analyser KungFuFurby is talking about. It's used to help determine the frequency (tuning) of the BRR sample when it played at 32kHz. To quickly fix the panic I am currently truncating the sample when analysing it. I'm still not sure how to properly analyse a large sample (or even if spectrum-analyzer is fit for the task). Truncating the sample works well enough for now. Thank you for creating and publishing the spectrum-analyzer crate. It allowed me to quickly build and test the sample analyser dialog over the course of 3 days. |
Hey, thanks for the interest in my project!
I think that this is exactly the way to do it, also in other libraries. Do in in multiple chunks.
Awesome!! |
c3f2f27
to
73904b7
Compare
Hey! Can you please
Thanks! I'm happy to merge once this is fixed |
3df9879
to
ad2fc81
Compare
Thanks for telling me about this, and very good call, given that my first commit was... a bit rushed, to put things simply, and didn't do enough of the research. So I discarded the first commit completely, since those sizes don't actually exist yet (and I seem to have realized that the lower layer stuff has a lot of lines to handle, so I can clearly tell how things are going exponentially-speaking). I also, in the process of doing conflict resolution, changed the microfft dependency to use a tilde requirement so that it can keep up to date as needed, and did a minor wording change to the changelog. And the formatting was modified courtesy of |
Great. Well done! ;) I'm going to fix the remaining CI error by myself |
Context for this pull request: The powers of 2 shown here go up to 16384, and I wish to add a few more power of 2's worth of buffer sizes to the mix.
The Terrific Audio Driver by Undisbeliever calls
samples_fft_to_spectrum
when analyzing a BRR sample (this is created for the SPC700, which has a 64KB limit) by calculating a spectrum in one go (thus, if the samples are large enough they tend to request a buffer size large enough). This works fine up to 16384 samples, but a BRR sample can theoretically be as large as 116496 (though in practice it is usually smaller than this). The result is a panic.Context for the code in question:
https://github.com/undisbeliever/terrific-audio-driver/blob/241c53fa3706bc89f3190ab7627145c29122c9ce/crates/tad-gui/src/sample_analyser.rs#L926
The code was since amended to limit the analysis size to 16384 samples for the time being. I determined that microfft only goes up to 32768...