Repository navigation
Move seekTarget into machine context (single source of truth) - #29
Conversation
The track machine and Track class had a dual-authority problem:
the machine owned lifecycle (states, transitions) but Track owned
timing truth (pausedAtTrackTime). Neither was the single source of
truth, causing bugs where SEEK events were dropped in idle/loading,
activation erased pending seeks, and the currentTime getter reported
stale values.
This refactor:
- Adds seekTarget to TrackContext as the single owner of "where
should playback resume"
- SEEK in idle/loading is now just assign({ seekTarget }) — no HTML5
side effects, no playback-method guards needed
- ACTIVATE/DEACTIVATE atomically reset seekTarget alongside other
timing state via clearSeekTarget
- freezePausedTime (webaudio pause) is now an assign that writes
seekTarget directly into context
- Splits activateAndPlayCurrent in the queue machine into
activateCurrent + playCurrent so GOTO does activate → seek → play
in the correct order (activate no longer erases the seek)
- _playHtml5 reconciles seekTarget with audio.currentTime at play
time, handling seek-before-load without needing idle-state side
effects
- Mid-stream crossover writes to _crossoverComputedOffset then
syncs via syncSeekTargetFromCrossover assign action
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying gapless with
|
| Latest commit: |
a946a30
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6ffd7f2b.gapless-e1l.pages.dev |
| Branch Preview URL: | https://refactor-seek-target-in-cont.gapless-e1l.pages.dev |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d158d0870f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const target = seekTarget; | ||
| this.audio.addEventListener('loadedmetadata', () => { | ||
| this.audio.currentTime = target; | ||
| }, { once: true }); |
There was a problem hiding this comment.
When metadata is still loading, this listener closes over the current seekTarget but survives DEACTIVATE/ACTIVATE. If a user starts a track at a nonzero offset, skips away, and returns before loadedmetadata fires, activation resets the track to zero and starts it normally, but the stale listener subsequently jumps the active playback back to the old offset. Remove or invalidate pending metadata handlers when resetting the track, or have the handler verify the current machine context before applying its captured target.
Useful? React with 👍 / 👎.
When a track is reset (ACTIVATE/DEACTIVATE), abort the seek signal so any pending loadedmetadata listener from a prior seek is removed rather than firing later and jumping playback to a stale position. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08665af7a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private _playHtml5(seekTarget: number): void { | ||
| if (this.audio.preload !== 'auto') this.audio.preload = 'auto'; | ||
| this.audio.playbackRate = this.queueRef.playbackRate; | ||
| if (isFinite(seekTarget) && seekTarget > this.audio.currentTime + 0.5) { |
There was a problem hiding this comment.
When gotoTrack(index, true, startTime) selects an unloaded track with a valid startTime of 0.5 seconds or less, activation resets audio.currentTime to zero and this tolerance check prevents the requested offset from being applied, so HTML5 playback starts at the beginning instead. Apply a newly assigned seek target regardless of this 0.5-second difference, or separately track whether the target is pending.
Useful? React with 👍 / 👎.
seekTarget is only non-zero when explicitly set, so we can use a tight threshold (0.01s) instead of 0.5s. This ensures small start offsets like gotoTrack(n, true, 0.3) are applied correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c57c9d0723
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (seekTarget > 0 && Math.abs(this.audio.currentTime - seekTarget) > 0.01) { | ||
| if (this.audio.readyState >= HTMLMediaElement.HAVE_METADATA) { | ||
| this.audio.currentTime = seekTarget; |
There was a problem hiding this comment.
When playback remains on HTML5 (especially in HTML5_ONLY, or while hybrid decoding is pending), seekTarget retains the last requested nonzero seek while audio.currentTime continues advancing. After a user seeks to 60 seconds, listens until 90 seconds, pauses, and resumes, this condition resets currentTime to 60 because the HTML5 PAUSE handler never refreshes seekTarget. Track the paused HTML5 position or only apply the target while an initial seek is pending so resume continues from the actual pause point.
Useful? React with 👍 / 👎.
Without this, seekTarget retained the last explicit seek position while audio.currentTime advanced during playback. On resume, _playHtml5 would jump back to the stale seekTarget. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: becb48795e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| on: { | ||
| PAUSE: { | ||
| actions: ['clearIsPlaying', 'pauseHtml5', 'stopProgressLoop', 'reportProgress'], | ||
| actions: ['clearIsPlaying', 'freezePausedTime', 'pauseHtml5', 'stopProgressLoop', 'reportProgress'], |
There was a problem hiding this comment.
When HTML5 playback is paused before loadedmetadata applies a requested nonzero seek, audio.currentTime is still typically 0, so freezePausedTime overwrites the pending seekTarget with 0. In HYBRID mode, if Web Audio decoding then completes before the metadata listener fires, the BUFFER_READY crossover uses that erased target and starts the decoded source from the beginning. Preserve the existing target while an HTML5 seek is still pending, and only freeze audio.currentTime after the seek has taken effect.
Useful? React with 👍 / 👎.
freezePausedTime now takes the max of the live position and the existing seekTarget. This prevents a pause before loadedmetadata from erasing a pending seek — the target is preserved until the audio element can actually apply it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Eliminates the dual-authority problem identified by Fable's architectural review: the track machine owned lifecycle but
Trackowned timing truth (pausedAtTrackTime), causing a class of bugs where seek state could be dropped, erased, or reported stale.seekTargetin TrackContext replacespausedAtTrackTimeas the single source of truth for "where should playback resume"assign({ seekTarget })— no HTML5 side effects, noisWebAudioOnlyguard, no staleloadedmetadatalistenersseekTargetalongside other timing stateactivateAndPlayCurrentsplit intoactivateCurrent+playCurrentin the queue machine so GOTO does activate → seek → play in correct orderfreezePausedTimeis now a providedassignthat writes directly into contextSupersedes #28 (closed). Addresses all findings from both Codex and Fable reviews.
Test plan
gotoTrack(n, true, 142.6)starts playback at 142.6s (the original bug)🤖 Generated with Claude Code