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

Fix(484) - ChannelVolume misbehaving #493

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `Sink.try_seek` now updates `controls.position` before returning. Calls to `Sink.get_pos`
done immediately after a seek will now return the correct value.

- `ChannelVolume` fixed to support numbers of channels apart from powers of two.

### Changed
- `SamplesBuffer` is now `Clone`

Expand Down
53 changes: 27 additions & 26 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions benches/resampler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn no_resampling(bencher: Bencher) {
(source.channels(), source.sample_rate(), source)
})
.bench_values(|(channels, sample_rate, source)| {
UniformSourceIterator::<_, i16>::new(source, channels, sample_rate)
UniformSourceIterator::<_, i16>::new(source, channels.unwrap(), sample_rate.unwrap())
.for_each(divan::black_box_drop)
})
}
Expand All @@ -36,7 +36,7 @@ fn resample_to(bencher: Bencher, target_sample_rate: u32) {
(source.channels(), source)
})
.bench_values(|(channels, source)| {
UniformSourceIterator::<_, i16>::new(source, channels, target_sample_rate)
UniformSourceIterator::<_, i16>::new(source, channels.unwrap(), target_sample_rate)
.for_each(divan::black_box_drop)
})
}
12 changes: 6 additions & 6 deletions benches/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ impl<T: rodio::Sample> Source for TestSource<T> {
None // forever
}

fn channels(&self) -> u16 {
self.channels
fn channels(&self) -> Option<u16> {
Some(self.channels)
}

fn sample_rate(&self) -> u32 {
self.sample_rate
fn sample_rate(&self) -> Option<u32> {
Some(self.sample_rate)
}

fn total_duration(&self) -> Option<Duration> {
Expand All @@ -54,8 +54,8 @@ impl TestSource<i16> {
.take_duration(duration);

TestSource {
channels: sound.channels(),
sample_rate: sound.sample_rate(),
channels: sound.channels().unwrap(),
sample_rate: sound.sample_rate().unwrap(),
total_duration: duration,
samples: sound.into_iter().collect::<Vec<_>>().into_iter(),
}
Expand Down
12 changes: 12 additions & 0 deletions examples/channel_volume.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use rodio::source::ChannelVolume;

fn main() {
let stream_handle = rodio::OutputStreamBuilder::open_default_stream().unwrap();
let sink = rodio::Sink::connect_new(&stream_handle.mixer());

let input = rodio::source::SineWave::new(440.0);
let chan_vol = ChannelVolume::new(input, vec![0.01, 0.0, 0.1, 0.1, 0.1, 0.5]);
sink.append(chan_vol);

sink.sleep_until_end();
}
16 changes: 9 additions & 7 deletions src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,13 @@ where
}

#[inline]
fn channels(&self) -> u16 {
self.channels
fn channels(&self) -> Option<u16> {
Some(self.channels)
}

#[inline]
fn sample_rate(&self) -> u32 {
self.sample_rate
fn sample_rate(&self) -> Option<u32> {
Some(self.sample_rate)
}

#[inline]
Expand All @@ -95,14 +95,16 @@ where
/// This jumps in memory till the sample for `pos`.
#[inline]
fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError> {
let curr_channel = self.pos % self.channels() as usize;
let new_pos = pos.as_secs_f32() * self.sample_rate() as f32 * self.channels() as f32;
let curr_channel = self.pos % self.channels().unwrap() as usize;
let new_pos = pos.as_secs_f32()
* self.sample_rate().unwrap() as f32
* self.channels().unwrap() as f32;
// saturate pos at the end of the source
let new_pos = new_pos as usize;
let new_pos = new_pos.min(self.data.len());

// make sure the next sample is for the right channel
let new_pos = new_pos.next_multiple_of(self.channels() as usize);
let new_pos = new_pos.next_multiple_of(self.channels().unwrap() as usize);
let new_pos = new_pos - curr_channel;

self.pos = new_pos;
Expand Down
18 changes: 17 additions & 1 deletion src/conversions/channels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ where
from: cpal::ChannelCount,
to: cpal::ChannelCount,
) -> ChannelCountConverter<I> {
assert!(from >= 1);
assert!(to >= 1);
assert!(from >= 1);

ChannelCountConverter {
input,
Expand Down Expand Up @@ -178,4 +178,20 @@ mod test {
let output = ChannelCountConverter::new(input.into_iter(), 2, 1);
assert_eq!(output.len(), 2);
}

#[test]
#[should_panic]
fn zero_input() {
let input = vec![1u16, 2, 3, 4, 5, 6];
// Panics because from is 0
let _output = ChannelCountConverter::new(input.into_iter(), 0, 3);
}

#[test]
#[should_panic]
fn zero_output() {
let input = vec![1u16, 2, 3, 4, 5, 6];
// Panics because to is 0
let _output = ChannelCountConverter::new(input.into_iter(), 3, 0);
}
}
7 changes: 5 additions & 2 deletions src/conversions/sample_rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ where
to: cpal::SampleRate,
num_channels: cpal::ChannelCount,
) -> SampleRateConverter<I> {
let from = from.0;
let from = match from.0 {
0 => to.0,
n => n,
};
let to = to.0;

assert!(num_channels >= 1);
Expand Down Expand Up @@ -353,7 +356,7 @@ mod test {

let to = SampleRate(to);
let source = SineWave::new(freq).take_duration(d);
let from = SampleRate(source.sample_rate());
let from = SampleRate(source.sample_rate().unwrap());

let resampled =
SampleRateConverter::new(source, from, to, 1);
Expand Down
8 changes: 4 additions & 4 deletions src/decoder/flac.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ where
}

#[inline]
fn channels(&self) -> u16 {
self.channels
fn channels(&self) -> Option<u16> {
Some(self.channels)
}

#[inline]
fn sample_rate(&self) -> u32 {
self.sample_rate
fn sample_rate(&self) -> Option<u32> {
Some(self.sample_rate)
}

#[inline]
Expand Down
16 changes: 8 additions & 8 deletions src/decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<R: Read + Seek> DecoderImpl<R> {
}

#[inline]
fn channels(&self) -> u16 {
fn channels(&self) -> Option<u16> {
match self {
#[cfg(all(feature = "wav", not(feature = "symphonia-wav")))]
DecoderImpl::Wav(source) => source.channels(),
Expand All @@ -130,12 +130,12 @@ impl<R: Read + Seek> DecoderImpl<R> {
DecoderImpl::Mp3(source) => source.channels(),
#[cfg(feature = "symphonia")]
DecoderImpl::Symphonia(source) => source.channels(),
DecoderImpl::None(_) => 0,
DecoderImpl::None(_) => None, // FIX-ME? I changed this from `0` to `None` instead of `Some(0)`
}
}

#[inline]
fn sample_rate(&self) -> u32 {
fn sample_rate(&self) -> Option<u32> {
match self {
#[cfg(all(feature = "wav", not(feature = "symphonia-wav")))]
DecoderImpl::Wav(source) => source.sample_rate(),
Expand All @@ -147,7 +147,7 @@ impl<R: Read + Seek> DecoderImpl<R> {
DecoderImpl::Mp3(source) => source.sample_rate(),
#[cfg(feature = "symphonia")]
DecoderImpl::Symphonia(source) => source.sample_rate(),
DecoderImpl::None(_) => 1,
DecoderImpl::None(_) => Some(1),
}
}

Expand Down Expand Up @@ -418,11 +418,11 @@ where
}

#[inline]
fn channels(&self) -> u16 {
fn channels(&self) -> Option<u16> {
self.0.channels()
}

fn sample_rate(&self) -> u32 {
fn sample_rate(&self) -> Option<u32> {
self.0.sample_rate()
}

Expand Down Expand Up @@ -516,12 +516,12 @@ where
}

#[inline]
fn channels(&self) -> u16 {
fn channels(&self) -> Option<u16> {
self.0.channels()
}

#[inline]
fn sample_rate(&self) -> u32 {
fn sample_rate(&self) -> Option<u32> {
self.0.sample_rate()
}

Expand Down
Loading
Loading