Repository navigation
Conversation
Two bugs fixed: 1. _startSourceNode used source.start(0) which dispatches to the audio render thread asynchronously. The gap between the main-thread currentTime read and the actual render-thread start (δ of 5-100ms depending on hardware) made playbackEndContextTime compute an end time δ too early, causing the next gapless track's source to start before the current one finished — brief audible overlap. Fix: schedule the source at an explicit future time (start(when)) with a lead of max(50ms, 2×baseLatency + 20ms). The anchor (_waRefCtxTime) now equals the actual start time, so end-time math is exact. The buffer offset is corrected by lead×rate so currentTime stays continuous and the crossover aligns both streams to the same position. 2. Queue.previous() when currentTime > 8 called ct.seek(0) + ct.play() directly on the Track, bypassing the queue state machine. This left _scheduledNextIndex set and the pre-scheduled next track's source node alive with a stale end time — full overlap when ctx.currentTime reached the old schedule. Fix: route through the queue machine via SEEK event, which triggers cancelAndRescheduleGapless. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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. |
Deploying gapless with
|
| Latest commit: |
05b757b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83309c75.gapless-e1l.pages.dev |
| Branch Preview URL: | https://fix-gapless-overlap-anchor.gapless-e1l.pages.dev |
![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: ae118d06c1
ℹ️ 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 rate = this.queueRef.playbackRate; | ||
| const maxOffset = this.audioBuffer.duration - this._bufferStartPaddingSec; | ||
| const correctedOffset = Math.min(offset + lead * rate, maxOffset); |
There was a problem hiding this comment.
When _startSourceNode is used for normal WebAudio play, resume, or seek, no HTML5 stream continues during lead, yet correctedOffset advances the source by lead * rate. Consequently every such operation silently discards at least 50 ms of audio—150 ms for a suspended context and potentially more with high baseLatency—while also reporting that skipped interval as played. The offset adjustment is appropriate only for the crossover path where HTML5 playback covers the lead; ordinary starts must retain the requested offset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch on the 50ms. Reduced the minimum lead to 20ms (2-4× typical desktop hardware callback period).
However, I kept the corrected offset for all paths rather than limiting it to crossover. Without it, currentTime drift accumulates across pause/resume cycles — each _startSourceNode call adds lead × rate of lag to pausedAtTrackTime. After N pause/resume cycles the total drift is N × lead × rate, which becomes visible in the progress display.
With the corrected offset applied universally, currentTime equals the requested offset immediately after each call — no accumulation. The trade-off is ≤20ms of audio skipped per start at 1× rate, which is below audible threshold.
Addressed in c844866.
Address review feedback: the corrected offset (advancing buffer start by lead×rate) must apply to all paths, not just crossover. Without it, position drift accumulates across pause/resume cycles — each start adds lead×rate of lag to currentTime. The skipped interval (≤20ms at 1× rate) is below audible threshold and prevents the alternative: visible position drift in the progress display after multiple pause/resume cycles. Also reduces the minimum lead from 50ms to 20ms, which is still 2-4× the typical hardware callback period on desktop systems. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c84486666b
ℹ️ 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".
| } else if (!this._html5GainNode) { | ||
| const savedVolume = this.audio.volume; | ||
| this.audio.volume = 0; | ||
| this.audio.pause(); | ||
| this.audio.volume = savedVolume; |
There was a problem hiding this comment.
When createMediaElementSource failed and _html5GainNode is null, the WebAudio source has just been scheduled for the future when, but this fallback pauses HTML5 immediately. This guarantees silence for the scheduling lead (at least 20 ms and 150 ms for a suspended context), followed by the 30 ms fade-in, precisely in the legacy/CORS fallback this branch is intended to support. Delay the pause until when or start the fallback source immediately.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Delayed the fallback HTML5 pause until when so it covers the scheduling lead. Addressed in b3c9f24.
When _html5GainNode is null (CORS blocked or legacy browser), the fallback crossover path paused HTML5 immediately while the WebAudio source was scheduled for the future `when`. This created a silence gap of at least 20ms (the scheduling lead). Now delays the pause until `when` so HTML5 covers the gap. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3c9f24829
ℹ️ 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".
| this._waRefCtxTime = when; | ||
| this._waRefTrackTime = effectiveOffset; | ||
| this.sourceNode.start(when, effectiveOffset + this._bufferStartPaddingSec); |
There was a problem hiding this comment.
If setPlaybackRate() is called during the new scheduling lead—especially the 150 ms suspended-context lead or a larger baseLatency lead—the existing rate-change logic treats this future anchor as though playback had already started and rewrites _waRefCtxTime/_waRefTrackTime, while the source still starts at the unchanged effectiveOffset. At when, the reported position therefore differs from the buffer position by (when - now) * (newRate - oldRate), and playbackEndContextTime reschedules the next track early or late, reintroducing an audible overlap or gap. The pre-start source offset/anchor needs to be adjusted separately when the rate changes before when.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Now guards the anchor rewrite with ctx.currentTime >= _waRefCtxTime — during the lead the source node's rate is updated directly (playbackEndContextTime reads it live) without touching the anchors. Addressed in d9f1832.
If setPlaybackRate() is called during the scheduling lead (before ctx.currentTime reaches _waRefCtxTime), the anchor rewrite computed a negative elapsed time, corrupting _waRefTrackTime and causing playbackEndContextTime to schedule the next track early or late. Now skips the anchor rewrite when ctx.currentTime < _waRefCtxTime. The source node's rate is still updated — playbackEndContextTime reads it live — so the end time stays correct without touching the anchors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Finding 1: WEBAUDIO_ONLY pendingPlay survives DEACTIVATE
- Clear pendingPlay on DEACTIVATE and ACTIVATE in both idle and
loading states of the track machine. Without this, a deactivated
track whose buffer arrives later would start its own source node
while another track is current — sustained overlap.
Finding 2: HTML5-clock prediction overlap
- Schedule a gain mute (5ms ramp to 0) on the current track's
_html5GainNode at endTime when scheduling gapless from an HTML5
track. If the HTML5 element stalls or runs late past the predicted
end, it's silenced instead of overlapping the next track.
- Cancel the mute in _cancelScheduledGapless.
Finding 3: _scheduledNextIndex set when SCHEDULE_GAPLESS is dropped
- scheduleGaplessStart now returns false if the track isn't in
idle/loading state. _tryScheduleGapless only sets
_scheduledNextIndex on success.
Finding 4: Residual δ from platform late-start
- Call sourceNode.stop(endTime) after start() in both
_startSourceNode and startScheduledSourceNode. This provides a
hard ceiling: any platform-level late start produces an inaudible
truncation instead of overlap with the next track.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05b757b63c
ℹ️ 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".
| // produces an inaudible truncation instead of overlap with the next track. | ||
| const bufRemaining = this.audioBuffer.duration - this._bufferStartPaddingSec - effectiveOffset; | ||
| if (bufRemaining > 0) { | ||
| this.sourceNode.stop(when + bufRemaining / rate); |
There was a problem hiding this comment.
When a WebAudio track is slowed down after it starts, this absolute stop time remains based on the rate captured at source creation, while setPlaybackRate() only updates the node's rate and the timing anchors. The node therefore stops at its original end time before the slowed buffer has finished; meanwhile gapless scheduling uses the new rate and starts the next track at the later calculated end, producing truncation followed by potentially substantial silence. Remove the hard stop or replace/reschedule the source when the rate changes.
Useful? React with 👍 / 👎.
1 participant
Summary
_startSourceNodeusedsource.start(0)which dispatches to the audio render thread asynchronously. The δ between the main-threadcurrentTimeread and the actual render-thread start (5-100ms depending on hardware/Bluetooth) madeplaybackEndContextTimecompute an end time δ too early — the next gapless track's source started before the current one finished, producing brief audible overlap. Now usesstart(when)with an explicit future time and corrected buffer offset so the anchor is exact.previous()leak fix: WhencurrentTime > 8,previous()calledseek(0)+play()directly on the Track, bypassing the queue state machine. The pre-scheduled next track's source node kept its stalewhen, producing full overlap whenctx.currentTimereached it. Now routes through the SEEK event which triggerscancelAndRescheduleGapless.Test plan
🤖 Generated with Claude Code