docs(plan): commit Phase 4.1 to Option A1 (Range headers, custom decoder)
Record the design-gate decision for HTTP Range support: Range headers in the JS fetch retaining the AudioBuffer decoder, rejecting MediaElement (loses early-playback) and synthesized-header-over-Range (breaks caching invariant). Add per-file shape, acceptance criteria, and the file-absolute offset constraint. Tighten 4.2 — disk-streaming already done on the default path; only the legacy offset branch remains.
This commit is contained in:
@@ -119,19 +119,34 @@ These follow from `CONTEXT.md §5`. Direction is strongly implied but no specifi
|
||||
|
||||
### 4.1 HTTP Range + CDN caching
|
||||
|
||||
- **What:** Today's `?offset=` query parameter defeats HTTP caching — a CDN sees `?offset=1234567` as a distinct URL from the un-offset request. The architecture re-invents byte-range on top of a custom query param.
|
||||
- **What:** Today's `?offset=` query parameter defeats HTTP caching — a CDN sees `?offset=1234567` as a distinct URL from the un-offset request. The architecture re-invents byte-range on top of a custom query param. Move the player's transport to standard HTTP `Range` headers against one canonical URL.
|
||||
- **Why it matters:** Material once the site has real listener traffic. Also relevant to non-WAV formats (1.2) where decoder-side seek is cheaper natively.
|
||||
- **Shape:** Two intertwined moves.
|
||||
- Server: `LoadResourceStreamAsync` returning an open `FileStream` instead of `LoadResourceAsync` materialising the whole buffer. `File(stream, mime, enableRangeProcessing: true)`. The `WavOffsetService` synthesised-header path becomes a special-case rather than the default.
|
||||
- Client: consider `MediaElementAudioSourceNode` instead of (or alongside) `decodeAudioData`-fed `AudioBufferSourceNode`s. Native seek, native range, native cache; FFT tap on the audio graph still works for the spectrum visualiser.
|
||||
- **Prerequisite:** None functionally, but the audit explicitly flagged this trade-off as architecture-intentional — the current path was chosen because spectrum analysis wants `AudioBuffer`s. Re-deciding the trade-off is itself part of the work.
|
||||
- **Constraint:** A move to `MediaElementAudioSourceNode` changes the early-playback story (the element handles buffering, not us). Worth a design pass.
|
||||
- **Chosen approach (design pass 2026-06-09): Option A1 — Range headers in the JS fetch, keep the custom `AudioBuffer` decoder.** Rejected Option B (`MediaElementAudioSourceNode`): it surrenders early-playback (the `minBuffersForPlayback` start-as-soon-as-buffered behaviour, a listed quality feature) and forces a redesign of the waveform-seek and early-play UX, while delivering no caching benefit beyond what the HTTP layer already gives. Also rejected A2 (synthesised header delivered over Range): keeping `WavOffsetService` on the hot path means each `bytes=X-` request produces a distinct synthesised prefix that can't share cache lineage with the canonical `bytes=0-` object, defeating half the caching win. A1 makes the cached object the *real file*, so every Range request is a true sub-range of one entity. Key enabling insight: `StreamDecoder` already synthesises a per-segment 44-byte header internally for every `decodeAudioData` call (`createWavFile`), so a Range continuation only needs to *retain* the parsed `WavHeader` and feed raw PCM — it does not need a header in the network stream.
|
||||
- **Shape (implementation direction):**
|
||||
- **Server (`DeepDrftAPI/Controllers/TrackController.cs` ~L407):** flip `enableRangeProcessing: false → true` on the no-offset seekable `FileStream` path; ASP.NET Core slices natively and emits `206` + `Content-Range`. Leave the `?offset=` / `WavOffsetService` branch reachable but off the player hot path — its removal is a clean follow-up commit, not part of this change.
|
||||
- **Proxy (`DeepDrftPublic/Controllers/TrackProxyController.cs` ~L175):** forward the incoming `Range` request header upstream; pass through upstream status (`206`/`200`/`416`) and the `Content-Range` / `Accept-Ranges` / `Content-Length` response headers verbatim. The proxy is a transparent relay — it does **not** slice the (non-seekable) upstream stream. Keep `ResponseHeadersRead` + `RegisterForDispose`.
|
||||
- **Client transport (`DeepDrftPublic.Client/Clients/TrackMediaClient`):** send `Range: bytes={byteOffset}-` instead of the `?offset=` query param (`byteOffset == 0` → `bytes=0-`, single code path). Confirm `TrackMediaResponse.ContentLength` carries the 206 remaining-length for continuations and full length for the initial request.
|
||||
- **JS decoder (`StreamDecoder.ts` — the real work):** add a continuation mode. Replace `reinitializeForOffset` (which nulls `wavHeader` and re-parses) with a `reinitializeForRangeContinuation(remainingByteLength)` that **retains** the parsed `WavHeader`, resets `rawChunks`/`totalRawBytes`/`processedBytes`/`streamComplete`, and routes incoming bytes straight to `addRawData` (the existing `if (!this.wavHeader)` branch already does this when the header is set). Add an `isContinuation` flag so `updateStreamCompleteFlag()` uses `totalRawBytes` **without** the `+ headerSize` addend on continuations. `createWavFile`, the decode pipeline, and the spectrum/level tap are all unchanged.
|
||||
- **`AudioPlayer.ts` / `index.ts`:** keep the public `reinitializeFromOffset` interop name (so `AudioInteropService` and the C# caller are untouched); internally call the continuation reinit. C# `StreamingAudioPlayerService.SeekBeyondBuffer` is otherwise unchanged.
|
||||
- **Acceptance criteria:**
|
||||
1. Initial load sends `Range: bytes=0-`; server responds `206`/`200` with `Accept-Ranges: bytes`; time-to-first-audio unchanged (early playback after `minBuffersForPlayback`).
|
||||
2. Seek-beyond-buffer sends `Range: bytes=X-` (block-aligned, file-absolute X) with **no `?offset=` anywhere**; server responds `206` + `Content-Range`; audio resumes with no click/pop and no header bytes leaking into PCM.
|
||||
3. Displayed total duration is unchanged across a seek (original full-track duration, not remaining-segment).
|
||||
4. A track seeked-near-end then played out fires the end callback exactly once (continuation `streamComplete` math correct).
|
||||
5. Spectrum visualiser and `LevelMeterFab` behave identically pre/post on a loud master (−3 dBFS).
|
||||
6. Same-URL invariant: two different-offset requests hit an identical URL differing only in the `Range` header (verifiable in the network panel; live CDN cache-hit verification is out of scope — no CDN in dev).
|
||||
7. No `MediaElement` introduced; the `AudioBufferSourceNode` graph remains the playback path.
|
||||
- **Constraints (non-obvious):**
|
||||
- **Range offset is file-absolute, not audio-relative.** The old `?offset=` contract was audio-data-relative (`WavOffsetService` added `HeaderSize` server-side). The Range offset must be `header.headerSize + blockAlignedAudioOffset`. Omitting `headerSize` lands the seek ~44 bytes early — audible click + position drift. **Most likely bug; verify first.**
|
||||
- Only the *continuation* skips header parse; the initial `bytes=0-` response still flows through `tryParseHeader` unchanged. Don't let the continuation flag bleed into initial load.
|
||||
- Proxy must pass `Accept-Ranges` / `Content-Range` (and a `416`) through verbatim — stripping them blinds the browser and any future CDN.
|
||||
- A1 preserves the multi-format (1.2) seam: the decoder stays the format integration point; the "retain format, skip header, treat bytes as frame data" pattern generalises (frame-boundary alignment differs per format). Add no new WAV-specific coupling in the transport/proxy layers beyond what already exists.
|
||||
|
||||
### 4.2 Server-side stream from disk (no buffer materialisation)
|
||||
|
||||
- **What:** `LoadResourceAsync<AudioBinary>` reads the entire file into memory before `File(file.Buffer, mimeType)` returns it. A 100 MB WAV is a 100 MB LOH allocation per request.
|
||||
- **Why it matters:** Scaling ceiling. Currently fine for a small audience and small library; not fine if either grows.
|
||||
- **Shape:** Folds into 4.1 — the same `LoadResourceStreamAsync` overload solves both. Listed separately because either could land without the other (you could stream from disk while still using the `?offset=` query path, or you could move to `Range` headers while still buffering).
|
||||
- **What:** The no-offset path **already** streams from disk — `TrackController` (~L390) takes `mediaStream.Stream` (a `FileStream` from `LoadResourceStreamAsync`), reads `streamLength` from `.Length`, and hands ownership to `File(...)`; no `LoadResourceAsync` buffer materialisation on the default path. The remaining buffer materialisation is **only** the legacy `?offset=` branch (~L414): `GetAudioBinaryAsync` loads the full `AudioBinary` into memory because `WavOffsetService` reslices over the in-memory buffer.
|
||||
- **Why it matters:** Scaling ceiling on the offset path specifically. Once 4.1 (A1) lands, the offset branch is off the player hot path, so its buffer cost stops mattering in practice.
|
||||
- **Shape:** Resolved for the default path. The only outstanding work is retiring the offset branch entirely — which is the 4.1 follow-up commit (remove the `?offset=` server branch, `WavOffsetService`, and the now-unused `ConcatStream`). No separate work item beyond that cleanup.
|
||||
|
||||
### 4.3 Dual-write rollback / dead-letter log
|
||||
|
||||
|
||||
Reference in New Issue
Block a user