refactor(player): E.16 — extract gapless-preload coordination state (#579)

Three coordinating mutables move into `src/store/gaplessPreloadState.ts`
behind a thin API:

  - `gaplessPreloadingId` / `bytePreloadingId` — guards so the runtime
    doesn't fire a chain-/byte-preload twice for the same id
  - `lastGaplessSwitchTime` — timestamp used by the 500–600 ms
    ghost-IPC suppression guards in `handleAudioEnded` and `playTrack`

Public API: `get…` / `set…` accessors plus `clearPreloadingIds`
(atomic clear of both guards — collapses three `= null; = null;`
duplications) and `markGaplessSwitch` (`Date.now()` stamp). The
mutables are no longer reachable from outside the module.

13 focused tests pin the get/set round-trips, independence between
the two guards, the atomic clear, and the timestamp progression.

playerStore 3208 → 3207 LOC (small delta because the `= null; = null;`
inlines compress to single `clearPreloadingIds()` calls but the
module itself adds the accessor functions to the import list).
This commit is contained in:
Frank Stellmacher
2026-05-12 14:53:37 +02:00
committed by GitHub
parent 2ad0be6a71
commit 3a99be4daa
3 changed files with 168 additions and 21 deletions
+91
View File
@@ -0,0 +1,91 @@
/**
* Three mutables that coordinate the gapless preloader. Most of the surface
* is straight get/set — the interesting bit is `clearPreloadingIds` (atomic
* clear of both) and `markGaplessSwitch` (timestamp side effect).
*/
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import {
_resetGaplessPreloadStateForTest,
clearPreloadingIds,
getBytePreloadingId,
getGaplessPreloadingId,
getLastGaplessSwitchTime,
markGaplessSwitch,
setBytePreloadingId,
setGaplessPreloadingId,
} from './gaplessPreloadState';
beforeEach(() => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2026-05-12T12:00:00Z'));
});
afterEach(() => {
_resetGaplessPreloadStateForTest();
vi.useRealTimers();
});
describe('initial state', () => {
it('is null / 0 for unread accessors', () => {
expect(getGaplessPreloadingId()).toBeNull();
expect(getBytePreloadingId()).toBeNull();
expect(getLastGaplessSwitchTime()).toBe(0);
});
});
describe('preloading-id accessors', () => {
it('round-trips through the gapless guard', () => {
setGaplessPreloadingId('t1');
expect(getGaplessPreloadingId()).toBe('t1');
});
it('round-trips through the byte guard', () => {
setBytePreloadingId('t2');
expect(getBytePreloadingId()).toBe('t2');
});
it('keeps the two guards independent', () => {
setGaplessPreloadingId('a');
setBytePreloadingId('b');
expect(getGaplessPreloadingId()).toBe('a');
expect(getBytePreloadingId()).toBe('b');
});
it('accepts null to clear a guard', () => {
setGaplessPreloadingId('a');
setGaplessPreloadingId(null);
expect(getGaplessPreloadingId()).toBeNull();
});
});
describe('clearPreloadingIds', () => {
it('clears both guards atomically', () => {
setGaplessPreloadingId('a');
setBytePreloadingId('b');
clearPreloadingIds();
expect(getGaplessPreloadingId()).toBeNull();
expect(getBytePreloadingId()).toBeNull();
});
it('does not touch the gapless-switch timestamp', () => {
markGaplessSwitch();
const before = getLastGaplessSwitchTime();
clearPreloadingIds();
expect(getLastGaplessSwitchTime()).toBe(before);
});
});
describe('markGaplessSwitch', () => {
it('stamps Date.now()', () => {
markGaplessSwitch();
expect(getLastGaplessSwitchTime()).toBe(Date.now());
});
it('overwrites on a later call', () => {
markGaplessSwitch();
const first = getLastGaplessSwitchTime();
vi.advanceTimersByTime(700);
markGaplessSwitch();
expect(getLastGaplessSwitchTime()).toBeGreaterThan(first);
});
});
+57
View File
@@ -0,0 +1,57 @@
/**
* Coordinates the gapless-chain preloader so the runtime doesn't pre-fetch
* the same track twice or fire a chain-switch while a previous one is
* still settling.
*
* - `gaplessPreloadingId` — track id last handed to `audio_chain_preload`
* - `bytePreloadingId` — track id last handed to `audio_preload`
* - `lastGaplessSwitchTime` — timestamp of the last gapless track-switch
* event from Rust. The 500600 ms guards in `handleAudioTrackSwitched`
* and the progress handler use this to suppress stale IPC arriving
* right after the switch.
*
* `clearPreloadingIds` collapses the repeated `= null; = null;` pattern
* that the store actions used to inline in five places.
*/
let gaplessPreloadingId: string | null = null;
let bytePreloadingId: string | null = null;
let lastGaplessSwitchTime = 0;
export function getGaplessPreloadingId(): string | null {
return gaplessPreloadingId;
}
export function setGaplessPreloadingId(id: string | null): void {
gaplessPreloadingId = id;
}
export function getBytePreloadingId(): string | null {
return bytePreloadingId;
}
export function setBytePreloadingId(id: string | null): void {
bytePreloadingId = id;
}
/** Atomic: clear both preloading guards. Called on track switch + on errors. */
export function clearPreloadingIds(): void {
gaplessPreloadingId = null;
bytePreloadingId = null;
}
export function getLastGaplessSwitchTime(): number {
return lastGaplessSwitchTime;
}
/** Record a gapless switch event. Subsequent guards compare against this. */
export function markGaplessSwitch(): void {
lastGaplessSwitchTime = Date.now();
}
/** Test-only: reset all three mutables. */
export function _resetGaplessPreloadStateForTest(): void {
gaplessPreloadingId = null;
bytePreloadingId = null;
lastGaplessSwitchTime = 0;
}
+20 -21
View File
@@ -95,6 +95,15 @@ import {
getLastQueueHeartbeatAt, getLastQueueHeartbeatAt,
syncQueueToServer, syncQueueToServer,
} from './queueSync'; } from './queueSync';
import {
clearPreloadingIds,
getBytePreloadingId,
getGaplessPreloadingId,
getLastGaplessSwitchTime,
markGaplessSwitch,
setBytePreloadingId,
setGaplessPreloadingId,
} from './gaplessPreloadState';
// Re-export so TauriEventBridge + persistence test keep their existing // Re-export so TauriEventBridge + persistence test keep their existing
// `from './playerStore'` imports. // `from './playerStore'` imports.
@@ -624,10 +633,6 @@ radioAudio.addEventListener('suspend', () => {
clearRadioReconnectTimer(); clearRadioReconnectTimer();
}); });
// Timestamp of the last gapless auto-advance (from audio:track_switched).
// Used to suppress ghost-commands from stale IPC arriving after the switch.
let lastGaplessSwitchTime = 0;
/** Coalesce concurrent `analysis_get_loudness_for_track` for one id+mode pair. The /** Coalesce concurrent `analysis_get_loudness_for_track` for one id+mode pair. The
* analysis:waveform-updated listener fires refreshWaveform + refreshLoudness in * analysis:waveform-updated listener fires refreshWaveform + refreshLoudness in
* parallel for every full-track analysis completion; without coalescing, gapless * parallel for every full-track analysis completion; without coalescing, gapless
@@ -821,11 +826,6 @@ async function promoteCompletedStreamToHotCache(track: Track, serverId: string,
} }
} }
// Track ID that has already been sent to audio_chain_preload (gapless chain).
let gaplessPreloadingId: string | null = null;
// Track ID that has already been sent to audio_preload (byte pre-download).
let bytePreloadingId: string | null = null;
// ─── Audio event handlers (called from initAudioListeners) ─────────────────── // ─── Audio event handlers (called from initAudioListeners) ───────────────────
function handleAudioPlaying(_duration: number) { function handleAudioPlaying(_duration: number) {
@@ -994,8 +994,8 @@ function handleAudioProgress(current_time: number, duration: number) {
const nextUrl = resolvePlaybackUrl(nextTrack.id, serverId); const nextUrl = resolvePlaybackUrl(nextTrack.id, serverId);
// Byte pre-download — runs early so bytes are cached by chain time. // Byte pre-download — runs early so bytes are cached by chain time.
if ((shouldBytePreload || shouldBytePreloadForGaplessBackup) && nextTrack.id !== bytePreloadingId) { if ((shouldBytePreload || shouldBytePreloadForGaplessBackup) && nextTrack.id !== getBytePreloadingId()) {
bytePreloadingId = nextTrack.id; setBytePreloadingId(nextTrack.id);
// Loudness cache only — do not call refreshWaveformForTrack(next): it writes global // Loudness cache only — do not call refreshWaveformForTrack(next): it writes global
// waveformBins and would replace the current track's seekbar while still playing it. // waveformBins and would replace the current track's seekbar while still playing it.
void refreshLoudnessForTrack(nextTrack.id, { syncPlayingEngine: false }); void refreshLoudnessForTrack(nextTrack.id, { syncPlayingEngine: false });
@@ -1017,8 +1017,8 @@ function handleAudioProgress(current_time: number, duration: number) {
} }
// Gapless chain — decode + chain into Sink 30s before track boundary. // Gapless chain — decode + chain into Sink 30s before track boundary.
if (shouldChainGapless && nextTrack.id !== gaplessPreloadingId) { if (shouldChainGapless && nextTrack.id !== getGaplessPreloadingId()) {
gaplessPreloadingId = nextTrack.id; setGaplessPreloadingId(nextTrack.id);
// Ensure loudness gain is already cached for the chained request payload. // Ensure loudness gain is already cached for the chained request payload.
void refreshLoudnessForTrack(nextTrack.id, { syncPlayingEngine: false }); void refreshLoudnessForTrack(nextTrack.id, { syncPlayingEngine: false });
const authState = useAuthStore.getState(); const authState = useAuthStore.getState();
@@ -1053,7 +1053,7 @@ function handleAudioProgress(current_time: number, duration: number) {
function handleAudioEnded() { function handleAudioEnded() {
// If a gapless switch happened recently, this ended event is stale — the // If a gapless switch happened recently, this ended event is stale — the
// progress task fired it for the OLD source before seeing the chained one. // progress task fired it for the OLD source before seeing the chained one.
if (Date.now() - lastGaplessSwitchTime < 600) { if (Date.now() - getLastGaplessSwitchTime() < 600) {
return; return;
} }
@@ -1101,8 +1101,8 @@ function handleAudioEnded() {
* touching the audio stream (no playTrack() call!). * touching the audio stream (no playTrack() call!).
*/ */
function handleAudioTrackSwitched(duration: number) { function handleAudioTrackSwitched(duration: number) {
lastGaplessSwitchTime = Date.now(); markGaplessSwitch();
gaplessPreloadingId = null; bytePreloadingId = null; // allow preloading for the track after this one clearPreloadingIds(); // allow preloading for the track after this one
isAudioPaused = false; isAudioPaused = false;
const store = usePlayerStore.getState(); const store = usePlayerStore.getState();
@@ -1727,8 +1727,7 @@ export const usePlayerStore = create<PlayerState>()(
scheduledResumeStartMs: null, scheduledResumeStartMs: null,
}); });
gaplessPreloadingId = null; clearPreloadingIds();
bytePreloadingId = null;
let gen = playGeneration; let gen = playGeneration;
const resyncEngine = Boolean(nextTrack) && !keepPlaybackFromPrior; const resyncEngine = Boolean(nextTrack) && !keepPlaybackFromPrior;
@@ -1955,7 +1954,7 @@ export const usePlayerStore = create<PlayerState>()(
isAudioPaused = false; isAudioPaused = false;
clearRadioReconnectTimer(); clearRadioReconnectTimer();
radioReconnectCount = 0; radioReconnectCount = 0;
gaplessPreloadingId = null; bytePreloadingId = null; clearPreloadingIds();
clearSeekFallbackRetry(); clearSeekFallbackRetry();
if (seekDebounce) { clearTimeout(seekDebounce); seekDebounce = null; } clearSeekTarget(); if (seekDebounce) { clearTimeout(seekDebounce); seekDebounce = null; } clearSeekTarget();
// Stop Rust engine in case a regular track was playing. // Stop Rust engine in case a regular track was playing.
@@ -2052,7 +2051,7 @@ export const usePlayerStore = create<PlayerState>()(
// Ghost-command guard: if a gapless switch happened within 500 ms, // Ghost-command guard: if a gapless switch happened within 500 ms,
// this playTrack call is likely a stale IPC echo — suppress it. // this playTrack call is likely a stale IPC echo — suppress it.
if (Date.now() - lastGaplessSwitchTime < 500) { if (Date.now() - getLastGaplessSwitchTime() < 500) {
return; return;
} }
@@ -2061,7 +2060,7 @@ export const usePlayerStore = create<PlayerState>()(
const gen = ++playGeneration; const gen = ++playGeneration;
isAudioPaused = false; isAudioPaused = false;
gaplessPreloadingId = null; bytePreloadingId = null; // new track — allow fresh preload for next clearPreloadingIds(); // new track — allow fresh preload for next
if (seekDebounce) { clearTimeout(seekDebounce); seekDebounce = null; } clearSeekTarget(); if (seekDebounce) { clearTimeout(seekDebounce); seekDebounce = null; } clearSeekTarget();
clearSeekFallbackRetry(); clearSeekFallbackRetry();
seekFallbackRestartAt = 0; seekFallbackRestartAt = 0;