Tone.Transport.Schedule callback sometimes triggers twice

[adrsh]

I'm making a simple web based DAW for a university project, and since this bug happens occasionally and is very noticeable I decided to track it down. This also happens in Tone@next, which I'm currently using. It seems like forEachAtTime can get called with the same tick twice, immediately after the other, and in turn trigger the callback twice.

I added a temporary fix in my project in forEachAtTime() to make sure that the current tick is not the same as the previous tick.

https://github.com/Tonejs/Tone.js/blob/053b5d4397b595ea804b5d6baf6108158c8e0696/Tone/core/util/Timeline.ts#L372

    previousTime

    forEachAtTime(time, callback) {
        if (this.previousTime !== time) {
            this.previousTime = time
            // iterate over the items in reverse so that removing an item doesn't break things
            const upperBound = this._search(time);
            ....

[gerardsmyth]

Hi there.

We have also hit this issue, so would appreciate any help in getting it resolved. From my testing there are certain points at which you can schedule an event which will cause it to be fired twice in an offline context, but not when using the live interactive context.

This simple example illustrates the problem. https://codepen.io/GerardS/pen/LYmRoNP

This schedules a callback at 10:0, and you can see this gets called twice if using an offline context.

I think the root cause of this is due to issues with tick to time conversion in the forEachTickBetween function inside TickSource.js.

For this specific example with 125 bpm, I think the 10:0 time represents tick 7680. When offline, this forEachTickBetween gets called twice around this time with start and end times of: startTime: 19.19709750566994 endTime: 19.20000000000101 startTime: 19.20000000000101 endTime: 19.202902494332076

In the first call, tick 7679 is determined to be at 19.1975 so is processed, and then tick 7680 is determined to be at 19.200000000000003 (so just before the end time) and is also processed. On the second call, the first tick is determined to be at 19.200000000001012, which is 7680 again. but as it is now after the incoming start time it is processed again.

These time differences are obviously very minor, and likely caused by JavaScript floating point issues etc, but are causing the schedule callback to be triggered twice. Would it make sense to compare the tick time again after finding the (rounded) tick number in this function somehow? Then it would always use the same time value for a given tick, and so only match against the start/end times once.

Interestingly the time for the 7680 tick seems to differ slightly between the interactive and offline contexts (as show in the codepen) which I guess explains why we are only seeing the issue when offline, but I haven’t looked into why the values are different.

I hope this is useful information. Please let me know if I can help further with getting this resolved. It looks like issue #1049 may also be related.

Regards Gerard dappledark.com

[yifanmai]

1098 may be another instance of this bug.

Here's a failing unit test for the bug:

// In Transport.test.ts
it("can loop events scheduled on the transport many times", () => {
    let invocations = 0;
    return Offline((context) => {
        const transport = new Transport({ context });
        transport.schedule((time) => {
            invocations++;
        }, 0);
        transport.setLoopPoints(0, 1).start(0);
        transport.loop = true;
    }, 120.1).then(() => {
        // Actually returns 124
        expect(invocations).to.equal(121);
    });
});

Also noting that deleting these lines in TickSource.ts "fixes" the bug:

// guard against floating point issues
offset = EQ(offset, 1) ? 0 : offset;

I don't entirely understand the logic of this line. This was apparently made to in this commit to fix #645. So deleting it would probably cause a regression, but unfortunately there wasn't an regression test included in the commit, so I can't be sure.

[pathime]

@yifanmai Did you ever discover any problems with deleting that line? It certainly seems to solve the duplicate event issue in the offline render for me.

[yifanmai]

I don't remember the details because it was a while ago, but I believe it caused some other unit tests to fail.