Repository navigation
Fix all gapless overlap paths - #25
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>
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>
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>
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>
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: 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 the user lowers the playback rate after this source starts, the absolute stop remains based on the original rate. setPlaybackRate() updates the source rate and playback anchor—and the queue schedules the next track from that updated end—but it never updates this stop time, so the source is cut off at its former wall-clock end, truncating the track and potentially leaving silence before the rescheduled successor. The same issue applies to the newly added stop in startScheduledSourceNode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Now re-issues sourceNode.stop() at the corrected end time in setPlaybackRate(). The spec allows repeated stop() calls (latest wins). Addressed in 8b8c1fe.
| // The skipped interval (≤20 ms at 1× rate) is below audible threshold. | ||
| const rate = this.queueRef.playbackRate; | ||
| const maxOffset = this.audioBuffer.duration - this._bufferStartPaddingSec; | ||
| const effectiveOffset = Math.min(offset + lead * rate, maxOffset); |
There was a problem hiding this comment.
When _startSourceNode handles an initial Web Audio play, resume, or seek, no other source covers the scheduling lead, so advancing the buffer offset by lead * rate permanently discards that audio. Every such operation skips at least 20 ms, and devices reporting higher baseLatency can lose substantially more; this correction is only appropriate during the HTML5 crossover, where HTML5 continues playing through the lead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Intentional trade-off — without the universal correction, currentTime drift accumulates across pause/resume cycles: each _startSourceNode call adds lead × rate to pausedAtTrackTime via the frozen-time capture. After N cycles the total drift is N × lead × rate, which becomes visible in the progress display.
With 20ms lead at 1× rate, the skipped audio per start is ≤20ms — below audible threshold and well below a single rAF frame. The alternative (drift accumulation) is worse for the user experience.
setPlaybackRate now re-issues sourceNode.stop() at the corrected end time after updating the rate. Without this, the stop remained based on the original rate — lowering the rate would truncate the track early, and raising it would leave the stale stop past the actual end. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deploying gapless with
|
| Latest commit: |
a7906b3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4fa259ff.gapless-e1l.pages.dev |
| Branch Preview URL: | https://fix-gapless-overlap-anchor.gapless-e1l.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b8c1feb58
ℹ️ 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._html5GainNode.gain.cancelScheduledValues(this.ctx.currentTime); | ||
| this._html5GainNode.gain.setValueAtTime(1, this.ctx.currentTime); |
There was a problem hiding this comment.
When a buffered current track crosses from HTML5 while an HTML5-predicted next start is already scheduled, the crossover first programs the HTML5 gain's 1→0 ramp, then TRACK_LOADED runs cancelAndRescheduleGapless, which reaches this method. cancelScheduledValues(currentTime) removes the newly programmed crossover ramp as well as the end mute and pins the HTML5 path at full gain, while Web Audio still fades from 0→1; the handoff therefore produces a brief amplified overlap followed by an abrupt cut instead of the intended crossfade. Track whether an end mute was scheduled and cancel only that automation without erasing the crossover ramp.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Now only calls cancelHtml5Mute() when the current track is still in HTML5 state — after crossover the gain node is owned by the crossfade ramp. Addressed in a7906b3.
After crossover, the HTML5 gain node's ramp (1→0) is managing the fade-out. Calling cancelScheduledValues there would wipe both the mute and the crossover ramp, pinning gain at 1 and producing an amplified overlap. Now guards the cancel with a playbackType check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review all commits |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7906b3f82
ℹ️ 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 delayMs = (when - this.ctx.currentTime + CROSSOVER_FADE_SEC) * 1000 + 5; | ||
| setTimeout(() => { | ||
| this.audio.pause(); |
There was a problem hiding this comment.
If the user pauses, seeks, skips, or changes tracks during the scheduling lead, the track machine has already transitioned to webaudio, but the HTML5 element remains audible until this timer fires. The webaudio PAUSE/SEEK/DEACTIVATE handlers only stop or replace the buffer source and do not pause HTML5 or cancel its pending gain automation, so a skipped track can overlap the replacement for the remainder of the lead; a quick pause/resume can also let the stale fade mute HTML5 before the replacement source starts, creating a gap. Track and cancel this pending crossover, and explicitly silence/pause HTML5 when the handoff is interrupted.
Useful? React with 👍 / 👎.
1 participant
Summary
Comprehensive fix for audio overlap during gapless transitions, addressing every identified path where two audio sources could play simultaneously.
Root cause: anchor timing error
_startSourceNodeusedsource.start(0)which dispatches to the audio render thread asynchronously. The δ between the main-threadcurrentTimeread and the actual start (5–100ms depending on hardware) madeplaybackEndContextTimecompute an end time δ too early — the next track started before the current one finished.Fix: Use
source.start(when)with an explicit future time (when = ctx.currentTime + lead). Lead =max(20ms, 2×baseLatency + 10ms). Buffer offset corrected bylead × rateto prevent position drift across pause/resume cycles.Additional overlap paths fixed
previous()bypass: WhencurrentTime > 8,previous()calledseek(0)+play()directly on Track, leaving the gapless schedule stale. Now routes through SEEK event →cancelAndRescheduleGapless.WEBAUDIO_ONLYpendingPlay leak:pendingPlaysurvived DEACTIVATE, causing a deactivated track to start its own source on BUFFER_READY. Now cleared on DEACTIVATE/ACTIVATE in idle and loading states.endTimeas a safety valve._scheduledNextIndexstale after dropped event:SCHEDULE_GAPLESSis only handled in idle/loading, but the queue set_scheduledNextIndexunconditionally. NowscheduleGaplessStartreturns false if the state rejects it.sourceNode.stop(endTime)called afterstart()as a hard ceiling — any render-thread late start produces inaudible truncation instead of overlap.whenin the no-MediaElementSourcefallback path.setPlaybackRateduring lead: Anchor rewrite guarded withctx.currentTime >= _waRefCtxTimeto prevent corruption during the scheduling lead.Test plan
🤖 Generated with Claude Code