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

setValueCurveAtTime when curve has non-finite values? #1171

Closed
rtoy opened this issue Mar 17, 2017 · 6 comments
Closed

setValueCurveAtTime when curve has non-finite values? #1171

rtoy opened this issue Mar 17, 2017 · 6 comments
Milestone

Comments

@rtoy
Copy link
Member

rtoy commented Mar 17, 2017

The spec currently doesn't say anything about the values contained in the curve for setValueCurveAtTime. Are we allowing any possible value for the curve, including NaN and Infinity?

Chrome currently signals an error if the curve contains non-finite values.

Allowing such values would probably very quickly cause NaN to be produced for the automation, probably also causing all downstream nodes to produce NaN for all audio samples. Figuring out why this happened is fairly hard.

@padenot
Copy link
Member

padenot commented Mar 21, 2017

I think this is taken care of already. Allowing NaN and Infinity in the ctor would be written as:

sequence<unrestricted float> imag;

We might want to change the factory method so that it uses sequence<float> as well ?

@rtoy
Copy link
Member Author

rtoy commented Mar 21, 2017

Yes, I think using sequence<float> everywhere instead of Float32Array would be a nice convenience.

@rtoy rtoy added the Needs Discussion The issue needs more discussion before it can be fixed. label Mar 21, 2017
@padenot
Copy link
Member

padenot commented Mar 21, 2017 via email

@rtoy
Copy link
Member Author

rtoy commented Mar 21, 2017

Can't think of any cases where sequence<float> would break anything. Not sure if Chrome's IDL compiler will actually check to see of the sequence only contained finite values, though. But that's an implementation issue, not a spec issue.

@joeberkovitz
Copy link
Contributor

sequence<float> is OK. We need to prevent propagating NaN/Infinity values.

@joeberkovitz joeberkovitz added V1 Blocker and removed Needs Discussion The issue needs more discussion before it can be fixed. labels Mar 30, 2017
@joeberkovitz joeberkovitz added this to the Web Audio V1 milestone Mar 30, 2017
@rtoy
Copy link
Member Author

rtoy commented Mar 31, 2017

Heh. This is a duplicate of #815, fixed by #819. But we seem to have lost the text about throwing errors. And #815 mentioned using sequence<float> but we never did.

Let's actually make it happen this time, with sequence<float>.

rtoy pushed a commit to rtoy/web-audio-api that referenced this issue Mar 31, 2017
Change the type of the curve parameter to sequence<float>.  This
specifies that only finite floats are allowed for the curve.  This is
a backward-compatible change since Float32Array is a sequence<float>
(if all the values in Float32Array are finite floats).
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

3 participants