From 02fd7ae7a42613b1556fcac76ae1dba5d248c812 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:26:29 -0400 Subject: [PATCH 01/10] fix: reader logic --- src/error/mod.rs | 12 ++++----- src/file/builder/event/mod.rs | 3 --- src/file/builder/mod.rs | 2 +- src/message/channel/voice.rs | 2 +- src/reader/error.rs | 18 +++++-------- src/reader/mod.rs | 49 +++++++++++++++++++++++++---------- tests/read_all.rs | 23 ++++++++-------- tests/smpte_offset.rs | 2 +- 8 files changed, 64 insertions(+), 47 deletions(-) diff --git a/src/error/mod.rs b/src/error/mod.rs index 203d7d4..e56739c 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -24,7 +24,7 @@ use thiserror::Error; // } /// All the ways parsing can go wrong -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum ParseError { /// Invalid databyte (leading 1) #[error("Invalid Data Byte: {0:0X}")] @@ -81,7 +81,7 @@ impl ParseError { } } /// Problems reading a file's header -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum HeaderError { /// Type 0 MIDI Format (SingleMultiChannel) defines multiple tracks #[error("Type 0 MIDI Format (SingleMultiChannel) defined multiple tracks")] @@ -107,7 +107,7 @@ impl From for ParseError { } /// Problems reading a file's metamessages -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum MetaMessageError { /// contains varlen inner, should be 1. #[error("varlen for channel count was {0}. Expected 1.")] @@ -129,7 +129,7 @@ impl From for ParseError { } /// problems reading a track -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum TrackError { /// Invalid event #[error("Invalid event found: {0:0X}")] @@ -143,7 +143,7 @@ impl From for ParseError { } /// Problems reading a chunk -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum ChunkError { /// Finding more than one header for a chunk #[error("Found more than one header for this chunk.")] @@ -157,7 +157,7 @@ pub enum ChunkError { } /// Problems with the file after reading it through -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum FileError { /// No format was found #[error("The file's format couldn't be determined")] diff --git a/src/file/builder/event/mod.rs b/src/file/builder/event/mod.rs index f3e026d..7bcc693 100644 --- a/src/file/builder/event/mod.rs +++ b/src/file/builder/event/mod.rs @@ -71,9 +71,6 @@ pub enum FileEvent<'a> { /// /// See [`TrackEvent`] for a detailed breakdown TrackEvent(TrackEvent<'a>), - - /// Yielded when no more bytes can be read - Eof, } impl From for FileEvent<'_> { diff --git a/src/file/builder/mod.rs b/src/file/builder/mod.rs index d0f7b19..8bda9f9 100644 --- a/src/file/builder/mod.rs +++ b/src/file/builder/mod.rs @@ -117,7 +117,7 @@ impl<'a> MidiFileBuilder<'a> { self.unknown_chunks.push(data); Ok(()) } - Eof => Err(ReaderErrorKind::OutOfBounds), + Eof => Err(ReaderErrorKind::Eof), } } /// Attempts to finish the midifile from the provided chunks. diff --git a/src/message/channel/voice.rs b/src/message/channel/voice.rs index e2c6f8e..1ada893 100644 --- a/src/message/channel/voice.rs +++ b/src/message/channel/voice.rs @@ -195,7 +195,7 @@ impl FromLiveEventBytes for ChannelVoiceMessage { let mut temp = Reader::from_byte_slice(data); let c = Controller::read(&mut temp).map_err(|e| match e.kind { ReaderErrorKind::ParseError(p) => p, - ReaderErrorKind::OutOfBounds => ParseError::MissingData, + ReaderErrorKind::OutOfBounds | ReaderErrorKind::Eof => ParseError::MissingData, })?; VoiceEvent::ControlChange(c) } diff --git a/src/reader/error.rs b/src/reader/error.rs index 6b90f55..ccf9c4b 100644 --- a/src/reader/error.rs +++ b/src/reader/error.rs @@ -13,14 +13,17 @@ pub struct ReaderError { } /// A kind of error that a reader can produce -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq, Eq)] pub enum ReaderErrorKind { /// Parsing errors #[error("Parsing {0}")] ParseError(#[from] ParseError), - /// Reading out of bounds. + /// Reading out of bounds. If the read is passed the length of the file, then [`ReaderErrorKind::Eof`] is returned. #[error("Read out of bounds!")] OutOfBounds, + /// The end of the file has been reached. + #[error("End of file read!")] + Eof, } impl ReaderErrorKind { @@ -34,10 +37,11 @@ impl ReaderError { pub const fn new(position: usize, kind: ReaderErrorKind) -> Self { Self { position, kind } } - /// True if out of bounds or unexpected end of file + /// True if out of bounds pub const fn is_out_of_bounds(&self) -> bool { matches!(self.kind, ReaderErrorKind::OutOfBounds) } + /// Returns the error kind of the reader. pub fn error_kind(&self) -> &ReaderErrorKind { &self.kind @@ -54,14 +58,6 @@ impl ReaderError { kind: ReaderErrorKind::ParseError(error), } } - - /// Create a new out of bounds error - pub const fn oob(position: usize) -> Self { - Self { - position, - kind: ReaderErrorKind::OutOfBounds, - } - } } /// The Read Result type (see [`ReaderError`]) diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 1bf2fad..3665679 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -153,20 +153,28 @@ impl<'slc> Reader> { //internal implementations impl<'slc, R: MidiSource<'slc>> Reader { - // Returns None if there's no bytes left to read + /// Reads the exact amount of bytes given the length. + /// + /// Errors with OutOfBounds if the result is beyond the total length of the source. pub(super) fn read_exact<'slf>(&'slf mut self, bytes: usize) -> ReadResult> where 'slc: 'slf, { if self.buffer_position() > self.reader.max_len() { - return Err(ReaderError::oob(self.buffer_position())); + return Err(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::OutOfBounds, + )); } let start = self.buffer_position(); let end = start + bytes; if end > self.reader.max_len() { - return Err(ReaderError::oob(self.buffer_position())); + return Err(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::OutOfBounds, + )); } self.state.increment_offset(bytes); @@ -185,7 +193,10 @@ impl<'slc, R: MidiSource<'slc>> Reader { let me = unsafe { &*ptr }; Ok(*me) } else { - Err(ReaderError::oob(self.buffer_position())) + Err(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::OutOfBounds, + )) } } @@ -196,7 +207,10 @@ impl<'slc, R: MidiSource<'slc>> Reader { let res = self .reader .get_byte(self.buffer_position()) - .ok_or(ReaderError::oob(self.buffer_position()))?; + .ok_or(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::OutOfBounds, + ))?; self.state.increment_offset(1); Ok(res) @@ -209,7 +223,10 @@ impl<'slc, R: MidiSource<'slc>> Reader { let res = self .reader .get_byte(self.buffer_position()) - .ok_or(ReaderError::oob(self.buffer_position()))?; + .ok_or(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::OutOfBounds, + ))?; self.state.increment_offset(1); DataByte::new(res).map_err(|e| ReaderError::parse_error(self.buffer_position(), e)) } @@ -274,14 +291,15 @@ impl<'slc, R: MidiSource<'slc>> Reader { ParseState::InsideMidi => { // expect only a header or track chunk let chunk = match self.read_exact(4) { - Ok(c) => c, - Err(e) => { - if e.is_out_of_bounds() { - return Ok(FileEvent::Eof); - } else { + Ok(chunk) => chunk, + Err(e) => match e.error_kind() { + ReaderErrorKind::OutOfBounds => { + return Err(ReaderError::new(e.position(), ReaderErrorKind::Eof)); + } + _ => { return Err(e); } - } + }, }; match chunk.as_ref() { @@ -322,7 +340,12 @@ impl<'slc, R: MidiSource<'slc>> Reader { let ev = TrackEvent::read(self, &mut running_status)?; break FileEvent::TrackEvent(ev); } - ParseState::Done => break FileEvent::Eof, + ParseState::Done => { + return Err(ReaderError::new( + self.buffer_position(), + ReaderErrorKind::Eof, + )); + } } }; diff --git a/tests/read_all.rs b/tests/read_all.rs index fe7a272..c259b50 100644 --- a/tests/read_all.rs +++ b/tests/read_all.rs @@ -1,16 +1,13 @@ -use midix::{file::builder::event::FileEvent, reader::Reader}; +use midix::reader::{Reader, ReaderErrorKind}; fn loop_through(bytes: &[u8]) { let mut reader = Reader::from_byte_slice(bytes); loop { - match reader.read_event() { - Ok(e) => { - if e == FileEvent::Eof { - break; - } - } - Err(e) => { + if let Err(e) = reader.read_event() { + if matches!(e.error_kind(), ReaderErrorKind::Eof) { + break; + } else { panic!("Error at {}, {:?}", reader.buffer_position(), e); } } @@ -47,9 +44,13 @@ fn read_pi_damaged() { let bytes = include_bytes!("../test-asset/PiDamaged.mid"); let mut reader = Reader::from_byte_slice(bytes); - while let Ok(e) = reader.read_event() { - if e == FileEvent::Eof { - panic!("Corrupted file should not have yielded an eof event") + loop { + if let Err(e) = reader.read_event() { + if matches!(e.error_kind(), ReaderErrorKind::Eof) { + panic!("Corrupted file should not have yielded an eof event") + } else { + return; + } } } } diff --git a/tests/smpte_offset.rs b/tests/smpte_offset.rs index 4684abe..3f18536 100644 --- a/tests/smpte_offset.rs +++ b/tests/smpte_offset.rs @@ -355,7 +355,7 @@ fn test_multiple_tracks_with_different_offsets() { let mut offsets = Vec::new(); while let Ok(event) = reader.read_event() { - println!("ok"); + println!("ok: {event:?}"); if let FileEvent::TrackEvent(track_event) = event && let TrackMessage::Meta(MetaMessage::SmpteOffset(offset)) = track_event.event() { From c850aee01bc1aa7e5ba11fd2afb88960b891b122 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:31:10 -0400 Subject: [PATCH 02/10] fix: overflow error --- src/file/meta/smpte_offset.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/file/meta/smpte_offset.rs b/src/file/meta/smpte_offset.rs index ec8229d..d22082a 100644 --- a/src/file/meta/smpte_offset.rs +++ b/src/file/meta/smpte_offset.rs @@ -90,9 +90,9 @@ impl SmpteOffset { /// # Returns /// The time offset in microseconds as a floating-point value pub const fn as_micros_with_override(&self, fps: SmpteFps) -> f64 { - ((((self.hour as u32 * 3600) + (self.minute as u32) * 60 + self.second as u32) * 1_000_000) + ((((self.hour as u64 * 3600) + (self.minute as u64) * 60 + self.second as u64) * 1_000_000) as f64) - + ((self.frame as u32) * 1_000_000) as f64 / fps.as_f64() + + ((self.frame as u64) * 1_000_000) as f64 / fps.as_f64() + ((self.subframe as u32) * 10_000) as f64 / fps.as_f64() } /// Convert this SMPTE offset to microseconds. @@ -102,9 +102,9 @@ impl SmpteOffset { /// minutes, seconds, frames, and subframes to provide a precise /// microsecond value. pub const fn as_micros(&self) -> f64 { - ((((self.hour as u32 * 3600) + (self.minute as u32) * 60 + self.second as u32) * 1_000_000) + ((((self.hour as u64 * 3600) + (self.minute as u64) * 60 + self.second as u64) * 1_000_000) as f64) - + ((self.frame as u32) * 1_000_000) as f64 / self.fps.as_f64() + + ((self.frame as u64) * 1_000_000) as f64 / self.fps.as_f64() + ((self.subframe as u32) * 10_000) as f64 / self.fps.as_f64() } From 80bc89ef71abf0a2f18836f8742c0a997f16a35e Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:32:22 -0400 Subject: [PATCH 03/10] fix: off by one header parse --- src/file/meta/smpte_offset.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/file/meta/smpte_offset.rs b/src/file/meta/smpte_offset.rs index d22082a..dcf184e 100644 --- a/src/file/meta/smpte_offset.rs +++ b/src/file/meta/smpte_offset.rs @@ -145,7 +145,7 @@ impl SmpteOffset { v => return Err(SmpteError::TrackFrame(v)), }; let hour = data[0] & 0b0001_1111; - if hour > 24 { + if hour > 23 { return Err(SmpteError::HourOffset(hour)); } let minute = data[1]; From bb4f019cb0f99bae259e4e67f8f599174f1e55b5 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:33:59 -0400 Subject: [PATCH 04/10] fix: docs --- src/file/builder/event/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/file/builder/event/mod.rs b/src/file/builder/event/mod.rs index 7bcc693..62652b8 100644 --- a/src/file/builder/event/mod.rs +++ b/src/file/builder/event/mod.rs @@ -19,7 +19,7 @@ This type is yielded by [`Reader::read_event`] and will be consumed by a Writer # Overview -Except [`FileEvent::Eof`] Events can be placed into two categories +Events can be placed into two categories: ## Chunk Events From cf1a4c80b0a3c959833ab7b714a178d18248e814 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:34:33 -0400 Subject: [PATCH 05/10] fix: Duration uses core --- src/micros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/micros.rs b/src/micros.rs index 20413e2..f92c9d7 100644 --- a/src/micros.rs +++ b/src/micros.rs @@ -6,7 +6,7 @@ pub trait DurationExt { fn to_micros(&self) -> UMicros; } -impl DurationExt for std::time::Duration { +impl DurationExt for core::time::Duration { fn to_micros(&self) -> UMicros { UMicros(self.as_micros() as u64) } From 56571484a2f61c99ae7a59c557c532ff2e16ff30 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:38:05 -0400 Subject: [PATCH 06/10] update ci --- .github/workflows/ci.yaml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bcaab94..844cb03 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -46,6 +46,7 @@ jobs: - uses: dtolnay/rust-toolchain@stable with: toolchain: ${{ env.RUST_TOOLCHAIN }} + components: clippy - name: Setup Rust cache uses: Swatinem/rust-cache@v2 - name: Install alsa and udev @@ -55,16 +56,17 @@ jobs: uses: actions-rs/cargo@v1 with: command: clippy + args: --no-default-features - name: Run cargo clippy (std) uses: actions-rs/cargo@v1 with: command: clippy - args: --features std + args: --no-default-features --features std - name: Run cargo clippy (bevy) uses: actions-rs/cargo@v1 with: command: clippy - args: --features bevy + args: --features "bevy bevy_asset tracing serde" test: strategy: matrix: From cd89ceadfb31add1a780ed31211b18648f2d28cc Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:38:59 -0400 Subject: [PATCH 07/10] update other runner ci --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 844cb03..c99b779 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -92,7 +92,7 @@ jobs: - name: Build & run unit and e2e tests (no features) run: cargo test --lib --tests --no-default-features - name: Build & run unit and e2e tests (all features) - run: cargo test --lib --tests --features all + run: cargo test --lib --tests --features "std bevy bevy_asset tracing serde" all-doc-tests: runs-on: ubuntu-latest steps: From 5bf690ffaf37653c8207f63330d32952f30af755 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:39:30 -0400 Subject: [PATCH 08/10] test doc --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c99b779..d369e32 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -112,4 +112,4 @@ jobs: - name: Install alsa and udev run: sudo apt-get update; sudo apt-get install --no-install-recommends libasound2-dev libudev-dev - name: Run doc tests with all features (this also compiles README examples) - run: cargo test --doc --features bevy + run: cargo test --doc From 6e66c34b576f821540acad632907cdfe029823f1 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:40:19 -0400 Subject: [PATCH 09/10] update version and bevy --- Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c161609..39537bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "midix" -version = "4.0.0-alpha.5" +version = "4.0.0-alpha.6" authors = ["dsgallups "] edition = "2024" description = "MIDI structures designed for humans" @@ -24,7 +24,7 @@ serde = ["dep:serde"] [dependencies.bevy] -version = "0.17.0-rc" +version = "0.17" optional = true default-features = false From 57f752eaeb822c30d3514a2e50c50599491294d5 Mon Sep 17 00:00:00 2001 From: dsgallups Date: Fri, 3 Oct 2025 06:45:05 -0400 Subject: [PATCH 10/10] bump version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 39537bf..8b269a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "midix" -version = "4.0.0-alpha.6" +version = "4.0.0-alpha.7" authors = ["dsgallups "] edition = "2024" description = "MIDI structures designed for humans"