mrchimp / tock

Timer Object/Class. Kickass!
MIT License
237 stars 33 forks source link

Callback skipped #41

Open jneuendorf opened 6 years ago

jneuendorf commented 6 years ago

Hey,

I've got the problem that sometimes a callback is skipped.

I created a minimal example where I get the current tick with Math.floor(tock.lap() / tock.interval)*. I'd expect the list of ticks to be a list natural numbers. But for some reason a tick is skipped at some point. Surprisingly, exactly until then the inaccuracy grows (see the console output).

Here's the example code:

import Tock from 'tocktimer'

const interval = 50
let lastTick = -1
let numberSkipped = 0

const clock = new Tock({
    interval,
    callback: () => {
        const elapsed = clock.lap()
        const tick = Math.floor(elapsed / interval)
        console.log('current tick:', tick, elapsed / interval, elapsed)
        if (tick !== lastTick + 1) {
            console.error(`skipped a callback. current tick=${tick}`)
            numberSkipped++
        }
        console.log('off by', elapsed - tick*interval, 'number of skipped ticks', numberSkipped)
        console.log('====================')
        lastTick = tick
    }
})

clock.start()

Is my assumption simply wrong or do you agree that this should be an invariant?

currentTick === lastTick + 1

Ok. I looked into it a bit more and the problem is the initial inaccuracy (until the 1st skipped tick). Because of that the tick value without rounding (per callback) is growing from the correct value to value + 1. Here an example:

[
// (actualTick, tickValue, roundedTickValue, flooredTickValue)
(0, 0.00, 0, 0),
(1, 1.15, 1, 1), // always +1.15 => inaccuracy is 0.15 per callback
(2, 2.30, 2, 2),
(3, 3.45, 3, 3),
(4, 4.60, 5, 4), // this is where rounding 'breaks' (3 -> 5)
(5, 5.75, 6, 5),
(6, 6.90, 7, 6),
(7, 8.05, 8, 8), // this is where flooring 'breaks' (6 -> 8)
// after this the value is correct enough so that no gaps happen...
]

An obvious workaround would be to have a closured tick variable that just gets incremented but the fact that all ticks before the 1st 'error' occurs are relatively inaccurate and all following ticks are good (as I would expect from the library) is weird...

jneuendorf commented 6 years ago

I am not quite sure yet why and how this worked (in https://github.com/mrchimp/tock/blob/master/tock.js#L50-L51)

var diff = (Date.now() - this.start_time) - this.time,
    next_interval_in = diff > 0 ? this.interval - diff : this.interval;

but I changed it to this (in my local copy - currently)

next_interval_in = this.start_time + this.time - Date.now()

and I think it works better.

mrchimp commented 6 years ago

I haven't got to the bottom of this yet but I've noticed a couple of things.

  1. I don't get the same numbers you do. I'm running on a Linux VM and I'm missing tick 1 and 2.
  2. If I change the interval to 1000ms I don't miss any ticks
  3. If I start the clock after a timeout of 100ms or more I don't miss any ticks.

Hmmm.

jneuendorf commented 6 years ago

Did your observations happen with the current or still with my changed/suggested code? If this code

next_interval_in = this.start_time + this.time - Date.now()

fixes the problem I'd create a PR.

mrchimp commented 6 years ago

That actually made things worse for me. I will try to find some time to look into this but I'm super busy at the moment.

I notice in the other thread that you're looking to use this for a drum machine. Tock's strength is that it doesn't drift over long periods of time, not that it is particularly accurate from tick to tick. It was originally made for a sports clock/scoreboard for example. it would be good for keeping a drum machine in time with another song, but not so great at making each individual beat be in time. This might be improved once I get around to implementing performance.now but - as much as I don't want to discourage you from using Tock - you might be better off using performance.now directly.

jneuendorf commented 6 years ago

Hm ok. It works pretty well for me with the above code. So I'll just keep using it for now. tock nicely abstracts from the setTimeout stuff and has a clean API. I'll use performance.now() soon anyway for better accuracy. If I find out anything new I'll let you know 😉