RustAudio / rodio

Rust audio playback library
Apache License 2.0
1.75k stars 227 forks source link

Sharing `get_pos()` between multiple threads? #610

Closed nednoodlehead closed 1 month ago

nednoodlehead commented 1 month ago

In an audio playing application (mainly talking about music players here), you'd want to have multiple threads, one that controls playing the music, one that controls the gui / event thread loop, and in the newest version of rodio, one that updates the time elapsed / scrubbing bar on the bottom of the application. (Which is great, I love this addition)

However, I am struggling to see how I can implement this for my application, since the mutex will only allow one thread at a time to read / write to the self.position field, this makes it difficult to adhere to the "multiple threads" concept above, since the music thread will own the sink, and the only way to view that value, is via the sink (sink.get_pos()), which the "time elapsed" thread does not have access to. (Cannot pass &Sink to the thread since iced requires message's contents to impl Debug)

I'm not sure if I am misunderstanding this, or if I am not following the architecture in mind when this api was designed, but a function to maybe return a handle to some wrapper that contains the self.position / Duration would be perfect. I could create a PR, but I have no clue what type I should wrap it in? Especially if we are very performance-conscious, since there would be some overhead writing to an ArcSwap or something, vs the regular mutex.

dvdsk commented 1 month ago

I had a look at your app. As I read it your app 'waits' until a change is needed try_recv then does something with the sink and gets back to checking for needed changes.

Quick sidenote, why try_recv and not recv().await?

What you could do is each few milliseconds run sink.get_pos() there and send that to the gui through a mpsc. I believe you already get one from iced: sender.

You could try wrapping the recv() call with a timeout (I dont get the difference between an io-future and a 'normal' future in async-std, so maybe this function is the wrong one). And set the timeout duration for about a frame of time. Or even better calculate the timeout such that you send a position every frame.

Then every time you time out you check the position and send it to the gui.

Good luck!

nednoodlehead commented 1 month ago

I got it working with what you suggested. I was concerned about the additional overhead of sending the values (and the two calculations that will be performed by the main blocking thread in fn update. I think I'm overthinking the 'performance hit' that occurs.

I'll close the issue, thanks.

dvdsk commented 4 weeks ago

I was concerned about the additional overhead of sending the values

A mpsc can easily handle millions of messages a second. Whenever you think something might be a performance hit just try it. Usually it isnt, Rust is great like that :)