Set BaseAudioContext::state by render thread to signal device readiness #419

orottier · 2024-01-02T19:46:22Z

- A context is instantiated in Suspended state
- Online context will change to Running at the first render quantum
- Online context will change to Closed when the render thread is dropped after a `close` call
- Online context suspend/resume state is still managed by the control thread - TODO
- Offline context will change to Running after startRendering
- Offline context will set state to suspended when suspending, running when resumed
- Offline context will set change to closed when rendering has finished

Fixes #410 #416 #101

It probably also fixes ircam-ismm/node-web-audio-api#61 (comment)

One thing that I found that is more a concern on the rust side, is that the first "resume" statechange event is sometimes triggered, sometimes not. I think this some kind of race condition but this is a bit weird from a user perspective.

Todo

find a solution for online AudioContext resume/suspend to signal device 'readiness'. Possible solution: first pause audio rendering by sending a ctrl msg to suspend the audio graph. When that message is handled, then emit the state_change event and update the state and suspend the cpal stream. Similar setup for resuming.
close_sync on online AudioContext is not truly sync. We don't guarantee the operation has finished before the function returns. Solution: use a channel to communicate if we are done
implement oncomplete for offline context - tricky! to be addressed in Emit oncomplete event for OfflineAudioContext #411
add async methods for suspend/resume/close on online AudioContext

Fixes #410 Except for suspending/resuming, I don't have a solution for that yet. In summary: - A context is instantiated in Suspended state - Online context will change to Running at the first render quantum - Online context will change to Closed when the render thread is dropped after a `close` call - Online context suspend/resume state is still managed by the control thread - TODO - Offline context will change to Running after startRendering

Fixes #416

Fixes #101

orottier · 2024-01-02T19:50:46Z

src/context/concrete_base.rs

@@ -333,14 +334,14 @@ impl ConcreteBaseAudioContext {
    /// Returns state of current context
    #[must_use]
    pub(super) fn state(&self) -> AudioContextState {
-        self.inner.state.load(Ordering::SeqCst).into()
+        self.inner.state.load(Ordering::Acquire).into()
    }

    /// Updates state of current context
    pub(super) fn set_state(&self, state: AudioContextState) {
        let current_state = self.state();
        if current_state != state {


A race condition may occur in concurrent calls to this method (e.g. making a massive amount of suspend calls) causing the statechange event to be sent too many times. This could be fixed with https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU64.html#method.compare_exchange but this method will probably be refactored away once we solve how to make suspend_sync better

b-ma · 2024-01-03T18:56:42Z

Cool! Looks all good to me

This will be very simple to implement #411 from this point, I was unsure of the behaviour of await

orottier · 2024-01-03T19:02:56Z

This will be very simple to implement #411 from this point

Ahah, that is what you think (and I thought), but this is especially tricky because we don't spawn an event loop in the case of the OfflineAudioContext. Something to reconsider maybe. In any case we can't use the existing methods to spawn the complete event. I tried but it does not emit the event properly.

b-ma · 2024-01-03T19:18:15Z

Ah damn I didn't realized that...

Something to reconsider maybe.

Indeed, and their will probably be another issue after that I just discovered yesterday... :) As we send the ended event for AudioScheduledSourceNode in the next render block, it might never be triggered in offline contexts if the rendering finishes on this block (there is a wpt test failling and I was thinking that was the actual issue)

…mpleted

orottier · 2024-01-03T19:25:06Z

Indeed that will also be an issue. But to be fair, these events simply don't make sense on an OfflineAudioContext because there is not actionable thing you can do with them.
What information does it give you that a AudioBufferSourceNode has ended when the offline context is rendering? You don't have time to modify the audio graph at that point anyway because the rendering will continue while you handle the event...
I think the only sensible (but deprecated) event for OfflineAudioContext is oncomplete. All the others (statechange, ended, processorerror) could be informational but not actionable..

b-ma · 2024-01-03T19:31:14Z

Hehe yup you are right, I guess the most important use case is actually testing and, eventually, some kind of monitoring (I do my best here :)...

orottier · 2024-01-03T19:56:21Z

Ready for merging

b-ma · 2024-01-03T20:14:05Z

src/context/online.rs

+        // Then, ask to resume rendering via a control message
+        let (sender, receiver) = oneshot::channel();
+        let notify = OneshotNotify::Async(sender);
+        let suspend_msg = ControlMessage::Resume { notify };


suspend_msg is a bit misleading, should be resume_msg?

b-ma · 2024-01-03T20:14:52Z

src/context/online.rs

+        // First, stop rendering via a control message
+        let (sender, receiver) = oneshot::channel();
+        let notify = OneshotNotify::Async(sender);
+        let suspend_msg = ControlMessage::Close { notify };


same close_msg?

b-ma · 2024-01-03T20:18:20Z

Nice!

orottier · 2024-01-04T10:04:36Z

Released in v0.41

orottier added 4 commits January 2, 2024 18:46

OfflineAudioContext should set state to suspended when suspending

caa6c51

Fixes #416

Validate panics in #101 are fixed (sending ctrl msgs to closed context)

fcbb67a

Fixes #101

Silence clippy

e54c5b7

orottier requested a review from b-ma January 2, 2024 19:46

orottier commented Jan 2, 2024

View reviewed changes

Ensure AudioContext suspend/resume_sync block control thread until co…

50e115f

…mpleted

orottier added 2 commits January 3, 2024 20:33

Ensure AudioContext close_sync blocks control thread until completed

ad24275

Add async versions of AudioContext suspend/resume/close

e9330cd

This was referenced Jan 3, 2024

Add async method for BaseAudioContext::decodeAudioData #423

Open

Draft: async interface (decodeAudioData, startRendering, suspend/resume/close) #177

Closed

b-ma reviewed Jan 3, 2024

View reviewed changes

orottier added 2 commits January 4, 2024 10:22

Fix confusing naming of suspend/resume/close control messages

480f6d1

Make test more robust by sleeping to prevent event thread racing

0644fe2

orottier merged commit d2e782c into main Jan 4, 2024
3 checks passed

orottier deleted the feature/device-readiness branch January 4, 2024 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set BaseAudioContext::state by render thread to signal device readiness #419

Set BaseAudioContext::state by render thread to signal device readiness #419

orottier commented Jan 2, 2024 •

edited

orottier Jan 2, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma Jan 3, 2024

b-ma Jan 3, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 4, 2024

Set BaseAudioContext::state by render thread to signal device readiness #419

Set BaseAudioContext::state by render thread to signal device readiness #419

Conversation

orottier commented Jan 2, 2024 • edited

orottier Jan 2, 2024

Choose a reason for hiding this comment

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma commented Jan 3, 2024

orottier commented Jan 3, 2024

b-ma Jan 3, 2024

Choose a reason for hiding this comment

b-ma Jan 3, 2024

Choose a reason for hiding this comment

b-ma commented Jan 3, 2024

orottier commented Jan 4, 2024

orottier commented Jan 2, 2024 •

edited