From 0f928798d72edfd007e333afcb49f78270803c72 Mon Sep 17 00:00:00 2001
From: Duncan Tourolle <duncan@tourolle.paris>
Date: Tue, 22 Sep 2026 21:00:32 -0400
Subject: [PATCH] docs(specs): download the original and fix the audio on
 device
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A Jellyfin transcode is generated as it is sent — no Content-Length, Range
ignored — so every interruption restarts it from byte zero, and three
concurrent downloads are three ffmpeg jobs on the server. Measured on the
tablet: a direct copy moves 2.06 GB in 142 s with no retries, a transcode
crawls at ~1 MB/s and cannot resume. The transcode is only ever requested
because of the audio track.

So: always fetch Static=true, and re-encode the audio on the device when the
source carries something the renderers cannot decode.

DR-171 keeps its diagnosis — a downloaded film played as picture in silence,
and offline there is no other source to fall back to — and loses its remedy,
which explicitly accepted the loss of byte-range resumability. Its other
finding survives and constrains this spec: a downloaded file outlives whatever
experimentalNativeVideo was set to when it arrived, which is why the fix is to
the bytes on disk rather than to one renderer.

Records the rejected alternatives with the specific reason each fails, and the
decision to accept HEVC staying HEVC (Android-first; recoverable by a setting,
unlike silent audio).
---
 docs/specs/README.md                |   4 +-
 docs/specs/on-device-audio-remux.md | 327 ++++++++++++++++++++++++++++
 2 files changed, 330 insertions(+), 1 deletion(-)
 create mode 100644 docs/specs/on-device-audio-remux.md

diff --git a/docs/specs/README.md b/docs/specs/README.md
index 1a8b95f63..71c4dbb0a 100644
--- a/docs/specs/README.md
+++ b/docs/specs/README.md
@@ -28,7 +28,8 @@ know how something *works*, read
 
 **Next free requirement ids** (always re-check
 [requirements.md](../requirements.md) before allocating): **UR-086**,
-**IR-036**, **JA-038**, **DR-289**. Three specs below suggested ids that have
+**IR-036**, **JA-038**, **DR-291** (DR-289/290 went to the download stall
+timeout and the transcode progress estimate in v0.12.2). Three specs below suggested ids that have
 since been taken by other work; each carries a ⚠️ note at the top — this line
 was itself stale by five, two and forty-seven until 2026-09-08, which is why the
 re-check is not optional.
@@ -49,6 +50,7 @@ re-check is not optional.
 |---|---|
 | [desktop-native-video.md](desktop-native-video.md) | mpv draws video on every desktop platform, then the webview `<video>` path and hls.js are deleted. Converts a measured 7% direct-play rate toward Android's 85%. Stacked on backend-owned stream selection. |
 | [backend-owned-stream-selection.md](backend-owned-stream-selection.md) | Rust owns direct-play-vs-transcode, transport and quality; players consume one `StreamSelection`. Partly built — `StreamSelection`, `Transport` and the `.m3u8` sniff removal have landed. |
+| [on-device-audio-remux.md](on-device-audio-remux.md) | Downloads fetch the original (`Static=true`, resumable — measured 14× faster) and a bundled FFmpeg re-encodes only the audio on device, replacing DR-171's server-side transcode. HEVC side effect accepted (Android-first; the webview video path is being retired). Android APK size increase still to be measured. |
 | [build-provenance.md](build-provenance.md) | `build.rs` is still bare. ⚠️ suggested id DR-093 is taken. |
 | [player-facade-enforcement.md](player-facade-enforcement.md) | ~60 `commands.player*` sites still outside the facade; no lint rule. ⚠️ suggested id DR-095 is taken. |
 | [windows-native-audio-backend.md](windows-native-audio-backend.md) | Blocked on the libmpv2 swap. ⚠️ suggested id IR-030 is taken. |
diff --git a/docs/specs/on-device-audio-remux.md b/docs/specs/on-device-audio-remux.md
new file mode 100644
index 000000000..522d42969
--- /dev/null
+++ b/docs/specs/on-device-audio-remux.md
@@ -0,0 +1,327 @@
+# Spec: downloads fetch the original file and fix the audio on device
+
+**Status:** Proposed
+**Requirements:** UR-071, UR-004 → DR-291 (new). Revises **DR-171**, which keeps
+its diagnosis and loses its remedy — see [Relationship to DR-171](#relationship-to-dr-171).
+**UX spec:** n/a — one new row state in the existing transfers list.
+**Destination on completion:** [06-downloads-and-offline.md](../architecture/06-downloads-and-offline.md)
+— a new "Post-download processing" section after the download worker, plus a
+rewrite of the audio-policy paragraph that currently describes DR-171's
+server-side remedy.
+
+## Summary
+
+A video download always fetches the server's original bytes (`Static=true`) and,
+when the source carries audio this app's renderers cannot decode, re-encodes
+that audio **on the device** with a bundled FFmpeg instead of asking the server
+to transcode the whole film on the fly. Downloads become byte-range resumable
+again — the property that makes them reliable — and the server stops spending a
+CPU-hour per saved film.
+
+## Motivation
+
+A Jellyfin transcode is generated as it is sent: chunked, no `Content-Length`,
+and `Range` ignored. Every interruption therefore restarts it from byte zero,
+and with the queue at three concurrent downloads it is also three FFmpeg jobs on
+the server.
+
+Measured on the ROD2-W09 tablet against the production server (2026-09-22):
+
+| Download kind | Throughput | Retries | Resumable |
+|---|---|---|---|
+| `Static=true` direct copy | **2.06 GB in 142 s** (~14.5 MB/s) | 0 | yes (HTTP 206) |
+| Server transcode | ~1 MB/s | restarts from 0 on any blip | no (HTTP 200) |
+
+That is a ~14× throughput difference, and the transcode's failure mode is
+unbounded: three retries × a full restart each is the whole file fetched four
+times, which is what DR-289 was masking and DR-290 was papering over.
+
+The transcode is only ever requested because of the audio track. Roughly every
+AC-3/E-AC-3 film hits it on this device (no `audio/ac3` or `audio/eac3` decoder
+in `MediaCodecList`), so on a typical library the slow, non-resumable path is
+the common one, not the exception.
+
+### Relationship to DR-171
+
+DR-171 is **right about the defect and wrong about the remedy**, and this spec
+keeps the first half intact.
+
+The defect: `Static=true` hands back the source untouched, E-AC-3 track
+included, and a downloaded film played as picture in silence while the same film
+had sound when streamed. Offline, the download is the only source a video has,
+so there is no working path to fall back to.
+
+DR-171's remedy was to ask the server for a transcode, and it explicitly
+accepted the cost: *"the transcode costs the byte-range resumability
+`Static=true` gives the download worker"*. That cost is now measured, and it is
+the dominant one.
+
+DR-171 also chose to judge the codec against the **webview's** list rather than
+the device's, reasoning that *"a downloaded file outlives whatever
+`experimentalNativeVideo` was set to when it arrived"*. **That reasoning
+survives this spec and constrains it**: whatever lands on disk must play on
+either renderer, on either platform, years after the setting that was active
+when it arrived. It is the reason this spec fixes the bytes on disk rather than
+teaching one renderer to cope (see [Rejected alternatives](#rejected-alternatives)).
+
+## Layer assignment
+
+| Logic / responsibility | Layer | Why it belongs there |
+|---|---|---|
+| Whether a source's audio needs re-encoding at all | **Rust** | Domain: it compares a Jellyfin-reported codec against renderer capability. Changes when Jellyfin's stream metadata or a platform's decoder set changes. Already Rust (`served_audio_codec` + `webview_can_decode_audio`); unchanged by this spec. |
+| Which download URL to request (`Static=true` vs a transcode) | **Rust** | Domain: a Jellyfin route with Jellyfin query semantics. This spec changes the *answer* (always static) but not the owner. |
+| Target codec/bitrate/channel layout of the local re-encode | **Rust** | Domain: it must satisfy the same renderer-capability rule as the streaming profile. A frontend-chosen value would be a second, drifting copy of DR-149's policy. |
+| Running FFmpeg, and the remux invariants (copy video, re-encode audio only, preserve subtitle/chapter streams) | **Rust** | Backend owns downloads end to end (CLAUDE.md). It is also the only side that can see the file. |
+| Deciding a row is not yet available offline while processing | **Rust** | Business rule about download state. Falls out for free: every offline query already gates on `status = 'completed'`. |
+| Re-encode progress as a fraction | **Rust** | Computed from FFmpeg's reported position against the item runtime — domain data. Emitted like download progress. |
+| Rendering the `processing` state (label, spinner, bar) | **Frontend** | Pure presentation; changes only if the transfers list is redesigned. |
+| Whether to show the transfers list sorted with processing rows first | **Frontend** | View ordering preference. |
+
+Borderline row, stated with its tie-breaker: **"is FFmpeg available on this
+build?"** could read as an environment/presentation concern. It is placed in
+**Rust** because the answer changes what URL is requested (a fallback build must
+go back to asking the server to transcode), and that is a domain decision, not a
+capability the UI reasons about. The frontend never sees the flag.
+
+## Design
+
+### Pipeline
+
+```
+resolve  →  download (Static=true, resumable)  →  [needs audio fix?]  →  play
+                                                        │ yes
+                                                        ▼
+                                              remux on device (FFmpeg)
+                                              status = 'processing'
+                                                        │
+                                                        ▼
+                                              replace file, status = 'completed'
+```
+
+1. `resolve_video_download` keeps judging the served audio codec exactly as
+   today, but the verdict no longer selects a URL. The URL is always
+   `Static=true` (plus `mediaSourceId`). The verdict is **persisted on the row**
+   as `needs_audio_remux` so the worker does not have to re-fetch the item, and
+   so a resumed download decided before a server change keeps its own verdict.
+2. The worker downloads as it does now. It gets a real `Content-Length` and
+   HTTP 206 on resume, so DR-290's estimate machinery becomes a fallback rather
+   than the normal case.
+3. On completion, a row with `needs_audio_remux = 1` moves to `processing`
+   rather than `completed`, and a new `download::remux` module runs FFmpeg:
+   `-c:v copy -c:s copy -c:a aac -b:a 384k -ac <min(source, 6)>`, writing to
+   `<target>.remux.mp4` and renaming over the original only on success.
+4. The row goes `completed` with `file_size` set to the **remuxed** file's size.
+
+The video stream is copied, so this is I/O plus an audio encode — minutes on a
+feature film, not the hours a video re-encode would take. Peak disk is ~2× the
+film for the duration of the remux; the spec's acceptance criteria include
+refusing to start a remux without that headroom.
+
+### Why `processing` is a status and not a flag
+
+Every offline query already reads `status = 'completed'` (`offline.rs:36`,
+`:65`, `:107`; `repository/offline.rs:321`, `:330`, `:970`, `:979`, `:1150`).
+A new status that is simply *not* `completed` therefore excludes a
+mid-remux file from offline browsing with **no query changes** — a file whose
+audio has not been fixed yet is exactly as unplayable as one still downloading,
+and the existing gate already says so.
+
+### Wire shapes
+
+Status vocabulary gains one value, on both sides:
+
+```rust
+// schema.rs comment + DownloadInfo
+status TEXT DEFAULT 'pending',  -- pending, downloading, processing, completed, failed, paused
+```
+
+```rust
+// download/events.rs — the existing kebab-case "download-event" channel
+#[serde(rename_all = "camelCase")]
+Processing {
+    download_id: i64,
+    item_id: String,
+    /// 0.0..=1.0, from FFmpeg's position against the item runtime.
+    progress: f64,
+},
+```
+
+```typescript
+// stores/downloads.ts
+status: "pending" | "downloading" | "processing" | "completed" | "failed" | "paused";
+type: … | "processing";
+```
+
+No new command is needed: the frontend already subscribes to `download-event`
+and renders whatever status the row carries. `bindings.ts` is regenerated from
+Rust (`cargo test export_typescript_bindings`), not hand-edited.
+
+**The status arrives twice and both payloads carry the new value**: once as
+`DownloadInfo.status` from `get_downloads` (the refresh path, e.g. after an app
+restart mid-remux) and once as the `Processing` event. A build that taught only
+the event about `processing` would show a correct live remux and a row stuck on
+"Downloading" after any refresh.
+
+New row column:
+
+```sql
+ALTER TABLE downloads ADD COLUMN needs_audio_remux INTEGER DEFAULT 0;
+```
+
+### Packaging FFmpeg
+
+Both platforms link **libav\*** from Rust rather than shelling out to an
+`ffmpeg` binary, so the remux is one code path with one set of tests.
+
+- **Android**: build a minimal FFmpeg for `arm64-v8a` (plus the other ABIs the
+  universal APK carries) enabling only what the job needs — decoders
+  `ac3,eac3,dts,truehd`, encoder `aac`, demuxers/muxers `mov,mp4,matroska`,
+  and the `copy` bitstream path. **The size of that build is an open number this
+  spec requires measuring before acceptance**, not an estimate to design
+  around; if it exceeds a budget the maintainer sets, drop TrueHD and DTS-HD
+  first (rarest, largest tables).
+- **Linux**: the AppImage already ships an FFmpeg stack behind libmpv, but those
+  are libmpv's private libraries and must not be linked against directly. Build
+  or link the same minimal set as Android.
+- **CI**: per CLAUDE.md, the FFmpeg toolchain and the prebuilt libraries live in
+  the builder image (`Dockerfile.builder`, rebuilt and pushed via
+  `scripts/build-builder-image.sh`). Nothing is fetched or compiled at job time.
+
+### Fallback, so this ships incrementally
+
+If the build has no FFmpeg, or the remux fails for any reason, the item falls
+back to **today's behaviour**: request the server-side transcode URL. This means
+the change can land platform by platform, a remux bug degrades to the current
+(working, slow) path rather than to a silent film, and the DR-171 defect cannot
+reappear. The fallback is logged at `warn` so it is visible rather than silent.
+
+## Rejected alternatives
+
+Recorded because each is the obvious next idea and each has a specific reason it
+fails — this is the half that gets folded into the architecture doc.
+
+- **Teach ExoPlayer to decode AC-3 via media3's FFmpeg decoder extension.**
+  Cheaper-looking: no file is rewritten, and it would fix streaming direct-play
+  for the same codecs. It fails DR-171's surviving constraint — a downloaded
+  file outlives the `experimentalNativeVideo` setting, and on Linux the webview
+  draws video, so the webview path would still be silent. It fixes one renderer;
+  the bytes on disk have to satisfy all of them. Worth doing **separately** as a
+  streaming optimisation, where the constraint does not apply.
+- **Use the device's own MediaCodec to decode AC-3.** The test device reports no
+  `audio/ac3` or `audio/eac3` decoder at all, which is the reason the transcode
+  is requested there in the first place. It is not a decoder we can assume.
+- **A pure-Rust decoder (Symphonia).** Symphonia does not implement AC-3, E-AC-3
+  or DTS. There is no pure-Rust path for the codecs that actually matter here.
+- **Download the server's HLS transcode segment by segment and remux locally.**
+  Segments are individually addressable, so this would restore resumability at
+  segment granularity without decoding anything on device. Rejected because it
+  still spends the server CPU this spec is trying to stop spending, still needs
+  an mp4 muxer on device (most of the same dependency), and HLS transcode
+  sessions expire — a download paused overnight would find its segments gone.
+- **Do nothing and raise the retry budget.** The transcode restarts from byte
+  zero, so each extra retry is another full-file fetch. More retries buys a
+  linear increase in bytes moved for a fixed, low probability of finishing.
+
+## Out of scope
+
+- **Files already downloaded stay as they are.** DR-171's closing note applies
+  unchanged: the bytes on disk are the wrong bytes, and only a re-download (or a
+  future one-off "repair downloads" pass) replaces them. If that pass is wanted,
+  it belongs beside the remux module as a command that re-runs it over existing
+  `completed` rows — noted there, not left to this file, which will be deleted.
+- **Streaming.** DR-149's server-side transcode for undecodable audio is
+  untouched; a stream has no file to fix and no resumability to lose. The media3
+  decoder extension above is the lever there.
+- **Video re-encoding on device.** HEVC sources that the *webview* cannot render
+  are currently converted to h264 as a side effect of the server transcode.
+  Under this spec an HEVC source with fine audio is copied verbatim and plays on
+  ExoPlayer but not in the webview.
+
+  **Decided 2026-09-22: accept it.** The project is Android-first until video
+  can be 100% mpv, and both ends of that road decode HEVC — ExoPlayer does today,
+  mpv will on the desktop ([desktop-native-video.md](desktop-native-video.md)).
+  The webview `<video>` element is the renderer being retired, so spending a
+  full server transcode of every HEVC film to keep a path alive that is on its
+  way out is the wrong trade. The exposure while it lasts: an HEVC film
+  downloaded on Android, then played with `experimentalNativeVideo` turned off,
+  shows no picture. Unlike DR-171's silent audio this is **recoverable without
+  re-downloading** — turn the setting back on — which is what makes it
+  acceptable where the audio case was not.
+- Quality presets (`high`/`medium`/`low`) keep asking the server to transcode —
+  they are a deliberate request for smaller files, the server does it better,
+  and their non-resumability is a known cost of a choice the user made.
+
+## Acceptance criteria
+
+- [ ] A film whose audio needs fixing downloads over `Static=true`, reports a
+      real `Content-Length`, and resumes from a kill -9 with HTTP 206.
+- [ ] That film, played offline afterwards, has sound on both ExoPlayer and the
+      webview `<video>` element.
+- [ ] Its row passes through `processing` and is absent from offline browsing
+      until the remux finishes.
+- [ ] A remux failure leaves the row `failed` with the partial output removed,
+      never `completed` over a half-written file.
+- [ ] A build without FFmpeg still downloads the same film correctly via the
+      server transcode, with a `warn` naming the fallback.
+- [ ] The Android APK size increase is measured and recorded in the PR.
+- [ ] `bun run check` and `bun run test` pass.
+- [ ] `cargo fmt` clean, `cargo clippy --all-targets -- -D warnings` clean,
+      `bun run test:rust` passes.
+- [ ] `bun run check:boundary` passes.
+- [ ] New requirement-implementing code carries `// TRACES:` comments and
+      `bun run traces:validate` passes.
+- [ ] `bindings.ts` regenerated from Rust.
+
+## Testing
+
+**Rust (`cargo test`)**
+
+- The URL builder returns `Static=true` for every quality-`original` case,
+  including the codecs that previously forced a transcode — the inverse of
+  today's `test_video_download_url_original_transcodes_undecodable_audio`, which
+  this spec **rewrites rather than deletes** (its `Static=true` assertion for
+  playable codecs stays).
+- `needs_audio_remux` is persisted from the same verdict the old URL choice used,
+  at all three resolution sites.
+- Remux argument construction is a pure function over (source codec, channel
+  count) and is tested without invoking FFmpeg: video is copied, audio is `aac`,
+  subtitles survive, no bitrate or scale filter appears.
+- A row in `processing` is excluded by `offline_is_available`.
+- Disk-headroom refusal: a remux is not started when free space is under the
+  output estimate.
+
+**Frontend (vitest)**
+
+- `describeProgress` and the transfers row render `processing` distinctly from
+  `downloading` — extend `downloadProgress.test.ts`.
+- The store carries `processing` through and does not treat it as terminal —
+  extend `downloads.test.ts`.
+
+**On device** — the acceptance criteria above are the manual pass; the tablet
+is the right target because it is the one with no AC-3 decoder.
+
+## TRACES
+
+| Piece | Tag |
+|---|---|
+| Always-static download URL + persisted remux verdict | `// TRACES: UR-071, UR-004 \| DR-291 \| UT-255` |
+| `download::remux` module (argument construction, invariants) | `// TRACES: UR-071 \| DR-291 \| UT-256` |
+| `processing` status + `Processing` event | `// TRACES: UR-071 \| DR-291 \| UT-257` |
+| Frontend `processing` rendering | `// TRACES: UR-071 \| DR-291 \| UT-258` |
+
+Allocate **DR-291** in [requirements.md](../requirements.md) (DR-290 is taken by
+the progress-estimate work) and **UT-255**–**UT-258**. Re-check the maxima
+before allocating: the specs index has been stale about this before.
+
+## Notes for the implementer
+
+- **A parallel Claude session may be active in this repo.** Run `git diff`
+  before "repairing" unexpected changes — see the CLAUDE.md gotchas.
+- DR-290's estimate code is not dead after this lands: quality presets still
+  transcode server-side, and the FFmpeg-less fallback still exists. Leave it.
+- The remux must not run on the tokio runtime's I/O threads — it is a long CPU
+  job. Use a blocking task, and honour the existing `download::stop` flag so a
+  cancel during processing is not ignored.
+- Editing Android sources means editing `src-tauri/android/src` and running
+  `scripts/sync-android-sources.sh`; never edit the `gen/` tree.
+- Do not run `./gradlew` directly in `gen/android` — build through `scripts/`.