Repository navigation
Key-based queue editing, repeat, snapshot/restore and onChange; tracks identified by reference - #31
Conversation
…(key) Every edit that keeps the current track goes through Queue._applyOrder, which reorders _tracks, reassigns Track.index and sends the machine a SYNC event. The scheduled gapless track is held by reference, and is cancelled and rescheduled only when the track after the current one changes. Callbacks from dropped tracks are ignored. Fuzz tests cover edits mixed with navigation, including an async HYBRID run with real gapless scheduling and a 'scheduled track is current + 1' invariant.
- setRepeat('off' | 'all' | 'one') and a repeat constructor option.
'all' wraps from the last entry to the first, gaplessly; 'one' restarts
the current entry when it ends (not sample-gapless) and fires
onStartNewTrack again. One helper, _nextIndexOf, answers 'what plays
after this' for next(), preloading, scheduling and _applyOrder.
- snapshot() / restore(): plain data (entries, currentKey, currentTime,
repeat) for persisting a queue and rebuilding it paused.
- onChange(listener): order, current entry or repeat changed. Batched into
a microtask, once per edit, so listeners can edit the queue.
- README documents the key-based API.
- Fuzz: setRepeat op, a repeat-aware scheduling invariant, FUZZ_SEEDS for
wider sweeps, and a 30 s timeout for loaded machines.
Codex Review Summary: ✅ CompletedThis 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: |
a81669b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c5e5b6b6.gapless-e1l.pages.dev |
| Branch Preview URL: | https://feat-play-order.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: 7ecb18c5f4
ℹ️ 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._orderVersion++; | ||
| this._actor.send({ type: 'ADD_TRACK' }); |
There was a problem hiding this comment.
When the last entry is playing under repeat 'all', the first entry may already have a scheduled gapless start. Calling addTrack() changes the next entry, but ADD_TRACK only increments the machine's track count and never cancels that wrap schedule. When the current track ends, the machine starts the newly appended entry while the first entry's scheduled source also starts, producing overlapping audio. Reconcile the scheduled successor after appending, as the key-based editing path does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2d0d7a6. addTrack now re-checks what plays next, using the same helper as setRepeat, so a scheduled wrap to the first track is cancelled and the appended track gets scheduled instead. Test: "addTrack while the last entry plays under repeat 'all' moves the schedule off the wrap".
| // setEntries keeps a surviving current track where it is; restore moves it. | ||
| if (snapshot.currentKey && current && current.key === snapshot.currentKey) { | ||
| this.goto(snapshot.currentKey, { play, startTime: snapshot.currentTime }); |
There was a problem hiding this comment.
When restoring into a paused queue whose current entry survives in the snapshot but differs from snapshot.currentKey, setEntries() preserves that current entry and ignores startKey and startTime. This condition then skips the corrective goto(), so neither the saved entry nor its position is restored. For example, restoring a snapshot at b:30 into a paused queue currently on a, with both entries present, leaves it on a. Perform the corrective navigation whenever reconciliation preserves the old current entry, including when its key differs from the saved key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2d0d7a6. restore() now runs the corrective goto(currentKey, { startTime }) whenever setEntries kept the old current track, whether or not it's the saved one. Test: "restore moves to the saved entry when a different current entry survives" (a → b at 30 s).
| metadata?: TrackMetadata; | ||
| }) { | ||
| this.index = opts.index; | ||
| this.key = opts.key ?? `gapless-${++_autoKey}`; |
There was a problem hiding this comment.
After a page reload, _autoKey resets to zero, but restoring entries with explicit keys does not advance it. Consequently, restoring a snapshot containing an automatically generated gapless-1 entry and then calling addTrack(url) generates another gapless-1. Key-based navigation and removal then cannot distinguish the entries, and a subsequent setEntries() reconciliation can reuse the same Track instance for both positions. Generate keys that cannot collide with keys already restored into the queue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2d0d7a6. Queue now generates keys itself and skips any already in the queue, so keys restored from a snapshot can't collide with new ones. The test restores exactly the keys the counter would generate next, then adds tracks. The same commit also makes setEntries, insert and addTrack throw on duplicate keys supplied by the caller, before changing anything. That covers the same corruption when the caller passes the key.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0b6aad8a9
ℹ️ 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._actor.send({ | ||
| type: 'GOTO', | ||
| index: startIndex, | ||
| playImmediately: opts.play ?? this.isPlaying, | ||
| startTime: opts.startTime, |
There was a problem hiding this comment.
When a playing queue's current entry is removed by setEntries(newEntries, { play: false }), the old track is stopped, but SYNC leaves the machine in playing. The subsequent GOTO takes the playing state's non-immediate branch, which also has no state transition and never starts the replacement track. The queue therefore remains silent while isPlaying reports true; calling play() cannot recover because the playing state has no PLAY handler. Explicitly transition to paused when replacing the current entry with playback disabled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 1ed74a5. The cause is older than this PR. On master, gotoTrack(i, false) while playing does the same thing: the playing state's non-immediate GOTO had no target, so the machine stayed in playing with nothing playing. That transition now targets paused, as it already did from idle and ended, which fixes both gotoTrack(i, false) and setEntries(..., { play: false }). Tests: "setEntries({ play: false }) that drops the current entry pauses on the new one", "gotoTrack(i, false) pauses on the new track", and a machine test split into the immediate and non-immediate cases.
…que keys - addTrack re-checks what plays next. Appending while the last track plays under repeat 'all' used to leave the first track's gapless start scheduled, so it and the new track both started. - restore() moves to the saved entry and time whenever setEntries kept the old current track, not only when it was the saved one. - Queue generates keys itself and skips keys already in the queue: after a reload the counter starts over and could collide with restored keys. - setEntries, insert and addTrack throw on duplicate keys before changing anything.
GOTO without playImmediately from 'playing' deactivated the old track
and didn't start the new one, but stayed in 'playing': silent, with
isPlaying true and no PLAY handler to recover. It now moves to 'paused',
as it already did from idle and ended. This affected gotoTrack(i, false)
on master, and setEntries(..., { play: false }) when it drops the
current entry.
When the order changes mid-track (a shuffle, insert or move), the old next track's gapless start was cancelled but its download kept going, competing with the new next track for bandwidth. Now every preload pass, and every reorder, aborts in-flight downloads outside the window (the current track plus the next preloadNumTracks in play order). This also covers a goto to a distant track. - Track.cancelPreload(): stops the fetch/decode child (aborting its fetch) and resets the loading state, only while resolving the URL or fetching: a decode can't be aborted and assigns the buffer itself, and by then the bytes are already here. Decoded buffers are kept. - The paused state's TRACK_ENDED didn't run preloadAhead, unlike every other move to a new track; it does now. - Fuzz: the async scenario uses slow downloads so they're in flight across edits, with a 'nothing outside the window is downloading' invariant. With cancelling disabled it fails within a few ops.
The machine held currentTrackIndex and trackCount, events carried
indices, and Track kept a mutable index that every reorder renumbered,
while the public API identified tracks by key. Now there's one model:
- The queue machine is generic over its entries. Its context is
{ current, list }, where list is a PlayList the queue implements
(next, previous, first, size, repeatsCurrent). Repeat lives only in
the queue; SET_REPEAT is gone.
- Events carry entries: GOTO { track }, REMOVE_TRACK { track, successor },
TRACK_LOADED { track }. SYNC and ADD_TRACK become LIST_CHANGED
{ current }: the queue says which entry is current.
- Track.index is gone. Queue._nextOf(track) is the one answer to 'what
plays next' (wrapping under repeat 'all'), for the machine, preloading,
scheduling and the preload window. onProgress and onPreloadReady
compare tracks, not indices.
- Positions are derived at the edge: currentTrackIndex, TrackInfo.index,
gotoTrack(i), removeTrack(i), queueSnapshot. The public API is
unchanged.
Tests: the machine tests use a string PlayList and cover wrap, repeat
'one', LIST_CHANGED and removing the current entry. Fuzz invariants now
read the machine's real current entry and check scheduling and the
preload window against an independent position spec, instead of calling
the helpers they test.
switz
changed the title
This adds a key-based API for editing the queue while it plays, plus repeat modes, snapshot/restore and change events. It's groundwork for Relisten's editable play queue ("Next up" plus the show or song list it came from).
It also changes how the queue identifies tracks internally: the state machine holds the current track by reference, and indices only mean positions. The public API is additive;
addTrack,removeTrack(index)andgotoTrack(index)still work.API
setEntries. Relisten keeps "Next up" out of the shuffle, and that rule belongs in the app.'all'wraps gaplessly from the last entry to the first.'one'restarts the entry and firesonStartNewTrackagain; the restart isn't sample-gapless.next()still moves on under'one'.onChangefires once per edit, in a microtask after it, so listeners can edit the queue.How it works
One identity model. Tracks are identified by reference, and indices are only positions:
{ current, list }, wherelistis aPlayListthe queue implements (next,previous,first,size,repeatsCurrent). Repeat lives only in the queue.GOTO { track },REMOVE_TRACK { track, successor },TRACK_LOADED { track }, andLIST_CHANGED { current }, where the queue says which entry is current after an edit.Track.indexis gone.Queue._nextOf(track)is the one answer to "what plays next" (it wraps under repeat'all'), for the machine, preloading, gapless scheduling and the preload window._scheduledNext), so a reorder can't point it at the wrong track.currentTrackIndex,TrackInfo.index,gotoTrack(i),removeTrack(i),queueSnapshot.Edits. Every edit that keeps the current track goes through
_applyOrder. It swaps the array, tells the machineLIST_CHANGED, and reschedules gapless only if the next track changed.Aborting displaced downloads. Every preload pass, and every reorder, calls
Track.cancelPreload()on tracks outside the window. That stops the fetch/decode child, which aborts itsfetch. It only cancels while the URL is being resolved or the file fetched: a decode can't be aborted, and by then the bytes have arrived.Behaviour changes
gotoTrack(i, false)while playing now pauses on the new track. Before, the old track stopped, the new one didn't start, and the queue stayedplayingwith nothing playing.play()couldn't recover. This already happened on master.setEntries,insertandaddTrackthrow on duplicate keys. They check before changing anything.Testing
play-order.test.ts: no interruption, schedule kept or redone, buffer reuse, stale callbacks;repeat-snapshot.test.ts: repeat, snapshot round-trip,onChange, review fixes;preload-cancel.test.ts: displaced downloads aborted, buffers kept, the paused case;queue.machine.test.ts: rewritten against a stringPlayList.FUZZ_SEEDS=60) passes all 1037 cases.LIST_CHANGEDwhile ended, emptying while playing, and the pausedTRACK_ENDEDpreload.'all'wrap is gapless, and'one'restarts every 2.00 s;net::ERR_ABORTED), and the new next track plays gaplessly.Not in this PR
The version bump and npm publish (4.5.0), after review.