diff --git a/MUSIC_API.md b/MUSIC_API.md index 22b5c50..f775ea7 100644 --- a/MUSIC_API.md +++ b/MUSIC_API.md @@ -30,30 +30,17 @@ 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` | - -- **`X-Audio-Duration` still ships, and no longer costs an `ffprobe`** (2026-08-19). It used to spawn - one per file on the first request — measured at 50ms warm, 315ms cold, paid before the first byte. - It is now read from the album's `meta.json` and cached per album: 1.5ms cold, 0.1ms warm, and the - same number, because the indexer got it from ffprobe too. - - It was briefly removed outright, on the reasoning that nothing in the web read it and `/meta` already - carried `durationSec`. The mobile player stopped playing, so it is load-bearing for at least one - client and stays in the contract. New clients should still prefer `durationSec` from `/meta` — it - arrives with the queue, before the first stream request. - - `meta.json` is the fast path, not the only one: `/stream` serves any home-relative path and does not - require indexing, so an unindexed album falls back to ffprobe (cached per file) rather than dropping - the header. Absent only if ffprobe is missing or the file is unreadable — as before. -- **`/stream` has no `ETag` and no `Cache-Control`.** One was added on 2026-08-19 and removed the same - day: the mobile player stopped playing, and after `X-Audio-Duration` was ruled out these headers were - the only remaining difference from the version that worked. A cache that keeps a validator after - evicting the body would send `If-None-Match`, receive `304`, and have no audio. - - `If-None-Match` on `/stream` is therefore ignored — never `304`, always bytes. `/cover`, `/meta`, - `/lyrics` and `/poster` DO have validators and are unaffected. +| 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` 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. ``` diff --git a/sidecar/stream-audio.ts b/sidecar/stream-audio.ts index 30334c4..7ae4034 100644 --- a/sidecar/stream-audio.ts +++ b/sidecar/stream-audio.ts @@ -1,8 +1,6 @@ import { stat } from 'node:fs/promises'; -import { basename, dirname, join, resolve, sep } from 'node:path'; +import { resolve, sep } from 'node:path'; import { homedir } from 'node:os'; -import type { IndexTrack } from './indexer'; -import { metaFilePath } from './indexer'; // All processing lives here (the platform is just a proxy). Files live under the owner's home — single // super user, so HOME_DIR. Paths from the app are home-relative (e.g. "Music/Artist/Album/track.mp3"), @@ -21,94 +19,15 @@ const CONTENT_TYPES: Record = { wma: 'audio/x-ms-wma', }; -// `X-Audio-Duration`: meta.json first, ffprobe only if that has no answer. +// NO ffprobe HERE, and no `X-Audio-Duration`. // -// ffprobe alone cost 50ms warm and 315ms cold, once per file per restart, paid BEFORE THE FIRST BYTE. +// 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. // -// It was removed outright on the reasoning that nothing in the web read the header and the indexer had -// already written `durationSec` into the album's `meta.json`. The mobile player stopped playing, which -// settles it: the header is part of the contract and something depends on it. -// -// Reading it from meta.json instead is the same number — the indexer got it from ffprobe at index time — -// for a file read rather than a process spawn. But it is only the same number FOR AN INDEXED ALBUM, and -// streaming does not require indexing: /stream serves any home-relative path, so a library that has never -// been indexed (alpha's, at the time of writing) would get no header at all. Replacing "always present" -// with "present if indexed" is the same regression again, one layer down. -// -// Hence both. The fast path covers the normal case and the spawn stays as the floor, so the header is -// present whenever it used to be, and costs a process only where meta.json cannot answer. - -/** - * `durationSec` for one track, from its album's `meta.json`. Undefined if the album is not indexed. - * - * Cached per album rather than per track: one read answers every track on the record, which is the access - * pattern — a queue plays an album in order. Keyed on the meta file's own mtime so a reindex invalidates - * it without anything having to remember to. - */ -const albumDurations = new Map>(); - -async function durationFromIndex(relPath: string): Promise { - // meta.json does NOT sit beside the audio — it lives under the sidecar's own cache root, mirroring the - // album's relative path. `metaFilePath` is the one place that knows that; do not re-derive it here. - const metaPath = metaFilePath(dirname(relPath.replace(/^\/+/, ''))); - if (!metaPath) return undefined; - - let key: string; - try { - key = `${metaPath}:${(await stat(metaPath)).mtimeMs}`; - } catch { - return undefined; // unindexed album — caller falls back to ffprobe - } - - let byFile = albumDurations.get(key); - if (!byFile) { - try { - const meta = (await Bun.file(metaPath).json()) as { tracks?: IndexTrack[] }; - byFile = new Map( - (meta.tracks ?? []).flatMap((t) => (t.durationSec ? [[t.file, t.durationSec] as [string, number]] : [])), - ); - albumDurations.set(key, byFile); - } catch { - return undefined; - } - } - return byFile.get(basename(relPath)); -} - -// 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; -} +// 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 { @@ -118,29 +37,11 @@ function resolveWithinRoot(relPath: string): string | null { return abs; } -/** - * Serve an audio file with byte-range support and an `X-Audio-Duration` header. - * - * ── No ETag, no Cache-Control, no 304 — and that is deliberate ── - * - * All three were added on 2026-08-19 to spare a re-download on replay and seek: /stream was the only - * route with no validator, so every seek re-read the file over the network. The reasoning still looks - * right and the measurement was real. - * - * The mobile player stopped playing. Restoring `X-Audio-Duration` did not fix it, and the range and - * whole-file branches are byte-identical to the version that worked, which left these headers as the - * only remaining difference. A cache that keeps a validator after evicting the body would send - * `If-None-Match`, get a 304, and have no audio — plausible, and not something to keep testing on - * someone's phone. - * - * So /stream is back to exactly what worked. `ifNoneMatch` is still accepted and deliberately unused: - * the caller passes it, and dropping the parameter would only have to be re-added. Re-introducing this - * needs a capture of what the player actually sends, not another guess. - */ +/** 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, + ifNoneMatch: string | null = null, ): Promise { const absPath = resolveWithinRoot(relPath); if (!absPath) return new Response('Invalid path', { status: 400 }); @@ -158,14 +59,29 @@ export async function streamAudioFile( const contentType = CONTENT_TYPES[ext] ?? 'application/octet-stream'; const file = Bun.file(absPath); - const duration = (await durationFromIndex(relPath)) ?? (await probeDuration(absPath, s.mtimeMs)); + // 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) {