sebpiq / WAAClock

A comprehensive event scheduling tool for Web Audio API.
MIT License
244 stars 32 forks source link

Script Processors evaluation order #15

Closed echo66 closed 9 years ago

echo66 commented 9 years ago

Greetings!

I noticed that WAAClock, instead of using window.setTimeout, window.setInterval or a web worker, uses a ScriptProcessorNode. But there are "problems" with the evaluation order of the clock processor node if there are other ScriptProcessorNodes at the same "depth" in the audio graph. Consider the following example:

var c = new AudioContext();

var sp1 = c.createScriptProcessor(256, 1, 1);
var sp2 = c.createScriptProcessor(256, 1, 1);
var sp3 = c.createScriptProcessor(256, 1, 1);

sp1.onaudioprocess = function () {
    console.log("sp1");
};
sp2.onaudioprocess = function () {
    console.log("sp2");
};
sp3.onaudioprocess = function () {
    console.log("sp3");
};

sp1.connect(c.destination);
sp2.connect(c.destination);
sp3.connect(c.destination);

Sometimes, the execution order makes the browser print: sp3 sp1 sp2

Other times: sp2 sp1 sp3

I guess you can see the problem in this.

sebpiq commented 9 years ago

Hi! Unless I've missed something, I think this isn't a problem for WAAClock. The ScriptProcessorNode on each onaudioprocess schedules things that should happen in the future.

echo66 commented 9 years ago

But if those things are related, for example, to starting AudioBufferNodes, at the same "depth" as the clock node, that would be a (small) problem, right?

sebpiq commented 9 years ago

I don't really understand what you mean by "depth"? I don't really understand the case you are talking about either. Could you illustrate with some code or an exact situation where there might be a problem?

echo66 commented 9 years ago
var context = new AudioContext();

var s1 = audioCtx.createBufferSource();
var s2 = audioCtx.createBufferSource();
var s3 = audioCtx.createBufferSource();

s1.connect(context.destination);
s2.connect(context.destination);
s3.connect(context.destination);

var clock = new WAAClock(context);

In this graph, all nodes and the clock are at the same "depth" in the audio tree.

sebpiq commented 9 years ago

Can you explain me how this affects WAAClock ? This is initialization code alright, but can you show me and explain how this audio graph leads to a problem??

echo66 commented 9 years ago

Sorry, I forgot to answer back, @sebpiq . :/

Imagine that your system sets the node execution order like this: [s1, clock, s2, s3]. If this happens and the callbacks (assigned to the clock tick) need to change some parameters of s1, s2 and s3, the node s1 will play before any callback is invoked (it won't be synchronized with s2 and s3, in terms of callbacks).

Of course, if the order is something like [clock, s1, s3, s2] or [s2, s1, s3, clock], we won't have this issue.

sebpiq commented 9 years ago

No worries :)

the callbacks (assigned to the clock tick) need to change some parameters of s1, s2 and s3, the node s1 will play before any callback is invoked

I see how that can be a problem, however there is nothing I can do in WAAClock to solve this. This is something that the people writing the spec must solve, if they believe it to be an issue.

Besides, the way WAAClock works, the callback is executed after all the ScriptProcessorNodes. So you shouldn't have a problem with the execution order : https://github.com/sebpiq/WAAClock/blob/master/lib/WAAClock.js#L161

The function process.nextTick causes the callback to be put on the queue of callbacks to be executed. I believe that this should schedule the callback to be executed after all the ScriptProcessorNodes have ran their callback. Even if it would be preferable to have it running first, at least it is always predictable, and that's probably the best I can do if the specification doesn't handle that issue...

echo66 commented 9 years ago

There is one way to solve this: use a second AudioContext for the clock and use the other for the your main nodes. Still, this option seems to be an approach similar to https://github.com/sebpiq/WAAClock/issues/16 . I just figured out that there might be a way to solve this, for the cases that all your "depth 0" nodes are ScriptProcessorNodes. Maybe I will hack something later to get some insights on this.

Regarding the spec, take a look at this issue: https://github.com/WebAudio/web-audio-api/issues/516