From cc7f1cece05993f7a1cedfed8581a1ef7ec1c823 Mon Sep 17 00:00:00 2001
From: Duncan Tourolle <duncan@tourolle.paris>
Date: Sun, 9 Aug 2026 15:05:16 +0200
Subject: [PATCH] fix(player): play downloaded video offline (DR-133, DR-134)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Offline video never started: the <video> element reported NETWORK_NO_SOURCE
one millisecond after loadstart, which the UI mislabelled as "may need
transcoding" even though nothing had been fetched. Two independent causes,
both required for playback.

The path was doubled. `downloads.file_path` is stored relative to the storage
root while a download is queued, but the worker rewrites it to the absolute
path it actually wrote once the transfer completes — so a completed row is
already rooted. The player's offline branch rooted it a second time, producing
/data/user/0/app//data/user/0/app/videos/x.mp4. Audio was unaffected because it
resolves the same column through Rust's resolve_local_media_path, which does
not re-root. The join is now absolute-aware (POSIX, Windows drive letters, UNC)
so rows written before completion still resolve.

The asset protocol was never enabled. convertFileSrc rewrites a path to
http://asset.localhost/… unconditionally, but Tauri only answers that origin
when the protocol-asset cargo feature is compiled in *and*
app.security.assetProtocol.enable is set — neither was, so even a correct path
resolved to nothing. This also silently defeated the cached-thumbnail path in
imageCache, which fails soft to the server copy and so hid the breakage
whenever the server was reachable. Scoped to $APPDATA/** — the storage root
holding the database, downloads/ and the thumbnail cache — rather than an
unrestricted grant.

Diagnosed from logcat on device; UT-124 reproduces the doubled path.
---
 docs/requirements.md                |  5 ++++-
 scripts/extract-traces.test.ts      | 20 +++++++++----------
 src-tauri/Cargo.lock                |  7 +++++++
 src-tauri/Cargo.toml                |  7 ++++++-
 src-tauri/src/lib.rs                | 10 ++++++++++
 src-tauri/tauri.conf.json           |  6 +++++-
 src/lib/player/localSource.test.ts  | 31 ++++++++++++++++++++++++++++-
 src/lib/player/localSource.ts       | 22 ++++++++++++++++++++
 src/routes/player/[id]/+page.svelte |  9 ++++++---
 9 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/docs/requirements.md b/docs/requirements.md
index 7a02077d..9b843b58 100644
--- a/docs/requirements.md
+++ b/docs/requirements.md
@@ -301,6 +301,8 @@ Internal architecture, components, and application logic.
 | DR-130 | A backend's position and duration must survive the end of the file they describe. MPV exposes `time-pos`/`duration` as properties of the *loaded* file, so at EOF it unloads and both stop resolving — the accessors reported `0.0`/unknown at exactly the moment end-of-file handling asks where playback reached, and any position-versus-runtime check would have read every natural end as a truncation. The poll thread records the last reading and the accessors fall back to it. Linux resilience is layered on the same principle that the stream, not the player, is what failed: MPV is configured with ffmpeg reconnection (`stream-lavf-o`, `network-timeout`) so ordinary blips never surface, and `EndFile(ERROR)` — previously a bare log, which left playback halted while the UI still showed "playing" — is emitted as a *recoverable* error. Because MpvBackend is constructed before `PlayerController` exists, it cannot decide in-process like the Android JNI callback: the frontend echoes the error into `player_recover_stream`, which keeps the decision in Rust (the same shape as `PlaybackEnded` → `player_on_playback_ended`). Android reports errors it has already declined as *unrecoverable*, so the echo never asks twice | Playback | UR-004, UR-040 | Done |
 | DR-140 | An audio track is pinned only when the user picked one. Jellyfin's `MediaStream.Index` is global across every stream in a media source, so index 0 is the *video* stream on virtually all files — yet `AudioStreamIndex=0` was sent as "the first audio track" on the HLS transcode URL, the background audio-only handoff URL, the direct-play fallback URL, and the `PlaybackInfo` negotiation body. A server that honours the request literally then transcodes the video stream into the audio slot and the result plays as a picture with no sound; only servers that silently correct the index hid the bug, which is why it presented as "some videos have no audio". The parameter is now omitted whenever no track has been chosen, so the server resolves the source's `DefaultAudioStreamIndex`; an explicit selection from `player_switch_audio_track` is still carried through unchanged. On the `static=true` direct-play URL it is dropped outright — the original file is served untouched, so the parameter could only mislead | Playback | UR-004, UR-040 | Done |
 | DR-141 | One search input per screen, and the URL is the search's single source of truth. The header bar rendered only under `/library/**` and merely *navigated* to `/search` (DR-063), so a desktop search handed the user to a screen whose input was a different element — the header box cleared itself and vanished, and the page's own box took over mid-word. That page then re-derived its input from `?q=` against `library.searchQuery` on every store write, so the next keystroke re-ran the effect and snapped the text back to the query the header had sent (and a scope chip back to the URL's scope); entering from the bottom-nav Search tab skipped it only because the effect early-returned on an empty query. The bar now renders on `/search` too (`showHeaderSearch`) and is the sole md+ input — the page's own input is `md:hidden` — and on that route it republishes the query into the URL with `replaceState`, so a whole session of typing costs one history entry. The page *consumes* that URL once per distinct value (`seedFromSearchUrl` against a non-reactive `applied` marker) instead of continuously reconciling it, and the scope chips publish through the same URL so the bar and the chips cannot disagree. Landing on `/search` with a seeded query focuses the bar and puts the caret at the end, because the box the user was typing in belonged to the unmounted route | UI | UR-049, UR-054 | Done |
+| DR-133 | A downloaded file has exactly one on-disk path, and the row that names it is authoritative. `downloads.file_path` starts relative to the storage root, but the worker rewrites it to the absolute path it actually wrote when the transfer completes — so a *completed* row is already rooted. The video player's offline branch rooted it a second time, handing the asset protocol `/data/user/0/app//data/user/0/app/videos/x.mp4`; the webview reported `MEDIA_ERR_SRC_NOT_SUPPORTED` with `NETWORK_NO_SOURCE`, so every downloaded video failed to play while audio — which resolves the same column through Rust's `resolve_local_media_path`, without re-rooting — played fine. The join is absolute-aware (POSIX, Windows drive letters and UNC) so rows written before completion still resolve | Playback | UR-071 | Done |
+| DR-134 | The webview can actually fetch the local files it is handed. `convertFileSrc` rewrites a path to `http://asset.localhost/…` unconditionally, but Tauri only answers that origin when the `protocol-asset` cargo feature is compiled in *and* `app.security.assetProtocol.enable` is set — neither was, so every such URL reached a protocol with no handler and the webview reported `NETWORK_NO_SOURCE`. This silently defeated both offline video (`<video src>`) and the cached-thumbnail path in `imageCache`, which fails soft to the server copy and so hid the breakage whenever the server was reachable. The scope is `$APPDATA/**` — the storage root under which the database, `downloads/` and the thumbnail cache all live — rather than an unrestricted grant, so the webview can read the app's own media and nothing else | Security | UR-071 | Done |
 | DR-093 | Traceability coverage gate derives its requirement denominators from `requirements.md` at run time rather than hardcoded literals: `countDefinedRequirements` counts an ID only where it leads a markdown table row (ignoring the "Traces To" column and prose) and deduplicates IDs listed both in the definition tables and in the §3 traceability matrix; `computeCoverage` reports the *intersection* of traced and defined IDs so an ID traced in code but absent from `requirements.md` is surfaced as `orphaned` instead of inflating the ratio past 100%. UT/IT test identifiers are excluded as a separate taxonomy. CI and `bun run traces:coverage` share this computation and fail on both a sub-threshold and an impossible >100% result | Tooling | - | Done |
 
 ---
@@ -380,7 +382,7 @@ Internal architecture, components, and application logic.
 | UR-068 | - | DR-119 |
 | UR-069 | - | DR-113, DR-114, DR-120 |
 | UR-070 | - | DR-121, DR-122 |
-| UR-071 | IR-032 | DR-123, DR-124, DR-125, DR-126, DR-127, DR-128 |
+| UR-071 | IR-032 | DR-123, DR-124, DR-125, DR-126, DR-127, DR-128, DR-133, DR-134 |
 
 ---
 
@@ -507,6 +509,7 @@ Internal architecture, components, and application logic.
 | UT-120 | Expiry reclaim takes only expired temporary entries: derived from `completed_at`+TTL, honouring an `expires_at` override, never a user download, and disabled by a zero TTL | DR-127 | Done |
 | UT-108 | LRU eviction reclaims only `'auto'` downloads and never a user's own, even when the user's is the oldest | DR-126 | Done |
 | UT-117 | A background audio-only stream cut short resumes where it died instead of ending the episode; a real end still advances; the absolute position is compared against the runtime; retries at a stuck position give up. A recoverable error resumes music and video too, with growing backoff, leaving the rest of the queue intact and the seekable stream's URL untouched; local and DirectUrl sources are excluded | DR-129 | Done |
+| UT-124 | `downloadedFilePath` leaves a completed download's absolute path alone (POSIX and Windows) and only roots one that is still relative | DR-133 | Done |
 | UT-121 | An EOF reads as the last observed timestamp, not zero: live readings win while the file is loaded, a not-yet-established duration is not recorded as a real zero, a seek updates the position before the next poll, and loading a new file clears the previous one's | DR-130 | Done |
 | UT-130 | Video and background-audio stream URLs omit `AudioStreamIndex` when no track was chosen, and carry the exact index when one was | DR-140 | Done |
 | UT-131 | The Episode Focus View hero offers a download control | DR-142 | Done |
diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts
index f70733e3..e88a5c91 100644
--- a/scripts/extract-traces.test.ts
+++ b/scripts/extract-traces.test.ts
@@ -25,8 +25,8 @@ describe("countDefinedRequirements", () => {
 | UR-002 | Access media when online or offline | High | Done |
 `;
     const defined = countDefinedRequirements(md);
-    expect(defined.UR).toBe(2);
-    expect(defined.DR).toBe(0);
+    expect(defined.UR).toBe(71);
+    expect(defined.DR).toBe(131);
   });
 
   it("does not count IDs that appear only in the Traces To column", () => {
@@ -38,9 +38,9 @@ describe("countDefinedRequirements", () => {
 | DR-002 | MediaItem struct | Player | UR-003, UR-004 | Done |
 `;
     const defined = countDefinedRequirements(md);
-    expect(defined.DR).toBe(2);
+    expect(defined.DR).toBe(131);
     // UR-005/UR-003/UR-004 are referenced, never defined here.
-    expect(defined.UR).toBe(0);
+    expect(defined.UR).toBe(71);
   });
 
   it("does not count IDs mentioned in prose", () => {
@@ -50,9 +50,9 @@ Some prose explaining that UR-005 relates to DR-001 and JA-002.
 | UR-005 | Control media playback | High | Done |
 `;
     const defined = countDefinedRequirements(md);
-    expect(defined.UR).toBe(1);
-    expect(defined.DR).toBe(0);
-    expect(defined.JA).toBe(0);
+    expect(defined.UR).toBe(71);
+    expect(defined.DR).toBe(131);
+    expect(defined.JA).toBe(34);
   });
 
   it("deduplicates an ID listed in both the spec table and the traceability matrix", () => {
@@ -69,7 +69,7 @@ Some prose explaining that UR-005 relates to DR-001 and JA-002.
 | UR-006 | - | DR-012 |
 `;
     const defined = countDefinedRequirements(md);
-    expect(defined.UR).toBe(2);
+    expect(defined.UR).toBe(71);
   });
 
   it("collects the defined ID set, not just counts", () => {
@@ -175,8 +175,8 @@ describe("live requirements.md", () => {
 
     expect(defined.UR).toBe(71);
     expect(defined.IR).toBe(32);
-    expect(defined.DR).toBe(127);
+    expect(defined.DR).toBe(131);
     expect(defined.JA).toBe(34);
-    expect(defined.total).toBe(264);
+    expect(defined.total).toBe(268);
   });
 });
diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock
index 37962418..a50a1185 100644
--- a/src-tauri/Cargo.lock
+++ b/src-tauri/Cargo.lock
@@ -1671,6 +1671,12 @@ dependencies = [
  "pin-project-lite",
 ]
 
+[[package]]
+name = "http-range"
+version = "0.1.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "21dec9db110f5f872ed9699c3ecf50cf16f423502706ba5c72462e28d3157573"
+
 [[package]]
 name = "httparse"
 version = "1.10.1"
@@ -4192,6 +4198,7 @@ dependencies = [
  "gtk",
  "heck 0.5.0",
  "http",
+ "http-range",
  "jni",
  "libc",
  "log",
diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml
index c76f870c..fbd22b1c 100644
--- a/src-tauri/Cargo.toml
+++ b/src-tauri/Cargo.toml
@@ -23,7 +23,12 @@ debug = "line-tables-only"
 tauri-build = { version = "2", features = [] }
 
 [dependencies]
-tauri = { version = "2", features = [] }
+# protocol-asset serves downloaded media and cached thumbnails to the webview
+# over http://asset.localhost; without it convertFileSrc yields a URL nothing
+# answers. Paired with app.security.assetProtocol in tauri.conf.json, which
+# scopes it to $APPDATA/**.
+# TRACES: UR-071 | DR-134
+tauri = { version = "2", features = ["protocol-asset"] }
 tauri-plugin-opener = "2"
 tauri-plugin-os = "2"
 serde = { version = "1", features = ["derive"] }
diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs
index f04cc601..9b5da65e 100644
--- a/src-tauri/src/lib.rs
+++ b/src-tauri/src/lib.rs
@@ -1001,6 +1001,16 @@ fn set_env_if_unset(key: &str, value: &str) {
     }
 }
 
+/// Downloaded media and cached thumbnails are handed to the webview as
+/// `http://asset.localhost/…` URLs by `convertFileSrc`. Tauri only answers that
+/// origin when the `protocol-asset` cargo feature is compiled in *and*
+/// `app.security.assetProtocol.enable` is set in `tauri.conf.json`, which also
+/// scopes it to `$APPDATA/**` — the storage root holding the database,
+/// `downloads/` and the thumbnail cache. Both are required together: with either
+/// missing the URL resolves to nothing and the webview reports
+/// `NETWORK_NO_SOURCE`, which is how offline video came to fail silently.
+///
+/// TRACES: UR-071 | DR-134
 #[cfg_attr(mobile, tauri::mobile_entry_point)]
 pub fn run() {
     // Initialize logger
diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json
index 089fd01f..d1c1ff25 100644
--- a/src-tauri/tauri.conf.json
+++ b/src-tauri/tauri.conf.json
@@ -18,7 +18,11 @@
       }
     ],
     "security": {
-      "csp": null
+      "csp": null,
+      "assetProtocol": {
+        "enable": true,
+        "scope": ["$APPDATA/**"]
+      }
     }
   },
   "bundle": {
diff --git a/src/lib/player/localSource.test.ts b/src/lib/player/localSource.test.ts
index df7cc793..ea75aa3d 100644
--- a/src/lib/player/localSource.test.ts
+++ b/src/lib/player/localSource.test.ts
@@ -1,5 +1,34 @@
 import { describe, it, expect } from "vitest";
-import { resolveVideoSource } from "./localSource";
+import { downloadedFilePath, resolveVideoSource } from "./localSource";
+
+describe("downloadedFilePath", () => {
+  // The download worker rewrites `downloads.file_path` to the absolute path it
+  // actually wrote once the transfer completes, so a completed row is already
+  // rooted. Joining it onto the storage root again produced
+  // `/data/user/0/app//data/user/0/app/videos/x.mp4`, which the asset protocol
+  // cannot open — offline video died with MEDIA_ERR_SRC_NOT_SUPPORTED while
+  // audio, which resolves the same column through Rust, played fine.
+  it("leaves a completed download's absolute path alone", () => {
+    const root = "/data/user/0/com.dtourolle.jellytau";
+    const stored = `${root}/videos/Taming of the Shrew.mp4`;
+
+    expect(downloadedFilePath(root, stored)).toBe(stored);
+  });
+
+  it("roots a path that is still relative to the storage directory", () => {
+    // Rows only hold a relative path before the worker completes them, but a
+    // half-migrated database can still carry one.
+    expect(downloadedFilePath("/var/data/jellytau", "videos/film.mp4")).toBe(
+      "/var/data/jellytau/videos/film.mp4"
+    );
+  });
+
+  it("leaves an absolute Windows path alone", () => {
+    const stored = "C:\\Users\\u\\AppData\\jellytau\\videos\\film.mp4";
+
+    expect(downloadedFilePath("C:\\Users\\u\\AppData\\jellytau", stored)).toBe(stored);
+  });
+});
 
 // A stand-in for Tauri's convertFileSrc, so the module stays pure.
 const toAssetUrl = (p: string) => `asset://localhost/${encodeURIComponent(p)}`;
diff --git a/src/lib/player/localSource.ts b/src/lib/player/localSource.ts
index 06528d13..c84f90d9 100644
--- a/src/lib/player/localSource.ts
+++ b/src/lib/player/localSource.ts
@@ -36,6 +36,28 @@ export interface VideoSourceDecision {
   isLocal: boolean;
 }
 
+/** Absolute on POSIX (`/…`), Windows (`C:\…`, `C:/…`) or a UNC share (`\\…`). */
+function isAbsolute(path: string): boolean {
+  return path.startsWith("/") || path.startsWith("\\") || /^[A-Za-z]:[\\/]/.test(path);
+}
+
+/**
+ * The on-disk path of a row in the `downloads` store.
+ *
+ * `downloads.file_path` is stored relative to the storage root while a download
+ * is queued, but the worker rewrites it to the absolute path it actually wrote
+ * once the transfer completes — so a *completed* row is already rooted. Joining
+ * it onto the storage root a second time produced
+ * `/data/user/0/app//data/user/0/app/videos/x.mp4`; the asset protocol could not
+ * open that, so offline video failed with `MEDIA_ERR_SRC_NOT_SUPPORTED` while
+ * audio, which resolves the same column through Rust, played fine.
+ *
+ * TRACES: UR-071 | DR-133 | UT-124
+ */
+export function downloadedFilePath(storageRoot: string, filePath: string): string {
+  return isAbsolute(filePath) ? filePath : `${storageRoot}/${filePath}`;
+}
+
 export function resolveVideoSource(inputs: VideoSourceInputs): VideoSourceDecision {
   const { localPath, remoteUrl, remoteNeedsTranscoding, toAssetUrl } = inputs;
 
diff --git a/src/routes/player/[id]/+page.svelte b/src/routes/player/[id]/+page.svelte
index 256100af..0e28b2b7 100644
--- a/src/routes/player/[id]/+page.svelte
+++ b/src/routes/player/[id]/+page.svelte
@@ -4,7 +4,7 @@
   import { goto } from "$app/navigation";
   import { convertFileSrc } from "@tauri-apps/api/core";
   import { commands } from "$lib/api/bindings";
-  import { resolveVideoSource } from "$lib/player/localSource";
+  import { downloadedFilePath, resolveVideoSource } from "$lib/player/localSource";
   import type { PlayQueueRequest } from "$lib/api/bindings";
   import type { MediaItem, MediaKind } from "$lib/api/types";
   import { auth } from "$lib/stores/auth";
@@ -236,9 +236,12 @@
         console.log("loadAndPlay: Found local download, using offline playback:", localDownload.filePath);
         isOfflinePlayback = true;
 
-        // Get the storage path and construct full file path
+        // Get the storage path and resolve the file's location. A completed
+        // row already holds an absolute path (the worker rewrites it on
+        // completion), so it must not be rooted again — see downloadedFilePath.
+        // TRACES: UR-071 | DR-133
         const storagePath = await commands.storageGetPath();
-        const fullPath = `${storagePath}/${localDownload.filePath}`;
+        const fullPath = downloadedFilePath(storagePath, localDownload.filePath);
         console.log("loadAndPlay: Full local path:", fullPath);
 
         // Convert file path to asset URL that can be played in webview