Intent to Implement and Ship: Selective Disconnection on AudioNode.disconnect()

24 views
Skip to first unread message

Hongchan Choi

unread,
Feb 9, 2015, 12:52:33 PM2/9/15
to blin...@chromium.org, rt...@chromium.org, cwi...@chromium.org
Contact emails

Spec

Summary
Currently AudioNode.disconnect() disconnects all existing connections out of node output(s). This change is to support disconnecting only one connection selectively.

Motivation
There have been numerous requests to support selective disconnection with disconnect() method and Audio WG agreed to change the spec to accommodate the feature request. With this change, it is possible to disconnect a signal path out of multiple connections to AudioNode input or AudioParam.
Currently the only way to disconnect one connection is to disconnect all the outputs and reconnect everything else except the connection to be dropped. This can cause audio glitches because it takes time to reconnect everything.

Compatibility Risk
None. We add new features to the existing disconnect() method and the change is backward-compatible. Mozilla also agreed with this change and will implement it in the near future.

Ongoing technical constraints
None.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS,  and Android, and Android WebView)?
Yes.

OWP launch tracking bug?

Link to entry on the feature dashboard

Requesting approval to ship?
Yes

Philip Jägenstedt

unread,
Feb 10, 2015, 4:23:56 AM2/10/15
to Hongchan Choi, blink-dev, rt...@chromium.org, cwi...@chromium.org
LGTM

There's already a "disconnect (optional unsigned long output = 0)"
which confused me at first, but an AudioNode's output can be connected
to multiple other AudioNode or AudioNodes. This API allows you to
disconnect only one of those.

Philip
> To unsubscribe from this group and stop receiving emails from it, send an
> email to blink-dev+...@chromium.org.

Jochen Eisinger

unread,
Feb 10, 2015, 10:51:38 AM2/10/15
to Philip Jägenstedt, Hongchan Choi, blink-dev, rt...@chromium.org, cwi...@chromium.org
LGTM2

Dimitri Glazkov

unread,
Feb 10, 2015, 12:01:59 PM2/10/15
to Jochen Eisinger, Philip Jägenstedt, Hongchan Choi, blink-dev, rt...@chromium.org, cwi...@chromium.org
LGTM3

Hongchan Choi

unread,
Mar 3, 2015, 1:13:35 PM3/3/15
to Dimitri Glazkov, Jochen Eisinger, Philip Jägenstedt, blink-dev, rt...@chromium.org, cwi...@chromium.org
Hello All,

While I was working on the actual implementation of this feature and I found some issues with the original proposal (which was approved by the spec WG). So I brought up the issue with the new proposal and it is approved by WG again today. WG agreed that this is a breaking change but the risk of compatibility is pretty low.

Please have a look at the new proposal (+ discussion) here: 

In short, the previous 'disconnect()' method was a bit broken. When the node has multiple outgoing connections and disconnects with no argument, it only disconnects the first output. (Because of optional = 0 in the method signature)

audioNode.disconnect(); // Currently it only disconnects the first output.

But with the new proposal, if the output index is unspecified then it disconnects all the connections at once.

audioNode.disconnect(); // (New) disconnects the all outgoing connections.
audioNode.disconnect(someNode); // (New) disconnects all the connections to the destination node.

Should I file the brand new intent email again? Or may I land the implementation to Blink with your approvals on this intent?

Thanks!
--
Hongchan Choi

Software Engineer
Google Chrome

Dimitri Glazkov

unread,
Mar 3, 2015, 1:22:33 PM3/3/15
to Hongchan Choi, Jochen Eisinger, Philip Jägenstedt, blink-dev, rt...@chromium.org, cwi...@chromium.org
As long as Mozilla folks are on board, I think you could.

:DG<

Hongchan Choi

unread,
Mar 3, 2015, 1:55:50 PM3/3/15
to Dimitri Glazkov, Jochen Eisinger, Philip Jägenstedt, blink-dev, rt...@chromium.org, cwi...@chromium.org
Yes - just got an LGTM from Paul (padenot@mozilla) on the new proposal.

Thanks Dimitri!
Reply all
Reply to author
Forward
0 new messages