From 643f58bf1d1a1134e6ca5dddd88daf536f5ce2f8 Mon Sep 17 00:00:00 2001 From: Billy Messenger Date: Sat, 30 Dec 2023 17:41:40 -0600 Subject: [PATCH] Remove unsafe (#47) * remove unsafe, bump deps, fix demos --- CHANGELOG.md | 12 +++++ Cargo.toml | 8 ++-- core/Cargo.toml | 4 +- core/src/lib.rs | 1 + core/src/read/data.rs | 40 +++++++++-------- core/src/read/decoder.rs | 24 +++++----- core/src/read/read_stream.rs | 82 ++++++++++++++++++---------------- core/src/read/server.rs | 16 ++++--- core/src/write/data.rs | 28 +++++------- core/src/write/encoder.rs | 12 +---- core/src/write/server.rs | 8 ++-- core/src/write/write_stream.rs | 27 +++++------ decode_symphonia/Cargo.toml | 4 +- decode_symphonia/src/lib.rs | 25 +++++------ demos/player/Cargo.toml | 6 +-- demos/player/src/main.rs | 15 ++++++- demos/player/src/output.rs | 1 + demos/player/src/ui.rs | 78 +++++++++----------------------- demos/writer/Cargo.toml | 6 +-- demos/writer/src/main.rs | 15 ++++++- demos/writer/src/output.rs | 1 + demos/writer/src/ui.rs | 59 ++++++------------------ encode_wav/Cargo.toml | 4 +- encode_wav/src/lib.rs | 35 +++++++++------ src/lib.rs | 1 + 25 files changed, 234 insertions(+), 278 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd2ae09..82b7496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Version History +## Version 1.2.0 (2023-12-28) + +### Breaking changes: + +- The trait methods `Decoder::decode()` and `Encoder::encode()` are no longer marked unsafe. Refer to the new documentation on how these trait methods should be implemented. + +### Other changes: + +- Removed all unsafe code +- Bumped `rtrb` to version "0.3.0" +- Updated demos to use latest version of `egui` + ## Version 1.1.0 (2023-07-11) - Added the ability to decode MP4/AAC/ALAC files diff --git a/Cargo.toml b/Cargo.toml index 6c7885b..d29b0ca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "creek" -version = "1.1.1" +version = "1.2.0" authors = ["Billy Messenger "] edition = "2021" license = "MIT OR Apache-2.0" @@ -57,9 +57,9 @@ decode-all = [ encode-wav = ["creek-encode-wav"] [dependencies] -creek-core = { version = "0.1.1", path = "core" } -creek-decode-symphonia = { version = "0.2.1", path = "decode_symphonia", optional = true } -creek-encode-wav = { version = "0.1.1", path = "encode_wav", optional = true } +creek-core = { version = "0.2.0", path = "core" } +creek-decode-symphonia = { version = "0.3.0", 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. [profile.dev] diff --git a/core/Cargo.toml b/core/Cargo.toml index 0153707..3509fda 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "creek-core" -version = "0.1.1" +version = "0.2.0" authors = ["Billy Messenger "] edition = "2021" license = "MIT OR Apache-2.0" @@ -13,4 +13,4 @@ repository = "https://github.com/RustyDAW/creek" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -rtrb = "0.2" +rtrb = "0.3.0" diff --git a/core/src/lib.rs b/core/src/lib.rs index 3baabc7..1f5676c 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -3,6 +3,7 @@ // TODO: #![warn(clippy::missing_panics_doc)] #![warn(clippy::clone_on_ref_ptr)] #![deny(trivial_numeric_casts)] +#![forbid(unsafe_code)] use std::time; diff --git a/core/src/read/data.rs b/core/src/read/data.rs index 86009eb..f7310e7 100644 --- a/core/src/read/data.rs +++ b/core/src/read/data.rs @@ -4,22 +4,27 @@ pub struct DataBlock { } impl DataBlock { - /// # Safety - /// - /// Using an allocated but uninitialized [`Vec`] is safe because the block data - /// will be always filled before it is sent to be read by the client. pub fn new(num_channels: usize, block_size: usize) -> Self { - let mut block: Vec> = Vec::with_capacity(num_channels); - for _ in 0..num_channels { - let mut data: Vec = Vec::with_capacity(block_size); - #[allow(clippy::uninit_vec)] // TODO - unsafe { - data.set_len(block_size) - }; - block.push(data); + DataBlock { + block: (0..num_channels) + .map(|_| Vec::with_capacity(block_size)) + .collect(), + } + } + + pub fn clear(&mut self) { + for ch in self.block.iter_mut() { + ch.clear(); } + } - DataBlock { block } + 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()); + } + } } } @@ -29,12 +34,11 @@ pub(crate) struct DataBlockCache { impl DataBlockCache { pub(crate) fn new(num_channels: usize, num_prefetch_blocks: usize, block_size: usize) -> Self { - let mut blocks: Vec> = Vec::with_capacity(num_prefetch_blocks); - for _ in 0..num_prefetch_blocks { - blocks.push(DataBlock::new(num_channels, block_size)); + Self { + blocks: (0..num_prefetch_blocks) + .map(|_| DataBlock::new(num_channels, block_size)) + .collect(), } - - Self { blocks } } } diff --git a/core/src/read/decoder.rs b/core/src/read/decoder.rs index 6bb09ad..95b4ad6 100644 --- a/core/src/read/decoder.rs +++ b/core/src/read/decoder.rs @@ -52,22 +52,18 @@ pub trait Decoder: Sized + 'static { /// set the read position the end of the file instead of returning an error. fn seek(&mut self, frame: usize) -> Result<(), Self::FatalError>; - /// Decode data into the `data_block` starting from the read position. This is streaming, - /// meaning the next call to `decode()` should pick up where the previous left off. + /// Decode data into the `data_block` starting from your current internal read position. + /// This is streaming, meaning the next call to `decode()` should pick up where the + /// previous left off. /// - /// If the end of the file is reached, fill data up to the end of the file, then set the - /// read position to the last frame in the file and do nothing. + /// Fill each channel in the data block with `block_size` number of frames (you should + /// have gotten this value from `Decoder::new()`). If there isn't enough data left + /// because the end of the file has been reached, then only fill up how ever many frames + /// are left. If the end of the file has already been reached since the last call to + /// `decode()`, then do nothing. /// - /// # Safety - /// - /// This is marked as `unsafe` because a `data_block` may be uninitialized, causing - /// undefined behavior if data is not filled into the block. It is your responsibility to - /// always fill the block (unless the end of the file is reached, in which case the server - /// will tell the client to not read data past that frame). - unsafe fn decode( - &mut self, - data_block: &mut DataBlock, - ) -> Result<(), Self::FatalError>; + /// Each channel Vec in `data_block` will have a length of zero. + fn decode(&mut self, data_block: &mut DataBlock) -> Result<(), Self::FatalError>; /// Return the current read position. fn current_frame(&self) -> usize; diff --git a/core/src/read/read_stream.rs b/core/src/read/read_stream.rs index cfb9acd..13c9fc6 100644 --- a/core/src/read/read_stream.rs +++ b/core/src/read/read_stream.rs @@ -767,6 +767,8 @@ impl ReadDiskStream { // 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]; @@ -791,22 +793,21 @@ impl ReadDiskStream { } }; - for i in 0..heap.read_buffer.block.len() { - let read_buffer_part = &mut heap.read_buffer.block[i][0..first_len]; - - if let Some(block) = current_block_data { - let from_buffer_part = &block.block[i] - [self.current_frame_in_block..self.current_frame_in_block + first_len]; - - read_buffer_part.copy_from_slice(from_buffer_part); - } else { - // Output silence. - read_buffer_part[..first_len].fill_with(Default::default); - }; + 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()); + } } - - // Keep this from growing indefinitely. - //self.current_block_start_frame = current_block_start_frame; } self.advance_to_next_block()?; @@ -840,18 +841,17 @@ impl ReadDiskStream { } }; - for i in 0..heap.read_buffer.block.len() { - let read_buffer_part = - &mut heap.read_buffer.block[i][first_len..first_len + second_len]; - - if let Some(block) = next_block_data { - let from_buffer_part = &block.block[i][0..second_len]; - - read_buffer_part.copy_from_slice(from_buffer_part); - } else { - // Output silence. - read_buffer_part[..second_len].fill_with(Default::default); - }; + 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; @@ -862,6 +862,8 @@ impl ReadDiskStream { // 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]; @@ -886,18 +888,20 @@ impl ReadDiskStream { } }; - for i in 0..heap.read_buffer.block.len() { - let read_buffer_part = &mut heap.read_buffer.block[i][0..frames]; - - if let Some(block) = current_block_data { - let from_buffer_part = &block.block[i] - [self.current_frame_in_block..self.current_frame_in_block + frames]; - - read_buffer_part.copy_from_slice(from_buffer_part); - } else { - // Output silence. - read_buffer_part[..frames].fill_with(Default::default); - }; + 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()); + } } } diff --git a/core/src/read/server.rs b/core/src/read/server.rs index 9fda75b..0d4a2c9 100644 --- a/core/src/read/server.rs +++ b/core/src/read/server.rs @@ -109,9 +109,11 @@ impl ReadServer { ), ); - // Safe because we assume that the decoder will correctly fill the block - // with data. - let decode_res = unsafe { self.decoder.decode(&mut block) }; + block.clear(); + + let decode_res = self.decoder.decode(&mut block); + + block.ensure_correct_size(self.block_size); match decode_res { Ok(()) => { @@ -181,9 +183,11 @@ impl ReadServer { // Fill the cache for block in cache.blocks.iter_mut() { - // Safe because we assume that the decoder will correctly fill the block - // with data. - let decode_res = unsafe { self.decoder.decode(block) }; + block.clear(); + + 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)); diff --git a/core/src/write/data.rs b/core/src/write/data.rs index 29e548f..788ce49 100644 --- a/core/src/write/data.rs +++ b/core/src/write/data.rs @@ -2,29 +2,15 @@ pub struct WriteBlock { pub(crate) block: Vec>, - pub(crate) written_frames: usize, pub(crate) restart_count: usize, } impl WriteBlock { - /// # Safety - /// - /// Using an allocated but uninitialized [`Vec`] is safe because the block data - /// will be always filled before it is sent to be written by the IO server. pub fn new(num_channels: usize, block_size: usize) -> Self { - let mut block: Vec> = Vec::with_capacity(num_channels); - for _ in 0..num_channels { - let mut data: Vec = Vec::with_capacity(block_size); - #[allow(clippy::uninit_vec)] // TODO - unsafe { - data.set_len(block_size) - }; - block.push(data); - } - WriteBlock { - block, - written_frames: 0, + block: (0..num_channels) + .map(|_| Vec::with_capacity(block_size)) + .collect(), restart_count: 0, } } @@ -34,7 +20,13 @@ impl WriteBlock { } pub fn written_frames(&self) -> usize { - self.written_frames + self.block[0].len() + } + + pub fn clear(&mut self) { + for ch in self.block.iter_mut() { + ch.clear(); + } } } diff --git a/core/src/write/encoder.rs b/core/src/write/encoder.rs index 8072c43..d9797ef 100644 --- a/core/src/write/encoder.rs +++ b/core/src/write/encoder.rs @@ -64,9 +64,6 @@ pub trait Encoder: Sized + 'static { /// Write a block of data to the file. /// - /// The block may contain less written frames than the length of the channel - /// `Vec`s, so be sure to only read up to `block.written_frames()`. - /// /// If the write was successful, return `WriteStatus::Ok`. /// /// If the codec has a maximum file size (i.e. 4GB for WAV), then keep track of @@ -75,14 +72,7 @@ pub trait Encoder: Sized + 'static { /// the file name (i.e. "_001" for the first file, "_002" for the second, etc.) /// This helper function `num_files_to_file_name_extension()` can be used to find /// this extension. - /// - /// # Safety - /// - /// This is marked as `unsafe` because a `data_block` may be uninitialized, causing - /// undefined behavior if unwritten data from the block is read. Please use the value - /// from `write_block.num_frames()` to know how many frames in the block are valid. - /// (valid frames are from `[0..num_frames]`) - unsafe fn encode( + fn encode( &mut self, write_block: &WriteBlock, ) -> Result; diff --git a/core/src/write/server.rs b/core/src/write/server.rs index b1a725b..5698376 100644 --- a/core/src/write/server.rs +++ b/core/src/write/server.rs @@ -89,12 +89,10 @@ impl WriteServer { // Don't use this block if it is from a previous discarded stream. if block.restart_count != self.restart_count { // Clear and send block to be re-used by client. - block.written_frames = 0; + block.clear(); self.send_msg(ServerToClientMsg::NewWriteBlock { block }); } else { - // Safe because we assume that the encoder will not try to use any - // unwritten data. - let write_res = unsafe { self.encoder.encode(&block) }; + let write_res = self.encoder.encode(&block); match write_res { Ok(status) => { @@ -105,7 +103,7 @@ impl WriteServer { } // Clear and send block to be re-used by client. - block.written_frames = 0; + block.clear(); self.send_msg(ServerToClientMsg::NewWriteBlock { block }); } Err(e) => { diff --git a/core/src/write/write_stream.rs b/core/src/write/write_stream.rs index 896ca8d..1e989e2 100644 --- a/core/src/write/write_stream.rs +++ b/core/src/write/write_stream.rs @@ -218,18 +218,17 @@ impl WriteDiskStream { // Check that there are available blocks to write to. if let Some(mut current_block) = heap.current_block.take() { if let Some(mut next_block) = heap.next_block.take() { - if current_block.written_frames + buffer_len > self.block_size { + let current_block_written_frames = current_block.block[0].len(); + + if current_block_written_frames + buffer_len > self.block_size { // Need to copy to two blocks. - let first_len = self.block_size - current_block.written_frames; - let second_len = buffer_len - first_len; + let first_len = self.block_size - current_block_written_frames; // Copy into first block. for (buffer_ch, write_ch) in buffer.iter().zip(current_block.block.iter_mut()) { - write_ch[current_block.written_frames..] - .copy_from_slice(&buffer_ch[0..first_len]); + write_ch.extend_from_slice(&buffer_ch[0..first_len]); } - current_block.written_frames = self.block_size; // Send the now filled block to the IO server for writing. // This cannot fail because we made sure there was a slot open in @@ -241,9 +240,8 @@ impl WriteDiskStream { // Copy the remaining data into the second block. for (buffer_ch, write_ch) in buffer.iter().zip(next_block.block.iter_mut()) { - write_ch[0..second_len].copy_from_slice(&buffer_ch[first_len..]); + write_ch.extend_from_slice(&buffer_ch[first_len..]); } - next_block.written_frames = second_len; // Move the next-up block into the current block. heap.current_block = Some(next_block); @@ -253,14 +251,13 @@ impl WriteDiskStream { } else { // Only need to copy to first block. - let end = current_block.written_frames + buffer_len; - for (buffer_ch, write_ch) in buffer.iter().zip(current_block.block.iter_mut()) { - write_ch[current_block.written_frames..end].copy_from_slice(buffer_ch); + write_ch.extend_from_slice(buffer_ch); } - current_block.written_frames = end; - if current_block.written_frames == self.block_size { + let current_block_written_frames = current_block.block[0].len(); + + if current_block_written_frames == self.block_size { // Block is filled. Sent it to the IO server for writing. // This cannot fail because we made sure there was a slot open in // a previous step. @@ -315,7 +312,7 @@ impl WriteDiskStream { let heap = self.heap_data.as_mut().unwrap(); if let Some(mut current_block) = heap.current_block.take() { - if current_block.written_frames > 0 { + if !current_block.block[0].is_empty() { // Send the last bit of remaining samples to be encoded. // Check that there is at-least one slot open. @@ -400,7 +397,7 @@ impl WriteDiskStream { let heap = self.heap_data.as_mut().unwrap(); if let Some(block) = &mut heap.current_block { - block.written_frames = 0; + block.clear(); } self.restart_count += 1; diff --git a/decode_symphonia/Cargo.toml b/decode_symphonia/Cargo.toml index 0a848c1..df2a2d5 100644 --- a/decode_symphonia/Cargo.toml +++ b/decode_symphonia/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "creek-decode-symphonia" -version = "0.2.1" +version = "0.3.0" authors = ["Billy Messenger "] 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.1.1", path = "../core" } +creek-core = { version = "0.2.0", path = "../core" } log = "0.4" symphonia = "0.5" diff --git a/decode_symphonia/src/lib.rs b/decode_symphonia/src/lib.rs index 799a206..540c4c0 100644 --- a/decode_symphonia/src/lib.rs +++ b/decode_symphonia/src/lib.rs @@ -3,6 +3,7 @@ #![warn(clippy::missing_panics_doc)] #![warn(clippy::clone_on_ref_ptr)] #![deny(trivial_numeric_casts)] +#![forbid(unsafe_code)] use std::fs::File; use std::path::PathBuf; @@ -230,10 +231,7 @@ impl Decoder for SymphoniaDecoder { Ok(()) } - unsafe fn decode( - &mut self, - data_block: &mut DataBlock, - ) -> Result<(), Self::FatalError> { + fn decode(&mut self, data_block: &mut DataBlock) -> Result<(), Self::FatalError> { if self.playhead_frame >= self.num_frames { // Do nothing if reached the end of the file. return Ok(()); @@ -260,8 +258,7 @@ impl Decoder for SymphoniaDecoder { for (dst_ch, src_ch) in data_block.block.iter_mut().zip(src_channels) { let src_ch_part = &src_ch[self.curr_decode_buffer_frame ..self.curr_decode_buffer_frame + num_frames_to_cpy]; - dst_ch[block_start_frame..block_start_frame + num_frames_to_cpy] - .copy_from_slice(src_ch_part); + dst_ch.extend_from_slice(src_ch_part); } block_start_frame += num_frames_to_cpy; @@ -413,9 +410,8 @@ mod tests { let (mut decoder, file_info) = decoder.unwrap(); let mut data_block = DataBlock::new(1, block_size); - unsafe { - decoder.decode(&mut data_block).unwrap(); - } + data_block.clear(); + decoder.decode(&mut data_block).unwrap(); let samples = &mut data_block.block[0]; assert_eq!(samples.len(), block_size); @@ -434,9 +430,8 @@ mod tests { 0.71875, 0.7421875, ]; - unsafe { - decoder.decode(&mut data_block).unwrap(); - } + data_block.clear(); + decoder.decode(&mut data_block).unwrap(); let samples = &mut data_block.block[0]; for i in 0..samples.len() { @@ -451,9 +446,9 @@ mod tests { // Seek to last frame decoder.seek(file_info.num_frames - 1 - block_size).unwrap(); - unsafe { - decoder.decode(&mut data_block).unwrap(); - } + data_block.clear(); + decoder.decode(&mut data_block).unwrap(); + let samples = &mut data_block.block[0]; for i in 0..samples.len() { assert_approx_eq!(f32, last_frame[i], samples[i], ulps = 2); diff --git a/demos/player/Cargo.toml b/demos/player/Cargo.toml index 943fde5..3e32d91 100644 --- a/demos/player/Cargo.toml +++ b/demos/player/Cargo.toml @@ -9,6 +9,6 @@ publish = false [dependencies] creek = { path = "../../" } -eframe = "0.9.0" -rtrb = "0.2" -cpal = "0.13" +eframe = "0.24.1" +rtrb = "0.3" +cpal = "0.15" diff --git a/demos/player/src/main.rs b/demos/player/src/main.rs index bc71216..6605b0f 100644 --- a/demos/player/src/main.rs +++ b/demos/player/src/main.rs @@ -30,8 +30,19 @@ fn main() { let (to_gui_tx, from_process_rx) = RingBuffer::::new(256); let (to_process_tx, from_gui_rx) = RingBuffer::::new(64); - let app = ui::DemoPlayerApp::new(to_process_tx, from_process_rx, file_path); let _cpal_stream = output::Output::new(to_gui_tx, from_gui_rx); - eframe::run_native(Box::new(app)); + eframe::run_native( + "creek demo player", + eframe::NativeOptions::default(), + Box::new(|cc| { + Box::new(ui::DemoPlayerApp::new( + to_process_tx, + from_process_rx, + file_path, + cc, + )) + }), + ) + .unwrap(); } diff --git a/demos/player/src/output.rs b/demos/player/src/output.rs index 88b3b66..7c58a5c 100644 --- a/demos/player/src/output.rs +++ b/demos/player/src/output.rs @@ -37,6 +37,7 @@ impl Output { move |err| { eprintln!("{}", err); }, + None, ) .unwrap(); diff --git a/demos/player/src/ui.rs b/demos/player/src/ui.rs index 84b80b2..03ca9ca 100644 --- a/demos/player/src/ui.rs +++ b/demos/player/src/ui.rs @@ -1,6 +1,6 @@ use creek::{Decoder, ReadDiskStream, ReadStreamOptions, SymphoniaDecoder}; -use eframe::{egui, epi}; -use rtrb::{Consumer, Producer, RingBuffer}; +use eframe::egui; +use rtrb::{Consumer, Producer}; use crate::{GuiToProcessMsg, ProcessToGuiMsg}; @@ -17,9 +17,6 @@ pub struct DemoPlayerApp { to_player_tx: Producer, from_player_rx: Consumer, - frame_close_tx: Producer<()>, - frame_close_rx: Option>, - loop_start: usize, loop_end: usize, @@ -33,6 +30,7 @@ impl DemoPlayerApp { mut to_player_tx: Producer, from_player_rx: Consumer, file_path: std::path::PathBuf, + _cc: &eframe::CreationContext<'_>, ) -> Self { // Setup read stream ------------------------------------------------------------- @@ -87,17 +85,12 @@ impl DemoPlayerApp { }) .unwrap(); - let (frame_close_tx, frame_close_rx) = RingBuffer::new(1); - Self { playing: false, current_frame: 0, num_frames, transport_control: Default::default(), - frame_close_tx, - frame_close_rx: Some(frame_close_rx), - to_player_tx, from_player_rx, @@ -111,31 +104,8 @@ impl DemoPlayerApp { } } -impl epi::App for DemoPlayerApp { - fn name(&self) -> &str { - "rt-audio-disk-stream demo player" - } - - fn update(&mut self, ctx: &egui::CtxRef, frame: &mut epi::Frame<'_>) { - if let Some(mut frame_close_rx) = self.frame_close_rx.take() { - // Spawn thread that calls a repaint 60 times a second. - - let repaint_signal = frame.repaint_signal().clone(); - - std::thread::spawn(move || { - loop { - std::thread::sleep(std::time::Duration::from_secs_f64(1.0 / 60.0)); - - // Check if app has closed. - if frame_close_rx.pop().is_ok() { - break; - } - - repaint_signal.request_repaint(); - } - }); - } - +impl eframe::App for DemoPlayerApp { + fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { while let Ok(msg) = self.from_player_rx.pop() { match msg { ProcessToGuiMsg::PlaybackPos(pos) => { @@ -147,6 +117,10 @@ impl epi::App for DemoPlayerApp { } } + // In a real app we should only repaint if something has changed, but since this is + // just a demo this is fine. + ctx.request_repaint(); + egui::CentralPanel::default().show(ctx, |ui| { egui::warn_if_debug_build(ui); @@ -177,10 +151,10 @@ impl epi::App for DemoPlayerApp { let mut loop_start = self.loop_start; let mut loop_end = self.loop_end; - ui.add(egui::Slider::usize(&mut loop_start, 0..=self.num_frames).text("loop start")); - ui.add(egui::Slider::usize(&mut loop_end, 0..=self.num_frames).text("loop end")); + ui.add(egui::Slider::new(&mut loop_start, 0..=self.num_frames).text("loop start")); + ui.add(egui::Slider::new(&mut loop_end, 0..=self.num_frames).text("loop end")); if (loop_start != self.loop_start || loop_end != self.loop_end) - && (ui.input().pointer.any_released() || ui.input().key_pressed(egui::Key::Enter)) + && ui.input(|i| i.pointer.any_released() || i.key_pressed(egui::Key::Enter)) { if loop_end <= loop_start { if loop_start == self.num_frames { @@ -203,16 +177,10 @@ impl epi::App for DemoPlayerApp { } if self.buffering_anim > 0 { - ui.label( - egui::Label::new("Status: Buffered") - .text_color(egui::Color32::from_rgb(255, 0, 0)), - ); + ui.colored_label(egui::Color32::from_rgb(255, 0, 0), "Status: Buffered"); self.buffering_anim -= 1; } else { - ui.label( - egui::Label::new("Status: Ready") - .text_color(egui::Color32::from_rgb(0, 255, 0)), - ); + ui.colored_label(egui::Color32::from_rgb(0, 255, 0), "Status: Ready"); } let (_, user_seeked) = self.transport_control.ui( @@ -233,12 +201,6 @@ impl epi::App for DemoPlayerApp { } } -impl Drop for DemoPlayerApp { - fn drop(&mut self) { - self.frame_close_tx.push(()).unwrap(); - } -} - struct TransportControl { rail_stroke: egui::Stroke, handle_stroke: egui::Stroke, @@ -254,7 +216,7 @@ impl Default for TransportControl { rail_stroke: egui::Stroke::new(1.0, egui::Color32::GRAY), handle_stroke: egui::Stroke::new(1.0, egui::Color32::WHITE), loop_stroke: egui::Stroke::new(1.0, egui::Color32::from_rgb(0, 255, 0)), - cache_stroke: egui::Stroke::new(1.0, egui::Color32::from_rgb(0, 0, 255)), + cache_stroke: egui::Stroke::new(1.0, egui::Color32::from_rgb(0, 255, 255)), seeking: false, } } @@ -273,7 +235,7 @@ impl TransportControl { cache_size: usize, ) -> (egui::Response, bool) { let (response, painter) = - ui.allocate_painter(ui.available_size_before_wrap_finite(), egui::Sense::drag()); + ui.allocate_painter(ui.available_size_before_wrap(), egui::Sense::drag()); let rect = response.rect; let mut shapes = vec![]; @@ -304,13 +266,13 @@ impl TransportControl { self.loop_stroke, )); - if let Some(press_origin) = ui.input().pointer.press_origin() { + if let Some(press_origin) = ui.input(|i| i.pointer.press_origin()) { if press_origin.x >= start_x && press_origin.x <= end_x && press_origin.y >= rail_y - 10.0 && press_origin.y <= rail_y + 10.0 { - if let Some(mouse_pos) = ui.input().pointer.interact_pos() { + if let Some(mouse_pos) = ui.input(|i| i.pointer.interact_pos()) { let handle_x = mouse_pos.x - start_x; *value = (((handle_x / rail_width) * max_value as f32).round() as isize) .max(0) @@ -322,8 +284,8 @@ impl TransportControl { } let mut changed: bool = false; - if ui.input().pointer.any_released() && self.seeking { - if let Some(mouse_pos) = ui.input().pointer.interact_pos() { + if ui.input(|i| i.pointer.any_released()) && self.seeking { + if let Some(mouse_pos) = ui.input(|i| i.pointer.interact_pos()) { let handle_x = mouse_pos.x - start_x; *value = (((handle_x / rail_width) * max_value as f32).round() as isize) .max(0) diff --git a/demos/writer/Cargo.toml b/demos/writer/Cargo.toml index b2f495a..63029bd 100644 --- a/demos/writer/Cargo.toml +++ b/demos/writer/Cargo.toml @@ -9,6 +9,6 @@ publish = false [dependencies] creek = { path = "../../" } -eframe = "0.9.0" -rtrb = "0.2" -cpal = "0.13" +eframe = "0.24.1" +rtrb = "0.3" +cpal = "0.15" diff --git a/demos/writer/src/main.rs b/demos/writer/src/main.rs index 75adf17..d08127a 100644 --- a/demos/writer/src/main.rs +++ b/demos/writer/src/main.rs @@ -28,7 +28,18 @@ fn main() { let (to_process_tx, from_gui_rx) = RingBuffer::::new(64); let (_cpal_stream, sample_rate) = output::Output::new(to_gui_tx, from_gui_rx); - let app = ui::DemoWriterApp::new(to_process_tx, from_process_rx, sample_rate); - eframe::run_native(Box::new(app)); + eframe::run_native( + "creek demo writer", + eframe::NativeOptions::default(), + Box::new(move |cc| { + Box::new(ui::DemoWriterApp::new( + to_process_tx, + from_process_rx, + sample_rate, + cc, + )) + }), + ) + .unwrap(); } diff --git a/demos/writer/src/output.rs b/demos/writer/src/output.rs index 8ce6b97..9e66df6 100644 --- a/demos/writer/src/output.rs +++ b/demos/writer/src/output.rs @@ -37,6 +37,7 @@ impl Output { move |err| { eprintln!("{}", err); }, + None, ) .unwrap(); diff --git a/demos/writer/src/ui.rs b/demos/writer/src/ui.rs index abae0be..102d039 100644 --- a/demos/writer/src/ui.rs +++ b/demos/writer/src/ui.rs @@ -1,6 +1,6 @@ use creek::{wav_bit_depth, WavEncoder, WriteDiskStream}; -use eframe::{egui, epi}; -use rtrb::{Consumer, Producer, RingBuffer}; +use eframe::egui; +use rtrb::{Consumer, Producer}; use crate::{GuiToProcessMsg, ProcessToGuiMsg}; @@ -22,9 +22,6 @@ pub struct DemoWriterApp { to_player_tx: Producer, from_player_rx: Consumer, - frame_close_tx: Producer<()>, - frame_close_rx: Option>, - freq: f32, sample_rate: u32, @@ -36,9 +33,8 @@ impl DemoWriterApp { to_player_tx: Producer, from_player_rx: Consumer, sample_rate: u32, + _cc: &eframe::CreationContext<'_>, ) -> Self { - let (frame_close_tx, frame_close_rx) = RingBuffer::new(1); - Self { file_active: false, bit_rate: BitRate::Int24, @@ -46,8 +42,6 @@ impl DemoWriterApp { written_frames: 0, to_player_tx, from_player_rx, - frame_close_tx, - frame_close_rx: Some(frame_close_rx), freq: 261.626, sample_rate, fatal_error: false, @@ -55,31 +49,8 @@ impl DemoWriterApp { } } -impl epi::App for DemoWriterApp { - fn name(&self) -> &str { - "rt-audio-disk-stream demo writer" - } - - fn update(&mut self, ctx: &egui::CtxRef, frame: &mut epi::Frame<'_>) { - if let Some(mut frame_close_rx) = self.frame_close_rx.take() { - // Spawn thread that calls a repaint 60 times a second. - - let repaint_signal = frame.repaint_signal().clone(); - - std::thread::spawn(move || { - loop { - std::thread::sleep(std::time::Duration::from_secs_f64(1.0 / 60.0)); - - // Check if app has closed. - if frame_close_rx.pop().is_ok() { - break; - } - - repaint_signal.request_repaint(); - } - }); - } - +impl eframe::App for DemoWriterApp { + fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { while let Ok(msg) = self.from_player_rx.pop() { match msg { ProcessToGuiMsg::FramesWritten(frames_written) => { @@ -93,6 +64,10 @@ impl epi::App for DemoWriterApp { } } + // In a real app we should only repaint if something has changed, but since this is + // just a demo this is fine. + ctx.request_repaint(); + egui::CentralPanel::default().show(ctx, |ui| { egui::warn_if_debug_build(ui); @@ -133,7 +108,7 @@ impl epi::App for DemoWriterApp { if ui .add( - egui::Slider::f32(&mut self.freq, 30.0..=20_000.0) + egui::Slider::new(&mut self.freq, 30.0..=20_000.0) .logarithmic(true) .text("pitch"), ) @@ -186,7 +161,7 @@ impl epi::App for DemoWriterApp { BitRate::Uint8 => { let write_stream = WriteDiskStream::>::new( - "./examples/demo_writer/out_files/output.wav", + "./target/output_u8.wav", 2, self.sample_rate, Default::default(), @@ -200,7 +175,7 @@ impl epi::App for DemoWriterApp { BitRate::Int16 => { let write_stream = WriteDiskStream::>::new( - "./examples/demo_writer/out_files/output.wav", + "./target/output_i16.wav", 2, self.sample_rate, Default::default(), @@ -214,7 +189,7 @@ impl epi::App for DemoWriterApp { BitRate::Int24 => { let write_stream = WriteDiskStream::>::new( - "./examples/demo_writer/out_files/output.wav", + "./target/output_i24.wav", 2, self.sample_rate, Default::default(), @@ -228,7 +203,7 @@ impl epi::App for DemoWriterApp { BitRate::Float32 => { let write_stream = WriteDiskStream::>::new( - "./examples/demo_writer/out_files/output.wav", + "./target/output_f32.wav", 2, self.sample_rate, Default::default(), @@ -247,9 +222,3 @@ impl epi::App for DemoWriterApp { }); } } - -impl Drop for DemoWriterApp { - fn drop(&mut self) { - self.frame_close_tx.push(()).unwrap(); - } -} diff --git a/encode_wav/Cargo.toml b/encode_wav/Cargo.toml index d8b7dc2..53226ee 100644 --- a/encode_wav/Cargo.toml +++ b/encode_wav/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "creek-encode-wav" -version = "0.1.1" +version = "0.2.0" authors = ["Billy Messenger "] edition = "2021" license = "MIT OR Apache-2.0" @@ -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.1.1", path = "../core" } +creek-core = { version = "0.2.0", path = "../core" } byte-slice-cast = "1.0.0" diff --git a/encode_wav/src/lib.rs b/encode_wav/src/lib.rs index 6abf692..9fda7b3 100644 --- a/encode_wav/src/lib.rs +++ b/encode_wav/src/lib.rs @@ -3,6 +3,7 @@ #![warn(clippy::missing_panics_doc)] #![warn(clippy::clone_on_ref_ptr)] #![deny(trivial_numeric_casts)] +#![forbid(unsafe_code)] use std::path::PathBuf; use std::{ @@ -120,13 +121,7 @@ impl Encoder for WavEncoder { file.write_all(header.buffer())?; file.flush()?; - let buf_len = usize::from(num_channels) * block_size; - let mut interleave_buf: Vec = Vec::with_capacity(buf_len); - // Safe because data will always be written to before it is read. - #[allow(clippy::uninit_vec)] // TODO - unsafe { - interleave_buf.set_len(buf_len); - } + let interleave_buf: Vec = Vec::with_capacity(block_size * usize::from(num_channels)); let max_file_bytes = u64::from(header.max_data_bytes()); let bytes_per_frame = u64::from(num_channels) * u64::from(format.bytes_per_sample()); @@ -154,15 +149,18 @@ impl Encoder for WavEncoder { )) } - unsafe fn encode( + fn encode( &mut self, write_block: &WriteBlock, ) -> Result { let mut status = WriteStatus::Ok; - if let Some(mut file) = self.file.take() { - let written_frames = write_block.written_frames(); + let written_frames = write_block.written_frames(); + if written_frames == 0 { + return Ok(status); + } + if let Some(mut file) = self.file.take() { if self.num_channels == 1 { self.bit_depth .write_to_disk(&write_block.block()[0][0..written_frames], &mut file)?; @@ -171,6 +169,12 @@ impl Encoder for WavEncoder { // Provide efficient stereo interleaving. let ch1 = &write_block.block()[0][0..written_frames]; let ch2 = &write_block.block()[1][0..written_frames]; + + if self.interleave_buf.len() < written_frames * 2 { + self.interleave_buf + .resize(written_frames * 2, Default::default()); + } + let interleave_buf_part = &mut self.interleave_buf[0..written_frames * 2]; for (i, frame) in interleave_buf_part.chunks_exact_mut(2).enumerate() { @@ -178,6 +182,11 @@ impl Encoder for WavEncoder { frame[1] = ch2[i]; } } else { + if self.interleave_buf.len() < written_frames * self.num_channels { + self.interleave_buf + .resize(written_frames * self.num_channels, Default::default()); + } + let interleave_buf_part = &mut self.interleave_buf[0..written_frames * self.num_channels]; @@ -246,11 +255,9 @@ impl Encoder for WavEncoder { status = WriteStatus::ReachedMaxSize { num_files: self.num_files, }; - - self.file = Some(file); - } else { - self.file = Some(file); } + + self.file = Some(file); } Ok(status) diff --git a/src/lib.rs b/src/lib.rs index 5987779..3d05354 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,7 @@ #![warn(clippy::missing_panics_doc)] #![warn(clippy::clone_on_ref_ptr)] #![deny(trivial_numeric_casts)] +#![forbid(unsafe_code)] pub use creek_core::*;