fix clippy warnings

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Version History
 
+## Version 1.2.2 (2024-1-5)
+
+- Fixed clippy warnings
+- Added panic documentation
+
 ## Version 1.2.1 (2024-1-3)
 
 - Improved performance when decoding near the end of a file

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "creek"
-version = "1.2.1"
+version = "1.2.2"
 authors = ["Billy Messenger <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -57,7 +57,7 @@ decode-all = [
 encode-wav = ["creek-encode-wav"]
 
 [dependencies]
-creek-core = { version = "0.2.1", path = "core" }
+creek-core = { version = "0.2.2", 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 }
 

core/Cargo.toml

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

core/src/lib.rs

@@ -1,6 +1,6 @@
 #![warn(rust_2018_idioms)]
 #![warn(rust_2021_compatibility)]
-// TODO: #![warn(clippy::missing_panics_doc)]
+#![warn(clippy::missing_panics_doc)]
 #![warn(clippy::clone_on_ref_ptr)]
 #![deny(trivial_numeric_casts)]
 #![forbid(unsafe_code)]

core/src/read/read_stream.rs

@@ -7,6 +7,7 @@ use super::{
     ClientToServerMsg, DataBlock, Decoder, HeapData, ReadData, ReadServer, ReadStreamOptions,
     ServerToClientMsg,
 };
+use crate::read::server::ReadServerOptions;
 use crate::{FileInfo, SERVER_WAIT_TIME};
 
 /// Describes how to search for suitable caches when seeking in a [`ReadDiskStream`].
@@ -34,6 +35,15 @@ pub enum SeekMode {
     NoCache,
 }
 
+struct ReadDiskStreamOptions<D: Decoder> {
+    start_frame: usize,
+    num_cache_blocks: usize,
+    num_look_ahead_blocks: usize,
+    max_num_caches: usize,
+    block_size: usize,
+    file_info: FileInfo<D::FileParams>,
+}
+
 /// A realtime-safe disk-streaming reader of audio files.
 pub struct ReadDiskStream<D: Decoder> {
     to_server_tx: Producer<ClientToServerMsg<D>>,
@@ -65,6 +75,11 @@ impl<D: Decoder> ReadDiskStream<D> {
     /// * `file` - The path to the file to open.
     /// * `start_frame` - The frame in the file to start reading from.
     /// * `stream_opts` - Additional stream options.
+    ///
+    /// # Panics
+    ///
+    /// This will panic if `stream_opts.block_size`, `stream_opts.num_look_ahead_blocks`,
+    /// or `stream_opts.server_msg_channel_size` is `0`.
     pub fn new<P: Into<PathBuf>>(
         file: P,
         start_frame: usize,
@@ -91,27 +106,32 @@ impl<D: Decoder> ReadDiskStream<D> {
 
         let file: PathBuf = file.into();
 
-        match ReadServer::new(
-            file,
-            start_frame,
-            stream_opts.num_cache_blocks + stream_opts.num_look_ahead_blocks,
-            stream_opts.block_size,
+        match ReadServer::spawn(
+            ReadServerOptions {
+                file,
+                start_frame,
+                num_prefetch_blocks: stream_opts.num_cache_blocks
+                    + stream_opts.num_look_ahead_blocks,
+                block_size: stream_opts.block_size,
+                additional_opts: stream_opts.additional_opts,
+            },
             to_client_tx,
             from_client_rx,
             close_signal_rx,
-            stream_opts.additional_opts,
         ) {
             Ok(file_info) => {
                 let client = ReadDiskStream::create(
+                    ReadDiskStreamOptions {
+                        start_frame,
+                        num_cache_blocks: stream_opts.num_cache_blocks,
+                        num_look_ahead_blocks: stream_opts.num_look_ahead_blocks,
+                        max_num_caches: stream_opts.num_caches,
+                        block_size: stream_opts.block_size,
+                        file_info,
+                    },
                     to_server_tx,
                     from_server_rx,
                     close_signal_tx,
-                    start_frame,
-                    stream_opts.num_cache_blocks,
-                    stream_opts.num_look_ahead_blocks,
-                    stream_opts.num_caches,
-                    stream_opts.block_size,
-                    file_info,
                 );
 
                 Ok(client)
@@ -120,24 +140,18 @@ impl<D: Decoder> ReadDiskStream<D> {
         }
     }
 
-    #[allow(clippy::too_many_arguments)] // TODO: Reduce number of arguments
-    pub(crate) fn create(
+    fn create(
+        opts: ReadDiskStreamOptions<D>,
         to_server_tx: Producer<ClientToServerMsg<D>>,
         from_server_rx: Consumer<ServerToClientMsg<D>>,
         close_signal_tx: Producer<Option<HeapData<D::T>>>,
-        start_frame: usize,
-        num_cache_blocks: usize,
-        num_look_ahead_blocks: usize,
-        max_num_caches: usize,
-        block_size: usize,
-        file_info: FileInfo<D::FileParams>,
     ) -> Self {
-        let num_prefetch_blocks = num_cache_blocks + num_look_ahead_blocks;
+        let num_prefetch_blocks = opts.num_cache_blocks + opts.num_look_ahead_blocks;
 
-        let read_buffer = DataBlock::new(usize::from(file_info.num_channels), block_size);
+        let read_buffer = DataBlock::new(usize::from(opts.file_info.num_channels), opts.block_size);
 
         // Reserve the last two caches as temporary caches.
-        let max_num_caches = max_num_caches + 2;
+        let max_num_caches = opts.max_num_caches + 2;
 
         let mut caches: Vec<DataBlockCacheEntry<D::T>> = Vec::with_capacity(max_num_caches);
         for _ in 0..max_num_caches {
@@ -152,15 +166,15 @@ impl<D: Decoder> ReadDiskStream<D> {
 
         let mut prefetch_buffer: Vec<DataBlockEntry<D::T>> =
             Vec::with_capacity(num_prefetch_blocks);
-        let mut wanted_start_frame = start_frame;
+        let mut wanted_start_frame = opts.start_frame;
         for _ in 0..num_prefetch_blocks {
             prefetch_buffer.push(DataBlockEntry {
                 use_cache_index: None,
                 block: None,
                 wanted_start_frame,
             });
 
-            wanted_start_frame += block_size;
+            wanted_start_frame += opts.block_size;
         }
 
         let heap_data = Some(HeapData {
@@ -178,18 +192,18 @@ impl<D: Decoder> ReadDiskStream<D> {
 
             current_block_index: 0,
             next_block_index: 1,
-            current_block_start_frame: start_frame,
+            current_block_start_frame: opts.start_frame,
             current_frame_in_block: 0,
 
             temp_cache_index,
             temp_seek_cache_index,
 
             num_prefetch_blocks,
-            prefetch_size: num_prefetch_blocks * block_size,
-            cache_size: num_cache_blocks * block_size,
-            block_size,
+            prefetch_size: num_prefetch_blocks * opts.block_size,
+            cache_size: opts.num_cache_blocks * opts.block_size,
+            block_size: opts.block_size,
 
-            file_info,
+            file_info: opts.file_info,
             fatal_error: false,
         }
     }
@@ -219,8 +233,10 @@ impl<D: Decoder> ReadDiskStream<D> {
     /// relied on, then any blocks relying on the oldest cache will be silenced. In this case, (false)
     /// will be returned.
     pub fn can_move_cache(&mut self, cache_index: usize) -> bool {
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_ref().unwrap();
+        let Some(heap) = self.heap_data.as_ref() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return false;
+        };
 
         let mut using_cache = false;
         let mut using_temp_cache = false;
@@ -266,8 +282,10 @@ impl<D: Decoder> ReadDiskStream<D> {
             return Err(ReadError::FatalError(FatalReadError::StreamClosed));
         }
 
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_mut().unwrap();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Ok(false);
+        };
 
         if cache_index >= heap.caches.len() - 2 {
             return Err(ReadError::CacheIndexOutOfRange {
@@ -373,8 +391,10 @@ impl<D: Decoder> ReadDiskStream<D> {
             return Err(ReadError::IOServerChannelFull);
         }
 
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_mut().unwrap();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Ok(false);
+        };
 
         let mut found_cache = None;
 
@@ -509,8 +529,10 @@ impl<D: Decoder> ReadDiskStream<D> {
             return Ok(false);
         }
 
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_ref().unwrap();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Ok(false);
+        };
 
         // Check if the next two blocks are ready.
 
@@ -631,8 +653,10 @@ impl<D: Decoder> ReadDiskStream<D> {
 
         // Retrieve any data sent from the server.
 
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_mut().unwrap();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Ok(());
+        };
 
         loop {
             // Check that there is at-least one slot open before popping the next message.
@@ -765,8 +789,10 @@ impl<D: Decoder> ReadDiskStream<D> {
 
             // 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();
+                let Some(heap) = self.heap_data.as_mut() else {
+                    // This will never return here because `heap_data` can only be `None` in the destructor.
+                    return Err(ReadError::IOServerChannelFull);
+                };
 
                 heap.read_buffer.clear();
 
@@ -782,8 +808,10 @@ impl<D: Decoder> ReadDiskStream<D> {
 
             // Copy from second block
             {
-                // This check should never fail because it can only be `None` in the destructor.
-                let heap = self.heap_data.as_mut().unwrap();
+                let Some(heap) = self.heap_data.as_mut() else {
+                    // This will never return here because `heap_data` can only be `None` in the destructor.
+                    return Err(ReadError::IOServerChannelFull);
+                };
 
                 copy_block_into_read_buffer(heap, self.current_block_index, 0, second_len);
             }
@@ -792,8 +820,10 @@ impl<D: Decoder> ReadDiskStream<D> {
         } else {
             // Only need to copy from current block.
             {
-                // This check should never fail because it can only be `None` in the destructor.
-                let heap = self.heap_data.as_mut().unwrap();
+                let Some(heap) = self.heap_data.as_mut() else {
+                    // This will never return here because `heap_data` can only be `None` in the destructor.
+                    return Err(ReadError::IOServerChannelFull);
+                };
 
                 heap.read_buffer.clear();
 
@@ -812,8 +842,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();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Err(ReadError::IOServerChannelFull);
+        };
 
         // This check should never fail because it can only be `None` in the destructor.
         Ok(ReadData::new(
@@ -824,8 +856,10 @@ impl<D: Decoder> ReadDiskStream<D> {
     }
 
     fn advance_to_next_block(&mut self) -> Result<(), ReadError<D::FatalError>> {
-        // This check should never fail because it can only be `None` in the destructor.
-        let heap = self.heap_data.as_mut().unwrap();
+        let Some(heap) = self.heap_data.as_mut() else {
+            // This will never return here because `heap_data` can only be `None` in the destructor.
+            return Ok(());
+        };
 
         let entry = &mut heap.prefetch_buffer[self.current_block_index];