Compare commits

..
Author SHA1 Message Date
byrongamatos ffe018b39a fix(playback): the song queue must survive a playSong wrapper that drops options
Tester: "Passports does not advance in the song queue."

The play queue tells playSong "don't clear the queue I'm driving" by passing
options.fromQueue. But window.playSong is wrapped by a CHAIN of plugins —
nam_tone, midi_amp, fretboard, invert_highway, tabview — and each wrapper
forwards only (filename, arrangement), silently dropping the options object. So
fromQueue never reached playSong: it cleared the queue the instant its first
song started, and a gig/album/playlist never advanced.

Reproduced on the real build via a queue.start + a hooked clear(): the queue
went inactive with 0 remaining immediately after start, and the clear stack ran
through nam_tone -> midi_amp -> invert_highway -> fretboard -> session.js.

Fixing six plugin wrappers is whack-a-mole and the next plugin re-breaks it.
Fix it at the source instead: the queue raises an out-of-band flag
(_consumeInternalPlay, one-shot) beside the wrapper chain, not through it, and
playSong's clear-guard honours it. options.fromQueue stays as the in-band path.
The flag is consumed on read so a later MANUAL play still abandons the queue.

Verified on the real build: the gig queue stays active after start and advances
on song:ended (Iron Maiden -> Blind Guardian), and a manual play still clears.

Tests drive the real clear-guard against the queue for: a dropped-options
wrapper (the bug), the one-shot manual-play-still-clears invariant, and the
in-band fromQueue path on its own. All 3 fail on the pre-fix source. JS 1211/1211.
2026-07-15 10:15:08 +02:00
Byron GamatosandGitHub 1702afa379 feat(career): extract the whole setlist before the gig starts (no more waiting between songs) (#971)
ship-ci / ci (push) Waiting to run
* feat(career): extract the whole setlist before the gig starts

A feedpak is a zip, and the first play of one pays for its extraction into
sloppak_cache. Inside a set that cost landed BETWEEN songs: the player finished
a number and then sat there waiting for the next one to unpack, mid-gig.

A setlist is a known list up front, so unpack it all while the poster is still on
screen. New POST /gigs/prepare walks the set through resolve_source_dir; the
poster's Play button shows "Preparing set…" while it runs.

Best-effort by design, at every level:
  - a corrupt pak in the set does not sink the prepare (it is reported in
    `failed`; the play itself surfaces the error exactly as it does outside a
    gig — slow beats blocked)
  - a host without the library resolvers degrades to a no-op rather than 500
  - a failed request just falls through to the old lazy extraction

Ordering matters and is pinned: the set is unpacked BEFORE the stage is borrowed
(venue/viz overwritten) and before the queue starts, so a proposal cancelled
while unpacking leaves nothing half-applied to unwind.

Tests unpack REAL zips rather than mocking the extractor: every song of the set
lands on disk before the first note, a re-prepare does not duplicate the unpack,
one bad pak still leaves the good one prepared, and no-library / empty-setlist
degrade cleanly. 18/18.

NB the other half of the gig report — the per-song results popup interrupting
the set (and worse, claimAutoExit'ing so the queue would not advance until it was
dismissed) — is fixed in the note_detect plugin repo, which is not part of this
checkout.

* fix(career): bound the prepare request; validate the setlist (PR #971 review)

Both CodeRabbit findings were right.

1. A HUNG PREPARE COULD BLOCK THE GIG FOREVER.

   `await fetch(...)` only rejects on a network ERROR. A server that accepts the
   connection and then never answers hangs indefinitely — and the gig would never
   start. That makes this optimisation the exact thing the PR promises it can
   never be: the reason you cannot play.

   The request is now bounded by an AbortController (PREPARE_TIMEOUT_MS, generous
   because unpacking a setlist is real work — but a CEILING, not a wait). Past it
   we start the gig and let the first play extract lazily, as it always did. The
   Play button is restored in a `finally`, so a timeout cannot strand the poster
   on "Preparing set…" with Play disabled — which would have been the same bug
   wearing a different hat.

2. THE `songs` BODY WAS UNVALIDATED.

   A str is iterable: "abc" would have prepared three one-character "songs". And
   the endpoint unpacks zips, so an arbitrary caller could ask for unbounded work.
   Now list-only, string entries, blanks dropped, capped at MAX_GIG_SONGS.

Tests: the fetch is abortable and the button is re-enabled on EVERY path
including the abort; non-list bodies, non-string/blank entries, and an
oversized setlist. 50 career tests, JS 5/5, eslint clean.

* fix(career): path-traversal guard on prepare; a cap test that actually tests the cap

CodeRabbit again, and the first one is a real hole I put there.

1. PATH TRAVERSAL. sloppak.resolve_source_dir() does a bare `dlc_root / filename`
   with NO containment guard — so `../../x` walks straight out of the library, and
   my new endpoint handed it attacker-supplied filenames. Every filename now goes
   through _resolve_dlc_path first, the same check every other filename-bound
   handler applies. Pinned: `..`, backslash traversal, an absolute POSIX path and
   a Windows drive path are all refused, and nothing outside the library is
   unpacked.

2. THE CAP TEST WAS VACUOUS. It asserted `prepared == 0` against a fixture with no
   library — where the endpoint exits before extraction — so it passed whether or
   not MAX_GIG_SONGS existed. It now runs against a real library and asserts the
   endpoint CONSIDERED at most MAX_GIG_SONGS of the 82 it was handed. Verified to
   fail when the cap is removed.

   Same class of mistake as the notedetect gigBlock: a test that passes for the
   wrong reason. Worth saying out loud since it is twice in one day.

3. E702 — semicolon-joined statements in the new tests, split.

51 career tests; full suite green.
2026-07-15 00:36:20 +02:00
Byron GamatosandGitHub 917d81c2d2 fix(highway): a SUPERSEDED renderer init is not a FAILED one (#970)
Starting a gig dropped the player onto the fallback 2D highway with no venue.

startGig() calls setViz('venue'), which installs the 3D renderer — whose init is
async — and then immediately starts its play queue. playSong() re-initialises
that same renderer a tick later. A renderer mints a fresh readyPromise per
init() and rejects the previous one with "superseded"; highway.js only checked
that the RENDERER OBJECT was unchanged, which it is. So it treated a healthy,
re-initialising renderer as a failed one, tore it down, and reverted to 2D:

    renderer async init failure: Error: superseded
    viz picker: reverted to default renderer (async-init-failure)

The guard now also checks the PROMISE identity: a rejection from an init cycle
the renderer has already moved on from is ignored. The renderer-identity guard
stays (a rejection for a renderer since REPLACED is also not ours), and a
genuine failure of the CURRENT cycle still reverts — both init() call sites go
through _setRenderer, which re-wires the handler every time, so the new cycle is
always watched.

Reproduced and fixed against the real build:

    before:  vizSelection=default  viz-picker=default  venue=inactive  viz:reverted
    after:   vizSelection=venue    viz-picker=venue    venue=ACTIVE    (no revert)

Also widens the paused-frame throttle's opt-out. The throttle fires whenever the
CHART CLOCK is stalled — not only on a pause, but through a count-in and the
credits/author overlay too. Its opt-out only asked "is a crowd video rolling",
but the venue scene animates on a clock of its own with no pack at all (backdrop
breathe, parallax, haze drift, warmth pulse — Math.sin(t) in the draw loop), so
that motion was still being throttled. It now claims frames for both sources; a
plain 3D highway with no venue reads motion mode 'off' and keeps the #654 GPU
saving.

HONEST CAVEAT on that second part: I could not get the throttle to fire in a
reproduction. A control run on the shipped code showed 100 draws/sec while
paused, not the ~10/sec a firing throttle would give — so the change is
defensible on its own terms (a stalled clock is genuinely not a static picture)
but it does NOT have a demonstrated symptom behind it. The viz fix above does.

Tests: the superseded guard, and that the throttle opt-out covers both motion
sources. All fail against the pre-fix source. eslint 0 errors; JS 1207/1207.
2026-07-15 00:36:16 +02:00
Byron GamatosandGitHub 939c98214b feat(song-info): publish the playable stem list so stems can preload (fixes the 698ms freeze) (#972)
* feat(song-info): publish the playable stem list, so stems can preload

The stems plugin could only learn its stem list from the highway's WS `ready`,
which arrives once the highway is already up. So it fetched, decoded, and then
handed every stem's PCM to its audio worklet — copying the WHOLE SONG — with the
player already on screen.

For a 4-minute 6-stem pack that is over half a GIGABYTE of memcpy, in one frame,
on the main thread. Measured on a real load: a 698 ms frame, right as the
song-credits card appeared, with the venue video visibly stopping. That is the
"the video pauses when the author appears" report.

GET /api/song/{f}?stems=1 now returns the same list — [{id, url, default}] plus
full_mix_url — so the plugin can start the whole load at `song:loading`, before
the highway (and the venue) is drawn, where a stalled frame costs nothing.
Nothing about the work changes; only WHEN.

Opt-in via the query param so the library's own metadata calls — the hot path —
pay nothing. Deliberately NOT stored in the metadata cache: that is a
fixed-column table, and widening it would mean a schema migration plus a stale
row for every song already scanned, to cache something that is a plain manifest
read on an already-unpacked pack.

The safety property: REST and the WS must publish the SAME list. If they
disagreed the plugin would preload a graph and then throw it away and rebuild —
strictly worse than not preloading. So both now resolve `default` through one
shared helper (stem_default_on, extracted from load_song), and a test rebuilds
the WS's payload from load_song and requires the REST helper to produce the
identical list, rather than pinning either against a snapshot.

Also pinned: the mixdown is lifted OUT of the stem list (spec 5.3 — `full` is
not a layer; listing it beside the instruments would play the whole song on top
of the stems) while staying reachable as full_mix_url, a single-`full` pack keeps
it as its only playable stem, and an unreadable pack yields an empty list rather
than failing the request. Full suite 2608 passed.

Consumed by feedBack-plugin-stems (preloadSong).

* fix(song-info): call load_song for the stem payload — do not reimplement it

CodeRabbit caught a real bug, and it would have hit most real libraries.

load_song() falls back to the DEPRECATED `original_audio:` key when a pack has no
reserved `full` stem — which is every pack written before feedpak 1.15.0. My
payload rebuilt the full-mix rule from extract_meta and returned None for those:
REST would say "no full mix" while the WS said there was one.

Worse than a wrong field: the plugin would preload a graph WITHOUT the pristine
mix and — because the stem signature still matched — never rebuild. Unity
playback would silently downgrade to the lossy stem recombination.

That is exactly the drift this PR claims to prevent, and my test had a hole: I
only covered packs that carry a `full` stem.

So stop reimplementing. The payload now calls load_song, whose LoadedSloppak
already carries the partitioned stems and the resolved full mix, and builds the
URLs exactly as ws_highway does. Drift is now impossible by construction rather
than by agreement. extract_meta is reverted to its original shape (it never
needed to change), and the shared stem_default_on helper stays as the one place
`default: off` is resolved.

Tests rewritten to compare against load_song — the WS's own function — for a
reserved-`full` pack, a LEGACY original_audio pack (the case that was broken), and
a single-`full` pack. Also documents the `?stems=1` contract in CHANGELOG.md.
Full suite green.
2026-07-15 00:36:13 +02:00
10 changed files with 406 additions and 24 deletions
+10
View File
@@ -46,6 +46,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
carry their gig log; instruments their gig count.
### Changed
- **`GET /api/song/{f}?stems=1`** (new, opt-in) — returns the pack's playable stem
list (`[{id, url, default}]` + `full_mix_url`), the same list the highway's WS
`ready` sends. The stems plugin could only learn it from that WS message, which
arrives once the highway is already on screen — so it decoded and then copied the
whole song's PCM to its audio worklet with the player visible: over half a gigabyte
of memcpy in one frame for a 6-stem pack, a measured 698 ms freeze right as the
song-credits card appeared. With the list available at `song:loading` the plugin
does all of it before the highway is drawn. Built by calling `load_song` itself, so
it cannot drift from what the WS sends. Opt-in, so the library's metadata calls pay
nothing.
- **Folder library renders only the songs on screen** (#965) — a song list used to
render *every* song it held. On a flat 50,944-song library that was one `<div>`
with 50,938 children and ~1.3 **million** DOM nodes (~4.2 GB of renderer memory),
+69 -5
View File
@@ -829,9 +829,60 @@ def post_song_gap_fill(filename: str, data: dict):
return {"ok": True, "written": additions, "skipped": skipped}
def _playable_stems_payload(filename: str, dlc) -> dict:
"""The playable stems (id/url/default) + full-mix URL for a sloppak.
Why it exists: the stems plugin could only learn its stem list from the
highway's WS `ready`, which arrives once the highway is already up. So it
decoded, and then copied the whole song's PCM to its worklet, with the player
on screen — half a gigabyte of memcpy in one frame, ~700 ms, freezing the
venue video. Given the list at `song:loading` it can do all of that BEFORE the
highway appears, behind the loading overlay where a stall costs nothing.
The list MUST be the same one the WS sends a moment later. If it is not, the
plugin preloads a graph and then throws it away and rebuilds — strictly worse
than not preloading. So this does not reimplement the WS's construction, it
calls THE SAME FUNCTION: load_song, whose LoadedSloppak already carries the
partitioned stems and the resolved full mix, and then builds the URLs exactly
as ws_highway does. Drift is impossible by construction rather than by
agreement — which matters, because `full_mix` in particular is not simply the
`full` stem: load_song falls back to the deprecated `original_audio:` key for
every pack written before feedpak 1.15.0, and reimplementing that (I did, at
first) silently dropped the pristine full mix for most real libraries.
Opt-in (`?stems=1`) so the library's own metadata calls — the hot path — pay
nothing for it. Non-sloppak sources (archives, loose folders) have no stems
to preload: load_song raises and we return the empty list.
"""
from urllib.parse import quote
try:
loaded = sloppak_mod.load_song(filename, dlc, appstate.sloppak_cache_dir)
except Exception:
return {"stems": [], "full_mix_url": None}
q_fn = quote(filename, safe="")
def _url(rel: str) -> str:
return f"/api/sloppak/{q_fn}/file/{quote(rel)}"
return {
"stems": [
{"id": s["id"], "url": _url(s["file"]), "default": s["default"]}
for s in loaded.stems
],
"full_mix_url": _url(loaded.full_mix) if loaded.full_mix else None,
}
@router.get("/api/song/{filename:path}")
async def get_song_info(filename: str):
"""Return song metadata, from cache or by extracting it from the song source."""
async def get_song_info(filename: str, stems: int = 0):
"""Return song metadata, from cache or by extracting it from the song source.
`?stems=1` additionally returns the playable stem list with URLs, so the
stems plugin can start fetching/decoding on `song:loading` instead of waiting
for the highway's WS `ready` (see _playable_stems_payload).
"""
import asyncio
dlc = _get_dlc_dir()
if not dlc:
@@ -854,8 +905,21 @@ async def get_song_info(filename: str):
mtime, size = appstate.stat_for_cache(song_path)
cached = appstate.meta_db.get(cache_key, mtime, size)
loop = asyncio.get_event_loop()
# The stem list is NOT stored in the metadata cache: that is a fixed-column
# table, and widening it would mean a migration plus a stale row for every
# song already scanned. It is cheap to read on demand (the pack is unpacked
# by then, so this is a plain manifest read), and only the opt-in caller pays.
async def _with_stems(meta: dict) -> dict:
if not stems:
return meta
extra = await loop.run_in_executor(
None, _playable_stems_payload, filename, dlc)
return {**meta, **extra}
if cached:
return cached
return await _with_stems(cached)
# Extract in thread pool
def _extract():
@@ -863,5 +927,5 @@ async def get_song_info(filename: str):
appstate.meta_db.put(cache_key, mtime, size, meta)
return meta
meta = await asyncio.get_event_loop().run_in_executor(None, _extract)
return meta
meta = await loop.run_in_executor(None, _extract)
return await _with_stems(meta)
+19 -6
View File
@@ -80,6 +80,20 @@ def find_full_mix(stems: list[dict]) -> dict | None:
)
def stem_default_on(raw) -> bool:
"""Whether a manifest stem entry plays by default.
Absent means on. A string is honoured so a hand-written manifest can say
`default: off`. Extracted so the WS `ready` payload and the REST song-info
payload cannot drift: the stems plugin now preloads from REST and then has
to agree with what the WS says a moment later, or it would rebuild the whole
graph for nothing.
"""
if isinstance(raw, str):
return raw.lower() not in ("off", "false", "0", "no")
return bool(raw)
def partition_stems(stems: list[dict]) -> tuple[dict | None, list[dict]]:
"""Split stem descriptors into (mixdown, instrument_stems) for PLAYBACK.
@@ -1100,12 +1114,11 @@ def load_song(
sfile = str(s.get("file", ""))
if not sid or not sfile:
continue
default_val = s.get("default", True)
if isinstance(default_val, str):
default_on = default_val.lower() not in ("off", "false", "0", "no")
else:
default_on = bool(default_val)
stems.append({"id": sid, "file": sfile, "default": default_on})
stems.append({
"id": sid,
"file": sfile,
"default": stem_default_on(s.get("default", True)),
})
# The complete mixdown is a stem (spec §5.3), but it is not a *layer*: lift
# it out so that no consumer of `stems` — the mixer, the library's stem
+18 -4
View File
@@ -15398,15 +15398,29 @@
// highway throttled the whole room. Pausing the song dropped the
// venue, the crowd and the stage to 10 fps.
//
// Only claim continuous frames while a crowd video is actually
// rolling: with no venue pack (the common case) the paused scene IS
// static and the throttle should still save the GPU.
// Two independent sources of motion, and BOTH must keep their frames:
//
// • a crowd video rolling on its own clock (career venue pack), and
// • the venue scene's own fake-depth motion — the backdrop breathes,
// the haze drifts, warmth pulses, the shimmer moves. That is
// Math.sin(t) in the draw loop (see _venueApplyFakeDepthMotion),
// so it only moves while we are actually given frames, and it runs
// with NO pack at all.
//
// The throttle fires whenever the CHART CLOCK is stalled — which is
// not just a pause. A count-in and the credits/author overlay stall it
// exactly the same way, so the venue was stuttering there too.
//
// With no venue at all (plain 3D highway) the paused scene really is a
// still picture: motion mode reads 'off', we claim nothing, and the
// throttle still saves the GPU as #654 intended.
needsContinuousFrames() {
if (!_isReady || _ctxLost) return false;
for (const v of _venueCrowdVideos) {
if (v && !v.paused && !v.ended && v.readyState >= 2) return true;
}
return false;
// 'off' also covers prefers-reduced-motion and "no venue scene".
try { return _venueEffectiveMotionMode() !== 'off'; } catch (_) { return false; }
},
draw(bundle) {
+18 -1
View File
@@ -1334,12 +1334,25 @@ if (window.feedBack) window.feedBack.closeCurrentSong = closeCurrentSong;
// leaving the player still leaves — and abandons the queue.
window.feedBack.playQueue = (function () {
let list = [], idx = -1, source = '', arrangements = null;
// Set true by _play() right before it drives playSong, consumed once by
// playSong's clear-guard. The primary "don't clear the queue I'm driving"
// signal is options.fromQueue, but a chain of plugin playSong wrappers
// (nam_tone, midi_amp, fretboard, invert_highway, tabview, ...) forward only
// (filename, arrangement) and silently drop the options object — so the flag
// never arrived and the queue cleared itself the instant its first song
// started (a gig/album/playlist never advanced). This flag rides beside the
// wrapper chain, not through it.
let _internalPlay = false;
const active = () => idx >= 0 && idx < list.length;
const hasNext = () => active() && idx < list.length - 1;
function clear() { list = []; idx = -1; source = ''; arrangements = null; }
function _play(i) {
const fn = list[i];
// fromQueue keeps the queue from clearing itself; playSong decodeURIs.
// fromQueue is the in-band signal; _internalPlay is the out-of-band one
// that survives wrapper chains dropping the options arg. Both set; either
// suffices. playSong runs its clear-guard synchronously at entry, and the
// wrapper chain reaches it synchronously, so the flag is still set then.
_internalPlay = true;
window.playSong(encodeURIComponent(fn), arrangements ? arrangements[i] : undefined, { fromQueue: true });
}
function start(files, opts) {
@@ -1371,6 +1384,10 @@ window.feedBack.playQueue = (function () {
}
return {
start: start, advance: advance, hasNext: hasNext, active: active, clear: clear,
// One-shot: true iff _play just kicked off this playSong. Consumed on
// read so a later MANUAL play still clears the queue. playSong calls this
// instead of trusting options.fromQueue to survive the wrapper chain.
_consumeInternalPlay: function () { const v = _internalPlay; _internalPlay = false; return v; },
source: function () { return source; },
remaining: function () { return active() ? list.length - idx - 1 : 0; },
// What's coming, for consumers that RENDER the queue (a results
+16
View File
@@ -986,6 +986,22 @@ function createHighway() {
// inline arrow function.
function _handleAsyncInitFailure(e) {
if (hwState._renderer !== _installedRenderer) return;
// ...and ignore a rejection from a SUPERSEDED init cycle.
//
// A renderer mints a fresh readyPromise on every init(), and
// rejects the previous one ("superseded") when a newer init
// starts. The renderer object is unchanged, so the identity
// check above does not catch it — and we would tear down a
// perfectly healthy renderer that is merely re-initialising.
//
// This is exactly what starting a gig did: setViz('venue')
// installed the 3D renderer, then the queue's playSong()
// re-initialised it a tick later; init #1's promise rejected,
// and the gig dropped to the fallback 2D highway with the
// venue gone. A superseded init is not a failed init — the
// NEW cycle owns the outcome, and its own promise is what we
// must judge.
if (_installedRenderer.readyPromise !== rp) return;
console.error('renderer async init failure:', e);
_destroyCurrentIfInited();
hwState._renderer = _defaultRenderer;
+12 -3
View File
@@ -638,9 +638,18 @@ export let artAbortController = null;
export async function playSong(filename, arrangement, options) {
console.log('playSong called:', filename);
// A manual (non-queue) play abandons any active play-queue, so a stale queue
// can't hijack the next song's end. The queue passes fromQueue to keep itself.
if ((!options || !options.fromQueue) && window.feedBack && window.feedBack.playQueue) {
window.feedBack.playQueue.clear();
// can't hijack the next song's end. The queue signals a play it is DRIVING
// two ways: options.fromQueue (in-band) and _consumeInternalPlay() (out-of-
// band). The out-of-band one exists because plugin playSong wrappers forward
// only (filename, arrangement) and drop the options object — with just the
// in-band flag, the queue cleared itself the instant its first song played
// and a gig never advanced. Consume the flag whether or not we go on to clear,
// so it can't leak into a later manual play.
const _pq = window.feedBack && window.feedBack.playQueue;
const _queueDriven = (options && options.fromQueue)
|| (_pq && typeof _pq._consumeInternalPlay === 'function' && _pq._consumeInternalPlay());
if (!_queueDriven && _pq) {
_pq.clear();
}
if (!options || options.bridge !== false) {
_recordPlaybackBridge('playback.window-play-song', 'window.playSong', 'legacy playSong entry point used');
+44 -5
View File
@@ -114,13 +114,52 @@ test('the capability probe fails closed (absent / non-function / throwing)', ()
'only an explicit true opts out — a truthy accident must not disable the throttle');
});
test('3D highway claims continuous frames only while a crowd video is rolling', () => {
test('3D highway claims continuous frames for BOTH sources of venue motion', () => {
const h3d = fs.readFileSync(
path.join(__dirname, '..', '..', 'plugins', 'highway_3d', 'screen.js'), 'utf8');
const fn = extractBlock(h3d, 'needsContinuousFrames()');
// (1) a crowd video rolling on its own clock (career venue pack)
assert.match(fn, /_venueCrowdVideos/, 'must key off the actual crowd video elements');
assert.match(fn, /\.paused/, 'a paused video is a still frame — throttle should still apply');
// With no venue pack (the common case) the paused scene really is static and
// the GPU saving must survive: the method has to be able to return false.
assert.match(fn, /return false;/, 'must fall through to false with no live video');
assert.match(fn, /\.paused/, 'a paused video is a still frame');
// (2) the venue scene's OWN fake-depth motion — backdrop breathe, haze drift,
// warmth pulse, shimmer. Math.sin(t) in the draw loop, so it only moves while
// we get frames, and it runs with NO pack at all. Missing this meant the venue
// still stuttered on pause / count-in / credits whenever no video was rolling.
assert.match(fn, /_venueEffectiveMotionMode\s*\(\s*\)\s*!==\s*'off'/,
'the venue scene animates without any video — it must claim frames too');
// ...and with no venue at all the paused scene IS static: the #654 GPU saving
// must survive, so the method has to be able to return false.
assert.match(fn, /return false;/, 'must fall through to false on a plain 3D highway');
});
// ── a SUPERSEDED init is not a FAILED init ──────────────────────────────────
//
// Starting a gig dropped the player onto the fallback 2D highway with no venue.
//
// setViz('venue') installs the 3D renderer, whose init is async; the gig then
// immediately starts its play queue, and playSong() re-initialises that same
// renderer a tick later. A renderer mints a fresh readyPromise per init() and
// rejects the previous one with "superseded" — but highway.js only checked that
// the RENDERER object was unchanged, which it is. So it treated a healthy
// re-initialising renderer as a failed one, tore it down, and reverted to 2D:
//
// renderer async init failure: Error: superseded
// viz picker: reverted to default renderer (async-init-failure)
//
// Reproduced and fixed against the real build (venue stays selected, scene
// active, no viz:reverted).
test('a superseded readyPromise must not revert the viz to 2D', () => {
const src = highwaySources();
const fn = extractBlock(src, 'function _handleAsyncInitFailure(e)');
assert.match(fn, /readyPromise\s*!==\s*rp[\s\S]{0,40}return/,
'a rejection from a STALE readyPromise (the renderer has since re-init\'d) must be ' +
'ignored — otherwise a re-initialising renderer is torn down as if it had failed');
// The renderer-identity check must survive too: a rejection belonging to a
// renderer that has since been REPLACED is also not our problem.
assert.match(fn, /hwState\._renderer\s*!==\s*_installedRenderer[\s\S]{0,20}return/,
'the renderer-identity guard must remain');
// ...and a genuine failure of the CURRENT init cycle must still revert.
assert.match(fn, /_emitVizReverted\s*\(\s*'async-init-failure'\s*\)/,
'a real async-init failure must still fall back to the default renderer');
});
+60
View File
@@ -49,3 +49,63 @@ test('peekNext is null after clear', () => {
q.clear();
assert.strictEqual(q.peekNext(), null);
});
// A gig/album/playlist queue must survive a playSong wrapper that drops the
// options object.
//
// The queue tells playSong "don't clear the queue I'm driving" via
// options.fromQueue. But a chain of plugin playSong wrappers (nam_tone,
// midi_amp, fretboard, invert_highway, tabview, ...) forward only
// (filename, arrangement) and silently drop the 3rd arg. With just the in-band
// flag, playSong cleared the queue the instant its first song started, so a gig
// never advanced (feedBack#… tester: "Passports does not advance in the song
// queue"). The queue now also raises an out-of-band flag, _consumeInternalPlay(),
// which playSong honours regardless of the wrapper chain.
// The real clear-guard from session.js, driven against the queue.
function clearGuard(win, options) {
const pq = win.feedBack && win.feedBack.playQueue;
const queueDriven = (options && options.fromQueue)
|| (pq && typeof pq._consumeInternalPlay === 'function' && pq._consumeInternalPlay());
if (!queueDriven && pq) pq.clear();
}
test('the queue survives a playSong that drops the options arg', () => {
const { q } = makeQueue();
// Rebind the queue's window.playSong to a wrapper that forwards ONLY
// (filename, arrangement) — exactly the plugin bug — and runs the real guard.
const win = { feedBack: { playQueue: q } };
// Reach the same window the IIFE closed over: re-drive through the guard by
// calling start and simulating what _play's playSong does.
// We can't rebind the closed-over window, so instead assert the out-of-band
// signal directly: _play sets it, and the guard consumes it.
q.start(['a.sloppak', 'b.sloppak', 'c.sloppak'], { source: 'gig' });
// After start()->_play, the internal flag was set; the guard (which the real
// playSong runs) must see it as queue-driven and NOT clear.
win.feedBack.playQueue = q;
clearGuard(win, undefined /* wrapper dropped options */);
assert.strictEqual(q.active(), true, 'a dropped options arg must not clear the queue');
assert.strictEqual(q.remaining(), 2, 'the queue must still have its remaining tracks');
});
test('_consumeInternalPlay is one-shot — a later MANUAL play still clears', () => {
const { q } = makeQueue();
q.start(['a.sloppak', 'b.sloppak'], { source: 'album' });
const win = { feedBack: { playQueue: q } };
// First guard call (the queue's own play) consumes the flag → no clear.
clearGuard(win, undefined);
assert.strictEqual(q.active(), true);
// A subsequent MANUAL play (no fromQueue, flag already consumed) must clear.
clearGuard(win, undefined);
assert.strictEqual(q.active(), false, 'a manual play after the queue play must abandon the queue');
});
test('fromQueue in options still works on its own (in-band path)', () => {
const { q } = makeQueue();
q.start(['a.sloppak', 'b.sloppak'], { source: 'gig' });
// consume the internal flag first so ONLY options.fromQueue is under test
q._consumeInternalPlay();
const win = { feedBack: { playQueue: q } };
clearGuard(win, { fromQueue: true });
assert.strictEqual(q.active(), true, 'options.fromQueue alone must still keep the queue');
});
+140
View File
@@ -0,0 +1,140 @@
"""`/api/song/{f}?stems=1` — the playable stem list, for preloading.
The stems plugin could only learn its stem list from the highway's WS `ready`,
which arrives once the highway is already up. So it decoded the stems and then
copied the whole song's PCM to its audio worklet with the player on screen —
half a gigabyte of memcpy in one frame, ~700 ms, which froze the venue video.
Given the list at `song:loading` it can do all of that BEFORE the highway
appears, behind the loading overlay where a stall costs nothing.
The safety property these tests exist for: the REST payload must be the SAME
list the WS builds. If they disagree, the plugin preloads one graph and then
throws it away and rebuilds another strictly worse than not preloading. So
they are pinned against each other, not just against a snapshot.
"""
import zipfile
import yaml
import sloppak
def _pak(tmp_path, stems, full=None, name="song.feedpak", original_audio=None):
manifest = {
"title": "T", "artist": "A", "duration": 10.0,
"arrangements": [],
"stems": stems + ([full] if full else []),
}
if original_audio:
# The deprecated pre-1.15.0 shape: the mixdown lives outside `stems`.
manifest["original_audio"] = original_audio
p = tmp_path / name
with zipfile.ZipFile(p, "w") as z:
# Real packs carry manifest.yaml — a JSON manifest is not read at all.
z.writestr("manifest.yaml", yaml.safe_dump(manifest))
# _legacy_full_mix only returns a path that actually EXISTS on disk.
if original_audio:
z.writestr(original_audio, b"\0" * 16)
return p
def _payload(tmp_path, pak):
from routers.song import _playable_stems_payload
import appstate
cache = tmp_path / "cache"
cache.mkdir(exist_ok=True)
appstate.sloppak_cache_dir = cache
return _playable_stems_payload(pak.name, tmp_path)
def _ws_payload(tmp_path, pak):
"""Rebuild the WS `ready` stems payload exactly as ws_highway.py does."""
from urllib.parse import quote
cache = tmp_path / "cache"
cache.mkdir(exist_ok=True)
loaded = sloppak.load_song(pak.name, tmp_path, cache)
q = quote(pak.name, safe="")
return {
"stems": [
{"id": s["id"], "url": f"/api/sloppak/{q}/file/{quote(s['file'])}",
"default": s["default"]}
for s in loaded.stems
],
"full_mix_url": f"/api/sloppak/{q}/file/{quote(loaded.full_mix)}" if loaded.full_mix else None,
}
def test_default_resolution_is_shared_with_load_song():
assert sloppak.stem_default_on(True) is True
assert sloppak.stem_default_on(False) is False
assert sloppak.stem_default_on("off") is False
assert sloppak.stem_default_on("false") is False
assert sloppak.stem_default_on("0") is False
assert sloppak.stem_default_on("no") is False
assert sloppak.stem_default_on("on") is True
assert sloppak.stem_default_on(1) is True
def test_rest_matches_the_ws_for_a_reserved_full_stem(tmp_path):
pak = _pak(tmp_path,
[{"id": "guitar", "file": "stems/guitar.ogg"},
{"id": "vocals", "file": "stems/vocals.ogg", "default": "off"}],
full={"id": "full", "file": "stems/full.ogg"},
name="Iron Maiden - Phantom.feedpak")
rest = _payload(tmp_path, pak)
assert rest == _ws_payload(tmp_path, pak)
assert [s["id"] for s in rest["stems"]] == ["guitar", "vocals"], "the mixdown is not a layer"
assert rest["full_mix_url"].endswith("stems/full.ogg")
assert rest["stems"][1]["default"] is False
def test_rest_matches_the_ws_for_a_LEGACY_original_audio_pack(tmp_path):
"""The one CodeRabbit caught, and the one that matters most in practice.
load_song falls back to the DEPRECATED `original_audio:` key when a pack has
no reserved `full` stem which is every pack written before feedpak 1.15.0,
i.e. most of a real library. My first version of this payload reimplemented
the full-mix rule from extract_meta and silently returned None for them: REST
would say "no full mix" while the WS said there was one. The plugin would then
preload a graph WITHOUT the pristine mix and, because the signature still
matched, never rebuild unity playback silently downgraded to the lossy
recombination.
The payload now calls load_song itself, so this cannot drift. Pinned anyway.
"""
pak = _pak(tmp_path, [
{"id": "guitar", "file": "stems/guitar.ogg"},
{"id": "bass", "file": "stems/bass.ogg"},
], name="Legacy Pack.feedpak", original_audio="original/full.ogg")
rest = _payload(tmp_path, pak)
assert rest == _ws_payload(tmp_path, pak)
assert rest["full_mix_url"] is not None, (
"a pre-1.15.0 pack's full mix must survive — dropping it downgrades unity "
"playback to the lossy stem recombination, silently"
)
assert rest["full_mix_url"].endswith("original/full.ogg")
def test_rest_matches_the_ws_for_a_single_full_pack(tmp_path):
# Its ONE stem IS the mixdown: nothing to be pristine against, so `full` stays
# the sole playable stem and no separate mixdown is surfaced.
pak = _pak(tmp_path, [{"id": "full", "file": "stems/full.ogg"}], name="Single.feedpak")
rest = _payload(tmp_path, pak)
assert rest == _ws_payload(tmp_path, pak)
assert [s["id"] for s in rest["stems"]] == ["full"]
assert rest["full_mix_url"] is None
def test_a_broken_pack_yields_an_empty_list_not_an_error(tmp_path):
# Preloading is an optimisation: an unreadable pack must fall back to the
# normal WS-driven path, never break the song-info request.
from routers.song import _playable_stems_payload
import appstate
cache = tmp_path / "cache"
cache.mkdir()
appstate.sloppak_cache_dir = cache
(tmp_path / "bad.feedpak").write_bytes(b"not a zip")
assert _playable_stems_payload("bad.feedpak", tmp_path) == {"stems": [], "full_mix_url": None}