-
Notifications
You must be signed in to change notification settings - Fork 167
[audioworklet] Number of channels in output buffers #1325
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
Comments
We can utilize dictionary AudioWorkletNodeOptions : AudioNodeOptions {
unsigned long numberOfInputs = 1;
unsigned long numberOfOutputs = 1;
sequence<unsigned long> outputChannelCount = [];
record<DOMString, double> parameterData;
} Consider these 4 cases: 1 input, 1 outputSimilar to GainNode. The input channel count is computed by n inputs, 1 outputSimilar to ChannelMergerNode. The input channel count will be identical for all inputs, once it's computed by 1 input, m outputsSimilar to ChannelSplitterNode. The input channel count is computed by n input, m outputsThe input channel count will be identical for all inputs, computed by To sum up, the dynamic channel configuration will not be supported on the output side when you have multiple inputs or outputs. Also |
Looks like it would work. Does this extend to 0 input n output (multi-channel source), or n input 0 output (multi-channel analyser)? I think it would works well. |
Yeah, I think it works correctly. |
The thrust of this seems right, but it feels underspecified to me. I think we need to say what happens across this matrix of possibilities in two different cases: 1) where So I would propose that the combinations behave like this: 0 input, 0 outputthrow an error 0 input, m outputsuse 1 input, 1 output(these cases are the same as per your observation that AudioNode n inputs, 1 outputuse (NOTE: Your comment "The input channel count will be identical for all inputs, once it's computed by 1 input, m outputsuse n input, m outputsuse |
One side point: why is the options dictionary called |
We were merely exchanging ideas here, but your last comment shaped this thread into the actual spec text.
Well, the spec of "up/downmixing" section is not really clear on how multiple inputs are handled. So my interpretation was to compute the channel count once based on the options, and apply that single value for all the inputs. This is where my confusion begins:
I think your interpretation is correct - and we should add more text in the spec to clarify that each input gets to have its own computed channel count. |
But it seems to me the spec is actually pretty clear that each input has a distinct count. I read the text here: https://webaudio.github.io/web-audio-api/#channel-up-mixing-and-down-mixing And at the end, what I see is a clear:
The words for each input to me convey that the computation of channel count is separate for each input. |
@joeberkovitz Oh, I missed that part. Then I think your comment above nicely sums up the behavior. Should we proceed to an actual PR with the more refined text? By the way, I think we might encounter some issues because of this dynamic mechanism. This means every render quantum will get a newly created object of |
@hoch: I don't agree that garbage is inevitable under this dynamic approach. There is a way to optimize this, and I think it's important to include something like the following as a non-normative guideline: The minimization of garbage in the audio thread is of great importance to the efficiency of AudioNode implementations. In the absence of connectivity changes to an AWN that affect the number of input channels, implementations are strongly encouraged to pool buffers, only allocating new ones when the channel counts are different from those in the previous quantum. If UAs generate garbage every quantum, it'll be a big step backward. |
@joeberkovitz Consider this example: let evilGlobal;
class myProcessor extends AudioWorkletProcessor {
constructor() {
super();
}
process(inputs, outputs, parameters) {
if (!evilGlobal)
evilGlobal = inputs[0]
}
} That's where "buffer pooling" fails. Without creating new From my point of view, The "let's make allocation more efficient" approach doesn't really help this situation. The allocation is allocation, and it'll accumulate rapidly when it happens. The good news is only one Perhaps we should open another issue for this matter. |
@hoch I'm still not seeing how this presents a problem (and it could be because I'm just plain slow). If we're worried that people will write broken code that just holds onto the buffer, that can be defeated in the UA by switching between different (but still pooled) sets of buffers, or by re-allocating them every N render quanta where N is large enough to really cut down on the garbage, but small enough to break code that tries to cache buffers. The sin of allowing code to cache buffers seems far outweighed by the sin of generating lots of garbage on every quantum. |
@joeberkovitz I completely agree with you on this, but I am only making the counter argument for the sake of discussion.
My concern is whether this "implicit yet visible" change in the buffer is acceptable from the architecture/specification perspective. I haven't seen any other Web API allows this behavior. Also randomizing underlying scope or storage seems to be rather critical due to the compat/interop issue: I don't think AudioWorklet poses such problem, but at least we have to address it before too late. |
Thanks, now I understand better. Yes, deterministic behavior could be a long-term architectural problem and we don't want it to get baked in. I'd agree that it deserves a separate issue. My sense is that some form of non-determinism would have to be specified regardless of whether it's random scopes, random re-allocations, random pool selections... but I don't think any of these need to change the API that we're designing in this issue right here... |
Returning back to the original discussion... While I was thinking about the efficient way of describing the behavior, I found some more corner cases. The following proposal is slightly different from the @joeberkovitz's idea above, but I think it is simpler: AudioWorkletNodeOptions.outputChannelCount
This way, we can make |
On Tue, Sep 26, 2017 at 2:12 PM, Hongchan Choi ***@***.***> wrote:
Returning back to the original discussion...
While I was thinking about the efficient way of describing the behavior, I
found some more corner cases. The following proposal is slightly different
from the @joeberkovitz <https://github.com/joeberkovitz>'s idea above,
but I think it is simpler:
AudioWorkletNodeOptions.outputChannelCount
- The range of valid channel count value is between 1 and the UA's
maximum number of channels supported. A NotSupportedError exception
will be thrown at the invalid channel count.
- IndexSizeError will be thrown The outputChannelCount.length does not
match numberOfOutputs.
1.
0 input, 0 output
NotSupportedError
2.
0 input, 1 output
Optional. The channel count will be mono if unspecified.
3.
0 input, m output
Optional. The channel count will be mono for all outputs if
unspecified.
4.
1 input, 0 output
N/A
5.
1 input, 1 output
Optional. The channel count will match the computed input channel
count if unspecified.
6.
1 input, m output
Optional. The channel count will match the computed input channel
count for all outputs if unspecified.
I would have guessed this would be mono, like for all other cases except
the 1 input 1 output case.
I think this would mean for any number of outputs, the count is one,
except for the one special case of single-input/single-output.
…
1.
2.
n input, 0 output
N/A
3.
n input, 1 output
Optional. The channel count will be mono if unspecified.
4.
n input, m output
Optional. The channel count will be mono for all outputs if
unspecified.
This way, we can make outputChannelCount optional for the majority of
cases while providing user with the sensible default channel count. Also I
believe this scheme is easier to grasp.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1325 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAofPE9sQLsekV2avQ0Piomb5ppvxnFZks5smWjCgaJpZM4Pbf29>
.
--
Ray
|
Hmm. My last comment didn't come out quite right. I think option 6 with 1 input and m outputs should default to mono outputs. Then it defaults to mono for all cases except the 1-input/1-output case. Also, what does N/A in, say, item 4 really mean? There are no outputs, but what if I specify a set of channel counts? You say earlier that an IndexSizeError is thrown if the number of outputs doesn't match the array length. |
@rtoy I'll tell you my reasoning for making the 1:m case use The reason is that if you want 1:m to use mono outputs, it's very easy to provide an explicit array |
@rtoy By "N/A", I meant |
@joeberkovitz Nothing wrong with multiple channels. I was just going for default behavior that is simple. I also couldn't (after a few seconds) think of a use-case where I would really want this kind of dynamic output count that couldn't be handled with multiple worklets with1-input/1-output dynamic case. |
WG resolution: leave 1:M case as mono output for each of the M channels |
Note: we need to file new issue for v.next to support adjustable multichannel count on AWN w multiple outputs |
Consider the following:
In the example above, what you get from
inputChannelCount
is determined by the channel count computing algorithm. (spec) However, what you get foroutputChannelCount
is not well-defined.The simplest approach is making the matching input/output buffers have the same channel count. For example, if
inputChannelCount
gives you n, you can expectedoutputChannelCount
to be n.Although there are more elegant solutions for this, I am against creating AudioBuffer objects (filling in
outputs
every render quantum) in theprocess
method because that will generate garbages fast and furious.The text was updated successfully, but these errors were encountered: