Repository navigation
Speed::try_seek incorrectly multiplies seek position by speed factor #876
Description
Hey, I found this while debugging a really frustrating issue in my music player (Sunder). When playing audio at non-1x speeds and seeking via the progress bar, tracks would immediately end as if the file was over.
After a lot of head-scratching, I traced it down to Speed::try_seek in src/source/speed.rs:
fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError> {
/* TODO: This might be wrong, I do not know how speed achieves its speedup
* so I can not reason about the correctness.
* <dvdsk noreply@davidsk.dev> */
// even after 24 hours of playback f32 has enough precision
let pos_accounting_for_speedup = pos.mul_f32(self.factor);
self.input.try_seek(pos_accounting_for_speedup)
}
The multiplication causes the inner decoder to receive a position that's factor times larger than intended. Since Speed achieves its speedup by modifying sample_rate() (not by resampling or skipping), the inner source still operates in source-time. Multiplying the position overshoots the actual file duration and the source immediately hits EOF.
Example:
215-second audio file, speed set to 2x
User seeks to the 150s mark: sink.try_seek(Duration::from_secs(150))
Speed::try_seek multiplies: 150 * 2.0 = 300s
The decoder receives try_seek(300s) on a 215s file, past EOF
sink.empty() returns true immediately, track skips
This happens for any seek past duration / speed. At 1x the bug is invisible since multiplying by 1.0 is a no-op, which is probably why it went unnoticed.
The TODO comment in the code already notes the uncertainty here, so I figured this was worth reporting.
Suggested fix:
Pass the position through unchanged, like Amplify, Stoppable, and all other sources that don't alter the time domain:
fn try_seek(&mut self, pos: Duration) -> Result<(), SeekError> {
self.input.try_seek(pos)
}
Current workaround:
Pre-divide the seek position by the speed factor before calling try_seek:
let seek_pos = source_secs / speed as f64;
sink.try_seek(Duration::from_secs_f64(seek_pos));
Activity
yara-blue commentedon Apr 20, 2026
Hey frog,
great report. Seems like you got the fix already. Feel free to open a PR! if you do make sure to add to the changelog that this got fixed :)
FrogSnot commentedon May 5, 2026
Hey @yara-blue, I’ve decided to close the PR because I realized the fix is more complicated than a one-liner.
While removing the multiplication fixes seeking, it makes the Speed wrapper internally inconsistent because total_duration() still returns a speed-adjusted duration (playback time). To fix this properly, the whole source would need to be refactored to consistently use 'Source Time' (unscaled) for both duration and seeking.
That’s a bit more of an architectural change than I have the capacity for right now, so I’m going to stick with the workaround in my own project for now. I’ll leave this issue open in case someone else wants to tackle the full refactor.
yara-blue commentedon May 5, 2026
Oh you're right, we do not know when the speedup started. That's a bummer :(
Only way to solve that would be incrementing a source time like you propose... With quiet a bit of overhead ...
Maybe we should just always seek without taking speed factor into account or this would get awefully complex. Since the overhead stops us from tracking the time we coud try a SpeedSeekable which can be created like this:
Speed::seekable() -> SpeedSeekable. That would only work if the source had not yet changed speed before calling::seekable... I guess we could check for that making it:Speed::seekable() -> Result<SpeedSeekable, AlreadySpedUp>...That is totally fine! Thank you for taking a look and figuring out these details!
roderickvd commentedon May 6, 2026
It's questionable whether it should. Think of your average media player: if you've got a 3'00'' audio or video clip, you can speed playback up or down, but the duration remains 3'00''.
xronocode commentedon Nov 2, 2026
Hey! I have also been looking into this, following the discussion here and in #934.
On master as of
685e0576,Speed::try_seekmaps the target with only the factor that happens to be active right now:total_duration()divides by the same current factor. After any mid-play speed change both go wrong, andtry_seek(get_pos())stops landing where it started once the two current-factor errors stop cancelling.There look to be two coherent ways out, and they differ in what a position means.
Source time, no bookkeeping. The direction FrogSnot sketched here and first tried in #881, now in #934 by @UnknownSuperficialNight:
track_position()beforespeed(), seek targets as media time.get_pos()andtry_seekbecome exact for any speed history, and nothing needs to track anything. The cost is a semantics change: today positions are playback time, andtotal_duration()stops adjusting, which matches the media player view roderickvd expressed above.Playback time, exact bookkeeping. The direction #638 itself pointed at ("correcting for this requires speed to keep track of all the speed changes"). Keep a small anchor history inside
Speed: on every real rate change, remember the frames consumed, the playback seconds elapsed, and the epoch's rates;try_seekwalks the epochs (a binary search) and converts through the owning epoch's rates;total_durationis the elapsed part from the anchors plus the remaining time at the current rate. The cost accounting and the edge cases (span-less sources, capacity, degenerate factors) are worked out and would go into the PR description.One honest caveat: this fixes
try_seekandtotal_durationonly.get_pos()stays an estimate after a mid-play speed change (#639 and #837 track this; #934 closes the latter), sotry_seek(get_pos())turns from an accidental no-op into landing on the estimate: an hour of media at 0.5x, then a switch to 2x, rewinds playback by 90 min on a span-less input (wav, flac). Under #934 reads become exact instead, which is a real argument for it.A flag keeps both mappings alive in one type and doubles the surface while containing none of the breakage, and the seekable-handle idea @yara-blue floated above stays valid only until the first speed change. That is why I went for bookkeeping inside
Speed.I know #901 is in flight and considers spans a mistake; the rate epochs here are span-independent, and the span-era parts drop out when it lands. The question of what a position means survives the rewrite either way, which is why it seems worth settling now.
My take: if changing the meaning of positions is acceptable, #934's source time is the simpler and cleaner fix, and I am happy to help it along. If playback time stays, the anchors make seeks exact without changing any public signatures. Either way the PR would update the module docs, add a changelog entry, a divan benchmark, and tests for epochs, paused and failed seeks, degenerate factors,
Delay, the round trip, and nestedSpeed.Which semantics do you want to keep?