restore the three streaming improvements
reverts0e08aef,b430a70and1170e41— stream-audio.ts and MUSIC_API.md are now byte-identical to2a70bc1. all three were unwound over two days chasing a mobile playback failure that turned out to have nothing to do with this plugin. the reasoning that undid them was elimination, not diagnosis: the duration header did not fix it, so the etag "must" be the cause. the option never considered was that none of it was mine, which is the one that was true. nothing here was measured wrong. removing the per-file ffprobe from the path before the first byte is the speedup, and it is confirmed in real use. what the mobile client should know, unchanged from MUSIC_API.md: x-audio-duration is gone, read durationSec from /meta instead — it arrives with the queue, before the first stream request.
This commit is contained in:
+10
-23
@@ -30,30 +30,17 @@ Byte-range streaming so the player can **seek without downloading the whole file
|
|||||||
|
|
||||||
| Case | Status | Headers |
|
| Case | Status | Headers |
|
||||||
| --------------------- | ------ | --------------------------------------------------------------------------------------------- |
|
| --------------------- | ------ | --------------------------------------------------------------------------------------------- |
|
||||||
| No `Range` | `200` | `Content-Type`, `Content-Length`, `Accept-Ranges: bytes`, `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`, `X-Audio-Duration` |
|
| With `Range: bytes=…` | `206` | `Content-Range`, `Content-Length`, `Accept-Ranges: bytes`, `Content-Type`, `ETag` |
|
||||||
|
|
||||||
- **`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.
|
|
||||||
|
|
||||||
|
- **`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.
|
- Errors: `400` invalid/missing path · `404` not found · `416` bad range.
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|||||||
+27
-111
@@ -1,8 +1,6 @@
|
|||||||
import { stat } from 'node:fs/promises';
|
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 { 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
|
// 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"),
|
// 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<string, string> = {
|
|||||||
wma: 'audio/x-ms-wma',
|
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
|
// Removed rather than cached harder: the fastest work is the work not done, and a second copy of a number
|
||||||
// already written `durationSec` into the album's `meta.json`. The mobile player stopped playing, which
|
// is a second thing that can disagree.
|
||||||
// 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<string, Map<string, number>>();
|
|
||||||
|
|
||||||
async function durationFromIndex(relPath: string): Promise<number | undefined> {
|
|
||||||
// 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<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;
|
|
||||||
}
|
|
||||||
|
|
||||||
/** Resolve a home-relative path within ROOT_DIR; null if it escapes (traversal). */
|
/** Resolve a home-relative path within ROOT_DIR; null if it escapes (traversal). */
|
||||||
function resolveWithinRoot(relPath: string): string | null {
|
function resolveWithinRoot(relPath: string): string | null {
|
||||||
@@ -118,29 +37,11 @@ function resolveWithinRoot(relPath: string): string | null {
|
|||||||
return abs;
|
return abs;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/** Serve an audio file with byte-range support and an mtime+size ETag. */
|
||||||
* 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.
|
|
||||||
*/
|
|
||||||
export async function streamAudioFile(
|
export async function streamAudioFile(
|
||||||
relPath: string,
|
relPath: string,
|
||||||
rangeHeader: string | null,
|
rangeHeader: string | null,
|
||||||
_ifNoneMatch: string | null = null,
|
ifNoneMatch: string | null = null,
|
||||||
): Promise<Response> {
|
): Promise<Response> {
|
||||||
const absPath = resolveWithinRoot(relPath);
|
const absPath = resolveWithinRoot(relPath);
|
||||||
if (!absPath) return new Response('Invalid path', { status: 400 });
|
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 contentType = CONTENT_TYPES[ext] ?? 'application/octet-stream';
|
||||||
const file = Bun.file(absPath);
|
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<string, string> = {
|
const baseHeaders: Record<string, string> = {
|
||||||
'Content-Type': contentType,
|
'Content-Type': contentType,
|
||||||
'Accept-Ranges': 'bytes',
|
'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) {
|
if (rangeHeader) {
|
||||||
const m = rangeHeader.match(/bytes=(\d*)-(\d*)/);
|
const m = rangeHeader.match(/bytes=(\d*)-(\d*)/);
|
||||||
if (m) {
|
if (m) {
|
||||||
|
|||||||
Reference in New Issue
Block a user