-
Notifications
You must be signed in to change notification settings - Fork 54
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
Wrong ParamRate for BufferSourceNode messages #338
Comments
Using types to avoid this problem sounds like a wise solution. |
Types can't fix this, the automationrate is mutable. It's some specific parameters where it's not. (In general: most knobs in WebAudio are tweakable in general and have constraints in specific cases) |
We actually sample these as k-rate anyway. We should still fix the rate here, but that's not actually going to affect anything; the real change is in libscript to introduce parameter constraints. |
I tried doing something like this: but idk how to go from |
Right, those checks belong in servo's script crate, at best we can have some debug assertions here IMO |
you mean now ARate is implemented as KRate? |
@khodzha hm? I mean that we sample them at the beginning of the block, not once per tick/frame. |
I've exposed node types via audio context, then they can be used in SetAutomationRate to do these checks |
…<try> WIP AutomationRate constraints [webaudio spec](https://webaudio.github.io/web-audio-api/#AudioParam-attributes) specifies that some nodes have constraints on their params automation rates. - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes (partially) fix servo/media#338 - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___
The spec says that
playbackRate
anddetune
of BufferSourceNode must be k-rate.But right now they are a-rate.
Also spec says it must be impossible to change them from k-rate to a-rate.
I suggest to put
ParamType
intoParam
struct so its possible to enforce these constraints directly inParam
The text was updated successfully, but these errors were encountered: