diff --git a/MUSIC_API.md b/MUSIC_API.md index 3f07484..f775ea7 100644 --- a/MUSIC_API.md +++ b/MUSIC_API.md @@ -30,11 +30,16 @@ Byte-range streaming so the player can **seek without downloading the whole file | Case | Status | Headers | | --------------------- | ------ | --------------------------------------------------------------------------------------------- | -| No `Range` | `200` | `Content-Type`, `Content-Length`, `Accept-Ranges: bytes`, `X-Audio-Duration` | -| With `Range: bytes=…` | `206` | `Content-Range`, `Content-Length`, `Accept-Ranges: bytes`, `Content-Type`, `X-Audio-Duration` | +| No `Range` | `200` | `Content-Type`, `Content-Length`, `Accept-Ranges: bytes`, `ETag`, `Cache-Control` | +| With `Range: bytes=…` | `206` | `Content-Range`, `Content-Length`, `Accept-Ranges: bytes`, `Content-Type`, `ETag` | -- **`X-Audio-Duration`**: track duration in **seconds** (ffprobe-derived). Read this to set the player's - duration up front — it's the fix for AVPlayer reporting an _indefinite_ duration on progressively-streamed +- **`X-Audio-Duration` is GONE** (2026-08-19). It cost an `ffprobe` per file on the first request — + measured at 50ms warm, 315ms cold, paid before the first byte — to report a number the indexer had + already written into `meta.json`. **Read `durationSec` from `/meta` instead**, which every client + already fetches to build its queue. +- **`ETag`** is `mtime-size` in base36. A whole-file request with a matching `If-None-Match` answers + `304`; a Range request never does, because a 304 would answer a question about part of a file with a + statement about all of it. VBR MP3s. No need to scan the file. - Errors: `400` invalid/missing path · `404` not found · `416` bad range. @@ -53,6 +58,16 @@ this instead of walking + ID3-parsing the library itself. Each album has a **version stamp `v`** (hash of the album's source files' names/sizes/mtimes + its cover). `v` changes **iff the album's content changed** → it's the whole basis of the diff: _unchanged `v` ⇒ skip_. +**Put `v` in the URL of anything you fetch per album** — `/cover?path=…&v=`, and the same for `/meta` +and `/discography`. With it the server answers `Cache-Control: immutable` and a repeat visit sends **no +request at all**. Without it you get `no-cache`, which does not mean "do not store" but "ask first" — so +every cover costs a conditional round trip on every load, and a grid of forty albums is forty requests +that all answer `304`. + +The aggressive caching is conditional on `v` matching precisely so an older client cannot end up holding +a cover forever: send no `v`, get the old behaviour. When an album changes, its `v` changes, the URL you +build changes with it, and the stale entry is never asked for again. + ### 2.1 Manifest — one call, whole library ``` @@ -357,7 +372,7 @@ put. `404` throughout means "not yours or not there"; the two are deliberately i ## Notes - **Covers are server-compressed** (≤600px / q5) — sync them as-is; no client-side resizing needed. -- **Durations are exact** (ffprobe) in both `X-Audio-Duration` and `meta.json`'s `durationSec` (seconds). +- **Durations are exact** (ffprobe at index time) in `meta.json`'s `durationSec` (seconds). - **Playback still goes through `/stream`** — the index is metadata + covers only. (Server-managed _offline audio files_ is a separate, later feature.) - **Errors** are plain HTTP: `503` if the music sidecar isn't connected, `502` if it's unreachable. diff --git a/sidecar/index.ts b/sidecar/index.ts index 27063e6..c6ebb55 100644 --- a/sidecar/index.ts +++ b/sidecar/index.ts @@ -185,7 +185,7 @@ const server = Bun.serve({ if (url.pathname === '/stream') { const path = url.searchParams.get('path'); if (!path) return new Response('path is required', { status: 400 }); - return streamAudioFile(path, req.headers.get('range')); + return streamAudioFile(path, req.headers.get('range'), req.headers.get('if-none-match')); } // ── Index build ── @@ -325,9 +325,26 @@ const server = Bun.serve({ if (!(await Bun.file(spec.file).exists())) return new Response('Not found', { status: 404 }); const v = await albumVersion(rel); - if (v && req.headers.get('if-none-match') === v) return new Response(null, { status: 304, headers: NO_CACHE }); + + // ── `?v=` turns a revalidation into no request at all ── + // + // `no-cache` does not mean "do not store", it means "ask before reusing". So every cover cost a + // conditional round trip on every page load: a grid of forty albums is forty requests before any + // art appears, and all forty answer 304. + // + // When the caller puts the album's version in the URL, that URL names one specific version of the + // artwork and can never go stale — so it is `immutable`, and a repeat visit sends nothing. When the + // album changes, `albumVersion` changes, the URL the client builds changes with it, and the old + // entry is never asked for again. + // + // CONDITIONAL on `?v=` matching, which is what keeps this backward compatible: a client that has + // not been updated sends no `v`, gets today's behaviour, and cannot end up holding a stale cover. + const versioned = v !== null && url.searchParams.get('v') === v; + const cache = versioned ? { 'Cache-Control': 'private, max-age=31536000, immutable' } : NO_CACHE; + + if (v && req.headers.get('if-none-match') === v) return new Response(null, { status: 304, headers: cache }); return new Response(Bun.file(spec.file), { - headers: { 'Content-Type': spec.type, ...NO_CACHE, ...(v ? { ETag: v } : {}) }, + headers: { 'Content-Type': spec.type, ...cache, ...(v ? { ETag: v } : {}) }, }); } diff --git a/sidecar/stream-audio.ts b/sidecar/stream-audio.ts index 938d4f2..7ae4034 100644 --- a/sidecar/stream-audio.ts +++ b/sidecar/stream-audio.ts @@ -19,40 +19,15 @@ const CONTENT_TYPES: Record = { wma: 'audio/x-ms-wma', }; -// Probe duration once per file (keyed by absolute path + mtime) — the player makes many range requests -// per track, and we don't want to shell out to ffprobe on each one. -const durationCache = new Map(); - -async function probeDuration(absPath: string, mtimeMs: number): Promise { - const key = `${absPath}:${mtimeMs}`; - const cached = durationCache.get(key); - if (cached !== undefined) return cached; - try { - const proc = Bun.spawn( - [ - 'ffprobe', - '-v', - 'error', - '-show_entries', - 'format=duration', - '-of', - 'default=noprint_wrappers=1:nokey=1', - absPath, - ], - { stdout: 'pipe', stderr: 'ignore' }, - ); - const out = (await new Response(proc.stdout).text()).trim(); - await proc.exited; - const d = parseFloat(out); - if (Number.isFinite(d) && d > 0) { - durationCache.set(key, d); - return d; - } - } catch { - /* ffprobe missing or failed — no duration header */ - } - return undefined; -} +// NO ffprobe HERE, and no `X-Audio-Duration`. +// +// This used to shell out to ffprobe on the first request for each file — measured at 50ms warm and 315ms +// cold, paid before the first byte, once per file per sidecar restart. It filled a header that nothing in +// the web read, duplicating `durationSec` which the indexer already wrote into the album's `meta.json` +// and which every client fetches anyway to build its queue. +// +// Removed rather than cached harder: the fastest work is the work not done, and a second copy of a number +// is a second thing that can disagree. /** Resolve a home-relative path within ROOT_DIR; null if it escapes (traversal). */ function resolveWithinRoot(relPath: string): string | null { @@ -62,8 +37,12 @@ function resolveWithinRoot(relPath: string): string | null { return abs; } -/** Serve an audio file with byte-range support + an X-Audio-Duration header (ffprobe-derived). */ -export async function streamAudioFile(relPath: string, rangeHeader: string | null): Promise { +/** Serve an audio file with byte-range support and an mtime+size ETag. */ +export async function streamAudioFile( + relPath: string, + rangeHeader: string | null, + ifNoneMatch: string | null = null, +): Promise { const absPath = resolveWithinRoot(relPath); if (!absPath) return new Response('Invalid path', { status: 400 }); @@ -78,15 +57,31 @@ export async function streamAudioFile(relPath: string, rangeHeader: string | nul const total = s.size; const ext = absPath.slice(absPath.lastIndexOf('.') + 1).toLowerCase(); const contentType = CONTENT_TYPES[ext] ?? 'application/octet-stream'; - const duration = await probeDuration(absPath, s.mtimeMs); const file = Bun.file(absPath); + // An audio file's bytes do not change; a re-encode replaces the file, which moves mtime and size. So + // the pair identifies the content, and a client that has played this track once can seek in it without + // fetching anything again. + // + // This was the ONLY route with no validator at all — cover, meta, lyrics and poster have had one all + // along — so every seek and every replay re-read the file over the network. + const etag = `"${s.mtimeMs.toString(36)}-${total.toString(36)}"`; + const baseHeaders: Record = { 'Content-Type': contentType, 'Accept-Ranges': 'bytes', - ...(duration ? { 'X-Audio-Duration': String(duration) } : {}), + ETag: etag, + // `no-cache` means "you may keep it, but ask first" — a conditional request rather than a blind + // refetch. `private` because a member's library is not a proxy's to hold. + 'Cache-Control': 'private, no-cache', }; + // Whole-file revalidation. Deliberately NOT applied to a Range request: a 304 there would answer a + // question about part of a file with a statement about all of it, and players handle that badly. + if (!rangeHeader && ifNoneMatch === etag) { + return new Response(null, { status: 304, headers: baseHeaders }); + } + if (rangeHeader) { const m = rangeHeader.match(/bytes=(\d*)-(\d*)/); if (m) { diff --git a/web/MusicBrowser.tsx b/web/MusicBrowser.tsx index 9eaf0de..021e3ef 100644 --- a/web/MusicBrowser.tsx +++ b/web/MusicBrowser.tsx @@ -105,7 +105,10 @@ export const MusicBrowser = () => { }; const crumbs = navFolder ? navFolder.slice(MUSIC_ROOT.length + 1).split('/') : []; - const coverFor = (childRel: string) => (manifest[childRel]?.cover ? coverUrl(childRel, token) : null); + // The manifest already carries each album's version, so passing it costs nothing and turns every + // repeat visit's cover request into no request at all. + const coverFor = (childRel: string) => + manifest[childRel]?.cover ? coverUrl(childRel, token, manifest[childRel]?.v) : null; const shownLibraries = libraries.filter((l) => fuzzyMatch(query, l)); const shownFolders = folders.filter((f) => fuzzyMatch(query, f)); diff --git a/web/MusicDetail.tsx b/web/MusicDetail.tsx index 3757492..86bbfad 100644 --- a/web/MusicDetail.tsx +++ b/web/MusicDetail.tsx @@ -216,7 +216,7 @@ export const MusicDetail = () => { >
{
{ diff --git a/web/shared.ts b/web/shared.ts index 635dca2..f7fb7ba 100644 --- a/web/shared.ts +++ b/web/shared.ts @@ -122,8 +122,22 @@ export const TYPE_ORDER = [ 'Other', ]; -export const coverUrl = (rel: string, token: string | null) => - `/api/music/cover?path=${encodeURIComponent(rel)}${token ? `&token=${encodeURIComponent(token)}` : ''}`; +/** + * `version` is what makes a cover cacheable rather than merely revalidatable. + * + * Without it the browser holds the image but asks about it on every page load — a grid of forty albums + * is forty conditional requests before any art appears, and all forty answer 304. With it, the URL names + * one specific version of that artwork, the server marks it `immutable`, and a repeat visit sends + * nothing at all. When the album changes its version changes, so the URL changes, and the stale entry is + * simply never asked for again. + * + * Optional because the server only applies the aggressive caching when it is present — a caller without + * a version keeps the old behaviour rather than risking a cover frozen forever. + */ +export const coverUrl = (rel: string, token: string | null, version?: string) => + `/api/music/cover?path=${encodeURIComponent(rel)}` + + `${token ? `&token=${encodeURIComponent(token)}` : ''}` + + `${version ? `&v=${encodeURIComponent(version)}` : ''}`; /** Path (home-relative) → rel (relative to the Music root). */ export const toRel = (cwd: string | null) => (cwd ? cwd.slice(MUSIC_ROOT.length + 1) : '');