WebAudio / web-audio-api

The Web Audio API v1.0, developed by the W3C Audio WG
https://webaudio.github.io/web-audio-api/
Other
1.05k stars 167 forks source link

Dynamic Lifetime section should include explicit lifetime of AudioWorker #475

Closed cwilso closed 7 years ago

cwilso commented 9 years ago

...capturing the importance of audio process event handler.

joeberkovitz commented 9 years ago

Any reason not to consider this Ready For Editing?

padenot commented 8 years ago

This is blocked on the audio worker text.

joeberkovitz commented 8 years ago

This problem of lifetime definition now needs to be revisited in concert with the AudioWorklet design of #869. Marking for WG review.

In the AudioWorker design, an AudioWorkerNode's lifetime was influenced by the existence of a live onaudioprocess callback reference (see https://lists.w3.org/Archives/Public/public-audio/2015JanMar/0040.html) This approach is no longer viable since an AudioWorkletProcessor does not use event dispatch to invoke its process() method, and there is no access to the object that maintains a live reference to the actual AudioWorkletProcessor itself.

Perhaps the WG needs to contemplate adding some sort of explicit method on AudioWorkletProcessor to remove all internal audio-thread references to it, which can be called from within its process() method. This could then be woven into the existing node-lifetime section.

joeberkovitz commented 8 years ago

Here is a rough proposal for us to discuss today regarding AudioWorklet lifetime, in reference to the Lifetime section at https://webaudio.github.io/web-audio-api/#lifetime-AudioNode:

joeberkovitz commented 8 years ago

In #959 I have proposed (echoing a suggestion by @hoch) that process() simply return a value that controls the existence of a special "active reference". The reference can be created and destroyed at will by this return value, and there is no immediate obligation on the part of the UA to immediately stop the processing of the node: its lifetime remains controlled by its references, and the active reference is only one such references among others.

This makes it very easy to implement an AudioWorkletNode that goes away when its inputs are gone: just don't return anything from process() in which case there is no additional active reference. To create an AudioWorkletNode that acts as a source in the absence of connection references, though, or to implement a tail-time concept, one must return true from process().

hoch commented 8 years ago

@joeberkovitz Just to make it clear, I believe that idea was suggested by @bfgeek.

I'll have to think about the idea of returning a boolean value from process() - I am not sure how the returned value gets processed by the system. So is it UA's responsibility to tear down a processor after its member method returns false? Why can't we just called something like terminate() to deactivate the processor? Not sure how this is different from returning a boolean, but it's more explicit and readable.

joeberkovitz commented 8 years ago

@hoch The returned value merely drives whether or not there is a reference that has the effect of retaining the node and its processor via the normal already-documented lifetime mechanism.

This does not mean that the UA is responsible for tearing anything down when the method returns false. It means that the UA does its usual thing of tracking node references, and allowing GC of a node whose references all go away. Only upon GC would process() cease to be called.

I'm not wedded to using a return value, but on thinking about an explicit terminate(), this seemed much less flexible than the ability to affect whether or not the node gets pinned into the graph.

(Sorry @bfgeek if I didn't attribute properly)

joeberkovitz commented 8 years ago

In particular, what's nice about this is that if you don't return true and pin the node in memory, you get a very useful behavior where an AudioWorkletNode goes away as soon as all connections and other JS references to it go away.

joeberkovitz commented 8 years ago

Here are some comments from today's WG call regarding this issue.

One main question is whether we use the existing mechanism of references or an approach requiring explicit termination by AWP code (e.g. terminate()). Given the cases of existing nodes which need to deal with retention or GC, a useful approach seemed to be to allow an AWP to have programmatic control over a reference which retains it. If you don't want explicit retention then you will still free the reference when other (input/output) references are removed

@rtoy also suggested a variation in which instead of a return value, there is a mutable boolean attribute of the AWP (e.g. active) that controls whether the node is active or not, and can be set or cleared by the code in process().

Finally @rtoy also pointed out that he is aware of implementation guidance that suggests it's generally a good idea to require developers to explicitly free any kind of heavyweight object (which, given its use of JS scopes, this is). Which might lead us to say, "this thing will live for ever unless you terminate it, references or not".

We decided to leave the approach open for the time being to give others a chance to comment.

hoch commented 8 years ago

@joeberkovitz

What if your JS reference is still there, but the process() method just returned false? I think we have same issue with the explicit terminate() call. You can terminate it even when you're actively using the node in the main thread code. This doesn't seem right.

After a little chat with @bfgeek, we thought this pattern might work for AudioWorklet.

class MyProcessor extends AudioWorkletProcessor {

  constructor (options) {
    this._isAlive = true;
  }

  process() {
    // do stuff.
  }

  keepAlive() {
    return this._isAlive?
  }

}

The UA will call keepAlive() at the beginning (or the end) of every rendering quantum, and it is to check if this processor is ready to GCed only when a) the associated audio node is dereferenced and b) the audio node does not have any active connection. Thus, developers can put their own heuristic in this method to implement behaviors like tail time and source nodes' automatic disconnection.

Actually this is roughly how the internal implementation works in Chrome's AudioNode. I find it useful because it gives you the ability to define its lifetime, but UA has the ultimate control of it.

rtoy commented 8 years ago

I think we also need to make it clear that once isAlive is set to false (or not true), any further changes are ignore. Then we don't have to worry about specifying what happens if set it false, the set it back to true some time later.

rtoy commented 8 years ago

I talked to a colleague at work who has implemented many GC systems, and his suggestion was essentially identical to Joe's suggestion about having process() return a value. One change was that he suggested that we explicitly check for true. Any other value means we're done.

After discussing this a bit with @hoch, this method is essentially identical to https://github.com/WebAudio/web-audio-api/issues/475#issuecomment-245960736. We basically take the return value from process() and stuff it into isAlive every rendering quantum.

joeberkovitz commented 8 years ago

This will be fixed by the forthcoming PR for AWN lifetime.

joeberkovitz commented 8 years ago

Sorry, this is lengthy. But in this case, TL;DR means "Too Long, Do Read".

I'm capturing here a set of use cases and "thought experiments" that came up at the F2F and need to be set out and understood. Although #959 proposes a solution, It will be helpful to describe these use cases in more detail before we go down a path.

The lifetime issue has two main areas of application: input processing nodes and source nodes.

  1. Some nodes process an input and have some ongoing state that continues for a while after that input is disconnected (the "tail-time"). Example: consider an AudioWorklet implementation of DelayNode.

It's a bit tricky though: inputs can be disconnected from such a node, and later new inputs can be connected to it. (This is not new to AudioWorkletNodes, it's true for ordinary DelayNodes as well.) Consider this case 1A:

  var delay = new AudioWorkletNode(ctx, "Delay", {tailTime: 0.1});
  osc.connect(delay);
  delay.connect(ctx.destination);  // osc -> delay -> destination
  setTimeout(500, function() {
    osc.disconnect();  // delay's tail time starts here
    delay = null;  // JS reference is annihilated. 0.1 sec later, delay can be GCed
  });

Contrast with this one, 1B:

  var delay = new AudioWorkletNode(ctx, "Delay", {tailTime: 0.1});
  osc.connect(delay);
  delay.connect(ctx.destination);  // osc -> delay -> destination
  setTimeout(500, function() {
    osc.disconnect();  // delay's tail time will elapse, but there's still a JS reference to it
    setTimeout(500, function() {
      osc.connect(delay);  // delay has inputs once again, and its output resumes
    });
  });

This has implications for the AWP implementation. In 1A, process() would monitor the connected status of its input (see #970) and return true up until the 0.1 sec tail-time elapses following osc's disconnection, then false after that. After that, the AWN/AWP pair can be GCed at some point of the UA's choosing.

But consider 1B. Here, process() presumably returns true and then false after the tail-time elapses, just like the previous case. However, there is a JS reference in the main thread, so the AWP must stay alive and continue to be called. Upon reconnection, process() must return true once again (and this cycle of disconnection/reconnection could be repeated indefinitely).

This thought experiment shows that returning false from process() (or any other AWP interface you like) cannot simply force the node to stop working immediately. It only indicates that the node no longer requires the engine to keep it alive. Hence the notion of returning true forcing the existence an "active reference" (similar to a tail-time reference) that pins the node in the graph.

  1. On to source nodes... Consider something like an NoiseGenerator that can stop and start at scheduled values of currentTime.
  var noise = new AudioWorkletNode(ctx, "Noise", {startTime: 5, stopTime: 10});
  noise.connect(ctx.destination);
  noise = null;

In this case, there are no JS references. The desire is for noise to stick around until its stop time is reached and then go away, just like an OscillatorNode. The AWP implementation must return true from process() until contextInfo.currentTime == 10, and then start returning false to indicate that the node can be GCed. Here again, the concept of process() return value governing some kind of active reference works out.

These two cases seem reasonably well behaved (and they seem like the main things people want from node lifetime), but murky questions arise: what if a node returns true and false in a sort of glitchy way, say alternately returning true and then false from successive render quanta? How quickly does the result of returning false from process() impact UA decisions about the node's lifetime?

My proposal is that we simply don't say. Instead, we say something like this:

That last guarantee is not very specific but it does seem to satisfy the use cases.

Comments?

rtoy commented 8 years ago

On Wed, Sep 28, 2016 at 1:16 PM, Joe Berkovitz notifications@github.com wrote:

Sorry, this is lengthy. But in this case, TL;DR means "Too Long, Do Read".

I'm capturing here a set of use cases and "thought experiments" that came up at the F2F and need to be set out and understood. Although #959 https://github.com/WebAudio/web-audio-api/pull/959 proposes a solution, It will be helpful to describe these use cases in more detail before we go down a path.

The lifetime issue has two main areas of application: input processing nodes and source nodes.

  1. Some nodes process an input and have some ongoing state that continues for a while after that input is disconnected (the "tail-time"). Example: consider an AudioWorklet implementation of DelayNode.

It's a bit tricky though: inputs can be disconnected from such a node, and later new inputs can be connected to it. (This is not new to AudioWorkletNodes, it's true for ordinary DelayNodes as well.) Consider this case 1A:

var delay = new AudioWorkletNode(ctx, "Delay", {tailTime: 0.1}); osc.connect(delay); delay.connect(ctx.destination); // osc -> delay -> destination setTimeout(500, function() { osc.disconnect(); // delay's tail time starts here delay = null; // JS reference is annihilated. 0.1 sec later, delay can be GCed });

Contrast with this one, 1B:

var delay = new AudioWorkletNode(ctx, "Delay", {tailTime: 0.1}); osc.connect(delay); delay.connect(ctx.destination); // osc -> delay -> destination setTimeout(500, function() { osc.disconnect(); // delay's tail time will elapse, but there's still a JS reference to it setTimeout(500, function() { osc.connect(delay); // delay has inputs once again, and its output resumes }); });

This has implications for the AWP implementation. In 1A, process() would monitor the connected status of its input (see #970 https://github.com/WebAudio/web-audio-api/issues/970) and return true up until the 0.1 sec tail-time elapses following osc's disconnection, then false after that. After that, the AWN/AWP pair can be GCed at some point of the UA's choosing.

But consider 1B. Here, process() presumably returns true and then false after the tail-time elapses, just like the previous case. However, there is a JS reference in the main thread, so the AWP must stay alive and continue to be called. Upon reconnection, process() must return true once again (and this cycle of disconnection/reconnection could be repeated indefinitely).

This thought experiment shows that returning false from process() (or any other AWP interface you like) cannot simply force the node to stop working immediately. It only indicates that the node no longer requires the engine to keep it alive. Hence the notion of returning true forcing the existence an "active reference" (similar to a tail-time reference) that pins the node in the graph.

On to source nodes... Consider something like an NoiseGenerator that can stop and start at scheduled values of currentTime.

var noise = new AudioWorkletNode(ctx, "Noise", {startTime: 5, stopTime: 10}); noise.connect(ctx.destination); noise = null;

In this case, there are no JS references. The desire is for noise to stick around until its stop time is reached and then go away, just like an OscillatorNode. The AWP implementation must return true from process() until contextInfo.currentTime == 10, and then start returning false to indicate that the node can be GCed. Here again, the concept of process() return value governing some kind of active reference works out.

​At least we can easily distinguish between the case of a pure source node (numberOfInputs = 0) and a processing node (numberOfInputs > 0). ​

These two cases seem reasonably well behaved (and they seem like the main things people want from node lifetime), but murky questions arise: what if a node returns true and false in a sort of glitchy way, say alternately returning true and then false from successive render quanta? How quickly does the result of returning false from process() impact UA decisions about the node's lifetime?

My proposal is that we simply don't say. Instead, we say something like this:

  • Returning true from process() enqueues a message to the control thread to retain an active reference to the AWN.
  • Returning false from process() enqueues a message to the control thread to remove any active reference to the AWN.
  • At a point of its choosing, the UA may remove an AWN that lacks any live references (note that these are not only active references, but also JS references and input-connection references as per the node Lifetime section). This also removes the AWP from the audio processing graph.
  • As a corollary to the message-queuing definitions above, if no live references to an AWN exist, and its AWP returns a consistent value, its live/dead state will eventually reflect that value.

​Is this possibly racy? If process() returns false, the control thread eventually removes the active reference. Later, process() returns true (for whatever reason). If GC has already started, the node will get collected even though the intent was to keep the node alive.​

​But if GC didn't happen, the node will stay alive, producing an output. This makes the behavior really flaky, depending on exactly when GC happens and when process returns true or false.​

That last guarantee is not very specific but it does seem to satisfy the use cases.

Comments?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WebAudio/web-audio-api/issues/475#issuecomment-250286515, or mute the thread https://github.com/notifications/unsubscribe-auth/AAofPCwFAzs3bQ8Fn8z2iLXKd3juQKGWks5qussQgaJpZM4DbswM .

Ray

joeberkovitz commented 8 years ago

@rtoy I guess I would say the behavior can be flaky insofar as the results from process() are flaky. If process() returns a consistent value given no change in external circumstances, then the UA will eventually respond in a predictable fashion. Given that there is an potential delay between the AWP signalling some intent and the UA deciding that a node is eligible to be GCed, I see no way to avoid this (but maybe someone else does).

Let's look at your case in more detail:

If process() returns false, the control thread eventually removes the active reference. Later, process() returns true (for whatever reason). If GC has already started, the node will get collected even though the intent was to keep the node alive.​

I think this scenario is not realistic, although it's a possible edge case. If process() returns true once again, the only sensible reason for this is because external circumstances changed and the node became reconnected, the AWP noticed this, and is effectively cancelling its tail-time and going back to "normal operation". In order for that to happen, there must have been a JS reference out there (to reconnect the node), in which case GC could not occur.

joeberkovitz commented 7 years ago

@rtoy I have amended #959 in an attempt to remove the flakiness and believe it's ready for review.

The new approach adopts a suggestion by @hoch and ends the AWP's active lifetime when process() returns false and there are no input connections to a node.

This is potentially problematic because new input connections could be supplied, at which point your typical native passthrough node would start working again. @hoch's original suggestion was that a 3rd condition be checked before ending an AWP's life: that there are no existing JS references to the node. But I didn't think it was possible to check that condition synchronously -- doesn't it amount to doing a GC?

rtoy commented 7 years ago

On Thu, Sep 29, 2016 at 7:42 AM, Joe Berkovitz notifications@github.com wrote:

@rtoy https://github.com/rtoy I guess I would say the behavior can be flaky insofar as the results from process() are flaky. If process() returns a consistent value given no change in external circumstances, then the UA will eventually respond in a predictable fashion. Given that there is an potential delay between the AWP signalling some intent and the UA deciding that a node is eligible to be GCed, I see no way to avoid this (but maybe someone else does).

Let's look at your case in more detail:

If process() returns false, the control thread eventually removes the active reference. Later, process() returns true (for whatever reason). If GC has already started, the node will get collected even though the intent was to keep the node alive.​

I think this scenario is not realistic, although it's a possible edge case. If process() returns true once again, the only sensible reason for this is because external circumstances changed and the node became reconnected, the AWP noticed this, and is effectively cancelling its tail-time and going back to "normal operation". In order for that to happen, there must have been a JS reference out there (to reconnect the node), in which case GC could not occur.

​But the processor can live without a reference to the node, right?

Yes, this is a corner case, but something that needs some consideration. I'm ok (mostly) if we say such behavior is undefined. I don't really like having undefined behavior, but sometimes that's the way things are. ​

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/WebAudio/web-audio-api/issues/475#issuecomment-250486580, or mute the thread https://github.com/notifications/unsubscribe-auth/AAofPAH8eye_NVhpHGOd4gk1CMNQtyBPks5qu85IgaJpZM4DbswM .

Ray

joeberkovitz commented 7 years ago

@rtoy I think I've finally identified an approach to AWN/AWP lifetime that avoids some of the unpredictability that bothered some of the folks with the previous approach, where process() returning a falsy value for an otherwise unreferenced AWN did not suppress subsequent calls to process(). It's an adaptation of some suggestions by @hoch.

The new concept has several parts:

  1. associate an active source flag with every AWN, which is initially true.
  2. invoke the AWP's process() method whenever active source is true OR there are connected inputs
  3. assign the return value of process() to the active source flag after each invocation
  4. retain any AWN in memory whose active source flag is set.

This behaves much the same as the earlier "active reference" proposal, with a couple of key differences:

I have updated #959 to reflect draft language describing this idea.

hoch commented 7 years ago

@joeberkovitz Sorry for the late response! In all, I like this approach in terms of writing the narrative in the spec because it's quite intuitive without mentioning another internal storage and all.

bfgeek commented 7 years ago

I talked with @hoch this morning, so I'll let him fill in more of the details, but one concern I have with returning true from process is that a lot of nodes will do this by default causing nodes which are not able to be collected.

An alternative api would be having a tailTime callback which is invoked whenever the source of a node is disconnected, e.g.

registerProcessor('delay', class {
  tailTime(parameters, properties) { // please name this better. :)
    return 1000; // This node has a tail time of 1000ms or some computation.
  }
});

This would be called whenever the node is disconnected, and the UA would be responsible for calling process for the specified period.

In this case the node doesn't get GC'd immediately after, it can be connected to a source during it's tail time, or after and should work as expected.

In this model the only time the node would get collected is when the javascript reference is dropped AND finished rendering its samples (or disconnected from graph). [1]

[1] @hoch can probably explain the lifetime here better.

hoch commented 7 years ago

Let me highlight two more points here:

One missing piece here is how this applies to the source-like AudioWorkletNode. The tail time works fine with the processor type node, but we haven't figured out the pattern for the source-like node.

Since we have two competing proposals: 1) using process() method and 2) introducing tailTime(). We'll (me, @bfgeek and @rtoy) explore the option 2) a bit more and follow up on this thread.

joeberkovitz commented 7 years ago

We don't quite have two competing proposals, until this other option supports source-like nodes as well as input-processing nodes. I look forward to seeing what shape that would take.

A couple of observations: