This makes ChannelCount NonZero<u16> and channels not zero asserts

I ran into a lot of bugs while adding tests that had to do with channel
being set to zero somewhere. While this change makes the API slightly
less easy to use it prevents very hard to debug crashes/underflows etc.

Performance might drop in decoders, the current implementation makes the
bound check every time `channels` is called which is once per span. This
could be cached to alleviate that.

Author: dvdsk

benches/pipeline.rs

@@ -1,6 +1,7 @@
 use std::time::Duration;
 
 use divan::Bencher;
+use rodio::ChannelCount;
 use rodio::{source::UniformSourceIterator, Source};
 
 mod shared;
@@ -31,7 +32,8 @@ fn long(bencher: Bencher) {
             .buffered()
             .reverb(Duration::from_secs_f32(0.05), 0.3)
             .skippable();
-        let resampled = UniformSourceIterator::new(effects_applied, 2, 40_000);
+        let resampled =
+            UniformSourceIterator::new(effects_applied, ChannelCount::new(2).unwrap(), 40_000);
         resampled.for_each(divan::black_box_drop)
     })
 }

benches/shared.rs

@@ -6,7 +6,7 @@ use rodio::{ChannelCount, Sample, SampleRate, Source};
 
 pub struct TestSource {
     samples: vec::IntoIter<Sample>,
-    channels: u16,
+    channels: ChannelCount,
     sample_rate: u32,
     total_duration: Duration,
 }

examples/mix_multiple_sources.rs

@@ -1,11 +1,12 @@
 use rodio::mixer;
 use rodio::source::{SineWave, Source};
 use std::error::Error;
+use std::num::NonZero;
 use std::time::Duration;
 
 fn main() -> Result<(), Box<dyn Error>> {
     // Construct a dynamic controller and mixer, stream_handle, and sink.
-    let (controller, mixer) = mixer::mixer(2, 44_100);
+    let (controller, mixer) = mixer::mixer(NonZero::new(2).unwrap(), 44_100);
     let stream_handle = rodio::OutputStreamBuilder::open_default_stream()?;
     let sink = rodio::Sink::connect_new(&stream_handle.mixer());
 

src/buffer.rs

@@ -6,7 +6,8 @@
 //!
 //! ```
 //! use rodio::buffer::SamplesBuffer;
-//! let _ = SamplesBuffer::new(1, 44100, vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
+//! use rodio::ChannelCount;
+//! let _ = SamplesBuffer::new(ChannelCount::new(1).unwrap(), 44100, vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
 //! ```
 //!
 
@@ -30,7 +31,6 @@ impl SamplesBuffer {
     ///
     /// # Panic
     ///
-    /// - Panics if the number of channels is zero.
     /// - Panics if the samples rate is zero.
     /// - Panics if the length of the buffer is larger than approximately 16 billion elements.
     ///   This is because the calculation of the duration would overflow.
@@ -39,13 +39,12 @@ impl SamplesBuffer {
     where
         D: Into<Vec<Sample>>,
     {
-        assert!(channels >= 1);
         assert!(sample_rate >= 1);
 
         let data = data.into();
         let duration_ns = 1_000_000_000u64.checked_mul(data.len() as u64).unwrap()
             / sample_rate as u64
-            / channels as u64;
+            / channels.get() as u64;
         let duration = Duration::new(
             duration_ns / 1_000_000_000,
             (duration_ns % 1_000_000_000) as u32,
@@ -89,14 +88,14 @@ impl Source for SamplesBuffer {
         // and due to the constant sample_rate we can jump to the right
         // sample directly.
 
-        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().get() as usize;
+        let new_pos = pos.as_secs_f32() * self.sample_rate() as f32 * self.channels().get() 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().get() as usize);
         let new_pos = new_pos - curr_channel;
 
         self.pos = new_pos;
@@ -123,36 +122,31 @@ impl Iterator for SamplesBuffer {
 #[cfg(test)]
 mod tests {
     use crate::buffer::SamplesBuffer;
+    use crate::math::ch;
     use crate::source::Source;
 
     #[test]
     fn basic() {
-        let _ = SamplesBuffer::new(1, 44100, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
-    }
-
-    #[test]
-    #[should_panic]
-    fn panic_if_zero_channels() {
-        SamplesBuffer::new(0, 44100, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
+        let _ = SamplesBuffer::new(ch!(1), 44100, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
     }
 
     #[test]
     #[should_panic]
     fn panic_if_zero_sample_rate() {
-        SamplesBuffer::new(1, 0, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
+        SamplesBuffer::new(ch!(1), 0, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
     }
 
     #[test]
     fn duration_basic() {
-        let buf = SamplesBuffer::new(2, 2, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
+        let buf = SamplesBuffer::new(ch!(2), 2, vec![0.0, 0.0, 0.0, 0.0, 0.0, 0.0]);
         let dur = buf.total_duration().unwrap();
         assert_eq!(dur.as_secs(), 1);
         assert_eq!(dur.subsec_nanos(), 500_000_000);
     }
 
     #[test]
     fn iteration() {
-        let mut buf = SamplesBuffer::new(1, 44100, vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
+        let mut buf = SamplesBuffer::new(ch!(1), 44100, vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0]);
         assert_eq!(buf.next(), Some(1.0));
         assert_eq!(buf.next(), Some(2.0));
         assert_eq!(buf.next(), Some(3.0));
@@ -172,7 +166,7 @@ mod tests {
         #[test]
         fn channel_order_stays_correct() {
             const SAMPLE_RATE: SampleRate = 100;
-            const CHANNELS: ChannelCount = 2;
+            const CHANNELS: ChannelCount = ch!(2);
             let mut buf = SamplesBuffer::new(
                 CHANNELS,
                 SAMPLE_RATE,
@@ -182,7 +176,10 @@ mod tests {
                     .collect::<Vec<_>>(),
             );
             buf.try_seek(Duration::from_secs(5)).unwrap();
-            assert_eq!(buf.next(), Some(5.0 * SAMPLE_RATE as f32 * CHANNELS as f32));
+            assert_eq!(
+                buf.next(),
+                Some(5.0 * SAMPLE_RATE as f32 * CHANNELS.get() as f32)
+            );
 
             assert!(buf.next().is_some_and(|s| s.trunc() as i32 % 2 == 1));
             assert!(buf.next().is_some_and(|s| s.trunc() as i32 % 2 == 0));

src/common.rs

@@ -1,8 +1,10 @@
+use std::num::NonZero;
+
 /// Stream sample rate (a frame rate or samples per second per channel).
 pub type SampleRate = u32;
 
-/// Number of channels in a stream.
-pub type ChannelCount = u16;
+/// Number of channels in a stream. Can never be Zero
+pub type ChannelCount = NonZero<u16>;
 
 /// Represents value of a single sample.
 /// Silence corresponds to the value `0.0`. The expected amplitude range is  -1.0...1.0.

src/conversions/channels.rs

@@ -11,7 +11,7 @@ where
     from: ChannelCount,
     to: ChannelCount,
     sample_repeat: Option<Sample>,
-    next_output_sample_pos: ChannelCount,
+    next_output_sample_pos: u16,
 }
 
 impl<I> ChannelCountConverter<I>
@@ -26,9 +26,6 @@ where
     ///
     #[inline]
     pub fn new(input: I, from: ChannelCount, to: ChannelCount) -> ChannelCountConverter<I> {
-        assert!(from >= 1);
-        assert!(to >= 1);
-
         ChannelCountConverter {
             input,
             from,
@@ -65,7 +62,7 @@ where
                 self.sample_repeat = value;
                 value
             }
-            x if x < self.from => self.input.next(),
+            x if x < self.from.get() => self.input.next(),
             1 => self.sample_repeat,
             _ => Some(0.0),
         };
@@ -74,11 +71,11 @@ where
             self.next_output_sample_pos += 1;
         }
 
-        if self.next_output_sample_pos == self.to {
+        if self.next_output_sample_pos == self.to.get() {
             self.next_output_sample_pos = 0;
 
             if self.from > self.to {
-                for _ in self.to..self.from {
+                for _ in self.to.get()..self.from.get() {
                     self.input.next(); // discarding extra input
                 }
             }
@@ -91,13 +88,13 @@ where
     fn size_hint(&self) -> (usize, Option<usize>) {
         let (min, max) = self.input.size_hint();
 
-        let consumed = std::cmp::min(self.from, self.next_output_sample_pos) as usize;
+        let consumed = std::cmp::min(self.from.get(), self.next_output_sample_pos) as usize;
 
-        let min = ((min + consumed) / self.from as usize * self.to as usize)
+        let min = ((min + consumed) / self.from.get() as usize * self.to.get() as usize)
             .saturating_sub(self.next_output_sample_pos as usize);
 
         let max = max.map(|max| {
-            ((max + consumed) / self.from as usize * self.to as usize)
+            ((max + consumed) / self.from.get() as usize * self.to.get() as usize)
                 .saturating_sub(self.next_output_sample_pos as usize)
         });
 
@@ -111,31 +108,37 @@ impl<I> ExactSizeIterator for ChannelCountConverter<I> where I: ExactSizeIterato
 mod test {
     use super::ChannelCountConverter;
     use crate::common::ChannelCount;
+    use crate::math::ch;
     use crate::Sample;
 
     #[test]
     fn remove_channels() {
         let input = vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 3, 2).collect::<Vec<_>>();
+        let output =
+            ChannelCountConverter::new(input.into_iter(), ch!(3), ch!(2)).collect::<Vec<_>>();
         assert_eq!(output, [1.0, 2.0, 4.0, 5.0]);
 
         let input = vec![1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 4, 1).collect::<Vec<_>>();
+        let output =
+            ChannelCountConverter::new(input.into_iter(), ch!(4), ch!(1)).collect::<Vec<_>>();
         assert_eq!(output, [1.0, 5.0]);
     }
 
     #[test]
     fn add_channels() {
         let input = vec![1.0, 2.0, 3.0, 4.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 1, 2).collect::<Vec<_>>();
+        let output =
+            ChannelCountConverter::new(input.into_iter(), ch!(1), ch!(2)).collect::<Vec<_>>();
         assert_eq!(output, [1.0, 1.0, 2.0, 2.0, 3.0, 3.0, 4.0, 4.0]);
 
         let input = vec![1.0, 2.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 1, 4).collect::<Vec<_>>();
+        let output =
+            ChannelCountConverter::new(input.into_iter(), ch!(1), ch!(4)).collect::<Vec<_>>();
         assert_eq!(output, [1.0, 1.0, 0.0, 0.0, 2.0, 2.0, 0.0, 0.0]);
 
         let input = vec![1.0, 2.0, 3.0, 4.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 2, 4).collect::<Vec<_>>();
+        let output =
+            ChannelCountConverter::new(input.into_iter(), ch!(2), ch!(4)).collect::<Vec<_>>();
         assert_eq!(output, [1.0, 2.0, 0.0, 0.0, 3.0, 4.0, 0.0, 0.0]);
     }
 
@@ -152,24 +155,24 @@ mod test {
             assert_eq!(converter.size_hint(), (0, Some(0)));
         }
 
-        test(&[1.0, 2.0, 3.0], 1, 2);
-        test(&[1.0, 2.0, 3.0, 4.0], 2, 4);
-        test(&[1.0, 2.0, 3.0, 4.0], 4, 2);
-        test(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0], 3, 8);
-        test(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0], 4, 1);
+        test(&[1.0, 2.0, 3.0], ch!(1), ch!(2));
+        test(&[1.0, 2.0, 3.0, 4.0], ch!(2), ch!(4));
+        test(&[1.0, 2.0, 3.0, 4.0], ch!(4), ch!(2));
+        test(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0], ch!(3), ch!(8));
+        test(&[1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0], ch!(4), ch!(1));
     }
 
     #[test]
     fn len_more() {
         let input = vec![1.0, 2.0, 3.0, 4.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 2, 3);
+        let output = ChannelCountConverter::new(input.into_iter(), ch!(2), ch!(3));
         assert_eq!(output.len(), 6);
     }
 
     #[test]
     fn len_less() {
         let input = vec![1.0, 2.0, 3.0, 4.0];
-        let output = ChannelCountConverter::new(input.into_iter(), 2, 1);
+        let output = ChannelCountConverter::new(input.into_iter(), ch!(2), ch!(1));
         assert_eq!(output.len(), 2);
     }
 }