Consider using maps for main->worklet inputs rather than creating individual properties on the AudioWorkletNode instance.

[bfgeek]

I've talked with some other folks on web platform, folks are pretty divided to whether we actually want to do this or not; we don't really have anything like this on the platform today, and it makes it difficult to expand the AudioWorkletNode later in time.

As a concrete example, it would limit a future spec from adding a new property to AudioWorkletNode.

One option Instead is having two maps for both params & properties.

For example:

const ctx = ...;
const node = new AudioWorkletNode(ctx, 'foo');
node.parameters.get('freq').value = 5;

node.properties.set('buffer', buffer);

interface AudioParamMap {
  readonly maplike<DOMString, AudioParam>;
};
interface AudioPropertyMap {
  readonly maplike<DOMString, *>;
};

partial interface AudioWorkletNode {
  readonly attribute AudioParamMap parameters;
  readonly attribute AudioPropertyMap properties;
};

cc/ @domenic @esprehn @cwilso @hoch

[joeberkovitz]

This seems to run against the goal of having custom nodes look very much like native ones.

[hoch]

I agree with @joeberkovitz. This completely breaks our convention. Although I see the benefit of this pattern, but not worth pushing this for V1.

[domenic]

It seems important to make this decision in v1, because if you go the current route, you will never be able to add any new properties to AudioWorkletNode in v2 (since they might be being used by author code).

[joeberkovitz]

The TAG also gave us feedback that subclassing built-in nodes should be possible. Since any subclass could add custom property getters and setters, doesn't your argument also apply to any subclasses of built-ins that developers might create?

[bfgeek]

No, it is subtle, and isn't quite the same; for example:

class Something {
  method() {}
}

class ExtendingClass extends Something {
  method() { super.method(); }
}

const thing = new ExtendingClass();
thing.method == ExtendingClass.prototype.method; // true

With the current spec:

class Something {
  constructor() { this.method = 5; } // I think CreateDataProperty effectively does this.
  method() {}
}

class ExtendingClass extends Something {
  method() { super.method(); }
}

const thing = new ExtendingClass();
thing.method == ExtendingClass.prototype.method; // false

This will mean that a class extending an AudioWorkletNode won't be able to intercept any properties set by the engine. You'll have to jump through hoops, (either with a proxy, or a Object.defineProperty magic).

A use-case for this might be validating on the main thread the value of a property (i.e. is 'sine', 'square', etc).

@domenic is the above code equiv. to CreateDataProperty?

[joeberkovitz]

@bfgeek I understand the case you are positing here, but I don't believe it should be the use case "driving the bus". We want to make properties/params of AudioWorkletNodes easy for developers to use and understand, and with behavior and syntax comparable to those in native nodes which already exist. (I believe this ability to code up comparable-to-native behavior purely in JS was another TAG request.) So support for the kind of same-name override of properties you described is nice, but perhaps not as important as this other goal.

Furthermore, subclasses of AudioWorkletNode are likely to be defined by the very same author that is creating the companion classes of AudioWorkletProcessor which expose the properties/params that might be overridden.

But... Let me elaborate what I meant by mentioning subclasses, which related to your original remark about futureproofing and not so much to the ability to intercept engine properties. Today, you can do this:

class MyOscillatorNode extends OscillatorNode {
  function get octaveDetune() { return this.detune / 1200;  }
  function set octaveDetune(octaves) { this.detune = octaves * 1200;  }
}

In the future, OscillatorNode could introduce its own nativeoctaveDetune property and perhaps define it differently, which would clash with the existing subclass definition. Yet, we're all OK with subclassing OscillatorNode (and I think we should be -- it's a good thing).

So I think the potential exists across the board for future name clashes. This, too, feels like it is acceptable in the light of increased ease of custom node development.

[esprehn]

I think there's a bunch of different things going on here that we need to address:

• This feature is using data properties on the instance. The platform doesn't use data properties though, and neither does the existing AudioNodes. These need to be getters and use hidden slots. They're still on the instance which is very strange, everything is on prototypes elsewhere in the platform. Note that the current spec is also creating writable data properties which also does not match existing nodes where these params are all readonly.
• This feature is vending polymorphic objects from it's constructors. I don't think any part of the platform does this today, this would be the very first time. I'm not very comfortable with that, it's hard to make fast in the engine, and it's the opposite of what we're telling authors to do.
• This feature is combining AudioParam inputs with generic inputs which are structured cloned. We need to unpack those into two different things. Having magic structured cloning of properties on instances of interfaces is not something I want to do in the platform.
• If you want classes that look like the regular AudioNodes, which seems totally reasonable, then you want something that synthesizes classes with the correct getters on the prototype. This is also important because unless we do this there's no way to check a node's type with instanceof since the current spec is just vending type specific instances without a type class.

ex.

var MyNode = AudioThinger.getWorkletClass("MyNode");
var node = new MyNode();
node instanceof MyNode;
class MyOtherNode extends MyNode { } // use the same worklet backend as the MyNode.

This is fine, but kind of strange. Instead if we use a map of params you can do this work yourself.

class MyNode extends AudioWorkletNode {
  constructor(context) {
    super(context, "MyNode");
  }
  get foo() { return this.params.get("foo"); }
  set bar(value) { this.params.get("foo").value = ...; }
} 

Doing it this way gets you a real API surface which looks identical to what other AudioNodes does, it also gets you a real class you can subclass and add instanceof checks against, it also avoids the polymorphic instances and the structured cloning of random properties on the object (which is not something I want to add to the platform).

So I think what we need to do here is decide if we want to have the platform vend classes like the first example, or if we want the platform to use two maps and you subclass to add pretty APIs yourself. I much prefer the second option, it's far less magical.

[joeberkovitz]

Interesting. Although I wouldn't describe what we have currently speced as "vending polymorphic objects" (because it's all happening at the instance level), yes, the practical intent was to support a kind of polymorphism for AudioWorkletNodes. The examples above do show that the way we're doing it right now has problems, although less from a futureproofing standpoint (Ian's initial concern) and more because we wind up with something that no longer can behave like a real subclass and expose properties like a subclass would do.

My feeling has always is that best practice that users would nearly always extend AudioWorkletNode to create their own "real API surface". Perhaps this makes the use of params and properties maps more palatable -- it is certainly still way easier to write a subclass that slaps a nice surface on these maps, then to implement a bunch of messaging back and forth.

[hoch]

Okay, then one more followup question: how does AudioWorkletProcessor access these maps? For AudioParam, the communication between AWN/AWP is taken care of by AudioParam's special infra. However, I am not sure how AWP can access AWN's property map. Are we going to have a special event for that?

[hoch]

Just to flesh out @esprehn's example:

class MyWorkletNode extends AudioWorkletNode {

  // The construction creates 'parameters' and 'properties' maps.
  constructor (context, options) {
    super(context, 'MyNode', options);
  }

  // Since |bitDepth| is an AudioParam, so we only need a getter.
  get bitDepth () { return this.parameters.get('bitDpeth'); }  

  get type () { return this.properties.get('type'); };
  set type (value) { this.properties.set('type', value); }
}

I believe this gives us pretty much what we want. So the usage would be:

var context = new AudioContext();
var myWorkletNode = new MyWorkletNode(context, { bitDepth: 8 });
myWorkletNode.bitDepth.value = 2;
myWorkletNode.bitDepth.linearRampToValueAtTime(12, 2.0);
myWorkletNode.type = myWorkletNode.type === 'softclip' ? 'hardclip' : 'saturate';

[joeberkovitz]

@hoch I think the answer to your question is:

AWPs need one of the following in order to be aware of properties:

• propertyChange(propName, value) event handling on an AWP. (base class implementation does nothing)
• a new process() parameter that is a map of all current property values: process(inputs, outputs, parameters, properties). Presumably properties that were never set are not bound in this map.

I sort of like the second option which @bfgeek proposed. It eliminates the need for the AWP to manage storage for properties, and it also eliminates any uncertainty about the timing at which a propertyChange() invocation occurs. Someone objected that properties don't change on a render-quantum basis, but presumably this map is managed by the engine.

[hoch]

Yeap. Just had a short chat with @esprehn, @bfgeek and @rtoy. Although its presentation is somewhat different to our convention, I am now convinced that this will be beneficial for developers in terms of the extensibility. The conventional interface can be easily built by a simple polyfill.

constructor (context, options) {
  super(context, 'MyNode', options);
  for (let [name, audioparam] of this.parameters)
    this[name] = audioparam;
  for (let [name, value] of this.properties)
    this[name] = value;
}

Also I missed Ian's second proposal about passing parameters/properties as arguments into process() method. It is easy to reason about and also removes the redundant storage in the AWP side. Good stuff.

[joeberkovitz]

Great. What is the recommended way to document the initialization of these maplike members in the description of the AudioWorkletNode constructor algorithm? I note that maplike doesn't seem to be official WebIDL just yet. Are we signing up for just set() and get() or the entire maplike proposed interface?

[hoch]

I think it's pretty well-defined and being used in other WD as well. Do we need a second opinion on this?

I think what @bfgeek suggests above is pretty much what we need. Naturally, the normative text for the instantiation process must be revised.

interface AudioParamMap {
  readonly maplike<DOMString, AudioParam>;
};

interface AudioPropertyMap {
  readonly maplike<DOMString, *>;
};

interface AudioWorkletNode : AudioNode {
  readonly attribute AudioParamMap parameters;
  readonly attribute AudioPropertyMap properties;
}

Two things:

1. AudioPropertyMap sounds too generic to me. Perhaps AudioNodePropertyMap?
2. AudioWorkletNode extends AudioNode and AudioNode has its own attributes (channelCount, channelCountMode, channelInterpretation). I think we can register them into the map when constructing the node.

Maybe, just maybe - we can apply this map-like attributes to all other AudioNodes? It makes sense for the AudioNode base class to have these generic interface. Currently developers have to go through few hassles to figure out the list of available parameters in the node and their names. This can be done easily with the new pattern:

for (let [name, value] of node.parameters) {
  // 1. You can properly obtain the 'name' of a parameter.
  // 2. You can properly get the enumeration of AudioParam objects in this node.
}

As @domenic pointed out, this has to happen soon (if WG agrees to it) since we just pushed out the new constructor pattern. Otherwise, many developers start to claim parameters or properties in their subclass.

[hoch]

Are we signing up for just set() and get() or the entire maplike proposed interface?

We should support the complete maplike feature since it has many useful methods/attributes. No? In terms of the normative steps, I think it's also well-defined in the ES spec.

https://tc39.github.io/ecma262/#sec-map-constructor https://tc39.github.io/ecma262/#sec-map.prototype.get https://tc39.github.io/ecma262/#sec-map.prototype.set

For the construction of the storage, I think these are all we need. @joeberkovitz WDYT?

[joeberkovitz]

SGTM.

But note that when we document how the properties map is initialized in the AWN constructor sequence, we can't call set() because that implies posting the change to the AWP in the audio thread (and we want the AWN/AWP to be completely synced up at initialization). Instead I think we need narrative prose that describes the initialization of the map entries. See https://heycam.github.io/webidl/#idl-maplike which states:

The map entries of an object implementing a maplike interface is empty at the of the object’s creation. Prose accompanying the interface can describe how the map entries of an object change.

[hoch]

Maybe I am missing something.

1. The AWN constructor invoked. an AWN instance created.
2. Create an AWP instance.
3. Add two empty maplike objects to the AWN instance.
4. Retrieve static parameters/properties from the AWP definition.
5. Iterate them to fill two maplike objects in the AWN instance.

Where do we need to 'post changes'? The construction process should be theoretically 'atomic', so shouldn't be an issue.

[joeberkovitz]

I think we're agreeing violently here. We want to avoid posting changes. My point is simply that your step 5 "Iterate them to fill two maplike objects in the AWN instance" should be described in terms of populating the maplikes, not in terms of calls to set() which would involve async posting of data.

[hoch]

@joeberkovitz @domenic @esprehn @bfgeek

I am raising a question out of the comment above, because I think it is important for us to resolve this issue:

AudioWorkletNode extends AudioNode which has own attributes (channelCount, channelCountMode, channelInterpretation, numberOfInputs, numberOfOutputs). How do we handle them in terms of the newly introduced property map?

Here is an example:

var node = new AudioWorkletNode(context, 'myNode');

console.log(node.numberOfInputs); // Should this work?
console.log(node.properties.get(numberOfInputs)); // How about this?

Here are options we can choose (feel free to throw ideas if you have anything else):

1. AudioWorkletNode still extends AudioNode, so we should keep these properties as they are, which are plain getters/setters. Then we leave the property map alone.
2. We put them in the property map and don't expose them as getter/setter.
3. We keep them in both: in the property map and getter/setter.

Sadly it won't be pretty no matter what we do, but I guess we have to pick one.

[joeberkovitz]

I think that these inherent AudioNode attributes fall into a separate categoryt because they are already exposed via normal getters and setters on the class. The architectural issue arose because we had a variety of getters and setters being dynamically attached on the instance making it impossible to refer to them or override them via normal polymorphism.

So I think the option that makes sense is option 1. As a corollary, the properties maplike refers only to custom properties defined by the registered AWP subclass.

I don't find this ugly. But maybe I just have low standards :-)

[hoch]

@joeberkovitz If we go down that road, we are sort of giving up this generic maplike interface for the AudioNode. I briefly mentioned the possibility of using this unified interface for all the AudioNode.

With that said, I am also fine with having the new maps only in AudioWorkletNode.

[joeberkovitz]

@hoch I missed that angle but now I understand what you mean. However, for AudioParams, we could easily have a generic maplike parameters. The problem only exists for properties/attributes as proposed in #988.

The properties problem goes a bit deeper, though.

First, at some point we inevitably have to make a distinction between properties that are clearly "audio-node-related" (e.g. AWN.someCustomProperty or OscillatorNode.type or AudioNode.channelCountInterpretation) and those which aren't (e.g. .prototype, to name a primitive Object property that is not even defined by AudioNode). So, wherever we draw the line, we're going to have some sort of distinction between attributes that belong in such a map, and attributes that don't.

Furthermore, some kinds of AWN properties do not belong in this map, namely those read-only properties which communicate state from the AWP out to the AWN. The proposed mechanism in #988 does not support that type of communication; it only supports writable properties that communicate from AWN to AWP. Given this issue, how would we decide which attributes should be exposed in the map by any particular native AudioNode subcalss?

Given these observations I wonder if it's best to do one of the following things:

1. Expose writable-properties and parameters maps in AWN (not in AudioNode) and explicitly state that they only expose AWP-declared properties and parameters.
2. Expose writable-properties maps in AWN (not in AudioNode), and expose a completely generic parameters map in AudioNode.
3. give up on declarative support of writable properties altogether (as well as the associated map) and say that AWN/AWP properties are a roll-your-own proposition in both directions, based on sendData(). Expose a completely generic parameters map in AudioNode.

[hoch]

I prefer to move fast with the option 1. I think we should try this new 'map-like' paradigm with AWN/AWP first then consider the wide adoption in the v.next.

Do we reduce any feature coverage by losing the 'read-only' property? Having a user-defined read-only property in a JS object sounds weird to me.

[joeberkovitz]

I agree that option 1 is a good place to start and I think it's what @bfgeek was suggesting.

I don't think we reduce any feature coverage by doing this. Read-only properties (e.g. the readout of a VU Meter) can be easily implemented by having the AWN expose a getter and no setter. (This doesn't seem weird to me and JS supports it.) The getter would return a value that is updated via sendData() from the AWP side when it changes -- similar to what we have in our VUMeterNode example now.

[joeberkovitz]

As per my comment https://github.com/WebAudio/web-audio-api/issues/988#issuecomment-258281334 I believe that we should restrict AWPs to declaring AudioParams exposed by AWNs, not settable properties. AWP properties (both settable and read-only) can be handled in a more general way by implementing regular setter methods in AWN subclasses that communicate by sending data to the AWP. This immediately removes a number of difficulties, and does not eliminate any functionality.

Let me recap the issues raised so far here, as I understand them, along with my current thinking about each one.

• Possible collisions between names of custom AudioParams/properties and future additions to base classes (@bfgeek).

I think that this issue can occur under any design. Even with maplike namespaces, people will (and, indeed, ought to) subclass AudioWorkletNode to directly expose getters, setters and methods for their custom AudioParams and properties that delegate to the maplikes. These can also collide with future additions. The TAG wanted such subclassing for any native node, and we also think it's important here too. A custom node should look like a native node.

• Use of data properties on the instance rather than getters (@esprehn)

Yes, this seems to me like a mistake in the current spec. To the extent that we expose anything on the instance, it should be either via slots or via maplike namespaces.

• Vending of polymorphic objects from constructors (@esprehn)

I agree that this feels weird, and it is another argument for maplikes in support of exposing AudioParams. Developers can implement getters directly on the AWN that hide the maplikes from clients of their custom nodes.

• This feature is combining AudioParam inputs with generic inputs which are structured cloned. We need to unpack those into two different things. Having magic structured cloning of properties on instances of interfaces is not something I want to do in the platform. (@esprehn)

I don't think the spec asks for this, so I'm a bit confused. What are the instances of interfaces that would be structured-cloned?

In summary, I now think a couple of good alternative options are...

1. Attach declared AudioParams to AWN getters on the instance
2. Expose read-only maps of AudioParams in a parameters maplike (which can be wrapped in custom-subclass getters for prettiness)

[hoch]

@joeberkovitz Thanks for the nice summary.

The settable properties represents the current state of the node/processor. On the other hand, the data produced by the AudioParam operation is volatile and temporary. So I think it is sensible to treat them differently. Feeding the audio param data directly into the process() method is quite nice for that reason.

So my preference is:

1. The map-like data structure for AudioParams.
2. Settable properties on both AWN and AWP, then use sendData()/ondata messaging channel to synchronize them.

[hoch]

After the discussion with @esprehn, we can use the design in my previous comment without introducing any difficulty in our existing platform.

The following is a simplified example:

Global Scope

class MyNode extends AudioWorkletNode {
  constructor() {
    super('my-node');
    this._prop = 0;
    this.ondata = event => {
      console.log('volume = ' + event.data.volume);
    }
  }

  set prop(value) {
    this._prop = value;
    this.sendData('prop', this._prop);
  }

  get prop() {
    return this._prop;
  }

  get param1() {
    return this.parameters.get('param1');
  }
}

AudioWorkletGlobalScope

registerProcessor('my-node', class extends AudioWorkletProcessor {
  static parameterDescriptors = [{
      name: 'param1',
      defaultValue: 0.5,
      maxValue: 1.0,
      minValue: 0.0
  }];

  constructor() {
    this._prop = 0;
    this._volume = 0;
    this.ondata = event => {
      this._prop = event.data.prop;
    }
  }

  process(input, output, parameters) {
    let param1Values = parameters['param1'];
    /* do something with param1Values and this._volume */
    this.sendData({ volume: this._volume });
  }
});

[joeberkovitz]

That seems great. Are you able to put together a PR that describes this? It looks like we'd need to rework the AWN/AWP constructor algorithm to initialize and then modify the AWN's parameters maplike, instead of setting AudioParam properties directly on the AWN itself.

[hoch]

Sure. I think the actual spec change is trivial. The issue was more of the capability of underlying infrastructure. Since this issue is already assigned to me, so I will submit a PR soon.

[hoch]

@joeberkovitz So is this ready for editing?

[joeberkovitz]

I think so, since we've already discussed this at length in the WG.

[joeberkovitz]

@hoch where will sendData/onData be addressed?

[hoch]

@joeberkovitz You mean we should file another issue for it? I think they are a part of the interface of AWN/AWP. I was thinking to describe them in the IDL just like what we did for postMessage/onmessage.

[joeberkovitz]

@hoch I just wanted to make sure they were going to get described in the IDL as part of some issue somewhere. Are you planning to do it as part of this one?

[hoch]

@joeberkovitz Yes, I think it should be. Do you envision otherwise? We can certainly land the PRs gradually/incrementally. Your call.

[joeberkovitz]

@hoch that's fine to do it in this issue, just want to get postMessage/onmessage replaced both in the IDL and in the examples.