From b430a702b4cc67a417c407ffbaf3cd88f522f908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 19 Aug 2026 15:28:54 +0000 Subject: [PATCH] keep ffprobe as the floor under the meta.json lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the previous commit read the duration from meta.json instead of spawning ffprobe. correct for an indexed album and wrong everywhere else: /stream serves any home-relative path and never required an index, so a library that has not been indexed got no header at all. alpha is exactly that case — its music cache is empty — which is how this was caught rather than shipped. "always present" replaced with "present if indexed" is the same regression as removing the header, one layer down. so both, in order: meta.json when it can answer, ffprobe when it cannot, each cached. verified on an indexed album (247 from meta, 0.3ms warm) and on the same file with its index deleted (7.03 from ffprobe, 83ms cold, 0.1ms warm). --- MUSIC_API.md | 4 ++- sidecar/stream-audio.ts | 66 ++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 15 deletions(-) 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,