From 18b88e3ae0b9923c4d56ec020d408aadfbaa3aee Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Tue, 12 May 2026 13:54:41 +0200 Subject: [PATCH] =?UTF-8?q?refactor(player):=20E.8=20=E2=80=94=20extract?= =?UTF-8?q?=20loudness-gain=20cache=20encapsulation=20(#571)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two parallel maps (`cachedLoudnessGainByTrackId`, `stableLoudnessGainByTrackId`) plus their helpers (`isReplayGainActive`, `loudnessCacheStateKeysForTrackId`, `clearLoudnessCacheStateForTrackId`, `loudnessGainDbForEngineBind`) move into `src/store/loudnessGainCache.ts`. The new module exposes a thin API: - `getCachedLoudnessGain` / `setCachedLoudnessGain` - `hasStableLoudness` / `markLoudnessStable` (atomic set + stable) - `forgetLoudnessGain` (single-key delete) vs `clearLoudnessCacheStateForTrackId` (two-form delete) - existing names kept for `isReplayGainActive`, `loudnessGainDbForEngineBind`, `loudnessCacheStateKeysForTrackId` playerStore's seven direct-access sites (delete pairs, set-pair, stable flag check, cached read, neighbour-cache write) become API calls — the mutables are no longer reachable from outside the module. All helpers were file-private; no caller-side changes outside playerStore's own imports. 22 focused tests pin the API surface including the partial-vs-stable visibility split and the two delete-shape variants. playerStore 3415 → 3389 LOC. --- src/store/loudnessGainCache.test.ts | 180 ++++++++++++++++++++++++++++ src/store/loudnessGainCache.ts | 92 ++++++++++++++ src/store/playerStore.ts | 74 ++++-------- 3 files changed, 296 insertions(+), 50 deletions(-) create mode 100644 src/store/loudnessGainCache.test.ts create mode 100644 src/store/loudnessGainCache.ts diff --git a/src/store/loudnessGainCache.test.ts b/src/store/loudnessGainCache.test.ts new file mode 100644 index 00000000..4a6365a0 --- /dev/null +++ b/src/store/loudnessGainCache.test.ts @@ -0,0 +1,180 @@ +/** + * Loudness-gain cache encapsulates two parallel maps and a small API that + * playerStore drives from the audio-event handlers + cache refresh path. + * The interesting behaviours: (a) stable-flag gating in + * `loudnessGainDbForEngineBind` (partial values are silently invisible to + * engine bind), (b) `clearLoudnessCacheStateForTrackId` expands across the + * `stream:` prefix while `forgetLoudnessGain` does NOT (preserves the + * existing direct-delete semantics from playerStore). + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { authState } = vi.hoisted(() => ({ + authState: { + normalizationEngine: 'off' as 'off' | 'replaygain' | 'loudness', + replayGainEnabled: false, + }, +})); + +vi.mock('./authStore', () => ({ + useAuthStore: { getState: () => authState }, +})); + +import { + _resetLoudnessGainCacheForTest, + clearLoudnessCacheStateForTrackId, + forgetLoudnessGain, + getCachedLoudnessGain, + hasStableLoudness, + isReplayGainActive, + loudnessCacheStateKeysForTrackId, + loudnessGainDbForEngineBind, + markLoudnessStable, + setCachedLoudnessGain, +} from './loudnessGainCache'; + +beforeEach(() => { + authState.normalizationEngine = 'off'; + authState.replayGainEnabled = false; +}); + +afterEach(() => { + _resetLoudnessGainCacheForTest(); +}); + +describe('loudnessCacheStateKeysForTrackId', () => { + it('returns bare + stream-prefixed form for a bare id', () => { + expect(loudnessCacheStateKeysForTrackId('abc')).toEqual(['abc', 'stream:abc']); + }); + + it('returns stream-prefixed + bare form for a stream id', () => { + expect(loudnessCacheStateKeysForTrackId('stream:abc')).toEqual(['stream:abc', 'abc']); + }); + + it('returns empty for an empty id', () => { + expect(loudnessCacheStateKeysForTrackId('')).toEqual([]); + }); + + it('returns only the stream-prefixed form when the bare portion is empty', () => { + expect(loudnessCacheStateKeysForTrackId('stream:')).toEqual(['stream:']); + }); +}); + +describe('getCachedLoudnessGain / setCachedLoudnessGain', () => { + it('round-trips a value through the cache', () => { + setCachedLoudnessGain('t1', -7.2); + expect(getCachedLoudnessGain('t1')).toBe(-7.2); + }); + + it('returns undefined for missing entries', () => { + expect(getCachedLoudnessGain('missing')).toBeUndefined(); + }); +}); + +describe('hasStableLoudness / markLoudnessStable', () => { + it('flags as stable only after markLoudnessStable', () => { + setCachedLoudnessGain('t1', -7); + expect(hasStableLoudness('t1')).toBe(false); + markLoudnessStable('t1', -7); + expect(hasStableLoudness('t1')).toBe(true); + }); + + it('markLoudnessStable writes the cached value atomically', () => { + markLoudnessStable('t1', -5); + expect(getCachedLoudnessGain('t1')).toBe(-5); + expect(hasStableLoudness('t1')).toBe(true); + }); +}); + +describe('forgetLoudnessGain (single-key delete)', () => { + it('clears both maps for the literal id only — does not touch the other form', () => { + markLoudnessStable('t1', -5); + markLoudnessStable('stream:t1', -6); + forgetLoudnessGain('t1'); + expect(getCachedLoudnessGain('t1')).toBeUndefined(); + expect(hasStableLoudness('t1')).toBe(false); + // Stream form must still be there — forget is intentionally narrow. + expect(getCachedLoudnessGain('stream:t1')).toBe(-6); + expect(hasStableLoudness('stream:t1')).toBe(true); + }); +}); + +describe('clearLoudnessCacheStateForTrackId (two-form delete)', () => { + it('clears both maps for both id forms', () => { + markLoudnessStable('t1', -5); + markLoudnessStable('stream:t1', -6); + clearLoudnessCacheStateForTrackId('t1'); + expect(getCachedLoudnessGain('t1')).toBeUndefined(); + expect(getCachedLoudnessGain('stream:t1')).toBeUndefined(); + expect(hasStableLoudness('t1')).toBe(false); + expect(hasStableLoudness('stream:t1')).toBe(false); + }); + + it('also works when invoked with the stream-prefixed form', () => { + markLoudnessStable('t1', -5); + markLoudnessStable('stream:t1', -6); + clearLoudnessCacheStateForTrackId('stream:t1'); + expect(getCachedLoudnessGain('t1')).toBeUndefined(); + expect(getCachedLoudnessGain('stream:t1')).toBeUndefined(); + }); +}); + +describe('loudnessGainDbForEngineBind', () => { + it('returns null without a stable flag (partial/placeholder values are hidden from engine bind)', () => { + setCachedLoudnessGain('t1', -5); + expect(loudnessGainDbForEngineBind('t1')).toBeNull(); + }); + + it('returns the cached value once the entry is stable', () => { + markLoudnessStable('t1', -5); + expect(loudnessGainDbForEngineBind('t1')).toBe(-5); + }); + + it('returns null when the cached value is non-finite', () => { + markLoudnessStable('t1', Number.NaN); + expect(loudnessGainDbForEngineBind('t1')).toBeNull(); + }); + + it('returns null for null / empty trackId input', () => { + expect(loudnessGainDbForEngineBind(null)).toBeNull(); + expect(loudnessGainDbForEngineBind(undefined)).toBeNull(); + expect(loudnessGainDbForEngineBind('')).toBeNull(); + }); +}); + +describe('isReplayGainActive', () => { + it('is false when normalization engine is off', () => { + authState.normalizationEngine = 'off'; + authState.replayGainEnabled = true; + expect(isReplayGainActive()).toBe(false); + }); + + it('is false when engine is replaygain but flag is disabled', () => { + authState.normalizationEngine = 'replaygain'; + authState.replayGainEnabled = false; + expect(isReplayGainActive()).toBe(false); + }); + + it('is true only when engine is replaygain AND the flag is enabled', () => { + authState.normalizationEngine = 'replaygain'; + authState.replayGainEnabled = true; + expect(isReplayGainActive()).toBe(true); + }); + + it('is false when engine is loudness (different normalization mode)', () => { + authState.normalizationEngine = 'loudness'; + authState.replayGainEnabled = true; + expect(isReplayGainActive()).toBe(false); + }); +}); + +describe('_resetLoudnessGainCacheForTest', () => { + it('wipes both maps', () => { + markLoudnessStable('t1', -5); + markLoudnessStable('t2', -6); + _resetLoudnessGainCacheForTest(); + expect(getCachedLoudnessGain('t1')).toBeUndefined(); + expect(getCachedLoudnessGain('t2')).toBeUndefined(); + expect(hasStableLoudness('t1')).toBe(false); + }); +}); diff --git a/src/store/loudnessGainCache.ts b/src/store/loudnessGainCache.ts new file mode 100644 index 00000000..5bbe448d --- /dev/null +++ b/src/store/loudnessGainCache.ts @@ -0,0 +1,92 @@ +import { useAuthStore } from './authStore'; + +/** + * In-memory cache of the per-track loudness normalization gain (dB). Two + * parallel maps: + * + * - `cachedLoudnessGainByTrackId` — the dB value last computed (from an + * `analysis_get_loudness_for_track` row, a partial-loudness event, or + * a placeholder-until-cache value). + * - `stableLoudnessGainByTrackId` — `true` once the value has been + * promoted to the final cached/analysis-confirmed form. Engine bind + * only trusts entries flagged stable; partial / placeholder values + * deliberately omit the flag so Rust uses its pre-trim default until + * the analysis catches up. + * + * Keys can land in either the bare Subsonic id form or the `stream:` + * prefixed form depending on which event surface wrote the entry — + * `loudnessCacheStateKeysForTrackId` returns the two forms a caller may + * need to look up or clear together. + */ + +const cachedLoudnessGainByTrackId: Record = {}; +const stableLoudnessGainByTrackId: Record = {}; + +/** Returns the two-form key list (bare id + `stream:`) for paired lookups. */ +export function loudnessCacheStateKeysForTrackId(trackId: string): string[] { + if (!trackId) return []; + const out: string[] = [trackId]; + if (trackId.startsWith('stream:')) { + const bare = trackId.slice('stream:'.length); + if (bare) out.push(bare); + } else { + out.push(`stream:${trackId}`); + } + return out; +} + +export function getCachedLoudnessGain(trackId: string): number | undefined { + return cachedLoudnessGainByTrackId[trackId]; +} + +export function setCachedLoudnessGain(trackId: string, gainDb: number): void { + cachedLoudnessGainByTrackId[trackId] = gainDb; +} + +export function hasStableLoudness(trackId: string): boolean { + return Boolean(stableLoudnessGainByTrackId[trackId]); +} + +/** Atomic: write the cached value AND mark it stable (analysis-confirmed). */ +export function markLoudnessStable(trackId: string, gainDb: number): void { + cachedLoudnessGainByTrackId[trackId] = gainDb; + stableLoudnessGainByTrackId[trackId] = true; +} + +/** Drop both maps for the literal track id (no stream-form expansion). */ +export function forgetLoudnessGain(trackId: string): void { + delete cachedLoudnessGainByTrackId[trackId]; + delete stableLoudnessGainByTrackId[trackId]; +} + +/** Drop both maps for each form of the track id (bare + `stream:`). */ +export function clearLoudnessCacheStateForTrackId(trackId: string): void { + for (const k of loudnessCacheStateKeysForTrackId(trackId)) { + delete cachedLoudnessGainByTrackId[k]; + delete stableLoudnessGainByTrackId[k]; + } +} + +/** + * Pass to `audio_play` / `audio_chain_preload` only — DB-backed gain. Omit + * partial hints so Rust uses pre-trim until `analysis:loudness-partial` + + * `audio_update_replay_gain`. + */ +export function loudnessGainDbForEngineBind(trackId: string | undefined | null): number | null { + if (!trackId) return null; + if (!stableLoudnessGainByTrackId[trackId]) return null; + const v = cachedLoudnessGainByTrackId[trackId]; + return Number.isFinite(v) ? v : null; +} + +/** True when ReplayGain is selected AND user has it enabled in Settings. */ +export function isReplayGainActive(): boolean { + const a = useAuthStore.getState(); + return a.normalizationEngine === 'replaygain' && a.replayGainEnabled; +} + +/** Test-only: wipe both maps so each spec starts clean. */ +export function _resetLoudnessGainCacheForTest(): void { + for (const k of Object.keys(cachedLoudnessGainByTrackId)) delete cachedLoudnessGainByTrackId[k]; + for (const k of Object.keys(stableLoudnessGainByTrackId)) delete stableLoudnessGainByTrackId[k]; +} diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index fc96b14f..ac205163 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -47,6 +47,17 @@ import { import { deriveNormalizationSnapshot } from './normalizationSnapshot'; import { emitNormalizationDebug } from './normalizationDebug'; import { isInOrbitSession } from './orbitSession'; +import { + clearLoudnessCacheStateForTrackId, + forgetLoudnessGain, + getCachedLoudnessGain, + hasStableLoudness, + isReplayGainActive, + loudnessCacheStateKeysForTrackId, + loudnessGainDbForEngineBind, + markLoudnessStable, + setCachedLoudnessGain, +} from './loudnessGainCache'; // Re-export the playback-progress public surface so existing call sites // (PlayerBar, FullscreenPlayer, WaveformSeek, LyricsPane, MobilePlayerView, @@ -335,8 +346,6 @@ let currentRadioArtistId: string | null = null; // the queue and the next Last.fm/topSongs response could re-add it. Reset // on `setRadioArtistId(other)` and on `clearQueue()`. Issue #500. let radioSessionSeenIds = new Set(); -let cachedLoudnessGainByTrackId: Record = {}; -let stableLoudnessGainByTrackId: Record = {}; let lastNormalizationUiUpdateAtMs = 0; /** Reload Rust audio to match a queue-undo snapshot (Zustand alone does not move the engine). */ @@ -726,38 +735,6 @@ function invokeAudioUpdateReplayGainDeduped(payload: { invoke('audio_update_replay_gain', payload).catch(console.error); } -function isReplayGainActive() { - const a = useAuthStore.getState(); - return a.normalizationEngine === 'replaygain' && a.replayGainEnabled; -} - -function loudnessCacheStateKeysForTrackId(trackId: string): string[] { - if (!trackId) return []; - const out: string[] = [trackId]; - if (trackId.startsWith('stream:')) { - const bare = trackId.slice('stream:'.length); - if (bare) out.push(bare); - } else { - out.push(`stream:${trackId}`); - } - return out; -} - -function clearLoudnessCacheStateForTrackId(trackId: string) { - for (const k of loudnessCacheStateKeysForTrackId(trackId)) { - delete cachedLoudnessGainByTrackId[k]; - delete stableLoudnessGainByTrackId[k]; - } -} - -/** Pass to `audio_play` / `audio_chain_preload` only — DB-backed gain. Omit partial hints so Rust uses pre-trim until `analysis:loudness-partial` + `audio_update_replay_gain`. */ -function loudnessGainDbForEngineBind(trackId: string | undefined | null): number | null { - if (!trackId) return null; - if (!stableLoudnessGainByTrackId[trackId]) return null; - const v = cachedLoudnessGainByTrackId[trackId]; - return Number.isFinite(v) ? v : null; -} - function resetLoudnessBackfillStateForTrackId(trackId: string) { for (const k of loudnessCacheStateKeysForTrackId(trackId)) { delete analysisBackfillInFlightByTrackId[k]; @@ -828,7 +805,7 @@ async function refreshWaveformForTrack(trackId: string) { } } -/** When `syncPlayingEngine` is false, only update `cachedLoudnessGainByTrackId` (e.g. queue neighbour) — do not call `audio_update_replay_gain` for the already-playing track. */ +/** When `syncPlayingEngine` is false, only update the loudness gain cache (e.g. queue neighbour) — do not call `audio_update_replay_gain` for the already-playing track. */ async function refreshLoudnessForTrack( trackId: string, opts?: { syncPlayingEngine?: boolean }, @@ -860,8 +837,7 @@ async function runRefreshLoudnessForTrack(trackId: string, syncEngine: boolean): return; } if (!row || !Number.isFinite(row.recommendedGainDb)) { - delete cachedLoudnessGainByTrackId[trackId]; - delete stableLoudnessGainByTrackId[trackId]; + forgetLoudnessGain(trackId); emitNormalizationDebug('refresh:miss', { trackId, row: row ?? null }); const auth = useAuthStore.getState(); const attempts = analysisBackfillAttemptsByTrackId[trackId] ?? 0; @@ -902,8 +878,7 @@ async function runRefreshLoudnessForTrack(trackId: string, syncEngine: boolean): }); return; } - cachedLoudnessGainByTrackId[trackId] = row.recommendedGainDb; - stableLoudnessGainByTrackId[trackId] = true; + markLoudnessStable(trackId, row.recommendedGainDb); analysisBackfillAttemptsByTrackId[trackId] = 0; emitNormalizationDebug('refresh:hit', { trackId, row }); usePlayerStore.setState({ @@ -917,8 +892,7 @@ async function runRefreshLoudnessForTrack(trackId: string, syncEngine: boolean): usePlayerStore.getState().updateReplayGainForCurrentTrack(); } } catch { - delete cachedLoudnessGainByTrackId[trackId]; - delete stableLoudnessGainByTrackId[trackId]; + forgetLoudnessGain(trackId); emitNormalizationDebug('refresh:error', { trackId }); usePlayerStore.setState({ normalizationDbgSource: 'refresh:error', normalizationDbgTrackId: trackId }); } @@ -1428,14 +1402,14 @@ export function initAudioListeners(): () => void { const payloadTrackId = normalizeAnalysisTrackId(payload.trackId); if (payloadTrackId && payloadTrackId !== current.id) return; if (!Number.isFinite(payload.gainDb)) return; - if (stableLoudnessGainByTrackId[current.id]) return; + if (hasStableLoudness(current.id)) return; // Skip when the cached gain is already within ~0.05 dB of the new payload — // float jitter from the partial-loudness heuristic would otherwise re-trigger // updateReplayGainForCurrentTrack → audio_update_replay_gain → backend echo // every PARTIAL_LOUDNESS_EMIT_INTERVAL_MS even when nothing audibly changed. - const existing = cachedLoudnessGainByTrackId[current.id]; - if (Number.isFinite(existing) && Math.abs(existing - payload.gainDb) < 0.05) return; - cachedLoudnessGainByTrackId[current.id] = payload.gainDb; + const existing = getCachedLoudnessGain(current.id); + if (Number.isFinite(existing) && Math.abs(existing! - payload.gainDb) < 0.05) return; + setCachedLoudnessGain(current.id, payload.gainDb); emitNormalizationDebug('partial-loudness:apply', { trackId: current.id, gainDb: payload.gainDb, @@ -1457,7 +1431,7 @@ export function initAudioListeners(): () => void { return; } // Backfill finished for another id (e.g. next in queue): refresh loudness cache only - // so `cachedLoudnessGainByTrackId` is ready before `audio_play` / gapless chain. + // so the cached gain is ready before `audio_play` / gapless chain. void refreshLoudnessForTrack(payloadTrackId, { syncPlayingEngine: false }); emitNormalizationDebug('backfill:applied', { trackId: payloadTrackId }); }), @@ -3295,9 +3269,9 @@ export const usePlayerStore = create()( : null; const normalization = deriveNormalizationSnapshot(currentTrack, queue, queueIndex); - const cachedLoud = cachedLoudnessGainByTrackId[currentTrack.id]; - const cachedLoudDb = Number.isFinite(cachedLoud) ? cachedLoud : null; - const haveStableLoud = !!stableLoudnessGainByTrackId[currentTrack.id]; + const cachedLoud = getCachedLoudnessGain(currentTrack.id); + const cachedLoudDb = Number.isFinite(cachedLoud) ? cachedLoud! : null; + const haveStableLoud = hasStableLoudness(currentTrack.id); const preEffForNorm = effectiveLoudnessPreAnalysisAttenuationDb( authState.loudnessPreAnalysisAttenuationDb, authState.loudnessTargetLufs, @@ -3324,7 +3298,7 @@ export const usePlayerStore = create()( volume, replayGainDb, replayGainPeak, - loudnessGainDb: currentTrack ? (cachedLoudnessGainByTrackId[currentTrack.id] ?? null) : null, + loudnessGainDb: currentTrack ? (getCachedLoudnessGain(currentTrack.id) ?? null) : null, preGainDb: authState.replayGainPreGainDb, fallbackDb: authState.replayGainFallbackDb, });