player: fill the album, report a cached track honestly, and stop double-appending

The album continuation appended exactly ONE track and then waited for the next
track start to append the one after, so a song started from search or an
artist page showed a queue of one or two entries however long the record was.
It still resolves one at a time -- a track cannot be queued before a peer with
it is found -- but it no longer stops after the first. My Name Is Jonas cold:
1 to all 10 in under 12 seconds, in album order. The resume anchor moved to
the LAST queued track so a fill cut short by going offline picks up where it
stopped instead of being locked out by the "next already queued" guard.

An adversarial review of that change found the hole it opened, and it is
fixed here rather than shipped: dropping the second clause of the bail-out was
wrong. streamSong installs autoAlbum only AFTER waitForStreamStart, which
polls the GLOBAL player status, so a second play issued during that buffering
window nulls autoAlbum and streamSong then re-installs its own context over
the top -- and the fill would go on appending one album's tracks to the other
album's live queue. The anchor must also still BE in the queue, which survives
our own appends and disappears exactly when the queue is replaced; and
streamSong no longer installs a context it no longer owns.

advanceGaplessly reported a pre-buffered track as zero-of-zero. streamTotal
=== 0 is the UI's "no byte counts yet" state, so a finished track showed the
indeterminate "Streaming from <peer>" sweep with no further event ever coming
to correct it. Gapless is only offered for a fully cached file, so its size IS
its received count. The buffering handler mirrors the store rather than
zeroing it for the same reason -- those counters are what a window restore
copies back, and zeroing them turned a complete transfer back into the sweep
on minimize and restore.

tryAppendNext set its appendedNextPath guard AFTER its await -- check then
act. Harmless with one caller; the moment the cached-play event gave it a
second same-tick caller both passed the guard and appended the same source to
the sink twice, which puts the panel a whole track ahead of the audio from the
next boundary on. It claims the slot before awaiting and gives it back on
failure.

ui.nowPlayingDragging is here because App.svelte's grid column has to leave
the 0.28s ease at the same instant the panel does.
This commit is contained in:
KeelrLeek committed 2026-07-29 08:12:07 -05:00
1 parent d460352973
commit 9cb19f42af
1 file changed
+86 -20
+86 -20
View File
@@ -55,6 +55,10 @@ export const ui = $state({
// flags: a width you set by hand and lost on every launch would be worse
// than not being able to set it at all.
nowPlayingWidth: readPanelWidth(),
// True only while the grip is held. The panel's width and the grid column
// it sits in share one clock (see App.svelte), so BOTH have to leave the
// 0.28s ease for a drag — the panel alone would outgrow its own column.
nowPlayingDragging: false,
// Full-window artwork lightbox. `null` when closed; otherwise a descriptor
// of whatever cover to show — { src, local, title, subtitle } — so any
// surface can open it, not only the playing track.
@@ -3483,12 +3487,20 @@ export async function streamSong(artist, title, opts = {}) {
quality: qualityOf(c.file),
},
];
const queuedItem = player.queue[0];
player.catalogRef = { albumId: opts.albumId ?? null, trackId: opts.trackId ?? null };
playQueueIndex(0);
if (await waitForStreamStart(15000)) {
// Keep the album going: resolve + pre-buffer the next album
// track in the background, Spotify-style.
if (opts.albumId) {
//
// Only if this song still owns the queue. `waitForStreamStart`
// polls the GLOBAL player status, so a play command issued while
// this one was buffering can satisfy it — and that command already
// cleared autoAlbum on its way past. Installing ours over the top
// would point the album fill at a queue belonging to something
// else entirely.
if (opts.albumId && player.queue[0] === queuedItem) {
autoAlbum = {
albumId: opts.albumId,
albumTitle: opts.album ?? null,
@@ -4179,8 +4191,16 @@ export function initEvents() {
player.position = 0;
latestPlayerPosition = 0;
latestPlayerDuration = player.duration;
latestStreamReceived = 0;
latestStreamTotal = 0;
// Mirror the store rather than zeroing it, the same way
// latestPlayerDuration does directly above. These counters are
// what a window restore copies back, and a fully cached track
// is reported COMPLETE before the audio thread ever says
// "buffering" — zeroing here threw that away, so minimising and
// restoring turned a finished transfer back into the
// indeterminate "Streaming from <peer>" sweep with no further
// event coming to correct it.
latestStreamReceived = player.streamReceived;
latestStreamTotal = player.streamTotal;
player.scrubbing = false;
player.error = "";
break;
@@ -4520,6 +4540,14 @@ async function tryAppendNext() {
const item = next >= 0 ? player.queue[next] : null;
if (!item?.stream || item.path === appendedNextPath) return;
if (player.status === "idle") return;
// Claim the slot BEFORE awaiting, and give it back only if the append
// fails. Setting it after the await is a check-then-act race: a fully
// cached track has two callers firing in the same tick — the stream
// event's `done` branch and play_stream's own `.then()` — and both would
// pass the guard above, append the same source to the sink twice, and
// leave the audio a whole track behind the panel from the next boundary on.
const claimed = appendedNextPath;
appendedNextPath = item.path;
try {
const ok = await api.preloadCachedStream(
item.stream.username,
@@ -4527,10 +4555,11 @@ async function tryAppendNext() {
item.stream.size,
item.duration ?? null,
);
if (ok) appendedNextPath = item.path;
if (!ok && appendedNextPath === item.path) appendedNextPath = claimed;
} catch {
// cache incomplete or command unavailable — the normal (non-gapless)
// advance still works.
if (appendedNextPath === item.path) appendedNextPath = claimed;
}
}
@@ -4550,11 +4579,19 @@ function advanceGaplessly() {
player.status = "playing";
appendedNextPath = null;
if (item.stream) {
// A pre-buffered stream track, playing from its completed cache
// file — no transfer to report.
// A pre-buffered stream track, playing from its completed cache file.
// Report it COMPLETE rather than zero-of-zero: `streamTotal === 0` is
// the UI's "no byte counts yet" state, so zeroing a finished track left
// the header showing the indeterminate "Streaming from <peer>" sweep
// with no further event ever coming to correct it. Gapless is only
// offered for a fully cached file — that is what made it appendable —
// so its size IS its received count.
const size = item.stream.size;
player.track = { title: item.title, sub: item.sub, source: "stream", path: item.path };
player.streamReceived = 0;
player.streamTotal = 0;
player.streamReceived = size;
player.streamTotal = size;
latestStreamReceived = size;
latestStreamTotal = size;
resetStreamRate();
if ("trackId" in item) player.catalogRef.trackId = item.trackId;
setAccentFromArtUrl(item.artUrl ?? null);
@@ -4571,17 +4608,28 @@ function advanceGaplessly() {
}
// ---- album continuation ----
// A single catalog song remembers its album: when it starts playing, the
// album's next track is resolved (background Soulseek search) and appended
// to the queue, one track per track start, so the album keeps going.
// A single catalog song remembers its album: when it starts playing, the rest
// of the album is resolved (a background Soulseek search per track) and
// appended to the queue, so the album keeps going.
//
// It used to append exactly ONE track and wait for the next track start to
// append the one after, which meant a song started from search or an artist
// page showed a queue of one or two entries however long the record was. It
// still resolves one at a time — a track cannot be queued before a peer with
// it is found — but it no longer stops after the first, so the queue fills in
// over the following minute instead of never.
let autoAlbum = null; // { albumId, albumTitle, cover, tracks, pending }
async function maybeExtendAlbumQueue() {
const ctx = autoAlbum;
if (!ctx || ctx.pending) return;
if (player.queueIndex !== player.queue.length - 1) return; // next already queued
const cur = player.queue[player.queueIndex];
if (!cur?.stream) return;
// Resume from the LAST thing queued, not from what is playing: the fill
// outlives the track it started on, and a run cut short (offline, a
// catalog hiccup) has to be able to pick up where it stopped on the next
// track start. When the album is already complete this finds nothing left
// to do and simply retires the context.
const anchor = player.queue[player.queue.length - 1];
if (!anchor?.stream) return;
ctx.pending = true;
try {
if (!ctx.tracks) {
@@ -4591,8 +4639,8 @@ async function maybeExtendAlbumQueue() {
ctx.albumTitle ??= page.album.title ?? null;
ctx.cover ??= page.album.cover ?? null;
}
let idx = ctx.tracks.findIndex((t) => t.id === cur.trackId);
if (idx < 0) idx = ctx.tracks.findIndex((t) => normText(t.title) === normText(cur.title));
let idx = ctx.tracks.findIndex((t) => t.id === anchor.trackId);
if (idx < 0) idx = ctx.tracks.findIndex((t) => normText(t.title) === normText(anchor.title));
if (idx < 0) {
autoAlbum = null;
return;
@@ -4615,8 +4663,24 @@ async function maybeExtendAlbumQueue() {
} else if (primary) {
artistIncluded = normText(primary).includes(normText(t.artist ?? ""));
}
// Bail if the user moved on while we were searching.
if (autoAlbum !== ctx || player.queue[player.queueIndex] !== cur) return;
// Bail if the user moved on while we were searching. BOTH tests
// are load-bearing:
//
// - context identity, because every path that replaces the queue
// clears autoAlbum;
// - the anchor still being IN the queue, because context identity
// alone is not enough. streamSong installs autoAlbum only after
// `await waitForStreamStart`, which polls the GLOBAL player
// status; a second play command issued during that window nulls
// autoAlbum first and streamSong then re-installs its own
// context over the top. The fill would go on appending one
// album's tracks to the other album's live queue.
//
// Not the currently-playing item: this loop deliberately outlives
// the track it started on, so advancing within the same album must
// not stop it. The anchor is a queue MEMBER, so it survives our own
// appends and disappears exactly when the queue is replaced.
if (autoAlbum !== ctx || !player.queue.includes(anchor)) return;
const best = songCandidates(
responses, t.title, true, ctx.albumTitle, t.duration_secs, t.artist, !artistIncluded,
)[0];
@@ -4634,11 +4698,13 @@ async function maybeExtendAlbumQueue() {
stream: { username: best.username, size: best.file.size },
quality: qualityOf(best.file),
});
// Both are guarded against repeat work (lastPreloadedPath,
// appendedNextPath), so calling them per append costs nothing once
// the immediate next track is already prepared.
preloadNextStream();
tryAppendNext();
return;
}
autoAlbum = null; // album finished
autoAlbum = null; // every remaining track queued or skipped
} catch {
// transient (offline, catalog hiccup) — retried on the next track start
} finally {