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

ChannelMergerNode in a cycle has unspecified behaviour #459

Closed
mark-buer opened this issue Dec 16, 2014 · 6 comments
Closed

ChannelMergerNode in a cycle has unspecified behaviour #459

mark-buer opened this issue Dec 16, 2014 · 6 comments

Comments

@mark-buer
Copy link
Contributor

What happens if the output of a ChannelMergerNode is fed back into it's input (via a DelayNode)?
The specification does not state the expected runtime behaviour.

As an illustration, please consider the following JS snippet where a ChannelMergerNode has two connected inputs - an oscillator, and the ChannelMergerNode itself (via a DelayNode):

var audioContext;
if (!audioContext) {
    if (window.AudioContext) {
        audioContext = new AudioContext();
    } else if (window.webkitAudioContext) {
        audioContext = new webkitAudioContext();  
    } else {
        throw new Error('AudioContext missing');
    }
}

var o = audioContext.createOscillator();
var m = audioContext.createChannelMerger(2);
var d = audioContext.createDelay();

o.connect(m, 0, 0);
d.connect(m, 0, 1);
m.connect(d);
m.connect(audioContext.destination);
d.delayTime.value = 0.5;
o.start(0);

The snippet produces silence in Blink and seems to correctly produce a tone in Mozilla.

@hoch
Copy link
Member

hoch commented Dec 17, 2014

@rtoy and I have found an interesting behavior of ChannelMergerNode.

From the spec:

There is a single output whose audio stream has a number of channels equal to the sum of the numbers of channels of all the connected inputs.

In the code snippet above, the merger node has 3 channels in input (1 OSC, 2 delay) so that means its output channels will be 3. Note that these 3 channels goes to the delay's input. Thanks to the current design of node input, the delay will automatically adapt itself to 3 channels after channel count propagation. Then the merger will have 4 channels in inputs (oscillator 1, delay 3). This goes on and on...

Surely this stops when the channel count reaches to 32. However, I think we need to reconsider the behavior of ChannelMerger since we're having another issue like #304.

@cwilso
Copy link
Contributor

cwilso commented Dec 17, 2014

Actually, it wouldn't NECESSARILY stop when the channel count reaches 32 - that's just the minimum required number of channels to support. :)

I think, to me, this calls out that #304 should just say "one channel per input, always, whether it's connected or not."

@hoch
Copy link
Member

hoch commented Dec 17, 2014

"One channel per input, always, whether it's connected or not."

I second that. It solves issues around the merger node - clearly and intuitively.

@rtoy
Copy link
Member

rtoy commented Dec 17, 2014

See also #312. Might be a small hassle if you want 32 channel output and you have to connect 32 inputs when all of your sources are 4 channels.

Or maybe channelCount (defaulting to 1) specifies how many channels each input should have, appropriately mixing the input to that number of channels, whether connected or not. I don't know if that would make sense with the interpretation of channelCountMode and channelInterpretation.

@cwilso
Copy link
Contributor

cwilso commented Dec 17, 2014

Could be just explicit on counts - but I would really prefer to strip, not downmix. I don't know that optimizing stacking 4 channel inputs into 32 channels is the right target; you can still do it, just need 8 splitter nodes.

@joeberkovitz
Copy link
Contributor

Fixed by changing behavior of ChannelMergerNode #304

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

6 participants