Fix audio distortion bug with SineWave source

[mlindner]

Fixes #207

This fixes a audio distortion bug caused by float rounding errors and by the sinewave frequency not matching the frequency of the driver.

[tomaka]

the sinewave frequency not matching the frequency of the driver.

Well, that shouldn't be a problem. If the sine wave doesn't match the expected frequency, it should get converted automatically.

[tomaka]

I agree with using cur_val and iterating this way, but the change to the frequency is probably a different problem.

[mlindner]

I can only assume that the algorithm being used to subsample 48khz to 44.1khz is incorrect. Any audible signal here would be below the Nyquist frequency so there shouldn't be any distortion. Rodio always uses 44.1khz as the output format type on my system because it picks the default format given by cpal device which on my system is a 44.1khz output.

One of the things I want to fix in a future pull request is have rodio pick the format that best approximates the sampling rate of the input signal so resampling isn't required, or punt the format selection up to the application user. Do you have a preference?

[tomaka]

One of the things I want to fix in a future pull request is have rodio pick the format that best approximates the sampling rate of the input signal so resampling isn't required, or punt the format selection up to the application user. Do you have a preference?

I'm a bit conflicted, because originally I wanted cpal and rodio to be able to handle hardware that doesn't come with any way to resample. Which is why it selects the frequency based on the output device, and not the input.
The other problem is that all inputs are mixed together, so if you have multiple different inputs of multiple frequencies, you have to convert at least one of them.

Ideally I'd say that there should be two modes of action: either we interface with a software engine (eg. ALSA), in which case we can open one new backend voice per input and thus pass the data through with the original frequency, or we are interfacing with some hardware, in which case we should select the most appropriate frequency based on the hardware, and do all the conversions ourselves.

[mlindner]

Many games even let you set the output device type and the output sample rate. I think limiting it to simply picking it automatically isn't helpful. It's trying to be too helpful. We can have the default value be automatic but it should also be allowed to specify the format.

Thoughts:

• By default I'd have rodio pick something automatically by looking at the sample rate of the output. (The default setting is the audio device's default format.) However it should be configurable if the user of rodio wishes to force a specific output format.
• At the next level up rodio should inspect the format that was selected either automatically or manually and insert resample filters for all incoming signals to that sample rate, but do no re-sample filtering for any samples that already have the correct sample rate.

This basically establishes two API layers. Later on down the line we should be able to transparently swap the "rodio picks the default format of the audio device" to "rodio passes raw samples to the lower level hardware layer and lets the hardware do the mixing".

Another thought, what I would suggest need to happen would have the lower level drivers like ALSA supply a list of traits that the driver implements up to cpal and cpal can then propagate those traits up to rodio and other applications. The traits would specify whether they can resample, whether they can mix, how many channels they can support (if for example we need to combine channels in software), etc. That should not conflict though with the other implementation of rodio that I just discussed. It will optionally fall back on doing some things itself if the lower level hardware doesn't support those features.

[jayanderson]

Is there a reason this change wasn't merged? I just found this project and started poking around and was about to submit a very similar pull request.

Two comments:

• Instead of creating its own constant PI_2 it could use std::f32::consts::TAU .
• No multiplication or division is needed within the loop. Instead the structure can maintain an increment value set at creation time: increment: TAU * sample_rate.recip() * freq. Then within the iterator: self.cur_val += self.increment.

That said, I agree that this change wouldn't fix the distortion issue. This is however a simpler, slightly more efficient implementation of a sine wave. It also makes it easier to change the frequency after it's started without pops and cracks (if such a feature is desired in the future).

[yara-blue]

The sine source will soon be deprecated. It is replaced by SignalGenerator which supports Sine. I do not think SignalGenerator suffers from the same issue but if it does please report it.

[PetrGlad]

@dvdsk I suspect current SignalGenerator has similar problem to this one. It also uses continuous sample counter that is then divided by period. So likely #207 still has to be fixed but for SignalGenerator. At the very least I'd used f64 there but I like this solution more as it has consistent behavior every sine period.

[PetrGlad]

I am not sure if that was intentional but int this patch roll over happens at $\pi \over 2$ where $sin$ is linear, so stitching should normally be seamless.