search

RustAudio / rodio

Rust audio playback library
Apache License 2.0
1.73k stars 223 forks source link

`SineWave::new` taking u32 instead of f32 #187

Open AntonHermann opened 6 years ago

AntonHermann commented 6 years ago

Hey there :) is there a particular reason why SineWave::new() takes an u32 when it stores a f32 internally? I'm not that familiar with sound and stuff so I can't estimate, if this minor accuracy loss is even noticeable, but to me there is no particular reason to limit it to u32

tomaka commented 5 years ago

This is mainly a quirk in SineWave that should eventually be fixed. Unfortunately, fixing it is a breaking change and requires a major version, which is why it isn't straight-forward to do so.

mlindner commented 5 years ago

I'm working on several changes to SineWave right now and will push up a pull request. Including fixing a float overflow issue that made it practically unusable.

hsandt commented 3 years ago

@mlindner Any news on this? I'm writing a program that output music notes for tuning so it would be convenient for me to drop either a table of decimal frequencies or a formula without having to round.

The freq member is private and when used, it's multiplied by PI, so the fact that the value is integer shouldn't affect too much. If the issue is about range/overflow as you said, maybe some clamping and/or warning can at least notify the user when passing crazy values (for negative values, you could silently take the opposite, or clamp to 0).

This conversion:

let value = 2.0 * 3.14159265 * self.freq * self.num_sample as f32 / 48000.0;

makes me wonder if self.freq was meant to be something else as well (it seems the product is already f32 and shouldn't need conversion).