Tonejs / Tone.js

A Web Audio framework for making interactive music in the browser.
https://tonejs.github.io
MIT License
13.44k stars 977 forks source link

inaccurate timing when firing midi events #805

Closed felixroos closed 3 years ago

felixroos commented 3 years ago

Hello, and thank you for this beautiful library ❤️

Similar to https://github.com/Tonejs/Tone.js/issues/196#issuecomment-620781830, I am having some timing inaccuracies when firing midi events using webmidi.js.

This little codesandbox shows the behaviour. The timing is perfect when triggering the events directly with webmidi.js. When scheduling the same events with Tone.Part, some events seem to stumble. I tried the suggested fix (and any combination I could think of), but the timing keeps stumbling, so I am considering this is a bug.

I am already happily using tone.js to playback events generated by my custom rhythm notation (like Tone.Sequence, but with polyphony and duration). If triggering MIDI events would work with the same precision, this would just blow my mind, as it enables to write flexible sequencers for external hardware, or record the output in a DAW.

tambien commented 3 years ago

thanks for the kind words.

I'm unable to reproduce this issue. I just had this loop back through a local midi device and i'm not hearing inaccuracies. In the example you shared, there was no content in "tetrisInSeconds.json" so i just used "tetrisInMs.json" and divided everything by 1000.

felixroos commented 3 years ago

In the example you shared, there was no content in "tetrisInSeconds.json" so i just used "tetrisInMs.json" and divided everything by 1000.

Sorry, it seems I forgot to save once more.. I updated the example to just use the millisecond variant + added the division.

I'm unable to reproduce this issue. I just had this loop back through a local midi device and i'm not hearing inaccuracies.

Hm that's odd. I am also just looping back using an IAC Driver (on macOS), playing an instrument inside Ableton. I just made a recording of how it sounds at my end: https://sndup.net/8jmc (first = without, second = with Tone.js)

As a visual reference, here is a comparison of the first two bars:

without tonejs

with tonejs

All I did was a midi recording of what was sent from the sandbox... the only variable here is Tone.js so i guess it must be the reason.. do you have any idea of what might be the cause?

tambien commented 3 years ago

Here's my results from recording MIDI from Tone.js:

image

Looks slightly closer when i use WebMidi.time instead of performance.now(). This issue seems to come from a clock difference. Syncing two clocks is complicated since they can drift from each other over time. Here you're trying to sync the Web Audio clock (that Tone.js uses) and the MIDI clock that Ableton Live uses. Looks like Tone.js is producing accurate timings according to its own clock, but there's no guarantee that those two clocks are in sync.

In your first example, you schedule all of the events ahead of time in the forEach loop so there's no place for the clocks to drift. Tone.js schedules events just-in-time right before they occur which would allow the MIDI clock and the Tone.js clock to get out of sync.

MIDI has a mechanism for syncing two clock called "MIDI Clock Sync". That would allow you to increment Ableton Live's clock from Tone.js to keep them in sync.

For now this doesn't appear to be a Tone.js issue, but follow up if there are updates to the API or pull requests which would make this task easier. It would be great to enable Tone.js to sync to the MIDI clock as well as the Web Audio clock.

felixroos commented 3 years ago

Thank you for looking at this more closely! I may have a limited knowledge of audio clocks, but from my understanding, the MIDI clock of Ableton does not do anything here, as I record the MIDI events directly, just like the input of a live instrument without syncing anything. The grid that you see in the screenshots does only line up because I set the correct tempo (32/14*60 = 137.14 bpm) and placed the one where the first note began. I did this to make the inaccuracies more obvious.

As you already said, it is of course much more precise when scheduling the events ahead of time, so I tested the same scenario with WAAClock, which also triggers the events just-in-time. The result: Using WAAClock, no audible errors occur. This somehow confirms that the time value from the tone callback is imprecise.

To eliminate that "somehow", I wrote a more generic timing test. The timing test works like this: It generates a number of beeps (36 by default), which all have the same time distance (100ms by default). For each beep, the actual passed time is measured to calculate the deviation from the target time distance (=100ms by default). After the last event, the average deviation is calculated to finally spit out a relative error percentage.

Some results:

distance Tone.js Error WAAClock Error
100ms 1.25% 2.73e-11%
10ms 7.2% 1.74e-11%
1000ms 5.78e-14% 0.017%

This makes it relatively clear. For the smaller time distances (which are more relevant), Tone.js has a much higher error. Also, the error gets worse when the distance gets smaller. Interestingly, the errors percentages stay consistent when running the tests multiple times. I think those errors make it impossible to play tight music.

Also, the errors are similar when not even emitting a MIDI message, so this issue does not have anything to do with midi, but with the accuracy of the given time value. What's still strange: When scheduling Tone.js instruments, the timing seems to be perfect, so I assume there is something else happening to the time value..

If I did not make a mistake in my reasoning, this is a rather fundamental flaw in the way Tone exposes time in scheduling callbacks. I made all the tests available in the codesandbox. The relevant functions are "timingTest", "testTone" and "testWaa". At the bottom of the page, they can be executed.

tambien commented 3 years ago

Thanks for following up with a very detailed report. i misidentified the issue here sorry about that. I think you're right that it doesn't have to do with the midi and web audio clocks being mismatched.

I should have noted in my original response when i saw that you were scheduling events with seconds instead of tempo-relative values Tone.js' timing is tick-aligned (unlike WAAClock). Which means that each event timing is rounded to the nearest tick (defined by Tone.Transport.PPQ). Aligning to this grid allows Tone.js to have tempo curves which WAAClock cannot do. The downside of this method is that your scheduled events need to align to that grid or they will get quantized to the wrong place.

Since the default tempo is 120 and the default PPQ is 192, that grid can't represent 0.1 or 0.01 seconds in your test. 0.1 seconds to ticks at 120 bpm would be something like 38.4 ticks (time / (60 / bpm / PPQ) == 0.1 / (60 / 120 / 192)) which would be rounded to just 38.

When you measured it, this is why the test at 1000ms is very accurate compared to the other two because 120bpm can perfectly represent 1000ms.

If you change you the tempo to something which includes 0.1 at an exact tick position by changing Tone.Transport.bpm.value = 100 (or bpm = 1000 for the 0.01 second test) or similarly adjust the PPQ so that those values are not rounded, these are the results that i get from running your tests:

distance Tone.js Error WAAClock Error
100ms 6.494804694057165e-13% 6.822320486321587e-12%
10ms 3.5022029918975664e-9% 6.593783059243613e-11%

I have been toying around with a solution which would not require the input timing to be tick-aligned while still enabling tempo-curves and tempo changes. Hopefully i'll have time to get to that change soon enough.

I would say in the mean-time you will likely get more accurate timing if you're able to set the Transport.bpm.value to the exact tempo of your song and also schedule the Tone.Part to start at time 0 instead of time "+0.1" (line 49 of your link) which will remove rounding errors since events get added to the timeline exactly at 0 instead of 0.1 (which will also be rounded to ticks).

felixroos commented 3 years ago

Ahh now it makes sense! So basically we are getting a moiré effect with the two grids not being aligned.. For my use case, having a fixed grid quantization is a little bit too limiting, as I need things like:

Maybe setting PPQ to a higher value will make the errors inaudible, but I wonder if this will drain the performance? EDIT: I guess if the correct tempo is set, only "playing back performances" would generate an audible problem here. But I think it should still work without needing to set a tempo..

There is only one thing bothering me, as I already pointed out:

What's still strange: When scheduling Tone.js instruments, the timing seems to be perfect, so I assume there is something else happening to the time value

To showcase that, I added the "Browser Synth vs MIDI timing" section to the sandbox. Pressing "play synth with tone.js" (playSynthWithTonejs function), the same tetris melody will be scheduled using Tone.Part, played back with a regular PolySynth.

The weird thing: It plays back perfectly, with no audible timing glitches, even without declaring the tempo or calculating a timing offset (EDIT: same happens with the timing test, when playing the synth instead of midi). To directly compare that, "play MIDI with tone.js without offset" (playWithTonejsNoOffset function) will do the exact same thing, except triggering midi messages instead of the PolySynth. Similar to the other midi tests, the playback will be glitchy.

From that, I conclude the PolySynth will somehow compensate the timing error (which we measured earlier) somewhere inside triggerAttackRelease. (EDIT: so I assume it is theoretically possible to get tight timing without bpm)

Aligning to this grid allows Tone.js to have tempo curves which WAAClock cannot do

What do you mean by tempo curves? WAAClock already allows changing the tempo.

tambien commented 3 years ago

PolySynth doesn't do any timing correction. My guess is that the error values are so small that you can't be heard easily. 1 tick at 120bpm is 60 / 120 / 192 = 0.0026 seconds, so the rounding error is going to be less 2.6ms. This shows up when you measure, but harder to hear that error in many circumstances.

By tempo curves i don't just mean the ability to change tempo from 120 to 60, but, for example, to ramp between 120 and 60 bpm with a linear curve over 5 seconds.

Tone.Transport.bpm.rampTo(60, 5)

There will be some performance difference when changing PPQ, but it might not be much. If you'd like to support triplet and quintuplet, set the PPQ to a multiple of both 3 and 5 like PPQ = 15 would be able to precisely schedule quarter note triplet and quarter note quintuplets. To support 8th notes and 8th notes triplet/quintuplet you could use PPQ = 30. Setting the tempo to 60 and PPQ to 1000 would give you millisecond timing accuracy.

felixroos commented 3 years ago

PolySynth doesn't do any timing correction. My guess is that the error values are so small that you can't be heard easily

My point is that you do not hear it with PolySynth, but you clearly hear it with midi, using the same code:

https://streamable.com/qxfu3b

In the video, the first beep test clearly skips around 3s (moiré effect). Using the exact same code, the second beep test does not skip (same time values). There has to be something going on...

Sorry for insisting so hard, but I would just be really happy about finding a way to keep using Tone.js scheduling instead of rewriting stuff to WAAClock. Setting PPQ manually according to the musical content is no option for me, as I want a more general solution, but I would'nt be offended if you won't care about covering that use case.

felixroos commented 3 years ago

I did some more research and found out that the PPQs are just a minor quirk but not the reason for the timing hiccups. The simple reason: When setting a grid aligned interval (e.g. 0.1s at 100bpm), the timing test results may look well (like you already showed)... but the problem remains: The midi still stutters at some point! I tried out different combinations of bpm and beep intervals, but I could not find a single combination below 1s that did not have an audible glitch..

Then I found the problem: Not the time value, but the callback invocation time has a hiccup:

callback time diffs tone

This chart shows the beep index on the x axis and the difference of audioContext.currentTime*1000 (which can be seen as the ms passed between last and current callback invocation) on the y axis. Most of the time, it will be more or less stable at 100ms (= beep interval), but at some point, the difference will drop down to 50ms at the exact spot where the audible hiccup appears.

Trying the same with WAAClock, the callback intervals are much more stable, without sudden hiccups:

callback time diffs waa

I have no idea where this hiccup is coming from, but it should be unrelated to the PPQ problem (as I am using grid aligned beep intervals).

Then I realized that the calculated offset time for the midi events was just wrong:

const offset = WebMidi.time - Tone.now() * 1000;
time = time * 1000 + offset
output.playNote("a4", 1, {
  time,
  duration: event.duration * 1000
});
const msUntilAttack = WebMidi.time - time;

The problem: msUntilAttack will eventually be lower than zero, causing the note to be attacked immediately. In this case, the callback invocation time will control the timing of the note, which will make the hiccup audible. As WAAClock's callback has no hiccup, no glitch will be audible (even if the notes are triggered by the not so precise callback).

TLDR; Solution: Using Tone.context.currentTime instead of Tone.now() solves the problem. I suppose the problem is that Tone.now() includes the lookAhead:

/**
* The current audio context time plus a short [[lookAhead]].
*/
now(): Seconds {
  return this._context.currentTime + this.lookAhead;
}

I am glad that I finally found the root of the problem which has been bugging me for days 😩 Maybe this will help someone in the future...

FYI: here's a fork of the previous sandbox, including the plot generation ("test timing with tone.js"). I also added the correct offset to finally listen to an accurate version of the tetris melody:

tetris correct

This does not even use the correct Tone.Transport.bpm.value, but still looks and sounds good..

UPDATE: I guess the hiccup comes from garbage collection:

heap vs hiccup

Maybe this is just a coincidence, but who cares anyway 🤦 I guess WAAClock has no hiccups because it has a smaller memory footprint, being much simpler

giohappy commented 2 years ago

@felixroos I've just come across this issue while searching for a robust way of scheduling MIDI.

After reading your tests I wondee why you calculate the absolute MIDI event time (from the audio and midi time offset) instead of just calculating a delay. Web MIDI supports delayed time (also the webmidi library, with the "+" notation). In, that case we don't care of the MIDI absolute time. Just calculating the difference between event time and the audio context time should be enough.

I wonder if I miss something and if there’s a valid reason to calculate absolute time.

felixroos commented 2 years ago

@giohappy I assume you mean this notation (from webmidi doc):

 // Play a note on channel 16 in 2 seconds (relative time)
 output.playNote("F5", 16, {time: "+2000"});

I already did that in the above mentioned sandbox, inside the function playWithoutTonejs:


function playWithoutTonejs(output) {
  tetrisMs.forEach(({ time, value, duration }) => {
    if (value === "r") {
      return;
    }
    time = "+" + (time + 100); // <- relative notation
    value = value.toUpperCase();
    output.playNote(value, 1, { time, duration });
  });
}

This works well timing wise, but it does not fit my goals, as I need dynamic midi scheduling, where user interaction can change / cancel the notes played in the future (as part of one of my passion projects). This is only possible using a callback that happens shortly before that note should sound (like Tone.js and waaclock). It could be possible with webmidi if there was a way to cancel all future events, but that is not possible atm.

If you find workaround or another way to control midi in real time, please let me know :)

giohappy commented 2 years ago

felixroos I mean inside your playWithTonejs doing:

time = '+' + (time - Tone.context.currentTime)*1000

instead of calculating the absolute MIDI time.