improve performance and seek accuracy (#48)

* Improved performance when decoding near the end of a file

* Improved the accuracy when seeking to a large frame value with the Symphonia decoder

* Bumped minimum supported Rust version to 1.65

.github/workflows/test.yaml

@@ -26,7 +26,7 @@ jobs:
 
           # Minimum Supported Rust Version (MSRV)
           - os: ubuntu-latest
-            rust: 1.62.0
+            rust: 1.65.0
             workspace-extra-args: "--exclude player --exclude writer"
 
     steps:

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Version History
 
+## Version 1.2.1 (2024-1-3)
+
+- Improved performance when decoding near the end of a file
+- Improved the accuracy when seeking to a large frame value with the Symphonia decoder
+- Bumped minimum supported Rust version to 1.65
+
 ## Version 1.2.0 (2023-12-28)
 
 ### Breaking changes:

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "creek"
-version = "1.2.0"
+version = "1.2.1"
 authors = ["Billy Messenger <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -17,7 +17,7 @@ include = [
     "LICENSE-MIT",
     "how_it_works.svg",
 ]
-rust-version = "1.62"
+rust-version = "1.65"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
@@ -57,8 +57,8 @@ decode-all = [
 encode-wav = ["creek-encode-wav"]
 
 [dependencies]
-creek-core = { version = "0.2.0", path = "core" }
-creek-decode-symphonia = { version = "0.3.0", path = "decode_symphonia", optional = true }
+creek-core = { version = "0.2.1", path = "core" }
+creek-decode-symphonia = { version = "0.3.1", path = "decode_symphonia", optional = true }
 creek-encode-wav = { version = "0.2.0", path = "encode_wav", optional = true }
 
 # Unoptimized builds result in prominent gaps of silence after cache misses in the demo player.

core/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "creek-core"
-version = "0.2.0"
+version = "0.2.1"
 authors = ["Billy Messenger <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"

core/src/read/data.rs

@@ -17,15 +17,6 @@ impl<T: Copy + Clone + Default + Send> DataBlock<T> {
             ch.clear();
         }
     }
-
-    pub(crate) fn ensure_correct_size(&mut self, block_size: usize) {
-        // If the decoder didn't fill enough frames, then fill the rest with zeros.
-        for ch in self.block.iter_mut() {
-            if ch.len() < block_size {
-                ch.resize(block_size, Default::default());
-            }
-        }
-    }
 }
 
 pub(crate) struct DataBlockCache<T: Copy + Clone + Default + Send> {

core/src/read/read_stream.rs

@@ -760,54 +760,22 @@ impl<D: Decoder> ReadDiskStream<D> {
         if end_frame_in_block > self.block_size {
             // Data spans between two blocks, so two copies need to be performed.
 
-            // Copy from first block.
             let first_len = self.block_size - self.current_frame_in_block;
             let second_len = frames - first_len;
+
+            // Copy from first block.
             {
                 // This check should never fail because it can only be `None` in the destructor.
                 let heap = self.heap_data.as_mut().unwrap();
 
                 heap.read_buffer.clear();
 
-                // Get the first block of data.
-                let current_block_data = {
-                    let current_block = &heap.prefetch_buffer[self.current_block_index];
-
-                    match current_block.use_cache_index {
-                        Some(cache_index) => {
-                            if let Some(cache) = &heap.caches[cache_index].cache {
-                                Some(&cache.blocks[self.current_block_index])
-                            } else {
-                                // If cache is empty, output silence instead.
-                                None
-                            }
-                        }
-                        None => {
-                            if let Some(block) = &current_block.block {
-                                Some(block)
-                            } else {
-                                // TODO: warn of buffer underflow.
-                                None
-                            }
-                        }
-                    }
-                };
-
-                if let Some(block) = current_block_data {
-                    for (read_buffer_ch, block_ch) in
-                        heap.read_buffer.block.iter_mut().zip(block.block.iter())
-                    {
-                        read_buffer_ch.extend_from_slice(
-                            &block_ch[self.current_frame_in_block
-                                ..self.current_frame_in_block + first_len],
-                        );
-                    }
-                } else {
-                    // Output silence.
-                    for ch in heap.read_buffer.block.iter_mut() {
-                        ch.resize(ch.len() + first_len, Default::default());
-                    }
-                }
+                copy_block_into_read_buffer(
+                    heap,
+                    self.current_block_index,
+                    self.current_frame_in_block,
+                    first_len,
+                );
             }
 
             self.advance_to_next_block()?;
@@ -817,45 +785,10 @@ impl<D: Decoder> ReadDiskStream<D> {
                 // This check should never fail because it can only be `None` in the destructor.
                 let heap = self.heap_data.as_mut().unwrap();
 
-                // Get the next block of data.
-                let next_block_data = {
-                    let next_block = &heap.prefetch_buffer[self.current_block_index];
-
-                    match next_block.use_cache_index {
-                        Some(cache_index) => {
-                            if let Some(cache) = &heap.caches[cache_index].cache {
-                                Some(&cache.blocks[self.current_block_index])
-                            } else {
-                                // If cache is empty, output silence instead.
-                                None
-                            }
-                        }
-                        None => {
-                            if let Some(block) = &next_block.block {
-                                Some(block)
-                            } else {
-                                // TODO: warn of buffer underflow.
-                                None
-                            }
-                        }
-                    }
-                };
-
-                if let Some(block) = next_block_data {
-                    for (read_buffer_ch, block_ch) in
-                        heap.read_buffer.block.iter_mut().zip(block.block.iter())
-                    {
-                        read_buffer_ch.extend_from_slice(&block_ch[0..second_len]);
-                    }
-                } else {
-                    // Output silence.
-                    for ch in heap.read_buffer.block.iter_mut() {
-                        ch.resize(ch.len() + second_len, Default::default());
-                    }
-                }
-
-                self.current_frame_in_block = second_len;
+                copy_block_into_read_buffer(heap, self.current_block_index, 0, second_len);
             }
+
+            self.current_frame_in_block = second_len;
         } else {
             // Only need to copy from current block.
             {
@@ -864,45 +797,12 @@ impl<D: Decoder> ReadDiskStream<D> {
 
                 heap.read_buffer.clear();
 
-                // Get the first block of data.
-                let current_block_data = {
-                    let current_block = &heap.prefetch_buffer[self.current_block_index];
-
-                    match current_block.use_cache_index {
-                        Some(cache_index) => {
-                            if let Some(cache) = &heap.caches[cache_index].cache {
-                                Some(&cache.blocks[self.current_block_index])
-                            } else {
-                                // If cache is empty, output silence instead.
-                                None
-                            }
-                        }
-                        None => {
-                            if let Some(block) = &current_block.block {
-                                Some(block)
-                            } else {
-                                // TODO: warn of buffer underflow.
-                                None
-                            }
-                        }
-                    }
-                };
-
-                if let Some(block) = current_block_data {
-                    for (read_buffer_ch, block_ch) in
-                        heap.read_buffer.block.iter_mut().zip(block.block.iter())
-                    {
-                        read_buffer_ch.extend_from_slice(
-                            &block_ch
-                                [self.current_frame_in_block..self.current_frame_in_block + frames],
-                        );
-                    }
-                } else {
-                    // Output silence.
-                    for ch in heap.read_buffer.block.iter_mut() {
-                        ch.resize(ch.len() + frames, Default::default());
-                    }
-                }
+                copy_block_into_read_buffer(
+                    heap,
+                    self.current_block_index,
+                    self.current_frame_in_block,
+                    frames,
+                );
             }
 
             self.current_frame_in_block = end_frame_in_block;
@@ -989,3 +889,55 @@ impl<D: Decoder> Drop for ReadDiskStream<D> {
         let _ = self.close_signal_tx.push(self.heap_data.take());
     }
 }
+
+fn copy_block_into_read_buffer<T: Copy + Default + Send>(
+    heap: &mut HeapData<T>,
+    block_index: usize,
+    start_frame_in_block: usize,
+    frames: usize,
+) {
+    let block_entry = &heap.prefetch_buffer[block_index];
+
+    let maybe_block = match block_entry.use_cache_index {
+        Some(cache_index) => heap.caches[cache_index]
+            .cache
+            .as_ref()
+            .map(|cache| &cache.blocks[block_index]),
+        None => {
+            block_entry.block.as_ref()
+
+            // TODO: warn of buffer underflow.
+        }
+    };
+
+    let Some(block) = maybe_block else {
+        // If no block exists, output silence.
+        for buffer_ch in heap.read_buffer.block.iter_mut() {
+            buffer_ch.resize(buffer_ch.len() + frames, Default::default());
+        }
+
+        return;
+    };
+
+    for (buffer_ch, block_ch) in heap.read_buffer.block.iter_mut().zip(block.block.iter()) {
+        // If for some reason the decoder did not fill this block fully,
+        // fill the rest with zeros.
+        if block_ch.len() < start_frame_in_block + frames {
+            if block_ch.len() <= start_frame_in_block {
+                // The block has no more data to copy, fill all frames with zeros.
+                buffer_ch.resize(buffer_ch.len() + frames, Default::default());
+            } else {
+                let copy_frames = block_ch.len() - start_frame_in_block;
+
+                buffer_ch.extend_from_slice(
+                    &block_ch[start_frame_in_block..start_frame_in_block + copy_frames],
+                );
+
+                buffer_ch.resize(buffer_ch.len() + frames - copy_frames, Default::default());
+            }
+        } else {
+            buffer_ch
+                .extend_from_slice(&block_ch[start_frame_in_block..start_frame_in_block + frames]);
+        };
+    }
+}

core/src/read/server.rs

@@ -113,8 +113,6 @@ impl<D: Decoder> ReadServer<D> {
 
                         let decode_res = self.decoder.decode(&mut block);
 
-                        block.ensure_correct_size(self.block_size);
-
                         match decode_res {
                             Ok(()) => {
                                 self.send_msg(ServerToClientMsg::ReadIntoBlockRes {
@@ -187,8 +185,6 @@ impl<D: Decoder> ReadServer<D> {
 
                     let decode_res = self.decoder.decode(block);
 
-                    block.ensure_correct_size(self.block_size);
-
                     if let Err(e) = decode_res {
                         self.send_msg(ServerToClientMsg::FatalError(e));
                         self.run = false;

decode_symphonia/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "creek-decode-symphonia"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Billy Messenger <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -14,7 +14,7 @@ repository = "https://github.com/RustyDAW/creek"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-creek-core = { version = "0.2.0", path = "../core" }
+creek-core = { version = "0.2.1", path = "../core" }
 log = "0.4"
 symphonia = "0.5"
 

decode_symphonia/src/lib.rs

@@ -95,12 +95,10 @@ impl Decoder for SymphoniaDecoder {
 
         // Seek the reader to the requested position.
         if start_frame != 0 {
-            let seconds = start_frame as f64 / f64::from(sample_rate.unwrap_or(44100));
-
             reader.seek(
                 SeekMode::Accurate,
                 SeekTo::Time {
-                    time: seconds.into(),
+                    time: frame_to_symphonia_time(start_frame as u64, sample_rate.unwrap_or(44100)),
                     track_id: None,
                 },
             )?;
@@ -199,12 +197,13 @@ impl Decoder for SymphoniaDecoder {
 
         self.playhead_frame = frame;
 
-        let seconds = self.playhead_frame as f64 / f64::from(self.sample_rate.unwrap_or(44100));
-
         match self.reader.seek(
             SeekMode::Accurate,
             SeekTo::Time {
-                time: seconds.into(),
+                time: frame_to_symphonia_time(
+                    self.playhead_frame as u64,
+                    self.sample_rate.unwrap_or(44100),
+                ),
                 track_id: None,
             },
         ) {
@@ -360,6 +359,17 @@ pub struct SymphoniaDecoderInfo {
     pub metadata: Option<MetadataRevision>,
 }
 
+fn frame_to_symphonia_time(frame: u64, sample_rate: u32) -> symphonia::core::units::Time {
+    // Doing it this way is more accurate for large inputs than just using f64s.
+    let seconds = frame / u64::from(sample_rate);
+    let fract_frames = frame % u64::from(sample_rate);
+    let frac = fract_frames as f64 / f64::from(sample_rate);
+
+    // TODO: Ask the maintainer of Symphonia to add an option to seek to an
+    // exact frame.
+    symphonia::core::units::Time { seconds, frac }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

encode_wav/Cargo.toml

@@ -13,5 +13,5 @@ repository = "https://github.com/RustyDAW/creek"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-creek-core = { version = "0.2.0", path = "../core" }
+creek-core = { version = "0.2.1", path = "../core" }
 byte-slice-cast = "1.0.0"