fix: animated artwork album cache poisoned by title-only fallback

Only cache artwork results that were matched by album. Previously the
first track looked up in an album decided the cached answer for every
other track: the title-only fallback (and its miss) was stored under the
album key, so later tracks in the album inherited it even when a
title-specific lookup would have found artwork.

fetchAnimatedArtwork now returns a scope key alongside the result — the
album key only when album-matched, else the title key. The dedupe skips
in artwork.ts and index.ts compare against that effective scope, so
skipping within an album stays free whenever the album matched.
This commit is contained in:
Daniel committed 2026-10-02 09:00:41 +02:00
1 parent fa309cb8b6
commit d76ed2ce55
3 files changed
+87 -39

No files matched your search

+36 -14
View File
@@ -207,6 +207,21 @@ export interface AnimatedArtwork {
url_tall: string; url_tall: string;
} }
export interface ArtworkLookup {
artwork: AnimatedArtwork | null;
/**
* Scope the result is valid for, as a dedupe key: the album key only when
* the answer was matched by album. A title-only fallback (or a miss) is
* specific to this track, so its key scopes to the title — the album's
* other tracks must not inherit it.
*/
key: string;
}
/** Title-scoped artwork key: a result only valid for this exact track. */
export const artworkTitleKey = (artist: string, title: string): string =>
`${artist}\u0000\u0000${title}`;
function artworkSearchUrl( function artworkSearchUrl(
title: string, title: string,
artist: string, artist: string,
@@ -219,6 +234,8 @@ function artworkSearchUrl(
} }
// Cached including misses; most tracks have none and the answer never changes. // Cached including misses; most tracks have none and the answer never changes.
// Only album-matched results go under the album key — a title-only fallback
// must not stand in for the album's other tracks (see ArtworkLookup).
const animatedArtworkCache = new Map<string, AnimatedArtwork | null>(); const animatedArtworkCache = new Map<string, AnimatedArtwork | null>();
const ARTWORK_CACHE_MAX = 200; const ARTWORK_CACHE_MAX = 200;
@@ -227,14 +244,16 @@ export async function fetchAnimatedArtwork(
artist: string, artist: string,
album?: string, album?: string,
signal?: AbortSignal, signal?: AbortSignal,
): Promise<AnimatedArtwork | null> { ): Promise<ArtworkLookup> {
// Keyed by album, matching artworkKey() in index.ts. const albumName = album?.trim() || "";
const cacheKey = const albumPresent = albumName !== "";
album && album.trim() !== "" // Album key, matching artworkKey() in index.ts; without an album the
? `${artist}\u0000${album.trim()}` // answer is only ever matched by title, so key by title directly.
: `${artist}\u0000\u0000${title}`; const cacheKey = albumPresent
? `${artist}\u0000${albumName}`
: artworkTitleKey(artist, title);
if (animatedArtworkCache.has(cacheKey)) { if (animatedArtworkCache.has(cacheKey)) {
return animatedArtworkCache.get(cacheKey) ?? null; return { artwork: animatedArtworkCache.get(cacheKey) ?? null, key: cacheKey };
} }
const controller = new AbortController(); const controller = new AbortController();
@@ -286,14 +305,17 @@ export async function fetchAnimatedArtwork(
const withAlbum = await tryFetch( const withAlbum = await tryFetch(
artworkSearchUrl(title, artist, album), artworkSearchUrl(title, artist, album),
); );
if (withAlbum) return remember(withAlbum); // Album-matched: cacheable, and its scope is the whole album.
if (album && album.trim() !== "") { if (withAlbum) return { artwork: remember(withAlbum), key: cacheKey };
const withoutAlbum = await tryFetch( if (albumPresent) {
artworkSearchUrl(title, artist), // Album lookup missed: the title-only answer — hit or miss — is
); // matched by title, so never cache it under the album key (that
if (withoutAlbum) return remember(withoutAlbum); // would poison the album's other tracks) and scope it to the title.
const artwork = await tryFetch(artworkSearchUrl(title, artist));
return { artwork, key: artworkTitleKey(artist, title) };
} }
return remember(null); // No album: the title key is already track-specific.
return { artwork: remember(null), key: cacheKey };
} finally { } finally {
clearTimeout(timeout); clearTimeout(timeout);
signal?.removeEventListener("abort", onAbort); signal?.removeEventListener("abort", onAbort);
+33 -21
View File
@@ -6,7 +6,7 @@ import { Tracer } from "@luna/core";
// Light build: no alt-audio/subtitles/EME needed here. Types in hls-light.d.ts. // Light build: no alt-audio/subtitles/EME needed here. Types in hls-light.d.ts.
import Hls from "hls.js/light"; import Hls from "hls.js/light";
import { fetchAnimatedArtwork } from "./api"; import { artworkTitleKey, fetchAnimatedArtwork } from "./api";
import { settings } from "./Settings"; import { settings } from "./Settings";
const { trace } = Tracer("[Radiant Lyrics]"); const { trace } = Tracer("[Radiant Lyrics]");
@@ -27,7 +27,10 @@ export class AnimatedArtworkLayer {
private resizeObs: ResizeObserver | null = null; private resizeObs: ResizeObserver | null = null;
private resizeRaf = 0; private resizeRaf = 0;
private artwork: { url: string; url_tall: string } | null = null; private artwork: { url: string; url_tall: string } | null = null;
/** See artworkKey() in index.ts. */ /**
* Scope key of the displayed artwork (see fetchAnimatedArtwork): the album
* key only when the answer was matched by album, else the title key.
*/
private artworkKey: string | null = null; private artworkKey: string | null = null;
private liveSrc: string | null = null; private liveSrc: string | null = null;
private loadToken = 0; private loadToken = 0;
@@ -109,52 +112,61 @@ export class AnimatedArtworkLayer {
/** /**
* Mount the artwork for `key`, leaving any playing video alone until a * Mount the artwork for `key`, leaving any playing video alone until a
* replacement resolves. False means no tile was on screen, so retry later. * replacement resolves. `done: false` means no tile was on screen, so
* retry later. Returns the scope `key` the result is valid under (see
* fetchAnimatedArtwork), which may be narrower than the request key.
*/ */
async load( async load(
key: string, key: string,
title: string, title: string,
artist: string, artist: string,
album: string | undefined, album: string | undefined,
): Promise<boolean> { ): Promise<{ done: boolean; key: string }> {
// Already showing this album's artwork — keep playing, touch nothing. // Already showing artwork that covers this track (album-matched, or the
if (key === this.artworkKey && this.artwork && this.video?.isConnected) { // same track again) — keep playing, touch nothing.
if (
(key === this.artworkKey ||
artworkTitleKey(artist, title) === this.artworkKey) &&
this.artwork &&
this.video?.isConnected
) {
this.ensurePlaying(); this.ensurePlaying();
return true; return { done: true, key: this.artworkKey };
} }
const token = ++this.loadToken; const token = ++this.loadToken;
this.fetchAbort?.abort(); this.fetchAbort?.abort();
this.fetchAbort = new AbortController(); this.fetchAbort = new AbortController();
const signal = this.fetchAbort.signal; const signal = this.fetchAbort.signal;
if (!settings.animatedArtwork) return true; if (!settings.animatedArtwork) return { done: true, key };
// Only check a tile exists; host after the fetch so the old video keeps playing. // Only check a tile exists; host after the fetch so the old video keeps playing.
if (!document.querySelector<HTMLElement>(ART_TILE_SELECTOR)) return false; if (!document.querySelector<HTMLElement>(ART_TILE_SELECTOR))
return { done: false, key };
const artwork = await fetchAnimatedArtwork(title, artist, album, signal); const lookup = await fetchAnimatedArtwork(title, artist, album, signal);
if (token !== this.loadToken) return true; if (token !== this.loadToken) return { done: true, key: lookup.key };
if (!this.tileAlive()) return false; if (!this.tileAlive()) return { done: false, key: lookup.key };
if (!artwork) { if (!lookup.artwork) {
trace.log(`Animated Artwork: no animated art for "${title}"`); trace.log(`Animated Artwork: no animated art for "${title}"`);
this.detach(false); this.detach(false);
// After detach(), which clears the key too. // After detach(), which clears the key too.
this.artworkKey = key; this.artworkKey = lookup.key;
return true; return { done: true, key: lookup.key };
} }
// Same stream already on screen: adopt the key, leave the video be. // Same stream already on screen: adopt the key, leave the video be.
const unchanged = const unchanged =
this.artwork?.url === artwork.url && this.artwork?.url === lookup.artwork.url &&
this.artwork?.url_tall === artwork.url_tall && this.artwork?.url_tall === lookup.artwork.url_tall &&
this.video?.isConnected === true; this.video?.isConnected === true;
this.artwork = artwork; this.artwork = lookup.artwork;
this.artworkKey = key; this.artworkKey = lookup.key;
if (unchanged) { if (unchanged) {
this.ensurePlaying(); this.ensurePlaying();
return true; return { done: true, key: lookup.key };
} }
this.mountVideo(false); this.mountVideo(false);
return true; return { done: true, key: lookup.key };
} }
/** Re-bind to the artwork tile after the Now Playing view (re)mounted. */ /** Re-bind to the artwork tile after the Now Playing view (re)mounted. */
+18 -4
View File
@@ -15,6 +15,7 @@ import {
type LyricsApiResponse, type LyricsApiResponse,
type WordLine, type WordLine,
type WordTiming, type WordTiming,
artworkTitleKey,
fetchLyrics as fetchLyricsApi, fetchLyrics as fetchLyricsApi,
flushLyrics as flushLyricsApi, flushLyrics as flushLyricsApi,
romanizeLyrics as romanizeLyricsApi, romanizeLyrics as romanizeLyricsApi,
@@ -1048,7 +1049,12 @@ let animatedArtworkLayer: AnimatedArtworkLayer | null = null;
let lastArtworkKey: string | null = null; let lastArtworkKey: string | null = null;
let lastArtworkTile: Element | null = null; let lastArtworkTile: Element | null = null;
/** Animated art is an album asset, so key by album and skips within one are free. */ /**
* Request key for an animated-artwork lookup: album-scoped when an album is
* present (skips within one are free). A result may still be scoped tighter —
* see fetchAnimatedArtwork() — so skipping compares lastArtworkKey, the
* effective scope, not this request key.
*/
const artworkKey = (t: TrackInfo): string => const artworkKey = (t: TrackInfo): string =>
t.album && t.album.trim() !== "" t.album && t.album.trim() !== ""
? `${t.artist}\u0000${t.album.trim()}` ? `${t.artist}\u0000${t.album.trim()}`
@@ -1072,20 +1078,28 @@ const updateAnimatedArtwork = async (
// Re-check after the await; another caller may have claimed this already. // Re-check after the await; another caller may have claimed this already.
if (!settings.animatedArtwork || !animatedArtworkLayer) return; if (!settings.animatedArtwork || !animatedArtworkLayer) return;
const key = artworkKey(target); const key = artworkKey(target);
if (key === lastArtworkKey) { // Free skip: the shown artwork's scope covers this track — it was matched
// by album (album key), or it is the same track again (title key).
if (
key === lastArtworkKey ||
artworkTitleKey(target.artist, target.title) === lastArtworkKey
) {
// Same artwork, but the view may have remounted underneath us. // Same artwork, but the view may have remounted underneath us.
if (rehost) animatedArtworkLayer.reattach(); if (rehost) animatedArtworkLayer.reattach();
return; return;
} }
lastArtworkKey = key; lastArtworkKey = key;
const done = await animatedArtworkLayer.load( const res = await animatedArtworkLayer.load(
key, key,
target.title, target.title,
target.artist, target.artist,
target.album, target.album,
); );
// Adopt the effective scope — a title-matched result (or a miss) must not
// stand in for the album's other tracks — unless a newer trigger claimed.
if (lastArtworkKey === key) lastArtworkKey = res.key;
// No tile on screen yet; clear the key so the tile observer retries. // No tile on screen yet; clear the key so the tile observer retries.
if (!done && lastArtworkKey === key) lastArtworkKey = null; if (!res.done && lastArtworkKey === res.key) lastArtworkKey = null;
}; };
(window as any).updateAnimatedArtwork = updateAnimatedArtwork; (window as any).updateAnimatedArtwork = updateAnimatedArtwork;