Address review comments

Drop a duplicated sentence in the docs for Fft::cutoff, and pin the
crossfade values that the docs for Slip::new quote with an assert, so they
cannot drift unnoticed.

Replace the float rounding used to size the FFT blocks and the sinc filter
with integer arithmetic. Dividing as f32 loses precision for large values,
and div_ceil says what is meant. Rounding down to zero blocks now falls
back to a single block instead of panicking further down.

Author: HEnquist

src/asynchro_sinc.rs

@@ -212,7 +212,7 @@ pub enum SincInterpolationType {
 
 /// Round the sinc length up to the multiple of 8 that the interpolators use.
 pub(crate) fn round_sinc_len(sinc_len: usize) -> usize {
-    8 * (((sinc_len as f32) / 8.0).ceil() as usize)
+    sinc_len.next_multiple_of(8)
 }
 
 /// Resolve the relative cutoff frequency of the sinc filter.

src/slip.rs

@@ -601,6 +601,10 @@ mod tests {
     /// leaving room for a correction (`chunk >= 2 * len + 2`).
     #[test]
     fn crossfade_len_scales_with_chunk() {
+        // The docs on Slip::new quote these two values, since they are what a caller
+        // needs to pick a chunk size. Keep them in sync if the target ever changes.
+        assert_eq!(MAX_CROSSFADE_LEN, 128);
+        assert_eq!(2 * MAX_CROSSFADE_LEN + 2, 258);
         // Capped at the target once the chunk is big enough to hold it.
         assert_eq!(crossfade_len_for(4096), MAX_CROSSFADE_LEN);
         assert_eq!(

src/synchro.rs

@@ -301,18 +301,21 @@ where
             FixedSync::Input => {
                 let min_chunk_in = sample_rate_input / gcd;
                 let wanted_subsize = chunk_size / sub_chunks;
-                (wanted_subsize as f32 / min_chunk_in as f32).ceil() as usize
+                wanted_subsize.div_ceil(min_chunk_in)
             }
             FixedSync::Output => {
                 let min_chunk_out = sample_rate_output / gcd;
                 let wanted_subsize = chunk_size / sub_chunks;
-                (wanted_subsize as f32 / min_chunk_out as f32).ceil() as usize
+                wanted_subsize.div_ceil(min_chunk_out)
             }
             FixedSync::Both => {
                 let min_chunk_in = sample_rate_input / gcd;
-                (chunk_size as f32 / min_chunk_in as f32).ceil() as usize
+                chunk_size.div_ceil(min_chunk_in)
             }
-        };
+        }
+        // Asking for more sub chunks than there are frames rounds down to zero blocks,
+        // which is not a usable resampler. Fall back to a single minimum sized block.
+        .max(1);
         let fft_size_out = fft_chunks * sample_rate_output / gcd;
         let fft_size_in = fft_chunks * sample_rate_input / gcd;
 
@@ -397,8 +400,6 @@ where
     /// A larger block moves it closer to Nyquist.
     /// When downsampling it is scaled down to keep it below the lower Nyquist
     /// frequency of the two sample rates.
-    ///
-    /// Multiply by `sample_rate_input / 2` to get the cutoff in Hz.
     pub fn cutoff(&self) -> f32 {
         self.resampler.cutoff
     }
@@ -412,21 +413,20 @@ where
     ) -> (usize, usize) {
         match fixed {
             FixedSync::Input => {
-                let subchunks_available: f32 =
-                    ((chunk_size + saved_frames) as f32 / fft_size_in as f32).floor();
-                let frames_available = (subchunks_available as usize) * fft_size_out;
+                let subchunks_available = (chunk_size + saved_frames) / fft_size_in;
+                let frames_available = subchunks_available * fft_size_out;
                 (chunk_size, frames_available)
             }
             FixedSync::Output => {
-                let subchunks_needed = ((chunk_size as f32 - saved_frames as f32)
-                    / fft_size_out as f32)
-                    .ceil()
-                    .max(0.0);
-                let frames_needed = (subchunks_needed as usize) * fft_size_in;
+                // Saturating, since more frames may be saved than the chunk needs.
+                let subchunks_needed = chunk_size
+                    .saturating_sub(saved_frames)
+                    .div_ceil(fft_size_out);
+                let frames_needed = subchunks_needed * fft_size_in;
                 (frames_needed, chunk_size)
             }
             FixedSync::Both => {
-                let subchunks_needed = (chunk_size as f32 / fft_size_in as f32).ceil() as usize;
+                let subchunks_needed = chunk_size.div_ceil(fft_size_in);
                 let frames_needed_in = subchunks_needed * fft_size_in;
                 let frames_needed_out = subchunks_needed * fft_size_out;
                 (frames_needed_in, frames_needed_out)
@@ -467,9 +467,7 @@ where
     ) -> usize {
         match fixed {
             FixedSync::Both | FixedSync::Input => chunk_size_in,
-            FixedSync::Output => {
-                (chunk_size_out as f32 / fft_size_out as f32).ceil() as usize * fft_size_in
-            }
+            FixedSync::Output => chunk_size_out.div_ceil(fft_size_out) * fft_size_in,
         }
     }
 
@@ -759,6 +757,25 @@ mod tests {
         assert!((maxval - 1.0).abs() < 0.1);
     }
 
+    #[test_log::test(test_matrix([FixedSync::Input, FixedSync::Output, FixedSync::Both]))]
+    fn fft_more_sub_chunks_than_frames(fixed: FixedSync) {
+        // Asking for more sub chunks than the chunk has frames rounds the sub chunk
+        // size down to zero. That used to give zero FFT blocks, and a panic on the
+        // first division by the block size.
+        let resampler = Fft::<f64>::new_custom(
+            44100,
+            48000,
+            100,
+            1000,
+            2,
+            WindowFunction::BlackmanHarris2,
+            fixed,
+        )
+        .unwrap();
+        assert_eq!(resampler.fft_size_in(), 147);
+        assert_eq!(resampler.fft_size_out(), 160);
+    }
+
     #[test_log::test(test_matrix(
         [512, 1024, 4096],
         [(44100, 48000), (48000, 44100), (44100, 88200), (88200, 44100), (44100, 192000), (192000, 44100)],