Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increasing delay between Madmom and C# reported time #23

Open
Ashoat opened this issue Sep 3, 2019 · 1 comment
Open

Increasing delay between Madmom and C# reported time #23

Ashoat opened this issue Sep 3, 2019 · 1 comment

Comments

@Ashoat
Copy link
Member

Ashoat commented Sep 3, 2019

We have three ways we can try to sync the time Madmom reports with Spectrum.

  1. In the initial version, for each beat, Madmom would report the number of seconds elapsed since the first detected beat. When we received that first detected beat on Spectrum, we would check the time, and then for each subsequent beat we would add the two numbers together. This had the issue of not accounting for any delay between the time Madmom reported and the time C# picked up the output
  2. To try and address this, we changed the code to use absolute timestamps. Madmom would output the number of milliseconds since the system was booted. This is a purely monotonic number, avoiding NTP clock sync issues, which we get from time.monotonic(). We would then compare those numbers to what we saw from C#'s Environment.TickCount, which is supposed to output the same number, and to our knowledge is sourced from the same internal APIs. However, after implementing this solution, we saw an issue where the skew between the Madmom-reported time and C#-reported time (at the time of reading a new Madmom-reported beat) would steadily increase. However, when watching animations performed to music, Madmom still seemed to be triggering beats at the correct intervals
  3. To address the skew issue described above on-playa, @revbucket changed the Madmom code so that instead of tracking the initial time.monotonic() and adding it on the reported elapsed time, we would just check time.monotonic() at every beat. I wasn't there to see whether this eliminated the clock skew issue, but Madmom did seem to perform well I can't find the code for this, see comment below

Ideally, I'd like to:

  • Understand why solution 2 above resulted in an increasing clock skew, and whether there is an issue with C# being increasingly delayed in picking up newly printed console lines from Madmom
  • Understand if solution 3 actually improved behavior over solution 2, given that both seemed to show well-synced animations
  • Ideally have C# actually compare its own readings from Environment.TickCount with what we get from Madmom
@Ashoat
Copy link
Member Author

Ashoat commented Sep 4, 2019

After looking through the commit log from the burn, some more notes:

  • 9ec8705 is the Spectrum side of solution 2 above
  • campmindshark/madmom@bd43084 and campmindshark/madmom@8f8762f are the Madmom side of solution 2 above
  • I can't find the code for solution 3, either in the spectrum folder or the spectrum_new folder that we were running off of on playa. I know I discussed it in person with @revbucket, but it's possible he decided to forgo solution 3 given that solution 2 seemed to work okay, despite the steadily increasing clock skew
  • d9dae0a is the commit that added code to print the skew so we can watch it increase. Once we fix this issue we should revert that commit
  • I can confirm that the skew still steadily increases when running the current latest version of Spectrum (which incorporates solution 2 above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant