TAG issue: Subclassing / Node composition

[chrislo]

The following issue was raised by the W3C TAG as part of their review of the Web Audio API

ISSUE: Subclassing

Related to a lack of constructors, but worth calling out independently, it's not currently possible to meaningfully compose node types, either through mixins or through subclassing. In JS, this sort of "is a" relationship is usually set up through the subclassing pattern:

var SubNodeType = function() {
  SuperNodeType.call(this);
};
SubNodeType.prototype = Object.create(SuperNodeType.prototype);
SubNodeType.prototype.constructor = SubNodeType;
// ...

There doesn't seem to be any way in the current design to enable this sort of composition. This is deeply unfortunate.

[juliangruber]

with sublassing capabilities writing custom nodes will be way easier and there won't be a fight of CustomNode(input) vs input.connect(customNode.node) etc.

[juliangruber]

For example, I have a multi channel analyzer node. If I were to expose a currently ideomatic webaudio api, it would be

sourceOne.connect(analyzer.node);
sourceTwo.connect(analyzer.node);
analyzer.on('data', function(data) {
  console.log(data); // [[ 0, 1, ... ], [ 1, 1, ... ]]
});

Having unnamed parameters sucks, and especially when you disconnect one node it becomes super confusing. So the api I have now is like this:

analyzer
  .add('one', sourceOne)
  .add('two', sourceTwo)
  .on('data', function(data) {
    console.log(data); // { one: [ 0, 1, ... ], two: [ 1, 1, ... ] }
  });

This works but isn't ideomatic webaudio style, i.e. it's not using .connect. What I'd ideally do is this:

sourceOne.connect(analyzer.input('one'));
sourceTwo.connect(analyzer.input('two'));
analyzer.on('data', function(data) {
  console.log(data); // { one: [ 0, 1, ... ], two: [ 1, 1, ... ] }
});

So I'd have be able to create custom audionodes.

[cwilso]

This is likely a meta-issue: make sure we can compose nodes somehow.

[kevinbarabash]

Would it be useful to a factory method for creating custom nodes? Maybe something like this:

AudioContext.prototype.createCustomNode = function(CustomNode, ...options) {
        var node = Object.create(CustomNode.prototype);
        Object.defineProperty(node, "context", {
            enumerable: true,
            configurable: false,
            writable: false,
            value: this
        });
        CustomNode.apply(node, options);
        return node;
    }

It would be nice if we could do write source.connect(customNode, [output], [input]). This would necessitate customNode to define some sort of inputs array that contains one or more AudioNode instances which source would look at when making the connection. The could be polyfilled by doing something like this:

var connect = AudioNode.prototype.connect;
AudioNode.prototype.connect = function(destination, output = 0, input = 0) {
   if (destination instanceof CustomNode) {
       this.connect(destination.inputs[input], output);
   } else if (destination instanceof AudioNode) {
       this.connect(destination, output, input);
   } else if (destination instanceof AudioParam) {
       this.connect(destination, output);
   } else {
       throw "not a valid destination";
   }
};

In the case of a custom node possessing multiple input nodes which have multiple inputs themselves a user would have to specify which of the inputs in the following way: node.connect(customNode.inputs[0], [output], [subInput])

[cwilso]

Even having a custom "compositorNode" that could have a graph internally would be a fix for this. It really, really sucks to not be able to properly do this, since it means you can't create (e.g.) a "ChorusEffectNode" that can be both connected to and connected.

[hoch]

I would really love to see this happen. I've already gone extra miles to have something like this in my WAAX library. Having this feature on API level would be much more useful then JS polyfills and prototype override/inheritance. If this happens, I can see my plug-ins can be ported with very little effort.

Also another useful thing to have is a collective AudioParam. Some parameters can be abstracted on top of a set of AudioParams. Redirection/collective control on them makes much more sense inside of CompositorNode.

@chris Do you have any idea/proposal for this?

[cwilso]

I'm not sure this should be v.next.

[joeberkovitz]

TPAC: @cwilso to verify that de facto subclassing/extension of native nodes can be done, then close.

[joeberkovitz]

I believe that there's no reason this would not be addressed by the existence of regular constructors as per #250, and the ability to delegate from a subclass to the prototype via Object.create(<superclass>.prototype) and <superclass>.call(this) in the subclass constructor. Does anyone disagree with this? If not I will close as a duplicate of #250.

[hoch]

@domenic I think the working group has reached the agreement. Could you take a look at the comments above and let us know TAG's decision?

[domenic]

I'm having a hard time understanding what the concrete proposal is for changing the spec based on the above comments. Would you be able to summarize?

[hoch]

@domenic Sorry about the confusion. Here is the summary:

• We are introducing a new constructor pattern for all types of audio nodes, that can be subclassed into a custom node by developers. An example would be:

class CustomGainNode extends GainNode {
  // Override an existing method.
  connect () {
    console.log(' connected!');
    super.connect.apply(this, arguments);
  }

  // Adding a new method.
  mute () {
    this.gain.setValueAtTime(0, 0);
  }
}

• In terms of the composition of a custom node (by assembling multiple built-in audio nodes), I have a separate proposal. It is unfortunate that we still need to use the shim, but I could not find a better way to do this.
• AudioWorkletNode class can be extended if you need to access the actual audio samples. Proposal 1 and 2 both does not provide this feature because the actual audio process method cannot be exposed to JS. The IDL proposal of AudioWorkletNode is here.

Hope this helps and let me know if you need more clarification.

[domenic]

That plans sounds pretty good!

A way to improve https://github.com/GoogleChrome/web-audio-samples/wiki/CompositeAudioNode to avoid having to override connect would be to add an overload to connect() that accepted something like a dictionary whose single member is input. Then, any subclass which exposed a public property named input (e.g. if you changed your example to use input instead of _input) would automatically work as an argument to connect().

(Alternately you could pick a name that is more specific to this adapter purpose like innerNode or connectableAudioNode or something, instead of input.)

[hoch]

@domenic Thanks for the idea. I will certainly make some improvements!

[hoch]

@joeberkovitz @mdjp What would be the next step here?

[joeberkovitz]

Seems to me we can just proceed on cleaning up the constructor definitions
as per the other issue filed on this point, and clarify that they are in
the global window namespace.

I don't think we need to do anything special to address the
composite-node-construction case for v1 although it's nice to get a better
pictre of how this might be done.

...Joe

. . . . . ...Joe

Joe Berkovitz
President
Noteflight LLC

+1 978 314 6271

49R Day Street
Somerville MA 02144
USA

"Bring music to life"
www.noteflight.com

On Fri, May 20, 2016 at 10:49 AM, Hongchan Choi notifications@github.com
wrote:

@joeberkovitz https://github.com/joeberkovitz @mdjp
https://github.com/mdjp What would be the next step here?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#251 (comment)

[hoch]

Then perhaps we can move this issue to the ready-for-editing status.

[joeberkovitz]

done!

[joeberkovitz]

Now we have constructors, this isn't an issue any more.

[dy]

Is there an info was the extending implemented by any browser? Chrome 91 doesn't seem to allow that.

[hoch]

Chrome 91 doesn't seem to allow that.

Can you elaborate what you tried and what did not work?

[dy]

@hoch I mean your example:

class CustomGainNode extends GainNode {
  mute () {
    this.gain.setValueAtTime(0, 0);
  }
}
let ctx = new AudioContext
let g = new CustomGainNode(ctx)
g.mute()
// VM207:1 Uncaught TypeError: g.mute is not a function
//    at <anonymous>:1:3

Can that be considered a chromium bug? Is that expected to be implemented?
Firefox, Safari support that.
Filed a bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1229154

[hoch]

I responded on the bug, but it seems working correctly for me. We can continue the discussion on the bug, not here.