Skip to content

AudioWorkletNodeOptions.outputChannelCount #1386

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

Merged
merged 5 commits into from
Oct 12, 2017

Conversation

hoch
Copy link
Member

@hoch hoch commented Sep 28, 2017

"AudioWorkletNodeOptions">numberOfOutputs</a> and
<a data-link-for=
"AudioWorkletNodeOptions">outputChannelCount</a>, various
channel configurations can be achieved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use a table somehow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I actually prefer this way. I think a list is better than using <table> element. We only have 3 cases with a single description for each.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

index.html Outdated
@@ -9267,7 +9327,8 @@ <h2 id="defining-a-valid-audioworkletprocessor">
<code>Float32Array</code> of audio samples for the
<code>m</code>th channel of <code>n</code>th input. While the
number of inputs is fixed at the construction, the number of
channels can be changed dynamically.
channels can be changed dynamically based on
<a>computedNumberOfChannels</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need period to end the sentence.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

index.html Outdated
@@ -9267,7 +9327,8 @@ <h2 id="defining-a-valid-audioworkletprocessor">
<code>Float32Array</code> of audio samples for the
<code>m</code>th channel of <code>n</code>th input. While the
number of inputs is fixed at the construction, the number of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not part of the PR, but noticed that "is fixed at the construction" -> "is fixed at construction"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

index.html Outdated
of channels can be changed dynamically.
for <code>m</code>th channel of <code>n</code>th output. The
number of channels in the output can be dynamically changed
only when the node has single output.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A link to the section describing this would be helpful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrased it with a link.

index.html Outdated
</dt>
<dd>
This is used to configure the number of channels in each
output. <code>IndexSizeError</code> MUST be thrown if the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still need to say a little bit more on what this means. Something like "outputChannelCount[n]" specifies the number of channels for output number n". Or something.

@hoch
Copy link
Member Author

hoch commented Oct 2, 2017

@joeberkovitz Could you take a look?

@hoch
Copy link
Member Author

hoch commented Oct 6, 2017

@padenot Could you take a look?

Copy link

@joeberkovitz joeberkovitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor clarifications

index.html Outdated
"AudioWorkletNodeOptions">numberOfOutputs</a>. A
<code>NotSupportedError</code> exception MUST be thrown if a
channel count is not in the valid range of <a data-link-for=
"AudioNode">channelCount</a>.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems confusing -- first of all, channelCount is not a range, it's a single number. Also, it's not clear if you mean the option dictionary value AudioNodeOptions.channelCount, or the read-only attribute AudioNode.channelCount (I'm guessing you mean the options value, but then you have to account for what happens if this was omitted, in which case there is no defined channel count for up/downmixing).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant was AudioNode.channelCount here. We have a defined range for that property and the same rules should be applied to each value in AudioWorkletNodeOptions.outputChannelCount array. I thought data-link-for was quite clear, but it's not visible in the actual page. So I edited the text to clarify what channelCount means here.

Also the behavior on unspecified (omitted) outputChannelCount is described in the following section.

index.html Outdated
"AudioWorkletNodeOptions">numberOfOutputs</a> = 0
<ul>
<li>
<code>NotSupportedError</code> MUST be thrown.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must be thrown... by the constructor

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@hoch hoch merged commit 2acb81c into WebAudio:gh-pages Oct 12, 2017
@hoch hoch deleted the 1325-output-channel-count branch March 10, 2021 16:03
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