Repository navigation
fix(player): skip count-in in backing track and external media modes instead of freezing - #2932
Open
leocaseiro wants to merge 6 commits into
Open
leocaseiro wants to merge 6 commits into
leocaseiro wants to merge 6 commits into
Conversation
…instead of freezing With PlayerMode.EnabledBackingTrack or PlayerMode.EnabledExternalMedia and countInVolume > 0, pressing play froze the page (CoderLine#2397). - MidiFileSequencer.fillMidiEventQueueToEndTime looped on the main state's time, but _fillMidiEventQueueLimited advances the current state. During the count-in the current state is the count-in state, so the loop never ended. It now loops on the state it advances. - AlphaSynthBase.play() started the count-in with a seek to 0, which BackingTrackPlayer forwards to the media, so starting at 30s rewound the media to the song start. These players have no synthesizer to play the count-in metronome, so they now skip the count-in through a new protected supportsCountIn hook (false in BackingTrackPlayer and ExternalMediaPlayer). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The supportsCountIn hook defaults to true, but every count-in test drives the media players where it is false, so flipping the default (or overriding it on AlphaSynth) kept the whole suite green. Play the real AlphaSynth with count-in from 1000ms and assert the reported position switches to the count-in's own clock (0). It uses only the public API: a test subclass of AlphaSynth would make the Kotlin transpiler emit the shipped class as open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… modes The keeps-position tests ignored play()'s result and both test doubles discarded play(), so a skip path that never started the media, or a half-skip that still entered the count-in and rewound the media on the next time update, passed both tests. Count play() calls on the doubles, assert play() succeeds, the player is Playing and the media was started once, then feed one more media time update and assert there is no seek back and the position advances. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ops advancing If fillMidiEventQueueToEndTime regresses to looping on the main state, fill-to-end-time-during-count-in spins forever: vitest's timeout cannot interrupt synchronous code, CI sets no job timeout, and the transpiled C# and Kotlin tests block the same way. The sequencer reads outSampleRate once per fill iteration, so the test synthesizer now counts the reads and throws after 100000 (the test needs 690), turning the hang into an immediate failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep one compact check per behavior, like the surrounding SyncPoint tests: drop the two count-in playback smoke tests (they passed with either part of the fix reverted), check only that playback keeps its position and does not seek the media back, and revert the play() counters on the test outputs. The fill-loop guard stays, so a regression of the sequencer loop still fails instead of hanging the run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
leocaseiro
commented
Oct 7, 2026
| public fillMidiEventQueueToEndTime(endTime: number) { | ||
| while (this._mainState.currentTime < endTime) { | ||
| if (this._fillMidiEventQueueLimited(endTime - this._mainState.currentTime)) { | ||
| while (this._currentState.currentTime < endTime) { |
Contributor
Author
There was a problem hiding this comment.
Sets the current clock, instead of the main one, to match fillMidiEventQueueToEndTime.
leocaseiro
commented
Oct 7, 2026
| expect(events.map(e => `${e.currentTime},${e.originalTempo},${e.modifiedTempo}`)).toMatchSnapshot(); | ||
| expect(testOutput.seekTimes).toMatchSnapshot(); | ||
| }); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
@Danielku15, I noticed you simplified my latest tests, so I tried to keep it simple this time.
Let me know if this provides good coverage or if you would prefer a different testing approach.
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
Partially fixes #2397: only the page freeze and the jump to the song start when count-in is enabled in backing-track / external-media mode, which @Danielku15 confirmed as a bug in #2397 (comment) (reported in #2397 (comment)).
Proposed changes
With
countInVolume > 0inPlayerMode.EnabledBackingTrackorPlayerMode.EnabledExternalMedia, pressing play froze the page, and starting playback mid-song rewound the media to the song start. Two changes:MidiFileSequencer.fillMidiEventQueueToEndTimeloops on the state it advances. The loop condition now reads_currentStateinstead of_mainState, so the loop ends whichever state (main, count-in, one-time MIDI) is active.AlphaSynthBase.play()starts the count-in only when a new protectedsupportsCountIngetter returnstrue(the default).BackingTrackPlayer, and with itExternalMediaPlayer, returnfalse: the media plays as-is and there is no synthesizer output for the count-in metronome. Without the count-in,play()no longer seeks to 0, so the media keeps its position. The synthesizer player is unchanged.New tests in
test/audio/SyncPoint.test.ts:count-in-keeps-position-backing-track/-external-media: start at 30 s with count-in. The media is not seeked back to the song start, and the next media time update continues past 30 s.fill-to-end-time-during-count-in: fill the sequencer to an end time while the count-in is active. The test synthesizer counts its reads and throws, so a regression fails instead of hanging the run.count-in-playback-synthesizer: the synthesizer player still starts its count-in.Out of scope:
playBeat/playNotein these modes still seek the media to the song start while the note plays (the sameupdateTimePosition(0, true)pattern; the loop change only removes the freeze it could cause at the song start).Root-cause analysis
timeUpdatehandler ofBackingTrackPlayercallsfillMidiEventQueueToEndTime(mediaTime). The loop compared_mainState.currentTimewith the end time, but it filled_currentState. While the count-in (or a one-time MIDI state) is active, the main state does not advance, so the loop never ended and blocked the main thread ("Page unresponsive") while the media kept playing.AlphaSynthBase.play()starts the count-in withupdateTimePosition(0, true).BackingTrackPlayerforwards every seek to the media (seekTo), so starting at 0:30 sent the media back to the song start.countInVolume > 0; the synthesizer mode is not affected. Skipping the count-in matches the current limitation that alphaTab cannot mix the synthesizer with media (see External Media Sync / Backing Tracks - Allow mixing with Synthesizer #2397).supportsCountIngives a future mixed mode one place to opt back in.protected virtual/overrideproperty in C# and toprotected open val/override valin Kotlin, the same shape as existing overridden getters such asLineBarRenderer.bottomGlyphOverflow.Testing done
packages/alphatab:npm testpasses (82 files, 1862 tests);npm run lintandnpm run typecheckare clean.SyncPointtests pass (28/28) on .NET 10 withDOTNET_ROLL_FORWARD=Major(the test project targets net8.0).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
countInVolumeis now ignored in the backing-track and external-media modes (before, it froze the page). Its API docs do not mention that yet; I can add a note if wanted.🤖 Generated with Claude Code