diff --git a/MUSIC_API.md b/MUSIC_API.md index ea68575..94cee14 100644 --- a/MUSIC_API.md +++ b/MUSIC_API.md @@ -43,7 +43,9 @@ Byte-range streaming so the player can **seek without downloading the whole file client and stays in the contract. New clients should still prefer `durationSec` from `/meta` — it arrives with the queue, before the first stream request. - Absent when the album has not been indexed, exactly as it was absent when ffprobe failed. + `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. - **`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. diff --git a/sidecar/stream-audio.ts b/sidecar/stream-audio.ts index da86cdf..529ba1f 100644 --- a/sidecar/stream-audio.ts +++ b/sidecar/stream-audio.ts @@ -21,22 +21,25 @@ const CONTENT_TYPES: Record = { wma: 'audio/x-ms-wma', }; -// `X-Audio-Duration` COMES FROM meta.json, NOT from ffprobe. +// `X-Audio-Duration`: meta.json first, ffprobe only if that has no answer. // -// 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. +// ffprobe alone cost 50ms warm and 315ms cold, once per file per restart, paid BEFORE THE FIRST BYTE. // -// It was briefly removed altogether, on the grounds that nothing in the web read the header and the -// indexer had already written `durationSec` into the album's `meta.json`. The mobile player disagreed by -// refusing to play, which is the answer that counts: it is a documented part of the contract and -// something out there depends on it. +// 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. // -// So the header stays and the ffprobe does not. The value is read from the same `meta.json` the client -// fetches anyway, cached per album, and it is the SAME number ffprobe produced — the indexer got it from -// ffprobe at index time. A file read instead of a process spawn. +// 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`. + * `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 @@ -44,7 +47,7 @@ const CONTENT_TYPES: Record = { */ const albumDurations = new Map>(); -async function durationFromMeta(relPath: string): Promise { +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(/^\/+/, ''))); @@ -54,7 +57,7 @@ async function durationFromMeta(relPath: string): Promise { try { key = `${metaPath}:${(await stat(metaPath)).mtimeMs}`; } catch { - return undefined; // unindexed album — no duration to report, same as ffprobe failing + return undefined; // unindexed album — caller falls back to ffprobe } let byFile = albumDurations.get(key); @@ -72,6 +75,41 @@ async function durationFromMeta(relPath: string): Promise { 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; +} + /** Resolve a home-relative path within ROOT_DIR; null if it escapes (traversal). */ function resolveWithinRoot(relPath: string): string | null { const clean = relPath.replace(/^\/+/, ''); @@ -110,7 +148,7 @@ export async function streamAudioFile( // along — so every seek and every replay re-read the file over the network. const etag = `"${s.mtimeMs.toString(36)}-${total.toString(36)}"`; - const duration = await durationFromMeta(relPath); + const duration = (await durationFromIndex(relPath)) ?? (await probeDuration(absPath, s.mtimeMs)); const baseHeaders: Record = { 'Content-Type': contentType,