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

Fixed ChannelVolume size_hint and added tests #440

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
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
64 changes: 63 additions & 1 deletion src/source/channel_volume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,25 @@ where

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
self.input.size_hint()
let map_size_hint = |input_hint: usize| -> usize {
// The input source provides a number of samples (input_hint / channels);
// We return 1 item per channel per sample => * channel_volumes
let input_provides =
input_hint / (self.input.channels() as usize) * self.channel_volumes.len();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering about this part, what happens if input_hint is not a multiple of the channel number? Maybe we should first multiply by channel_volumes.len and then do the division? Or is that wrong?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching the order would be wrong, we have to ceil the division.
If we divide first, a hint of 5 when mapping from stereo to stereo would result in 5 again, but 6 values will be returned.

So I will change the division to ceil, thank you for noticing this mistake.


// In addition, we may be in the process of emitting additional values from
// self.current_sample
let current_sample = if self.current_sample.is_some() {
self.channel_volumes.len() - self.current_channel
} else {
0
};

input_provides + current_sample
};

let (min, max) = self.input.size_hint();
(map_size_hint(min), max.map(map_size_hint))
}
}

Expand Down Expand Up @@ -139,3 +157,47 @@ where
self.input.total_duration()
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::buffer::SamplesBuffer;

const SAMPLES: usize = 100;

fn dummysource(channels: usize) -> SamplesBuffer<f32> {
let data: Vec<f32> = (1..=(SAMPLES * channels)).map(|v| v as f32).collect();
SamplesBuffer::new(channels as _, 1, data)
}

fn make_test(channels_source: usize, channels_result: usize) {
let original = dummysource(channels_source);
assert_eq!(original.size_hint().0, SAMPLES * channels_source);

let mono = ChannelVolume::new(original, vec![1.0; channels_result]);

let (hint_min, hint_max) = mono.size_hint();
assert_eq!(Some(hint_min), hint_max);

let actual_size = mono.count();
assert_eq!(hint_min, actual_size);
}

#[test]
fn size_stereo_mono() {
make_test(2, 1);
}
#[test]
fn size_mono_stereo() {
make_test(1, 2);
}

#[test]
fn size_stereo_eight() {
make_test(2, 8);
}
#[test]
fn size_stereo_five() {
make_test(2, 5);
}
}