Re: Audio Workers - please review

On Thu, Sep 11, 2014 at 6:30 PM, Ehsan Akhgari <ehsan@mozilla.com> wrote:

> OK, I think we should remove terminate().  It seems that the exact same
> effect can be achieved by AudioNode.disconnect().
>

Not entirely; disconnect() does not abort the script and cancel any pending
tasks (e.g. setTimeouts, etc.)  Terminate is still a valuable thing to do
on a worker.  Ceasing firing of onaudioprocess is only one of those things.

c) How would different AudioWorkerNodes allocated through different
>>> AudioContext objects interact with each other?  (Here is an example of
>>> something that is not clear, should it be allowed to send an AudioParam
>>> belonging to a node form context 1 to a worker node from context 2?  The
>>> spec is currently silent about this.)
>>>
>>
>> You are correct, it in.  In general, the spec is silent on how AudioNodes
>> created from different AudioContext objects interact with each other.  I've
>> known this was an issue, but had not ensured that we had an issue filed.
>>  Now we do: https://github.com/WebAudio/web-audio-api/issues/351.  I
>> don't think this is specific to Audio Workers.
>>
>
> Hmm.  So I'm a bit confused here.  I think I remember that the spec used
> to prohibit connecting one node from one context to another one from a
> different context, so that was what I had in mind when I wrote the above.
> But checking the spec again, I don't see a mention of that.  Has that
> changed in the spec, or is my memory faulty here?
>

It has not changed in the spec.  We need to specify this.  No,  I don't
expect to be able to connect a node from one context to a node from another
context (and I also don't expect that audio workers in different contexts
to be in the same thread or communicate with each other; but it's not
observable anyway).

I do expect we should be able to share AudioBuffers across threads, though.

e) What is the order in which these worker nodes receive the audioprocess
>>> events (assuming obviously that one such node is not an indirect
>>> input/output of the other)?  Note that with the currently specified API, I
>>> think it is possible for one of these workers to send a MessagePort through
>>> the main thread to another one, and therefore be able to communicate with
>>> the other AudioWorkerNode workers through MessagePort.postMessage, so the
>>> order of execution is observable to script.  (I admit that I don't have a
>>> good solution for this -- but I also don't know what use cases for
>>> transferring information back and forth between these nodes and the main
>>> thread we're trying to address here.)
>>>
>>
>> I don't think this is true; MessagePort is asynchronous, and the firing
>> of onaudioprocess events would be synchronous (more to the point, the
>> system would, I expect, resolve the entire audio graph for a block before
>> processing messages).  I don't expect Audio Workers to communicate with
>> each other, and I don't believe they can observe each others' behavior.
>>
>
> I'm not sure what you mean by MessagePort being asynchronous.  Obviously,
> MessagePort.postMessage() is an asynchronous operation (I think that's what
> you meant, please correct me if I'm wrong.)  What you describe above is my
> understanding of the intention as well, but I don't think that's currently
> specified.
>

MessagePort.postMessage is asynchronous.  That makes any observation of
ordering you could do through communicating with another Audio Worker
through a MessagePort irrelevant; its timing would not let you detect which
is processed first, for example.  Or am I misunderstanding how you would
use MessagePort?


> 2. I feel very strongly against exposing this node type as a web worker.
>>> I think that has a number of undesired side effects.  Here are some
>>> examples:
>>>
>>> a) Even though the HTML web worker termination algorithm says nothing
>>> about killing an underlying worker thread, I think that is what authors
>>> would typically expect to happen, but obviously doing that would not be an
>>> option here.  It is also not specified what needs to be output from the
>>> node after terminate() has been called on it (the input unmodified?
>>> silence? something else?)  Also, given the fact that you can already
>>> disconnect the node from the graph, why do we need the terminate() method
>>> in the first place?
>>>
>>
>> I'm not sure why that's not an option, and the terminate() description
>> DOES say the worker thread should be terminated.  (not the underlying
>> system thread, but that's not intended.)  As Alex said, terminate() is
>> there for uniformity with Workers, and for clarity in enforcing "no more
>> onaudioprocess events, please."
>>
>
> That effect can be achieved by just disconnecting the node.
>

See above.  disconnecting will not halt other processing the audio worker
might be doing.


> b) The API gives you the illusion that multiple AudioWorkerNode's will run
>>> on different DedicatedWorkerScopes.  That, in Web Workers world, would mean
>>> different threads, but that will not be the case here.  I think that
>>> discrepancy is problematic.
>>>
>>
>> Why?
>>
>
> Honestly I can't quite imagine how authors would perceive this.  Here is
> an example: do you think it's unreasonable for an author to expect that
> worker nodes on the same audio context use the same global object?  The
> answer to that question is observable to script.
>

Yes, I think it's unreasonable for an author to expect that worker nodes
would share a global object.  The global object contains the onaudioprocess
event handler; it has a one-to-one correspondence with a single node.  It
HAS to.  Node implementations cannot share data (except through the main
thread, or possibly through a sub-shared worker.


> c) Using DedicatedWorkerGlobalScope is a bit weird in terms of APIs that
>>> are exposed to workers.  For example, should these workers support
>>> onlanguagechange?  What about IndexedDB on workers?  What about nested Web
>>> Workers?
>>>
>>
>> Yes, I would think they would support these.  Particularly nested Web
>> Workers, e.g.
>>
>
> What is the use case in supporting these?
>

Creating worker threads to do multi-threaded audio processing (introducing
latency, of course, to get the data across thread boundaries).  This has
already been requested.


> d) At least on Gecko, Web Workers have specific implementation concerns in
>>> terms of their message queue, event processing model and so on.  It might
>>> be a lot of effort for us to properly implement the observable behavior of
>>> these workers running inside our audio processing thread code (which has a
>>> completely different event processing model, etc.)
>>>
>>
>> That's the point of having this discussion, yes.
>>
>>
>>> e) The topic of whether or not synchronous APIs must be allowed on
>>> workers is being debated on public-script-coord, and it seems like there is
>>> no consensus on that yet.  But I find the possibility of running
>>> synchronous XHR on the audio processing thread unacceptable for example,
>>> given its realtime requirements.
>>>
>>
>> Of course.  That (using synchronous XHR) would be a dumb thing to do. So
>> would "while (true) ;".
>>
>
> Well, labeling these concerns as dumb doesn't really address the issue.  I
> agree with you completely that those are dumb, but the point is that we're
> making them possible, so we should be thinking very hard about the
> consequences of that.  There is always a chance of people doing these
> things.
>

Yees; but that's precisely my point.  People can "while (true) ;" today.
 This isn't new, it isn't unique to using workers or even being in the
audio thread; in practical terms, I'm not convinced it would be any better
to have a constantly-glitching ScriptProcessor in the main thread (because
it's CPU-starved) than to not be able to service the whole audio graph due
to a CPU-starved Audio Worker.  I think they end up being pretty
functionally equivalent.


> I think tying this node to Web Workers opens a huge can of worms.  It will
>>> also keep biting us in the future as more and more APIs are exposed to
>>> workers.  I am wondering if we can get away with a completely different API
>>> that only tries to facilitate the things that authors would typically need
>>> to do during audio processing without attempting to make that a Web Worker?
>>>
>>
>> Only by using some other language to represent an "audio shader", and
>> having that language runtime run in the audio thread.  Anything else would
>> add inherent latency, which would destroy the ability of this to
>> self-describe Web Audio.
>>
>
> Is inventing a new language really our only alternative?  I don't think
> so.  Another option is to run these workers under a restricted and new
> global object that exposed built-in Ecmascript objects, and expose any
> additional APIs that addresses a use case that makes sense to audio
> processing.
>

What do you think is in Workers that won't be needed in audio workers?  In
looking through the list, I'd say pretty much everything could be
interesting.


> 3. What is the purpose of addParameter and removeParameter?  It seems to
>>> me that if we defined a structured clone algorithm for AudioParam (which is
>>> another detail missing from the spec that needs to be clarified anyway) and
>>> make it transferable, then the author would be able to postMessage() an
>>> AudioParam just as easily.  Is there a good reason to have a specialized
>>> method for what is effectively posting an AudioParam to the worker?  (Note
>>> that we'd probably need to add methods to AudioParam for extracting a-rate
>>> and k-rate values at any given time in that case, so that the worker script
>>> can get the right values when it needs them.)
>>>
>>
>> No.  To have AudioParams be connectable (i.e. connect an Oscillator into
>> an AudioParam) with zero latency, they must be evaluated in the audio
>> thread, only in the current block, only when they are needed.  You can't
>> extract ahead of time, or ask for them later; their value buffers need to
>> be handed to the onaudioprocess when it fires.
>>
>
> That's exctly what I meant.  I was not suggesting adding methods for
> arbitrarily peeking into the AudioParam values from the main thread.
>
> (Note that my suggestion may mean exposing these objects through another
> interface to the worker.)
>

I'm confused.  This sounds, then, precisely like what the Audio Worker
exposes (the parameters object's arrays described at
http://webaudio.github.io/web-audio-api/#widl-AudioProcessEvent-parameters
).


> 4. More on the idea of transferring an AudioParam to a worker, that will
>>> probably involve some kind of neutering the object.  It might make sense to
>>> introduce a clone() method on AudioParam as a way for authors to be able to
>>> keep a copy of the object around on the main thread.  That could of course
>>> be a future enhancement idea.
>>>
>>
>> In essence, the design of AudioParams here is much like AudioNodes
>> themselves - one part sits in the main thread, one part sits in the audio
>> thread.  The latter does the heavy lifting.
>>
>
> Well, my point was more towards the state that is stored inside the
> AudioParam (namely the automation events.)  We need to specify what happens
> if you hand off an AudioParam to a worker, and then add or remove an
> automation event to/from it after that.
>

What do you mean by automation events?  The audio processing system knows
how to create a value array from an AudioParam with its connected inputs
and any schedule events; it hands those value arrays to the onaudioprocess
handler.

5. Still more on the idea of transferring an AudioParam, another thing that
>>> we need to worry about is what happens if you try to transfer an AudioParam
>>> that is currently being used somehow (either through being a property on
>>> another AudioNode, or being used as an input to one.)
>>>
>>
>> How would you make such a transfer?
>>
>
> Through addParameter?
>

That's not a transfer; you can only create another parameter.  addParameter
takes a name, not another AudioParam.