Skip to content
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

Questions in termusic repo #637

Open
dvdsk opened this issue Nov 11, 2024 · 7 comments
Open

Questions in termusic repo #637

dvdsk opened this issue Nov 11, 2024 · 7 comments

Comments

@dvdsk
Copy link
Collaborator

dvdsk commented Nov 11, 2024

These questions originated in the discussions part of another repo which uses rodio: tramhao/termusic#203 and where asked by @hasezoey

I have copied some of the rodio api related questions here so they (and the answers/discussion that might follow) can also benefit other rodio users.

send a message on EOS of a source (see this code in our implementation) (suggestion would be to get the handle to the sleep_until_end directly, or a function which accepts a callback and is similar to source::Done)

You might be able to do that using EmptyCallback. Its a source that will never play audio but will execute arbitrary code when it runs. You can append an EmptyCallback after each source. Let me know if that sounds good

possibly a way to add / replace other source modifiers after all the other processing in Sink::append

You already seem to be using PeriodicCallback. Adding effects is done by compiling the source with those effects applied but configured such that they have no effect. For example a speed of 1.0 does nothing to the underlying source. Some source effects like automatic_gain_control have an enable/disable toggle.

there is likely a possible de-sync between the track_position getting for events and the Sink::get_pos which is after speed / delay

That must be a bug in rodio. Please open an issue and provide a minimal example where the bug is easy to see so we can fix it.

@hasezoey
Copy link

You might be able to do that using EmptyCallback. Its a source that will never play audio but will execute arbitrary code when it runs. You can append an EmptyCallback after each source. Let me know if that sounds good

That does sound like a good idea. Alternatively i think its possible to custom-add something similar to source::Done which is not necessarily in rodio itself.

You already seem to be using PeriodicCallback. Adding effects is done by compiling the source with those effects applied but configured such that they have no effect. For example a speed of 1.0 does nothing to the underlying source. Some source effects like automatic_gain_control have an enable/disable toggle.

I meant it in the context of still being able to use Sink::controls (and by extension Sink::set_speed) without having to extra-wrap it for a case like replacing speed with our soundtouch source modifier.

there is likely a possible de-sync between the track_position getting for events and the Sink::get_pos which is after speed / delay

That must be a bug in rodio. Please open an issue and provide a minimal example where the bug is easy to see so we can fix it.

I did not test if there actually is a desync, but i noted it as possible as, if i understand correctly, track_position is affected by speedup / delay / other source modifiers (as per this comment and this doc-comment)


After having a look at those things again, with the current rodio structure it seems possible that we also could just make a custom Sink impl instead of using the rodio provided one, or would this be a problem?
My thinking is because everything rodio::Sink uses is also exported pub (like queue::SourcesQueueInput pub cpal re-export and all sources)

@dvdsk
Copy link
Collaborator Author

dvdsk commented Nov 11, 2024

Alternatively i think its possible to custom-add something similar to source::Done which is not necessarily in rodio itself.

Yes anything that implements the Source trait will work. If you can think of something that specifically works better with termusic go for it!

@dvdsk
Copy link
Collaborator Author

dvdsk commented Nov 11, 2024

I meant it in the context of still being able to use Sink::controls (and by extension Sink::set_speed) without having to extra-wrap it for a case like replacing speed with our soundtouch source modifier.

So you would like Sink to add controls for Sources defined outside of rodio?

I am also intrigued by the soundtouch source modifier what does it do?

@dvdsk
Copy link
Collaborator Author

dvdsk commented Nov 11, 2024

I did not test if there actually is a desync, but i noted it as possible as, if i understand correctly, track_position is affected by speedup / delay / other source modifiers (as per this comment and this doc-comment)

Yes but only those applied after track_position. If you use the speed option from the Sink then track position will report the position in the underlying source and neglect the speedup.

From the Sink::get_pos docs:

Example: if you apply a speedup of *2* to an mp3 decoder source and
[`get_pos()`](Sink::get_pos) returns *5s* then the position in the mp3
recording is *10s* from its start.

wait... thats wrong.... Okay I am going to check this out and report back!

@dvdsk
Copy link
Collaborator Author

dvdsk commented Nov 11, 2024

wait... thats wrong.... Okay I am going to check this out and report back!

Nope its correct. try_seek takes speedup's into account. We do that because otherwise mixing could become terribly confusing. That only works if the speedup is always the same though...... Thinking about it now that might have been a bad design choice. Tracked in new issue #638.

@dvdsk
Copy link
Collaborator Author

dvdsk commented Nov 11, 2024

After having a look at those things again, with the current rodio structure it seems possible that we also could just make a custom Sink impl instead of using the rodio provided one, or would this be a problem?
My thinking is because everything rodio::Sink uses is also exported pub (like queue::SourcesQueueInput pub cpal re-export and all sources)

Yes, that is a very good idea. Originally that would have been the only possibility. I think Rodio's Sink was made because users found that hard to do and "just wanted to play some music". Its design however is questionable. We are going to rewrite it and make breaking changes to it, some discussion here: #624 and you have seen the user stories issue where we are gathering feedback so we can decide what the new sink like thing should be.

Making your own Sink like structure is quite doable. I have explained part of it here: #634 (second paragraph of my first response). Let me know if you need any help.

@hasezoey
Copy link

I am also intrigued by the soundtouch source modifier what does it do?

The soundtouch source modifier is currently just a wrapper for soundtouch which is a ffi for c++ soundtouch.
TL;DR: speed up/down with no pitch change (which rodio's current speed impl does)

So you would like Sink to add controls for Sources defined outside of rodio?

Not quite, for this specific example, i would like to just replace rodio's speed modifier (or at least disable it from being changed) and re-use Sink::controls's speed for the soundtouch speed modifier (see this disabling and this enabling) without some extra wrapper around Sink. This could likely be solved with the referenced #624 by exposing more controls or directly accessing source modifier's controls.
And otherwise i just meant to apply modifiers after the other changes which might depend on it, like after speed (which there currently, aside from periodic_access, we dont have) (a example could be to have a periodic_access which should run regardless of if it is paused or not). Which has also, as i have seen, been part of #624, though more broadly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants