Skip to content

Fix #1299: add audio param tables#1359

Merged
rtoy merged 16 commits into
WebAudio:gh-pagesfrom
rtoy:1299-add-audio-param-tables
Oct 9, 2017
Merged

Fix #1299: add audio param tables#1359
rtoy merged 16 commits into
WebAudio:gh-pagesfrom
rtoy:1299-add-audio-param-tables

Conversation

@rtoy
Copy link
Copy Markdown
Member

@rtoy rtoy commented Sep 25, 2017

For each AudioParam, add a simple table giving the values that the AudioParam object would have by default. Also mention whether the param is a-rate or k-rate. While not necessary, I think it makes it easier to see what the AudioParam values should be and how the nominal range is really the min and max values of the parameter.

And this makes the description of the AudioParam consistent everywhere.


Preview | Diff

@rtoy rtoy requested review from hoch, joeberkovitz and padenot October 3, 2017 16:01
@rtoy
Copy link
Copy Markdown
Member Author

rtoy commented Oct 3, 2017

PTAL. Don't think it's controversial; it just formalizes the default, min, and max values for each AudioParam in a readable table, and also adds a note on whether the parameter is a-rate or k-rate. Removes a bunch of text and makes it all uniform.

@mdjp
Copy link
Copy Markdown
Member

mdjp commented Oct 5, 2017

2.17.1 Attributes

These all still include the old description

eg This parameter is a-rate when used with a PannerNode that has a panningModel set to "equalpower" , k-rate otherwise. Its nominal range is (−∞,∞).

Same with
2.23.2 Attributes (release)
2.24.2 Gain

I'm assuming this is unintentional duplication

@rtoy
Copy link
Copy Markdown
Member Author

rtoy commented Oct 5, 2017

Thanks for catching those errors. I thought I had fixed them all. :-(

@mdjp
Copy link
Copy Markdown
Member

mdjp commented Oct 9, 2017

Looks good to me now @rtoy

@rtoy
Copy link
Copy Markdown
Member Author

rtoy commented Oct 9, 2017

Thanks for the quick review @mdjp

@rtoy rtoy merged commit 604a010 into WebAudio:gh-pages Oct 9, 2017
@padenot
Copy link
Copy Markdown
Member

padenot commented Jul 18, 2018

Changing to -FLT_MIN and FLT_MIN is a breaking change. It would. have been good to open an issue about it. I'm not opposed to it though, I think it makes more sense.

@rtoy
Copy link
Copy Markdown
Member Author

rtoy commented Jul 18, 2018

Why is this a breaking change? Previously we said the nominal range was (-infinity, +infinity), so the limits aren't included.

@padenot
Copy link
Copy Markdown
Member

padenot commented Jul 18, 2018

I see a couple with [-Infinity, Infinity], and Firefox was returning -Infinity and Infinity for those.

I found this because the idl-harness.html test uses playbackRate to check the existence of minValue and maxValue.

Pretty minor though, and now that I think of it, it must have been a typo from one of use to have square brackets here. I'm uploading the patch to align with the spec (and Chrome) as we speak.

I find uses on github though: https://github.com/craigharvi3/audibly/blob/9433b4e7c1bc9361aa46a9f14e03d657f47ca214/src/js/API/Audibly/Node/Source/AudiblyAudioBufferSourceNode.js#L49

@rtoy
Copy link
Copy Markdown
Member Author

rtoy commented Jul 18, 2018

Oops. Sorry about that. I think allowing infinity for the limit is a bad idea because it probably very quickly causes audio to be infinity or, more likely, NaN. Of course, FLT_MAX will also probably quickly produce that too, but that's a different issue.

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

Successfully merging this pull request may close these issues.

3 participants