From a5aadeea67dcbd3288363f4b47e6d7f8cf0fd1a0 Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Tue, 12 May 2026 18:08:09 +0200 Subject: [PATCH] =?UTF-8?q?refactor(player):=20E.25=20=E2=80=94=20extract?= =?UTF-8?q?=20audio-orchestration=20cluster=20(#589)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two thematic cuts in one PR: - `src/store/queueUndoAudioRestore.ts` — `queueUndoRestoreAudioEngine` (~70 LOC). Reload the Rust audio engine to match a queue-undo snapshot: audio_play with snapshot track params, optional audio_seek to the snapshot position, audio_pause if the snapshot captured a paused state. Generation-guard bails on concurrent playTrack. Drives recordEnginePlayUrl, setDeferHotCachePrefetch, and touchHotCacheOnPlayback. - `src/store/loudnessPrefetch.ts` — `prefetchLoudnessForEnqueuedTracks`. Warms the loudness cache for the current track + next-N window after a bulk enqueue, no-op when normalization isn't loudness. Both file-private; no caller-side changes outside playerStore's own imports. Removes the unused `collectLoudnessBackfillWindowTrackIds` import that was only feeding the moved prefetch helper. 11 tests across the two modules pin the orchestration: audio_play payload + seek-when-near-zero + wantPlaying=false → audio_pause + generation-mismatch bail + .finally clears the hot-cache gate even on errors; engine guard + window forwarding + sync-flag for prefetch. playerStore 2865 → 2779 LOC. --- src/store/loudnessPrefetch.test.ts | 76 ++++++++++ src/store/loudnessPrefetch.ts | 27 ++++ src/store/playerStore.ts | 90 +----------- src/store/queueUndoAudioRestore.test.ts | 182 ++++++++++++++++++++++++ src/store/queueUndoAudioRestore.ts | 95 +++++++++++++ 5 files changed, 382 insertions(+), 88 deletions(-) create mode 100644 src/store/loudnessPrefetch.test.ts create mode 100644 src/store/loudnessPrefetch.ts create mode 100644 src/store/queueUndoAudioRestore.test.ts create mode 100644 src/store/queueUndoAudioRestore.ts diff --git a/src/store/loudnessPrefetch.test.ts b/src/store/loudnessPrefetch.test.ts new file mode 100644 index 00000000..51ffa357 --- /dev/null +++ b/src/store/loudnessPrefetch.test.ts @@ -0,0 +1,76 @@ +/** + * `prefetchLoudnessForEnqueuedTracks` warms the loudness cache for the + * current + next-N tracks after a bulk enqueue. Tests pin the engine + * guard, the window collection, and the no-sync-engine flag on each + * refresh call. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Track } from './playerStore'; + +const hoisted = vi.hoisted(() => { + const auth = { normalizationEngine: 'loudness' as 'off' | 'replaygain' | 'loudness' }; + const player = { currentTrack: null as Track | null }; + return { + auth, + player, + refreshMock: vi.fn(async () => undefined), + collectMock: vi.fn((_q: Track[], _i: number, _c: Track | null): string[] => []), + }; +}); + +vi.mock('./authStore', () => ({ useAuthStore: { getState: () => hoisted.auth } })); +vi.mock('./playerStore', () => ({ + usePlayerStore: { getState: () => hoisted.player }, +})); +vi.mock('./loudnessRefresh', () => ({ + refreshLoudnessForTrack: hoisted.refreshMock, +})); +vi.mock('./loudnessBackfillWindow', () => ({ + collectLoudnessBackfillWindowTrackIds: hoisted.collectMock, +})); + +import { prefetchLoudnessForEnqueuedTracks } from './loudnessPrefetch'; + +function track(id: string): Track { + return { id, title: id, artist: 'A', album: 'X', albumId: 'X', duration: 100 }; +} + +beforeEach(() => { + hoisted.auth.normalizationEngine = 'loudness'; + hoisted.player.currentTrack = null; + hoisted.refreshMock.mockClear(); + hoisted.collectMock.mockReset(); + hoisted.collectMock.mockReturnValue([]); +}); + +describe('prefetchLoudnessForEnqueuedTracks', () => { + it("is a no-op when engine isn't loudness", () => { + hoisted.auth.normalizationEngine = 'off'; + hoisted.collectMock.mockReturnValueOnce(['t1']); + prefetchLoudnessForEnqueuedTracks([track('t1')], 0); + expect(hoisted.refreshMock).not.toHaveBeenCalled(); + expect(hoisted.collectMock).not.toHaveBeenCalled(); + }); + + it('forwards each window id to refreshLoudnessForTrack with syncPlayingEngine=false', () => { + hoisted.collectMock.mockReturnValueOnce(['t1', 't2', 't3']); + prefetchLoudnessForEnqueuedTracks([track('t1'), track('t2'), track('t3')], 0); + expect(hoisted.refreshMock).toHaveBeenCalledTimes(3); + expect(hoisted.refreshMock).toHaveBeenCalledWith('t1', { syncPlayingEngine: false }); + expect(hoisted.refreshMock).toHaveBeenCalledWith('t2', { syncPlayingEngine: false }); + expect(hoisted.refreshMock).toHaveBeenCalledWith('t3', { syncPlayingEngine: false }); + }); + + it('passes the queue + currentTrack through to the window collector', () => { + hoisted.player.currentTrack = track('cur'); + const q = [track('cur'), track('next')]; + prefetchLoudnessForEnqueuedTracks(q, 0); + expect(hoisted.collectMock).toHaveBeenCalledWith(q, 0, hoisted.player.currentTrack); + }); + + it('handles empty window list gracefully', () => { + hoisted.collectMock.mockReturnValueOnce([]); + prefetchLoudnessForEnqueuedTracks([], 0); + expect(hoisted.refreshMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/store/loudnessPrefetch.ts b/src/store/loudnessPrefetch.ts new file mode 100644 index 00000000..9dfff334 --- /dev/null +++ b/src/store/loudnessPrefetch.ts @@ -0,0 +1,27 @@ +import { useAuthStore } from './authStore'; +import { collectLoudnessBackfillWindowTrackIds } from './loudnessBackfillWindow'; +import { refreshLoudnessForTrack } from './loudnessRefresh'; +import { usePlayerStore, type Track } from './playerStore'; + +/** + * After a bulk enqueue (queue replace, append-many, lucky-mix) warm the + * loudness cache for the current track + the next N entries so the + * gapless `audio_chain_preload` payload sees a real cached gain instead + * of the startup trim. No-op when normalization isn't on `loudness` — + * other engines don't need the cache populated proactively. + * + * Calls don't sync the playing engine (`syncPlayingEngine: false`) — the + * already-playing track is unaffected; we're only filling the cache for + * the upcoming ones. + */ +export function prefetchLoudnessForEnqueuedTracks( + mergedQueue: Track[], + queueIndex: number, +): void { + if (useAuthStore.getState().normalizationEngine !== 'loudness') return; + const currentTrack = usePlayerStore.getState().currentTrack; + const ids = collectLoudnessBackfillWindowTrackIds(mergedQueue, queueIndex, currentTrack); + for (const id of ids) { + void refreshLoudnessForTrack(id, { syncPlayingEngine: false }); + } +} diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index 74dfacc0..884cfc90 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -70,7 +70,6 @@ import { bumpWaveformRefreshGen } from './waveformRefreshGen'; import { touchHotCacheOnPlayback } from './hotCacheTouch'; import { applySkipStarOnManualNext } from './skipStarRating'; import { resetLoudnessBackfillStateForTrackId } from './loudnessBackfillState'; -import { collectLoudnessBackfillWindowTrackIds } from './loudnessBackfillWindow'; import { flushPlayQueuePosition, flushQueueSyncToServer, @@ -156,6 +155,8 @@ import { isInfiniteQueueFetching, setInfiniteQueueFetching, } from './infiniteQueueState'; +import { queueUndoRestoreAudioEngine } from './queueUndoAudioRestore'; +import { prefetchLoudnessForEnqueuedTracks } from './loudnessPrefetch'; // Re-export so TauriEventBridge + persistence test keep their existing // `from './playerStore'` imports. @@ -406,96 +407,9 @@ type NormalizationStatePayload = { // ─── Module-level playback primitives ───────────────────────────────────────── -/** Reload Rust audio to match a queue-undo snapshot (Zustand alone does not move the engine). */ -function queueUndoRestoreAudioEngine(opts: { - generation: number; - track: Track; - queue: Track[]; - queueIndex: number; - atSeconds: number; - wantPlaying: boolean; -}): void { - const { generation, track, queue, queueIndex, atSeconds, wantPlaying } = opts; - const authState = useAuthStore.getState(); - const vol = usePlayerStore.getState().volume; - const coldPrev = queueIndex > 0 ? queue[queueIndex - 1] : null; - const coldNext = queueIndex + 1 < queue.length ? queue[queueIndex + 1] : null; - const replayGainDb = resolveReplayGainDb( - track, coldPrev, coldNext, - isReplayGainActive(), authState.replayGainMode, - ); - const replayGainPeak = isReplayGainActive() ? (track.replayGainPeak ?? null) : null; - const url = resolvePlaybackUrl(track.id, authState.activeServerId ?? ''); - recordEnginePlayUrl(track.id, url); - usePlayerStore.setState({ - currentPlaybackSource: playbackSourceHintForResolvedUrl(track.id, authState.activeServerId ?? '', url), - }); - const keepPreloadHint = usePlayerStore.getState().enginePreloadedTrackId === track.id; - setDeferHotCachePrefetch(true); - invoke('audio_play', { - url, - volume: vol, - durationHint: track.duration, - replayGainDb, - replayGainPeak, - loudnessGainDb: loudnessGainDbForEngineBind(track.id), - preGainDb: authState.replayGainPreGainDb, - fallbackDb: authState.replayGainFallbackDb, - manual: false, - hiResEnabled: authState.enableHiRes, - analysisTrackId: track.id, - streamFormatSuffix: track.suffix ?? null, - }) - .then(() => { - if (getPlayGeneration() !== generation) return; - if (keepPreloadHint) { - usePlayerStore.setState({ enginePreloadedTrackId: null }); - } - const dur = track.duration && track.duration > 0 ? track.duration : null; - const seekTo = Math.max(0, atSeconds); - const canSeek = seekTo > 0.05 && (dur == null || seekTo < dur - 0.05); - const afterSeek = () => { - if (getPlayGeneration() !== generation) return; - if (!wantPlaying) { - invoke('audio_pause').catch(console.error); - setIsAudioPaused(true); - usePlayerStore.setState({ isPlaying: false }); - } else { - setIsAudioPaused(false); - } - }; - if (canSeek) { - void invoke('audio_seek', { seconds: seekTo }).then(afterSeek).catch(afterSeek); - } else { - afterSeek(); - } - }) - .catch((err: unknown) => { - if (getPlayGeneration() !== generation) return; - console.error('[psysonic] queue-undo audio_play failed:', err); - usePlayerStore.setState({ isPlaying: false }); - }) - .finally(() => { - setDeferHotCachePrefetch(false); - }); - touchHotCacheOnPlayback(track.id, authState.activeServerId ?? ''); -} - // Streaming fallback seek guard: coalesce repeated "not seekable" recoveries. -function prefetchLoudnessForEnqueuedTracks( - mergedQueue: Track[], - queueIndex: number, -) { - if (useAuthStore.getState().normalizationEngine !== 'loudness') return; - const currentTrack = usePlayerStore.getState().currentTrack; - const ids = collectLoudnessBackfillWindowTrackIds(mergedQueue, queueIndex, currentTrack); - for (const id of ids) { - void refreshLoudnessForTrack(id, { syncPlayingEngine: false }); - } -} - // ─── Audio event handlers (called from initAudioListeners) ─────────────────── function handleAudioPlaying(_duration: number) { diff --git a/src/store/queueUndoAudioRestore.test.ts b/src/store/queueUndoAudioRestore.test.ts new file mode 100644 index 00000000..c3972e12 --- /dev/null +++ b/src/store/queueUndoAudioRestore.test.ts @@ -0,0 +1,182 @@ +/** + * Smoke test for the queue-undo engine-restore orchestrator. Verifies + * the audio_play payload shape, the seek follow-up that fires when + * atSeconds > 0.05, the wantPlaying=false branch that issues audio_pause, + * and the generation-mismatch bail-out. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Track } from './playerStore'; + +const hoisted = vi.hoisted(() => { + const auth = { + activeServerId: 'srv', + replayGainMode: 'track' as 'track' | 'album', + replayGainPreGainDb: 0, + replayGainFallbackDb: -6, + enableHiRes: false, + }; + const player = { + volume: 0.8, + enginePreloadedTrackId: null as string | null, + }; + return { + auth, + player, + invokeMock: vi.fn(async (_cmd: string, _args?: Record) => undefined), + setDeferHotCachePrefetchMock: vi.fn(), + resolvePlaybackUrlMock: vi.fn((id: string) => `https://mock/${id}`), + resolveReplayGainDbMock: vi.fn(() => -6), + isReplayGainActiveMock: vi.fn(() => false), + loudnessGainDbForEngineBindMock: vi.fn(() => null as number | null), + recordEnginePlayUrlMock: vi.fn(), + playbackSourceHintMock: vi.fn(() => 'stream'), + touchHotCacheOnPlaybackMock: vi.fn(), + playerSetStateMock: vi.fn(), + setIsAudioPausedMock: vi.fn(), + getPlayGeneration: vi.fn(() => 1), + }; +}); + +vi.mock('@tauri-apps/api/core', () => ({ invoke: hoisted.invokeMock })); +vi.mock('../utils/hotCacheGate', () => ({ setDeferHotCachePrefetch: hoisted.setDeferHotCachePrefetchMock })); +vi.mock('../utils/resolvePlaybackUrl', () => ({ resolvePlaybackUrl: hoisted.resolvePlaybackUrlMock })); +vi.mock('../utils/resolveReplayGainDb', () => ({ resolveReplayGainDb: hoisted.resolveReplayGainDbMock })); +vi.mock('./authStore', () => ({ useAuthStore: { getState: () => hoisted.auth } })); +vi.mock('./engineState', () => ({ + getPlayGeneration: hoisted.getPlayGeneration, + setIsAudioPaused: hoisted.setIsAudioPausedMock, +})); +vi.mock('./hotCacheTouch', () => ({ touchHotCacheOnPlayback: hoisted.touchHotCacheOnPlaybackMock })); +vi.mock('./loudnessGainCache', () => ({ + isReplayGainActive: hoisted.isReplayGainActiveMock, + loudnessGainDbForEngineBind: hoisted.loudnessGainDbForEngineBindMock, +})); +vi.mock('./playbackUrlRouting', () => ({ + playbackSourceHintForResolvedUrl: hoisted.playbackSourceHintMock, + recordEnginePlayUrl: hoisted.recordEnginePlayUrlMock, +})); +vi.mock('./playerStore', () => ({ + usePlayerStore: { + getState: () => hoisted.player, + setState: hoisted.playerSetStateMock, + }, +})); + +import { queueUndoRestoreAudioEngine } from './queueUndoAudioRestore'; + +function track(id: string, duration = 100): Track { + return { id, title: id, artist: 'A', album: 'X', albumId: 'X', duration }; +} + +beforeEach(() => { + hoisted.invokeMock.mockReset(); + hoisted.invokeMock.mockResolvedValue(undefined); + hoisted.setDeferHotCachePrefetchMock.mockClear(); + hoisted.touchHotCacheOnPlaybackMock.mockClear(); + hoisted.playerSetStateMock.mockClear(); + hoisted.setIsAudioPausedMock.mockClear(); + hoisted.recordEnginePlayUrlMock.mockClear(); + hoisted.getPlayGeneration.mockReturnValue(1); + hoisted.player.enginePreloadedTrackId = null; +}); + +describe('queueUndoRestoreAudioEngine', () => { + it('issues audio_play with the snapshot track parameters', async () => { + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 0, + wantPlaying: true, + }); + expect(hoisted.invokeMock).toHaveBeenCalledWith('audio_play', expect.objectContaining({ + url: 'https://mock/t1', + durationHint: 100, + manual: false, + analysisTrackId: 't1', + })); + expect(hoisted.recordEnginePlayUrlMock).toHaveBeenCalledWith('t1', 'https://mock/t1'); + expect(hoisted.setDeferHotCachePrefetchMock).toHaveBeenCalledWith(true); + expect(hoisted.touchHotCacheOnPlaybackMock).toHaveBeenCalledWith('t1', 'srv'); + }); + + it('fires audio_seek when atSeconds > 0.05', async () => { + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 30, + wantPlaying: true, + }); + await Promise.resolve(); + await Promise.resolve(); + expect(hoisted.invokeMock).toHaveBeenCalledWith('audio_seek', { seconds: 30 }); + }); + + it('skips audio_seek when atSeconds is near zero', async () => { + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 0, + wantPlaying: true, + }); + await Promise.resolve(); + await Promise.resolve(); + const seekCall = hoisted.invokeMock.mock.calls.find(c => c[0] === 'audio_seek'); + expect(seekCall).toBeUndefined(); + }); + + it('issues audio_pause when wantPlaying=false', async () => { + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 0, + wantPlaying: false, + }); + await Promise.resolve(); + await Promise.resolve(); + expect(hoisted.invokeMock).toHaveBeenCalledWith('audio_pause'); + expect(hoisted.setIsAudioPausedMock).toHaveBeenCalledWith(true); + }); + + it('bails out of the .then chain when generation has moved on', async () => { + hoisted.getPlayGeneration.mockReturnValue(2); // user navigated, new gen + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 30, + wantPlaying: false, + }); + await Promise.resolve(); + await Promise.resolve(); + // Should NOT have called audio_seek or audio_pause — gen moved on. + const seekCall = hoisted.invokeMock.mock.calls.find(c => c[0] === 'audio_seek'); + const pauseCall = hoisted.invokeMock.mock.calls.find(c => c[0] === 'audio_pause'); + expect(seekCall).toBeUndefined(); + expect(pauseCall).toBeUndefined(); + }); + + it('clears the deferHotCachePrefetch gate in .finally even on error', async () => { + hoisted.invokeMock.mockRejectedValueOnce(new Error('rust down')); + queueUndoRestoreAudioEngine({ + generation: 1, + track: track('t1'), + queue: [track('t1')], + queueIndex: 0, + atSeconds: 0, + wantPlaying: true, + }); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + expect(hoisted.setDeferHotCachePrefetchMock).toHaveBeenCalledWith(false); + }); +}); diff --git a/src/store/queueUndoAudioRestore.ts b/src/store/queueUndoAudioRestore.ts new file mode 100644 index 00000000..7baec0e8 --- /dev/null +++ b/src/store/queueUndoAudioRestore.ts @@ -0,0 +1,95 @@ +import { invoke } from '@tauri-apps/api/core'; +import { setDeferHotCachePrefetch } from '../utils/hotCacheGate'; +import { resolvePlaybackUrl } from '../utils/resolvePlaybackUrl'; +import { resolveReplayGainDb } from '../utils/resolveReplayGainDb'; +import { useAuthStore } from './authStore'; +import { getPlayGeneration, setIsAudioPaused } from './engineState'; +import { touchHotCacheOnPlayback } from './hotCacheTouch'; +import { isReplayGainActive, loudnessGainDbForEngineBind } from './loudnessGainCache'; +import { playbackSourceHintForResolvedUrl, recordEnginePlayUrl } from './playbackUrlRouting'; +import { usePlayerStore, type Track } from './playerStore'; + +/** + * Reload the Rust audio engine to match a queue-undo snapshot. Zustand + * alone can rewrite the queue + currentTrack, but the engine is still + * playing whatever cold-started before the undo — so we need a full + * `audio_play` (+ optional `audio_seek` to the snapshot position) to + * line the audible playback back up with the restored UI state. + * + * Captures the play-generation at start so a later concurrent `playTrack` + * (e.g. user clicks another track) invalidates the seek/pause follow-up + * without clobbering the new engine state. + */ +export function queueUndoRestoreAudioEngine(opts: { + generation: number; + track: Track; + queue: Track[]; + queueIndex: number; + atSeconds: number; + wantPlaying: boolean; +}): void { + const { generation, track, queue, queueIndex, atSeconds, wantPlaying } = opts; + const authState = useAuthStore.getState(); + const vol = usePlayerStore.getState().volume; + const coldPrev = queueIndex > 0 ? queue[queueIndex - 1] : null; + const coldNext = queueIndex + 1 < queue.length ? queue[queueIndex + 1] : null; + const replayGainDb = resolveReplayGainDb( + track, coldPrev, coldNext, + isReplayGainActive(), authState.replayGainMode, + ); + const replayGainPeak = isReplayGainActive() ? (track.replayGainPeak ?? null) : null; + const url = resolvePlaybackUrl(track.id, authState.activeServerId ?? ''); + recordEnginePlayUrl(track.id, url); + usePlayerStore.setState({ + currentPlaybackSource: playbackSourceHintForResolvedUrl(track.id, authState.activeServerId ?? '', url), + }); + const keepPreloadHint = usePlayerStore.getState().enginePreloadedTrackId === track.id; + setDeferHotCachePrefetch(true); + invoke('audio_play', { + url, + volume: vol, + durationHint: track.duration, + replayGainDb, + replayGainPeak, + loudnessGainDb: loudnessGainDbForEngineBind(track.id), + preGainDb: authState.replayGainPreGainDb, + fallbackDb: authState.replayGainFallbackDb, + manual: false, + hiResEnabled: authState.enableHiRes, + analysisTrackId: track.id, + streamFormatSuffix: track.suffix ?? null, + }) + .then(() => { + if (getPlayGeneration() !== generation) return; + if (keepPreloadHint) { + usePlayerStore.setState({ enginePreloadedTrackId: null }); + } + const dur = track.duration && track.duration > 0 ? track.duration : null; + const seekTo = Math.max(0, atSeconds); + const canSeek = seekTo > 0.05 && (dur == null || seekTo < dur - 0.05); + const afterSeek = () => { + if (getPlayGeneration() !== generation) return; + if (!wantPlaying) { + invoke('audio_pause').catch(console.error); + setIsAudioPaused(true); + usePlayerStore.setState({ isPlaying: false }); + } else { + setIsAudioPaused(false); + } + }; + if (canSeek) { + void invoke('audio_seek', { seconds: seekTo }).then(afterSeek).catch(afterSeek); + } else { + afterSeek(); + } + }) + .catch((err: unknown) => { + if (getPlayGeneration() !== generation) return; + console.error('[psysonic] queue-undo audio_play failed:', err); + usePlayerStore.setState({ isPlaying: false }); + }) + .finally(() => { + setDeferHotCachePrefetch(false); + }); + touchHotCacheOnPlayback(track.id, authState.activeServerId ?? ''); +}