-
Notifications
You must be signed in to change notification settings - Fork 16
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
Update AudioParam automation rate #88
Comments
Marking it a prerequisite for v1.0, but only because of the possible breaking API change in the last bullet |
My two cents:
Maybe a new field in // node.rs
let param_options = {
// ...
constraint: Some(|rate: AutomationRate| -> AutomationRate { ... })
}
// param.rs
set_automation_rate(rate: AutomationRate) {
if self.constraint.is_some() {
rate = self.constraint.unwrap()(rate);
}
self.automation_rate = rate;
} if you think it's a good approach, I can try to propose something.
This question is more tricky somehow and unfolds other ones I have the impression. The spec does not even say that the length is related to
At first sight, it kind of sounds to me more like: "Chrome people (or whoever) did that and they don't want to change their implementation, so let's consider it's valid because it doesn't hurt" :) My impression here is that we could also be defensive for v1.0: deliberately avoid the extensibility problem (be it called In any case, let me know what you think I can also have a look on that point, I'm pretty confident the unit tests will allow us to see quite fast if some unexpected problem reveals itself. Beyond the fact we decide or not to keep this |
Thanks for the comments! Regarding the constraints, your solution is fine but maybe it is a bit over-engineered. Reading the spec, I think we can get away with a simple option I'm looking at the current signature of the audio param values:
Then we can don't need to make the final decision before release v1.0 |
Yep, you are right, I didn't check the spec and I was thinking there was more special cases. I'm not completely sure the flag will be enough for panner with HRTF (but my solution neither... :), but HRFT will be a tricky thing anyway.
Yep, that looks like a good tradeoff
Yes I think it worth the case to at least try to see what we get there, I'd really like the advance on the Oscillator sub-quantum scheduling first but I will check that after (assigning myself to the issue as it is somehow related to #20) |
Regarding the discussion at #98 I'm removing the V1 milestone here |
https://www.w3.org/TR/webaudio/#dom-audioparam-automationrate
The automation rate, barring some constraints, can be changed by the control thread.
set_automation_rate
toimpl AudioParam
AudioParamValues.get(..)
. It now returns a slice of full render quantum length always. But the spec says it can be length 1 too (for k-rate and for constant pieces) [1]. For type safety we can also make it return an enum.[1] https://www.w3.org/TR/webaudio/#audioworkletprocess-callback-parameters
The text was updated successfully, but these errors were encountered: