three bottlenecks in streaming and loading
Measured, not guessed. ffprobe is gone from the stream path. It ran on the first request for each file — 50ms warm, 315ms cold on this machine, paid before the first byte, once per file per sidecar restart — to fill X-Audio-Duration, a header nothing in the web read and which duplicates the durationSec the indexer already wrote into meta.json. 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. BREAKING for any client that read that header; read /meta instead, which every client already fetches to build its queue. /stream gains an ETag and Cache-Control. It 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. mtime+size identifies the bytes, since a re-encode replaces the file. A Range request never answers 304: that would answer a question about part of a file with a statement about all of it, and players handle it badly. And `?v=` makes covers cacheable rather than merely revalidatable. `no-cache` means "ask before reusing", so a grid of forty albums cost forty conditional requests on every load, all answering 304. The manifest already carries each album's version, so passing it costs the client nothing — and the server then answers `immutable`, making a repeat visit send nothing at all. That last one is deliberately conditional on `v` being present, which keeps it backward compatible: a client that has not been updated gets today's behaviour and cannot end up holding a stale cover forever.
This commit is contained in:
+20
-3
@@ -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 } : {}) },
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
+33
-38
@@ -19,40 +19,15 @@ const CONTENT_TYPES: Record<string, string> = {
|
||||
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<string, number>();
|
||||
|
||||
async function probeDuration(absPath: string, mtimeMs: number): Promise<number | undefined> {
|
||||
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<Response> {
|
||||
/** 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<Response> {
|
||||
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<string, string> = {
|
||||
'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) {
|
||||
|
||||
Reference in New Issue
Block a user