Implements Seek #513

dvdsk · 2023-10-04T10:47:19Z

Adresses #176, see #176 (comment) for the motivations behind the design.

Adds a fail-able method fn try_seek(&mut self, pos: Duration) -> Result<(), SeekNotSupported> to all sources and decoders. The error struct SeekNotSupported contains the type name of the source that did not support seek. This is particularly useful when you have a chain of sources.

I commented out some working functionality that could not be merged yet:

Mixing sources are not seek-able, they need to know the current position in a source to be able to roll back when one of the sources fails to seek.
I added seeking to Minimp3 however the minimp3_fixed maintainer has engaged with my PR yet. I want to give them some more time to do so before looking at other options.

dvdsk · 2023-10-04T10:47:35Z

Note: this is currently still a draft, early reviews are welcome though :)

src/source/mix.rs

src/decoder/vorbis.rs

tests/seek.rs

dvdsk

Yet another thought, now we've got them implemented everywhere can_seek and try_seek should get default implementations. That way this wont break the build for everyone implementing Source on their own type.

dvdsk · 2023-10-07T12:53:34Z

So the motivation behind can_seek was to use it in mixing sources. That way we can prevent scenarios where we seek in one source (A) and cant in the other (B). The problem is that the seek can still fail if can_seek returns true.

A better approach is to use #510 to seek back in A if B returns a (non critical) error while seeking. Such an error could be our SeekError::NotSupported or something like symphonia's SeekErrorKind::Unseekable.

examples/seek_mp3.rs

naglis

Thank you for your work!

I lack the expertise to give a thorough review of the actual implementation, so these are mostly typos I've noticed.

src/conversions/sample.rs

src/conversions/sample_rate.rs

src/decoder/mp3.rs

src/decoder/symphonia.rs

src/source/mod.rs

src/source/sine.rs

src/spatial_sink.rs

tests/seek.rs

dvdsk · 2023-10-12T17:05:10Z

Thank you for your work!

I lack the expertise to give a thorough review of the actual implementation, so these are mostly typos I've noticed.

I applied all the spelling/grammar suggestions directly, thanks a lot for them! The others are great too they just require a little more attention. I will apply them tomorrow!

…named the trait to SeekableSource

… decoderimpl a bit

add todo about can_seek to mix Co-authored-by: naglis <[email protected]>

document why Sink::can_seek does not exist

This reverts commit 80add81.

seek is broken, RustAudio/lewton#73. We could work around it by: - using unsafe to create an instance of Self - use mem::swap to turn the &mut self into a mut self - take out the underlying Read+Seek - make a new self and seek If this issue is fixed use the implementation in commit: 3bafe32

This adds two dependencies, they are only needed for the test suite. Compile time will not increase for users. The extra overhead when running the test suite is worth it imho. The test file is significantly shorter and there is less code duplication. A run time solution would decrease the test interface (you would have to manually find out which params caused the test).

More readable, prep for using the same lines in new refine_position fn. Since that is already pretty complex this needed to be made simpler. This code retries on all errors not only decode errors. Retries will not work on anything else then a decode error however they also wont cause any problems. Not checking the result does however make the code simpler.

Houndie · 2024-04-05T11:38:25Z

Just wanted to drop props for @dvdsk and everyone else who has reviewed this, this is great work and I'll be excited to see this land!

Was needed before a cargo update call. Update to cpal made the type copy though.

dvdsk · 2024-04-06T10:34:32Z

Thanks to @Rintse for his extensive review! We caught a few cases where seek could be extended. The review also inspired me to extend the test suite with as result a ton of bugs discovered 😄.

Those have now all been fixed! Time to merge this PR 🎉 🥳 🎈.

Whats next?

In a few days I plan to cut a new release and push that to crates.io and maybe an announcement on users.rust-lang.org and the reddit.

In the long term I am planning a few other features to make rodio more suitable as a backend for music players. They will land over the next year probably (no promises). Meanwhile I'll be spending a little time every week to work through the PR backlog. So if you see anything you would like in rodio, do not hesitate to open an issue. If it fits for rodio we can work on it together.

dvdsk marked this pull request as draft October 4, 2023 10:47

dvdsk mentioned this pull request Oct 4, 2023

Seeking #176

Closed

naglis reviewed Oct 4, 2023

View reviewed changes

src/source/mix.rs Outdated Show resolved Hide resolved

dvdsk mentioned this pull request Oct 4, 2023

Elapsed Duration functionality for the Decoder/LoopedDecoder #510

Closed

dvdsk commented Oct 5, 2023

View reviewed changes

src/decoder/vorbis.rs Outdated Show resolved Hide resolved

dvdsk commented Oct 5, 2023

View reviewed changes

tests/seek.rs Show resolved Hide resolved

dvdsk commented Oct 6, 2023

View reviewed changes

dvdsk force-pushed the seek_runtime_err branch from d7012ae to b2940a2 Compare October 10, 2023 00:26

This was referenced Oct 11, 2023

Reverb example rans into error with symphonia vorbis decoder #516

Closed

symphonia impl total_duration #405

Open

dvdsk marked this pull request as ready for review October 12, 2023 12:07

naglis reviewed Oct 12, 2023

View reviewed changes

examples/seek_mp3.rs Outdated Show resolved Hide resolved

naglis reviewed Oct 12, 2023

View reviewed changes

dvdsk force-pushed the seek_runtime_err branch from ab2d6f1 to afa1b76 Compare October 13, 2023 13:23

dvdsk and others added 13 commits October 13, 2023 15:44

seek implemented through SourceExt trait

fafe4ba

request pos now uses mutable self

023e833

switch to fork for cpal

204a3f8

remove seek trait from source mods, added it to symphonia decoder, re…

d47842f

…named the trait to SeekableSource

removes SeekableSource adds failable try_seek to Source

745db82

adds try_seek for sink and all sources

202687b

removes default try_seek impl, impl try_seek for decoders + refactors…

1f3f36a

… decoderimpl a bit

refactors seektest and adds more formats

5b933d7

Fix seeking for mix source

961c3ef

add todo about can_seek to mix Co-authored-by: naglis <[email protected]>

refactors seek test, now covers all decoders/formats

e1092f7

fixes symphonia seek div by zero

a3c55b8

add seek for lewton

d8a8be4

add can_seek method to source

9a4dcb0

document why Sink::can_seek does not exist

dvdsk added 3 commits April 3, 2024 11:45

test(seek) add test for channel order

9ae1c55

test(seek) improve channel order test

04c6957

fix(seek) vorbis decoder now respects channel order

1c82136

dvdsk force-pushed the seek_runtime_err branch from 11afd32 to 1c82136 Compare April 3, 2024 10:41

dvdsk added 5 commits April 3, 2024 13:02

commit to save work on vorbis try_seek

5562241

removes seeking from vorbis decoder, can not be implemented

80add81

Vorbis actually can be implemented since the sample rate is constant

4e14b0a

This reverts commit 80add81.

refactor(seek) explain why seek to sample works

3bafe32

dvdsk force-pushed the seek_runtime_err branch from 78c9289 to 26e9db7 Compare April 4, 2024 10:54

dvdsk added 6 commits April 4, 2024 21:22

fix(seek) hound(wav) now keeps channel order consistent

f846cdf

refactor(decoder/symphonia) logic reorderd

a52a41b

test(seek) made channel order more brittle

1fcf4b8

fix(decoder/symphonia) seek is no longer off

6034af3

dvdsk added 2 commits April 5, 2024 13:59

seek/error symphonia seekerror is now more precise

aa0880d

fix(seek/delay) seek < delay duration ate up the delay

7cf0451

dvdsk force-pushed the seek_runtime_err branch from ed9b48a to 7cf0451 Compare April 5, 2024 13:14

dvdsk added 6 commits April 6, 2024 12:00

Merge branch 'master' into seek_runtime_err

6f1f44f

style clippy fixes

34366fe

fix(seek) conditional compilation

a4d167f

Merge branch 'master' into seek_runtime_err

b49b22a

update changelog

bbc8f00

style, removes a needless clone()

1ed1197

Was needed before a cargo update call. Update to cpal made the type copy though.

dvdsk merged commit d103517 into RustAudio:master Apr 6, 2024
10 checks passed

dvdsk mentioned this pull request Apr 10, 2024

Calculate duration for mp3s #481

Closed

ivanstepanovftw mentioned this pull request Jun 16, 2024

Audio sink seek bevyengine/bevy#13869

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements Seek #513

Implements Seek #513

dvdsk commented Oct 4, 2023 •

edited

Loading

dvdsk commented Oct 4, 2023 •

edited

Loading

dvdsk left a comment

dvdsk commented Oct 7, 2023

naglis left a comment

dvdsk commented Oct 12, 2023 •

edited

Loading

Houndie commented Apr 5, 2024

dvdsk commented Apr 6, 2024

Implements Seek #513

Implements Seek #513

Conversation

dvdsk commented Oct 4, 2023 • edited Loading

dvdsk commented Oct 4, 2023 • edited Loading

dvdsk left a comment

Choose a reason for hiding this comment

dvdsk commented Oct 7, 2023

naglis left a comment

Choose a reason for hiding this comment

dvdsk commented Oct 12, 2023 • edited Loading

Houndie commented Apr 5, 2024

dvdsk commented Apr 6, 2024

Whats next?

dvdsk commented Oct 4, 2023 •

edited

Loading

dvdsk commented Oct 4, 2023 •

edited

Loading

dvdsk commented Oct 12, 2023 •

edited

Loading