AudioWorkletNode #385
AudioWorkletNode #385
Conversation
examples/worklet.rs
Outdated
| outputs: &mut [AudioRenderQuantum], | ||
| params: AudioParamValues<'_>, | ||
| _scope: &RenderScope, | ||
| inputs: &'b [&'a [f32]], |
There was a problem hiding this comment.
These different lifetimes are necessary unfortunately
There was a problem hiding this comment.
A bit odd/complex indeed, but not really shocking neither in my opinion as this is boilerplate code...
Do you think it is due to the fact that you need to deal with the existing AudioWorkletRenderer so this could potentially be removed once (and if) all nodes are ported to this new API, or is it something we will have to live with forever (with an additional depth when several inputs/outputs are supported) in any case?
|
Hey, I'll try to have a close look in the next few days |
And perform the initialization in the render thread - todo remove the `Send` bounds on the trait.
|
It looks quite nice indeed! Great work I read the spec and the code quite a few time (...but probably not enough :) and here are a few questions / remarks:
So if I understand well you consider the instantiation should occur in render thread (which is confirmed by the spec by the way) because you think processors should be only stack allocated and thus deallocation should be free? But I still don't really get how this new API would help to solve our issues with e.g. delay ring buffer (but maybe that's not the point yet...)? If I'm not missing something the bitcrusher example could already be implemented no? I can try to do it if you want, would be an interesting test maybe. In any case, that's really interesting! Let me know if I can help in any way (maybe porting some existing nodes see were it gets us?) |
even though we don't use it only in the case of AudioWorkletNode
|
Thanks for having a look. I added a bit more work so right now I think it would be very interesting if you could try to port the bitcrusher example and see if it works out. The VU-meter is not possible as of yet I think because it needs to extend the AudioWorkletNode which is not possible in rust and you should then still use the raw AudioProcessor trait
Yes definitely
I'm worried that this is a bridge to far, this will not help us get rid of the Box to support polymorphism of the processors
I understand, I'm making things messier before I make them nicer ;) I will need to port the construct-on-render-thread code to the AudioProcessor trait as well. Then we can remove some unsafe snippets :)
I think the current AudioWorkletProcessor trait has a performance hit so I'm not sure if we should do this at all. Provided I port the nice to have features from AudioWorkletProcessor to AudioProcessor |
Examples: implement bit crusher worklet
src/node/worklet.rs
Outdated
| params: crate::render::AudioParamValues<'_>, | ||
| scope: &RenderScope, | ||
| ) -> bool { | ||
| // Bear with me, to construct a &[&[&[f32]]] we first build a backing vector of all the |
There was a problem hiding this comment.
To get rid of these allocations in safe rust is going to be tricky. But I will try a few more things
|
/bench |
|
|
/bench |
|
I'm happy to see the new worklet overhead is quite low compared to the processors that handle the RenderQuantum raw. |
|
Are you okay with this mod layout? |
Yup quite impressive indeed, congrats! (I would be quite curious to test this with other nodes as well at some point)
I think I would simply put it in But no strong opinion here
On the one side I don't feel it has exactly the same responsibility as in the spec. But on the other side I don't think we can really have such "scope" behavior in Rust (?). So, maybe that's the closest thing we can provide and somehow it makes sens... Maybe that's just a matter of properly documenting the tradeoff with the spec? I think you read the spec more than me here, what your guts are telling you? :) Another thing I'm a bit unsure is the argument order for: fn process<'a, 'b>(
&mut self,
scope: &'b RenderScope,
inputs: &'b [&'a [&'a [f32]]],
outputs: &'b mut [&'a mut [&'a mut [f32]]],
params: AudioParamValues<'b>,
) -> bool {Why did you decided to put |
But it also contains
I haven't put much thought in it, but it felt like an extension of |
Hum right, then let's go with |
|
I have decided to merge this current work and leave the leftovers for a new issue #391 |
Hey, this is already getting out of hand so I wanted to show you the current progress.
I think mainly the
examples/worklet.rsfile would be interesting to inspect. It should resemble the spec way more than the AudioProcessor trait we used before.Lots of work to do:
Sendbound from the traitname: String