From 22059c24ca7ef229498fe970992411173bc20c06 Mon Sep 17 00:00:00 2001 From: Michael Freno Date: Thu, 13 Aug 2026 21:04:12 -0400 Subject: [PATCH] fix(visualizer): show loading spinner on resume until fresh bars arrive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resume re-arms a pipeline whose ffmpeg pass was killed at pause, so the pre-pause bars are stale until fresh frames flow. Three changes: - resumeVisualization always sets the loading state (previously only for positions outside decoded coverage) and records the resume point; renderFrame clears it only once the position clock advances past that point — a player still re-buffering after a long pause keeps the spinner instead of serving static cached bars. - stopVisualization clears barData so cold restarts (unload, disable, episode change) show the spinner rather than stale bars, and never suppress it. - renderFrame detects a frozen position clock while playing (STALL_DETECT_MS) and surfaces it as a loading state; recovery clears it. Tests: resume-into-undecoded-audio shows loading until bars land; frozen position clock surfaces a stall and recovery clears it; disable/enable pins barData cleared on stop and the restart loading flash. --- src/pages/Player/RealtimeWaveform.tsx | 18 +++-- src/stores/visualizer.ts | 99 +++++++++++++++++++++++- tests/visualizer-store.test.ts | 105 +++++++++++++++++++++++++- 3 files changed, 212 insertions(+), 10 deletions(-) diff --git a/src/pages/Player/RealtimeWaveform.tsx b/src/pages/Player/RealtimeWaveform.tsx index f469ab3..3f30344 100644 --- a/src/pages/Player/RealtimeWaveform.tsx +++ b/src/pages/Player/RealtimeWaveform.tsx @@ -8,8 +8,9 @@ * * This component only subscribes to store state, reports the width-derived * bar count (terminal resize re-inits the running pipeline), and renders: - * a braille spinner while the pipeline is loading its first frames, the - * frequency bars once frames arrive, and a dotted placeholder when idle. + * a braille spinner while the pipeline is loading its first frames or the + * player is stalled (re-buffering), the frequency bars once frames arrive, + * and a dotted placeholder when idle. */ import { createEffect, on } from "solid-js"; @@ -53,12 +54,13 @@ export function RealtimeWaveform() { const bars = viz.barData(); const count = numBars(); - // Loading state: the braille spinner shows while the pipeline warms - // up — but only when there are no bars to render yet (first play / - // after an unload). On resume/seek the last bars stay on screen - // until fresh frames arrive, so the waveform never blanks out for - // the (multi-second, network-bound) cold start. - if (bars.length === 0 && viz.isLoading()) { + // Loading state: the braille spinner shows while the pipeline is + // warming up — cold start (first play / after an unload), resume + // into undecoded audio, or a stalled position clock (mpv + // re-buffering after a long pause on a network stream). The store + // clears it the moment the first fresh frame renders, so stale + // bars never masquerade as live data while the pipeline re-arms. + if (viz.isLoading() || viz.isStalled()) { return ; } diff --git a/src/stores/visualizer.ts b/src/stores/visualizer.ts index 6e04b75..a6f1a03 100644 --- a/src/stores/visualizer.ts +++ b/src/stores/visualizer.ts @@ -25,6 +25,12 @@ * after the Player tab stops being focused it tears down. Reads outside * decoded coverage return empty — the renderer simply holds the last frame * until the decode frontier arrives. + * + * Loading semantics: `isLoading` is true from any pipeline start (cold + * start, resume into undecoded audio) until the first complete FFT frame, + * and `isStalled` while playback claims to be live but the position clock + * is frozen (player re-buffering). The component renders the spinner for + * either; bars replace it the moment fresh frames arrive. */ import { @@ -55,6 +61,14 @@ const FRAME_INTERVAL = 33; /** Number of PCM samples to read per frame (512 is a good FFT window) */ const SAMPLES_PER_FRAME = 512; +/** + * How long the position clock may stay frozen while the UI believes + * playback is live before the waveform reports a stall (loading state). + * mpv polls time-pos every ~150ms, so a frozen clock means the player is + * re-buffering — the long-pause-then-resume case on network streams. + */ +const STALL_DETECT_MS = 2000; + /** Timer handle as returned by setTimeout/setInterval in this runtime. */ type TimerHandle = ReturnType; @@ -65,6 +79,10 @@ export interface VisualizerStore { barData: () => number[]; /** True from pipeline start until the first complete FFT frame renders. */ isLoading: () => boolean; + /** True while playback claims to be live but the position clock has + * been frozen past STALL_DETECT_MS (player re-buffering, e.g. after a + * long pause on a network stream). */ + isStalled: () => boolean; /** True while the ~30fps render loop is armed. */ isRunning: () => boolean; /** Report whether the Player tab is the visible tab. */ @@ -82,6 +100,10 @@ function createVisualizerStore(): VisualizerStore { // True from pipeline start until the first complete FFT frame renders. const [isLoading, setIsLoading] = createSignal(false); + // True while playback is live but the position clock is frozen + // (player re-buffering) — see STALL_DETECT_MS. + const [isStalled, setIsStalled] = createSignal(false); + // Whether the Player tab is the visible tab (fed by PlayerPage). const [focused, setFocused] = createSignal(false); @@ -103,6 +125,20 @@ function createVisualizerStore(): VisualizerStore { let sampleBuffer: Float64Array | null = null; let unloadTimer: TimerHandle | null = null; + // Stall tracker: last observed position-signal value and when it moved. + // Any change (forward, backward, seek) re-arms the clock; a frozen + // signal while playing trips isStalled after STALL_DETECT_MS. + let lastRenderPos = -1; + let lastPosMoveAt = 0; + + // Resume point: the position a paused pipeline was re-armed at. The + // loading state set by resume only clears once the position clock has + // advanced PAST this — while the player is still re-buffering, the + // cache can serve the same window forever and the stale pre-pause bars + // must not masquerade as live data. -1 = cold start (clear on the + // first produced frame, regardless of the clock). + let resumePos = -1; + // What the running pipeline was started with — lets the playback effect // tell "nothing changed, stay warm" from "must restart". let activeUrl = ""; @@ -200,9 +236,19 @@ function createVisualizerStore(): VisualizerStore { lastPolledPosition = position; lastPolledAt = performance.now(); + // Seed the stall tracker: a fresh pipeline should not report a + // stall just because the first position poll hasn't landed. + lastRenderPos = position; + lastPosMoveAt = performance.now(); + + // Cold start: the loading state clears on the first produced frame + // (see renderFrame) — no resume-position gating. + resumePos = -1; + activeUrl = url; activeBars = barCount(); setIsLoading(true); + setIsStalled(false); frameTimer = setInterval(renderFrame, FRAME_INTERVAL); }; @@ -224,6 +270,14 @@ function createVisualizerStore(): VisualizerStore { } sampleBuffer = null; setIsLoading(false); + setIsStalled(false); + // Drop the last rendered frame: after a stop the bars are stale (a + // different episode, a different position) and would masquerade as + // live data while the next cold start warms up — and, because the + // component only shows the spinner while bars are empty, they'd + // also suppress the loading state. Cold restarts re-render fresh + // bars within the first frame. + setBarData([]); }; // ── Pause: freeze the loop, keep the cache ────────────────────────── @@ -248,6 +302,7 @@ function createVisualizerStore(): VisualizerStore { // (still cold-starting when paused), the component should fall back // to the placeholder, not freeze on a spinner. setIsLoading(false); + setIsStalled(false); }; // ── Resume: re-arm the render loop, top up the cache ─────────────── @@ -269,6 +324,20 @@ function createVisualizerStore(): VisualizerStore { lastPolledPosition = pos; lastPolledAt = performance.now(); + // Re-arm the stall tracker from the resume position (a long pause + // left the old timestamps stale — they'd trip the stall detector on + // the very first frame otherwise). + lastRenderPos = pos; + lastPosMoveAt = performance.now(); + + // Resume re-arms a pipeline whose ffmpeg pass was killed at pause: + // the pre-pause bars are stale until fresh frames flow, so show the + // loading state IN THEIR PLACE. It clears only once the position + // clock has advanced past the resume point (see renderFrame) — a + // player still re-buffering after a long pause keeps the spinner + // instead of serving static cached bars. + resumePos = pos; + setIsLoading(true); frameTimer = setInterval(renderFrame, FRAME_INTERVAL); return true; }; @@ -282,6 +351,26 @@ function createVisualizerStore(): VisualizerStore { // coverage (decode cold start, seek into a hole) the read is empty // and the LAST FRAME simply holds — never clamped/repeated junk. const target = smoothPosition(); + + // Stall detection: while the UI believes playback is live, the + // position signal must keep advancing (useAudio polls it every + // ~150ms). A frozen clock with a warm pipeline means the player is + // re-buffering — the classic long-pause-then-resume on a network + // stream — and without this the waveform shows dead-looking static + // bars for the whole stall. Report it as loading; the first frame + // after the clock moves again clears it. + const rawPos = audioPlaybackSignals.position(); + if (rawPos !== lastRenderPos) { + lastRenderPos = rawPos; + lastPosMoveAt = performance.now(); + if (isStalled()) setIsStalled(false); + } else if ( + audioPlaybackSignals.isPlaying() && + performance.now() - lastPosMoveAt > STALL_DETECT_MS + ) { + setIsStalled(true); + } + const count = pcm.readWindow(sampleBuffer, target); // Never feed a partial FFT window to cava. if (count < sampleBuffer.length) return; @@ -290,7 +379,14 @@ function createVisualizerStore(): VisualizerStore { // Normalize against the running peak and copy to a new array setBarData(scaler(output)); - if (isLoading()) setIsLoading(false); + // Fresh frames only count once the position clock has moved past + // the resume point: while the player is still re-buffering after a + // long pause, the cache serves the same window and the spinner must + // stay in place of the stale bars. Cold starts (resumePos < 0) + // clear on the first frame as before. + if (isLoading() && (resumePos < 0 || rawPos > resumePos)) { + setIsLoading(false); + } }; // ── Playback subscription ────────────────────────────────────────── @@ -425,6 +521,7 @@ function createVisualizerStore(): VisualizerStore { // state barData, isLoading, + isStalled, isRunning: () => frameTimer !== null, // inputs setFocused, diff --git a/tests/visualizer-store.test.ts b/tests/visualizer-store.test.ts index 52b5d03..13a4536 100644 --- a/tests/visualizer-store.test.ts +++ b/tests/visualizer-store.test.ts @@ -230,15 +230,118 @@ test.skipIf(skip)( app.updateVisualizer({ enabled: false }); await waitFor(() => !viz.isRunning(), 10000); expect(viz.isLoading()).toBe(false); + // Stopping the pipeline must drop the last rendered frame — a cold + // restart (re-enable, unload, episode change) would otherwise show + // stale bars from the previous run and never reach the loading + // state (the spinner only shows while bars are empty). + expect(viz.barData().length).toBe(0); app.updateVisualizer({ enabled: true }); - await waitFor(() => viz.isRunning(), 10000); + // The restart surfaces the loading state before the first frame. + await waitFor(() => viz.isLoading(), 5000); await waitFor(() => !viz.isLoading() && viz.barData().length > 0, 10000); expect(viz.barData().length).toBe(64); }, { timeout: 20000 }, ); +// A pause followed by a seek while paused, then resume, lands OUTSIDE the +// decoded sliding window: the cache can't serve bars instantly, so the +// store must surface the warm-up as a loading state instead of silently +// holding the stale pre-pause frame. Regression: resumeVisualization never +// set isLoading, so the last frame froze with no feedback until the +// re-decode's first frame landed. +test.skipIf(skip)( + "resume into undecoded audio shows the loading state until bars land", + async () => { + const viz = useVisualizer(); + await startPlaying(); + expect(viz.barData().length).toBe(64); + + // Pause, then seek far ahead while paused (outside the ~10s of + // decoded coverage), then resume. + setIsPlaying(false); + await waitFor(() => !viz.isRunning(), 10000); + setPosition(30); + setIsPlaying(true); + + // The resume position isn't decoded yet — loading, not frozen bars. + await waitFor(() => viz.isLoading(), 5000); + expect(viz.isRunning()).toBe(true); + + // Playback advances past the resume point (mpv moves the clock); + // once the re-decode covers it, fresh bars replace the stale + // pre-pause frame (chirp spectrum at 30s ≠ 2s) and the loading + // state clears. + setPosition(31); + const barsBefore = viz.barData(); + await waitFor( + () => !viz.isLoading() && viz.barData() !== barsBefore, + 15000, + ); + expect(viz.barData().length).toBe(64); + }, + { timeout: 30000 }, +); + +// After a long pause on a network stream, the player (mpv) re-buffers: +// `isPlaying` stays true but the position clock freezes. Without +// detection the waveform rendered the same cached window forever — static +// bars and no feedback. The render loop must report the stall as a +// loading state and clear it the moment the clock moves again. +test.skipIf(skip)( + "a frozen position clock while playing surfaces a stall; recovery clears it", + async () => { + const viz = useVisualizer(); + await startPlaying(); + expect(viz.isStalled()).toBe(false); + + // Freeze the position: isPlaying stays true, the clock never moves. + await waitFor(() => viz.isStalled(), 10000); + + // Player recovers — the clock advances again. + setPosition(4); + await waitFor(() => !viz.isStalled(), 3000); + expect(viz.isRunning()).toBe(true); + }, + { timeout: 20000 }, +); + +// Resume re-arms a pipeline whose ffmpeg pass was killed at pause: the +// stale pre-pause bars must not masquerade as live data while the player +// recovers. The spinner shows IN THEIR PLACE until the position clock +// advances past the resume point — a frozen clock (mpv re-buffering after +// a long pause) keeps the spinner even though the cache can serve the +// same window. +test.skipIf(skip)( + "resume shows the loading state in place of stale bars until the position clock advances", + async () => { + const viz = useVisualizer(); + await startPlaying(); + expect(viz.isLoading()).toBe(false); + + // Pause, then resume against the still-covered position. + setIsPlaying(false); + await waitFor(() => !viz.isRunning(), 10000); + setIsPlaying(true); + + // The spinner replaces the bars immediately on resume. + await waitFor(() => viz.isLoading(), 5000); + expect(viz.isRunning()).toBe(true); + + // Position clock stays frozen at the resume point (re-buffering): + // the loading state must persist, not yield to static cached bars. + await Bun.sleep(250); + expect(viz.isLoading()).toBe(true); + + // Player recovers — the clock advances → fresh bars, spinner gone. + setPosition(3); + await waitFor(() => !viz.isLoading(), 3000); + expect(viz.barData().length).toBe(64); + }, + { timeout: 20000 }, +); + // ── Teardown ───────────────────────────────────────────────────────────── afterAll(() => {