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

AudioWorklet improvements #127

Open
2 of 11 tasks
b-ma opened this issue May 15, 2024 · 5 comments
Open
2 of 11 tasks

AudioWorklet improvements #127

b-ma opened this issue May 15, 2024 · 5 comments

Comments

@b-ma
Copy link
Collaborator

b-ma commented May 15, 2024

Remaining todos and ideas following #124

  • Review rules and fix number of channels outputs when input change
  • Use a pool of Float32Array channels to avoid allocation and GC (as much as possible)
  • Ensure we run run_audio_worklet_global_scope and setImmediate only once per render quantum
  • Use the fact that parameterDescriptors are ordered to avoid transferring parameter keys, i.e. NapiAudioWorlketProcessor::param_values could just be Vec<&'static [f32]>
  • Review how message passing is done between audio and worklet threads (possibly use parking_lot crate ?)
  • Dry run process functions to force the JIT compiler to optimize them ? (might break some wpt checks) could create issues If the worklet buffer some values
  • Review and annotate unsafe sections
  • Change the thread_priority crate to https:/mozilla/audio_thread_priority
  • Use a CondVar instead of crossbeam channel for Worker/Rust message passing
  • Propagate back error to rust thread in case of WorkletAbruptCompletionResult to remove the Worklet from graph
  • ...
@orottier
Copy link
Collaborator

I added some more todos following the discussion over at https://app.slack.com/client/T04PVA27V/CGS5YDU0J

@orottier
Copy link
Collaborator

orottier commented May 23, 2024

Ensure we run run_audio_worklet_global_scope and setImmediate only once per render quantum

I would rather not add something to the base lib to facilitate this. Instead, we could just add an unconnect custom AudioWorkletNode of our own in de the node lib to each AudioContext. It won't affect audio processing, and will tick exactly once per render quantum. This we can use to yield to V8.

@b-ma
Copy link
Collaborator Author

b-ma commented May 23, 2024

I understand your point, but can we be sure it will be called last? I'm a bit unsure of the graph sorting rules but I think in particular if we have a second user defined unconnected worklet, we might end up in a situation where the graph needs to wait for the JS tick to end, no? (that may be no huge deal and certainly better than the current situation in any case)

@orottier
Copy link
Collaborator

Yes indeed, we cannot guarantee it is called before or after the audio destination node. (unless we connect the destination node to the canary node - perhaps not a bad idea although the end user could theoretically disconnect them)

I agree that ideally these two operations run concurrently in their respective threads

  • shipping the samples to the speaker (audio thread)
  • spin the V8 event loop (worker thread)

But I think that is something to look into later. In any case we go from many event loop spins to 1 per render quantum, which is beneficial always.

Let me whip up some code and we can discuss further :)

@b-ma
Copy link
Collaborator Author

b-ma commented May 23, 2024

Hehe, yup let's go :)

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