From 77896cc49854ecbe8ce5bb7d8fbf48f67b8060e2 Mon Sep 17 00:00:00 2001 From: Methapon2001 <61303214+Methapon2001@users.noreply.github.com> Date: Sun, 24 Nov 2024 20:41:50 +0700 Subject: [PATCH 1/5] Fix wrong total duration cause by divided by zero --- src/decoder/symphonia.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 127c720..28ee4c9 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -170,8 +170,9 @@ impl Source for SymphoniaDecoder { #[inline] fn total_duration(&self) -> Option { - self.total_duration - .map(|Time { seconds, frac }| Duration::new(seconds, (1f64 / frac) as u32)) + self.total_duration.map(|Time { seconds, frac }| { + Duration::new(seconds, if frac > 0.0 { (1f64 / frac) as u32 } else { 0 }) + }) } fn try_seek(&mut self, pos: Duration) -> Result<(), source::SeekError> { From 94a8197273dd688be58092f988779fd275374c97 Mon Sep 17 00:00:00 2001 From: Methapon2001 <61303214+Methapon2001@users.noreply.github.com> Date: Sun, 24 Nov 2024 21:00:17 +0700 Subject: [PATCH 2/5] Add time to duration tests --- src/decoder/symphonia.rs | 46 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 28ee4c9..3ce43a9 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -170,9 +170,7 @@ impl Source for SymphoniaDecoder { #[inline] fn total_duration(&self) -> Option { - self.total_duration.map(|Time { seconds, frac }| { - Duration::new(seconds, if frac > 0.0 { (1f64 / frac) as u32 } else { 0 }) - }) + self.total_duration.map(time_to_duration) } fn try_seek(&mut self, pos: Duration) -> Result<(), source::SeekError> { @@ -306,6 +304,17 @@ fn skip_back_a_tiny_bit( Time { seconds, frac } } +fn time_to_duration(time: Time) -> Duration { + Duration::new( + time.seconds, + if time.frac > 0.0 { + (1f64 / time.frac) as u32 + } else { + 0 + }, + ) +} + impl Iterator for SymphoniaDecoder { type Item = i16; @@ -332,3 +341,34 @@ impl Iterator for SymphoniaDecoder { Some(sample) } } + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use symphonia::core::units::Time; + + use crate::decoder::symphonia::time_to_duration; + + #[test] + fn test_time_to_dur_zero_frac() { + let time = Time { + seconds: 7, + frac: 0.0, + }; + let duration = Duration::new(7, 0); + + assert_eq!(time_to_duration(time), duration); + } + + #[test] + fn test_time_to_dur_non_zero_frac() { + let time = Time { + seconds: 7, + frac: 0.3, + }; + let duration = Duration::new(7, (1.0 / 0.3) as u32); + + assert_eq!(time_to_duration(time), duration); + } +} From d18070d2f7d7d20cad9da367ea47e9e664a927ce Mon Sep 17 00:00:00 2001 From: Methapon2001 <61303214+Methapon2001@users.noreply.github.com> Date: Sun, 24 Nov 2024 21:13:44 +0700 Subject: [PATCH 3/5] Update test to use value from Time instead --- src/decoder/symphonia.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 3ce43a9..31577ce 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -356,7 +356,14 @@ mod tests { seconds: 7, frac: 0.0, }; - let duration = Duration::new(7, 0); + let duration = Duration::new( + time.seconds, + if time.frac > 0.0 { + (1f64 / time.frac) as u32 + } else { + 0 + }, + ); assert_eq!(time_to_duration(time), duration); } @@ -367,7 +374,14 @@ mod tests { seconds: 7, frac: 0.3, }; - let duration = Duration::new(7, (1.0 / 0.3) as u32); + let duration = Duration::new( + time.seconds, + if time.frac > 0.0 { + (1f64 / time.frac) as u32 + } else { + 0 + }, + ); assert_eq!(time_to_duration(time), duration); } From 4ebb2ab8f2c2ed73a4d761232f0a7554770405b3 Mon Sep 17 00:00:00 2001 From: Methapon2001 <61303214+Methapon2001@users.noreply.github.com> Date: Wed, 27 Nov 2024 07:13:00 +0700 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5cbfb9..ee9544e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Breaking: `Sink::try_new` renamed to `connect_new` and does not return error anymore. `Sink::new_idle` was renamed to `new`. +### Fixed + +- Symphonia decoder `total_duration` incorrect value caused by conversion from `Time` to `Duration`. + # Version 0.20.1 (2024-11-08) ### Fixed From de0ffdc9050ed6a0b4ecf1ce7ad981829c97e82e Mon Sep 17 00:00:00 2001 From: dvdsk Date: Wed, 27 Nov 2024 18:57:40 +0100 Subject: [PATCH 5/5] removes symphonia duration unit tests In principle we do not have unit tests. See contributor guidlines for the reasoning. --- src/decoder/symphonia.rs | 45 ---------------------------------------- 1 file changed, 45 deletions(-) diff --git a/src/decoder/symphonia.rs b/src/decoder/symphonia.rs index 31577ce..5705445 100644 --- a/src/decoder/symphonia.rs +++ b/src/decoder/symphonia.rs @@ -341,48 +341,3 @@ impl Iterator for SymphoniaDecoder { Some(sample) } } - -#[cfg(test)] -mod tests { - use std::time::Duration; - - use symphonia::core::units::Time; - - use crate::decoder::symphonia::time_to_duration; - - #[test] - fn test_time_to_dur_zero_frac() { - let time = Time { - seconds: 7, - frac: 0.0, - }; - let duration = Duration::new( - time.seconds, - if time.frac > 0.0 { - (1f64 / time.frac) as u32 - } else { - 0 - }, - ); - - assert_eq!(time_to_duration(time), duration); - } - - #[test] - fn test_time_to_dur_non_zero_frac() { - let time = Time { - seconds: 7, - frac: 0.3, - }; - let duration = Duration::new( - time.seconds, - if time.frac > 0.0 { - (1f64 / time.frac) as u32 - } else { - 0 - }, - ); - - assert_eq!(time_to_duration(time), duration); - } -}