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

Behavior of multiple connections to same node needs to be explicitly defined #143

Closed
olivierthereaux opened this issue Sep 11, 2013 · 13 comments

Comments

Projects
None yet
2 participants
@olivierthereaux
Copy link
Contributor

commented Sep 11, 2013

Originally reported on W3C Bugzilla ISSUE-17795 Tue, 17 Jul 2012 18:40:12 GMT
Reported by Chris Wilson
Assigned to

I noticed that the spec doesn't say what should happen when you multiply connect() a node to another node, e.g.

nodeA.connect( nodeB );
nodeA.connect( nodeB );

Given the fan-out/sum-in model, it could be expected you could have multiple independent connections. The current Chrome behavior is that the second line above is a no-op; I prefer this behavior, but it needs to be explicitly stated in the AudioNode.connect() description.

Suggested text:

There can only be one connection between a given output of one specific node and a given input of another specific node; multiple connections with the same termini are ignored. For example,

nodeA.connect( nodeB );
nodeA.connect( nodeB );

will have the same effect as

nodeA.connect( nodeB );

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Robert O'Callahan (Mozilla) on W3C Bugzilla. Thu, 19 Jul 2012 05:07:19 GMT

This seems like an arbitrary limitation. It would be simpler for connect() to always create a new connection than for connect() to sometimes not create a new connection. What's the motivation for making the second connect() a no-op?

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Wilson on W3C Bugzilla. Thu, 19 Jul 2012 15:24:50 GMT

If you allow multiple connections between two nodes*, then you'll have to provide an identifier for that connection, and enable the disconnect() to take that identifier (and require developers to hang on to that identifier). Or, said another way - if connect() always adds another connection, even if it already exists, then what does disconnect() do, and how do you know when you are really disconnected? It seems like enabling multiple connections would complexify the connect/disconnect part of the API significantly.

If there's a scenario for making multiple connections between two nodes*, I'm open to hearing it; I just couldn't think of any. I would presume it would just effectively double the gain, which of course is easy to do in other ways.

*Technically, these should both read "the same input/output pair of two nodes." Of course you can make connections between different i/o pairs.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Robert O'Callahan (Mozilla) on W3C Bugzilla. Thu, 19 Jul 2012 15:41:15 GMT

(In reply to comment #2)

If you allow multiple connections between two nodes*, then you'll have to
provide an identifier for that connection, and enable the disconnect() to take
that identifier (and require developers to hang on to that identifier). Or,
said another way - if connect() always adds another connection, even if it
already exists, then what does disconnect() do, and how do you know when you
are really disconnected?

disconnect() could just remove one of the connections.

(I do think having identifiers for connections is a good idea though. MSP has them --- InputPorts --- and it lets you set parameters on connections, which seems useful.)

If there's a scenario for making multiple connections between two nodes*, I'm
open to hearing it; I just couldn't think of any. I would presume it would
just effectively double the gain, which of course is easy to do in other ways.

The API should behave predictably even when used in unexpected ways, even ways that aren't the best approach to any given use-case.

Imagine if some module builds an AudioNode A, and then two other modules independently get hold of A and try to play it by connecting it to the DestinationNode. It would be confusing to have one of those operations unexpectedly fail.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Wilson on W3C Bugzilla. Thu, 19 Jul 2012 16:17:43 GMT

(In reply to comment #3)

disconnect() could just remove one of the connections.

I would expect. But we'd have to alter the API to return the number of existing connections, or something, to tell if the nodes are still connected. (So you'd have some way of saying "REALLY disconnect these nodes".)

(I do think having identifiers for connections is a good idea though. MSP has
them --- InputPorts --- and it lets you set parameters on connections, which
seems useful.)

I'm not fundamentally opposed to the idea, but I think the API should be usable in most cases without having to keep track of those identifiers.

If there's a scenario for making multiple connections between two nodes*, I'm
open to hearing it; I just couldn't think of any. I would presume it would
just effectively double the gain, which of course is easy to do in other ways.

The API should behave predictably even when used in unexpected ways, even ways
that aren't the best approach to any given use-case.

The API IS predictable. All I was saying is that the behavior needs to be explicit in the spec.

Imagine if some module builds an AudioNode A, and then two other modules
independently get hold of A and try to play it by connecting it to the
DestinationNode. It would be confusing to have one of those operations
unexpectedly fail.

I'm not suggesting at all that it WOULD fail. It doesn't today - it just doesn't create an ADDITIONAL, duplicate connection (thereby doubling the gain, which would be just as unpredictable, and IMO less usable.)

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Robert O'Callahan (Mozilla) on W3C Bugzilla. Thu, 19 Jul 2012 16:37:30 GMT

Imagine if some module builds an AudioNode A, and then two other modules
independently get hold of A and try to play it by connecting it to the
DestinationNode. It would be confusing to have one of those operations
unexpectedly fail.

I'm not suggesting at all that it WOULD fail. It doesn't today - it just
doesn't create an ADDITIONAL, duplicate connection (thereby doubling the gain,
which would be just as unpredictable, and IMO less usable.)

Depends on your point of view I guess. In this case I think the author would expect connect() to cause a change in the output, and it won't.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Wilson on W3C Bugzilla. Thu, 19 Jul 2012 16:48:51 GMT

(In reply to comment #5)

Depends on your point of view I guess. In this case I think the author would
expect connect() to cause a change in the output, and it won't.

I guess I translate connect() as "make sure these nodes are connected". I see the model you're describing, and it's consistent; I think we just need to choose the simplicity of the model (no identifiers) vs the utility of multiple connections between node i/o points.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Rogers on W3C Bugzilla. Mon, 30 Jul 2012 20:30:47 GMT

My view is that connecting the same thing twice to another thing will not create a duplicate connection since the connection has already been made. Think about this in the real world - what does it mean if I plug my guitar into my guitar amp with a cable - then plug it in again to the exact same amp input with another different cable! I'm quite certain developers won't be confused by this, because I think it's an anomaly and doing nothing will be exactly what they expect. We will need to add more details to the spec explaining this...

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Robert O'Callahan (Mozilla) on W3C Bugzilla. Mon, 30 Jul 2012 21:42:03 GMT

(In reply to comment #7)

Think about this in the real world - what does it mean if I plug my guitar into
my guitar amp with a cable - then plug it in again to the exact same amp
input with another different cable!

Presumably it's just like plugging in another guitar which happens to be producing the same output as the first one.

I'm quite certain developers won't be
confused by this, because I think it's an anomaly and doing nothing will be
exactly what they expect.

The real world doesn't have code modularity and first-class node objects that can be passed around and shared through arbitrary control and data flow. I suspect in the real world you don't shout "here's the output of my guitar, do what you want with it" and throw a lead over to someone on the other side of a crowded room you haven't ever met (who then shares it with several of her friends).

We're not building guitars and amps, we're building software APIs. These APIs will be used by thousands of people who have never plugged a guitar into an amp. They will not limit themselves to using Web Audio to simulate "real world" artifacts.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Tony Ross [MSFT] on W3C Bugzilla. Tue, 31 Jul 2012 23:23:43 GMT

The "do nothing" behavior for duplicate calls to connect() is also consistent with other DOM APIs like addEventListener (which silently ignores duplicate registrations). As such, I think it's a reasonable approach for this scenario.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Robert O'Callahan (Mozilla) on W3C Bugzilla. Tue, 31 Jul 2012 23:43:16 GMT

That's an interesting point. However, the addEventListener behavior is also bad. It means

var f = function() { alert("hello"); };
for (var i = 0; i < 2; ++i) {
document.documentElement.addEventListener("click", f, false);
}

behaves differently from

for (var i = 0; i < 2; ++i) {
document.documentElement.addEventListener("click", function() { alert("hello"); }, false);
}

and

document.documentElement.addEventListener("click", f, false);
document.documentElement.removeEventListener("click", f, false);
document.documentElement.addEventListener("click", f, false);

behaves differently from

document.documentElement.addEventListener("click", f, false);
document.documentElement.addEventListener("click", f, false);
document.documentElement.removeEventListener("click", f, false);

I don't think being consistent with badness is a good thing.

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Rogers on W3C Bugzilla. Tue, 09 Oct 2012 22:03:27 GMT

Fixed:
https://dvcs.w3.org/hg/audio/rev/351b64e571ba

@olivierthereaux

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2013

Original comment by Chris Wilson on W3C Bugzilla. Tue, 09 Oct 2012 22:06:14 GMT

Looks good to me.

@padenot

This comment has been minimized.

Copy link
Member

commented Sep 16, 2013

(This should have been set as closed).

@padenot padenot closed this Sep 16, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.