Repository navigation
fix(player): keep sync point times exact to avoid drift on non-integer tempos - #2960
Merged
Merged
Conversation
…r tempos (#2952) MidiUtils.ticksToMillis truncated its result to whole milliseconds. The MIDI generator accumulates the synthesizer time of sync points and beat timers bar by bar with this helper, so every bar whose duration is not an integer millisecond value (e.g. 1621.62ms for a 4/4 bar at 148 BPM) lost the fractional part. The sequencer maps synth time back to ticks with exact math, so the position reported for a given backing track or external media time fell further behind over the song (~123ms after 200 bars at 148 BPM). ticksToMillis now returns exact milliseconds (multiplying before dividing keeps whole-number results exact). The Guitar Pro writer rounds beat timers since the format stores integer milliseconds. The alphaTex \sync tag now parses its millisecond offset as float so fractional offsets also work without parentheses (e.g. `\sync 1 0 1621.62` was previously split into a sync point and a note). The interpolated end sync point of the sync point test file now keeps the sync BPM of the previous segment, updating the related snapshots and the audio export reference. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
AI-authored disclosure (
alphatab-ai-authored-v1)Portions of this content were authored by an AI agent. The agent has read
AGENTS.md and the human submitter accepts responsibility for
compliance with the rules in that document.
Issues
Fixes #2952
Proposed changes
Root cause
MidiUtils.ticksToMilliscut its result down to whole milliseconds (| 0).MidiFileGeneratoruses it to add up the synthesizer time of sync points (synthTime) and beat timers bar by bar. Any bar whose length isn't a whole number of milliseconds loses the decimal part every time. For example, a 4/4 bar at 148 BPM lasts 1621.62 ms but is counted as 1621 ms.The sequencer converts synth time back to ticks with exact math, using its tempo table. So the shortened
synthTimeon every sync point makes the reported position fall further behind the backing track or external media as the song goes on. In the issue's repro that is 292 ticks (~123 ms) by bar 200 at 148 BPM. At 120 and 100 BPM a bar is a whole number of milliseconds, so there is no drift. The error builds up from the start of the song and doesn't reset at sync points, which explains why the gap depends on the bar, not on how long playback has run.Changes
MidiUtils.ticksToMillisreturns exact milliseconds. It multiplies before dividing (ticks * 60000 / (tempo * 960)), so values that should be whole numbers stay exact (2000 instead of 2000.0000000000002). Fixing it here fixes all its callers: sync point times, the interpolated end sync point, beat timers, tick-lookup bar durations, scroll durations and the generated audio end time.GpifWriterrounds<Timer>: beat timers can now have decimals, and the Guitar Pro format stores whole milliseconds.\sync: the millisecond offset is now read as a decimal number (RequiredAsFloat, changed inpackages/alphatexand regenerated). Sync points from Guitar Pro already have decimal offsets, and the exporter writes the parenthesized form, which already worked. The bare form\sync 1 0 1621.62was previously split into\sync 1 0 1621, a.separator and a note62.Changes to existing test references
The existing sync point test file (
syncpoints-testfile.gp) has an interpolated end sync point. It was computed from a truncated duration, so it landed at 42004.67 ms and distorted the sync BPM of the last segment (80.0178). It now lands at 42005.44 ms, and the last segment keeps the sync BPM of the segment before it (80.0109), which is what the generator intends. The updatedSyncPointsnapshots, one seek target that is now exact (36006→36006.2585), and theexport-sync-points.pcmreference all follow from that. The new audio is identical to the old reference for the first ~39.7 s; the last stretch is shifted slightly and ends in ~1.45 ms of extra silence.Tests added
SyncPointTests > non-integer-bar-duration: 201 bars at 148 BPM with sync points placed exactly where the tempo puts them. It checks the generated sync point times, and that theExternalMediaPlayerandBackingTrackPlayerreport a position within 1 tick of each bar start, at every bar. It also uses a bare decimal\syncoffset.MidiFileGeneratorTest > beat-timer-non-integer-bar-duration: beat timers stay exact over 200 bars at 148 BPM.AlphaTexImporterTest > sync-float-millisecond-offset: bare and parenthesized decimal offsets, a decimal bar position, an unchanged note count, and an export round trip.Gp7ExporterTest > beat-timer-as-integer:<Timer>is written as a whole number.Every new test except
beat-timer-as-integerfails without the fix. TS: 1985/1985 passing. C#: 1979/1979 passing, including the new tests. Kotlin was not run locally.Checklist
AI authorship disclosure
(
alphatab-ai-authored-v1) is present at the top of this body, and I havepersonally reviewed every change and can explain each one
Further details
🤖 Generated with Claude Code