-
Notifications
You must be signed in to change notification settings - Fork 293
Add TrackPosition to Source #585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| use std::time::Duration; | ||
|
|
||
| use crate::{Sample, Source}; | ||
|
|
||
| use super::SeekError; | ||
|
|
||
| /// Internal function that builds a `TrackPosition` object. | ||
| pub fn track_position<I>(source: I) -> TrackPosition<I> { | ||
| TrackPosition { | ||
| input: source, | ||
| samples_counted: 0, | ||
| offset_duration: 0.0, | ||
| current_frame_sample_rate: 0, | ||
| current_frame_channels: 0, | ||
| current_frame_len: None, | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct TrackPosition<I> { | ||
| input: I, | ||
| samples_counted: usize, | ||
| offset_duration: f64, | ||
| current_frame_sample_rate: u32, | ||
| current_frame_channels: u16, | ||
| current_frame_len: Option<usize>, | ||
| } | ||
|
|
||
| impl<I> TrackPosition<I> { | ||
| /// Returns a reference to the inner source. | ||
| #[inline] | ||
| pub fn inner(&self) -> &I { | ||
| &self.input | ||
| } | ||
|
|
||
| /// Returns a mutable reference to the inner source. | ||
| #[inline] | ||
| pub fn inner_mut(&mut self) -> &mut I { | ||
| &mut self.input | ||
| } | ||
|
|
||
| /// Returns the inner source. | ||
| #[inline] | ||
| pub fn into_inner(self) -> I { | ||
| self.input | ||
| } | ||
| } | ||
|
|
||
| impl<I> TrackPosition<I> | ||
| where | ||
| I: Source, | ||
| I::Item: Sample, | ||
| { | ||
| /// Returns the position of the source. | ||
| #[inline] | ||
| pub fn get_pos(&self) -> f64 { | ||
| self.samples_counted as f64 / self.input.sample_rate() as f64 / self.input.channels() as f64 | ||
| + self.offset_duration | ||
| } | ||
|
|
||
| #[inline] | ||
| fn set_current_frame(&mut self) { | ||
| self.current_frame_len = self.current_frame_len(); | ||
| self.current_frame_sample_rate = self.sample_rate(); | ||
| self.current_frame_channels = self.channels(); | ||
| } | ||
| } | ||
|
|
||
| impl<I> Iterator for TrackPosition<I> | ||
| where | ||
| I: Source, | ||
| I::Item: Sample, | ||
| { | ||
| type Item = I::Item; | ||
|
|
||
| #[inline] | ||
| fn next(&mut self) -> Option<I::Item> { | ||
| // This should only be executed once at the first call to next. | ||
| if self.current_frame_len.is_none() { | ||
| self.set_current_frame(); | ||
| } | ||
|
|
||
| let item = self.input.next(); | ||
| if item.is_some() { | ||
| self.samples_counted += 1; | ||
|
|
||
| // At the end of a frame add the duration of this frame to | ||
| // offset_duration and start collecting samples again. | ||
| if Some(self.samples_counted) == self.current_frame_len() { | ||
| self.offset_duration += self.samples_counted as f64 | ||
| / self.current_frame_sample_rate as f64 | ||
| / self.current_frame_channels as f64; | ||
|
|
||
| // Reset. | ||
| self.samples_counted = 0; | ||
| self.set_current_frame(); | ||
| }; | ||
| }; | ||
| item | ||
| } | ||
|
|
||
| #[inline] | ||
| fn size_hint(&self) -> (usize, Option<usize>) { | ||
| self.input.size_hint() | ||
| } | ||
| } | ||
|
|
||
| impl<I> Source for TrackPosition<I> | ||
| where | ||
| I: Source, | ||
| I::Item: Sample, | ||
| { | ||
| #[inline] | ||
| fn current_frame_len(&self) -> Option<usize> { | ||
| self.input.current_frame_len() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after
Feel free to experiment with other approaches to find what fits best. Performance matters but as always premature optimization is the root of all evil. An if statement that fires every
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you happen to know which decoder currently handle this correctly? I've tried with FLAC using https://github.com/ietf-wg-cellar/flac-test-files/blob/main/uncommon/01%20-%20changing%20samplerate.flac with the symphonia decoder and after switching to the next sample rate it stops working and with claxon it does work, but doesn't seem to record the new sample rate (just reports the 32 kHz all the way trough) and it ends up 'pitching' the audio. (FWIW, VLC also borks on this only mpv seems to somewhat handle it but has incorrect position after switching the sample rate and has the incorrect total duration). Shouldn't be hard to handle these cases correctly, but at least the FLAC decoder is stopping me from testing this and I don't know of any other format that supports variable sample rate 😅
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've gone ahead and opened PRs against claxon and symphonia to correctly handle such cases. ruuda/claxon#35 & pdeljanov/Symphonia#287. For the claxon support a single line would need to be added to rodio once the PR is merged: diff --git a/src/decoder/flac.rs b/src/decoder/flac.rs
index 9dbc0c4..b3b69dd 100644
--- a/src/decoder/flac.rs
+++ b/src/decoder/flac.rs
@@ -118,6 +118,7 @@ where
let buffer = mem::take(&mut self.current_block);
match self.reader.blocks().read_next_or_eof(buffer) {
Ok(Some(block)) => {
+ self.sample_rate = block.sample_rate();
self.current_block_channel_len = (block.len() / block.channels()) as usize;
self.current_block = block.into_buffer();
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
On the rodio side at least the symphonia and vorbis code takes care to handle this. I had no idea it was broken in symphonia & claxon (rodio's test suit is kinda sparse atm). Like us they are desperate for maintainers/contributers :).
nice! thanks a lot for that 👍
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once the decoders are fixed 0e6d5a1 should make the position tracking work correctly with. Doing a few local tests doesn't hint at performance problems (but only a profiler would be able to confirm that) and tracking the position of https://github.com/ietf-wg-cellar/flac-test-files/blob/main/uncommon/01%20-%20changing%20samplerate.flac works flawlessly. |
||
| } | ||
|
|
||
| #[inline] | ||
| fn channels(&self) -> u16 { | ||
| self.input.channels() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn sample_rate(&self) -> u32 { | ||
| self.input.sample_rate() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn total_duration(&self) -> Option<Duration> { | ||
| self.input.total_duration() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError> { | ||
| let result = self.input.try_seek(pos); | ||
| if result.is_ok() { | ||
| self.offset_duration = pos.as_secs_f64(); | ||
| // This assumes that the seek implementation of the codec always | ||
| // starts again at the beginning of a frame. Which is the case with | ||
| // symphonia. | ||
| self.samples_counted = 0; | ||
| } | ||
| result | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use std::time::Duration; | ||
|
|
||
| use crate::buffer::SamplesBuffer; | ||
| use crate::source::Source; | ||
|
|
||
| #[test] | ||
| fn test_position() { | ||
| let inner = SamplesBuffer::new(1, 1, vec![10i16, -10, 10, -10, 20, -20]); | ||
| let mut source = inner.track_position(); | ||
|
|
||
| assert_eq!(source.get_pos(), 0.0); | ||
| source.next(); | ||
| assert_eq!(source.get_pos(), 1.0); | ||
|
|
||
| source.next(); | ||
| assert_eq!(source.get_pos(), 2.0); | ||
|
|
||
| assert_eq!(source.try_seek(Duration::new(1, 0)).is_ok(), true); | ||
| assert_eq!(source.get_pos(), 1.0); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am planning to overhaul this whole module, the every 5ms check for things to do feels very iffy. One might want more precision. Though that is for later right now. If you have ideas on how to redo this module I would love to hear them.