dtourolle/jellytau
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 52 Wiki Activity

fix(player): mpv draws all Linux video, and no longer runs text from a URL

Browse Source 
Security (DR-298, DR-299):
- The pinned libmpv crate's Mpv::command joins its arguments and calls
  mpv_command_string, which parses `;` as a command separator. Stream
  URLs carry server-controlled ids and TranscodingUrl, and a download's
  file:// path carries its track title, so a crafted title could run any
  mpv command, `run` included. Every call now goes through
  mpv_command::command, an argv built for mpv_command. The same parse
  broke loadfile for every downloaded title containing a space.
- mpv's tls-verify defaults to no, and its URLs carry the ApiKey. Every
  handle is now hardened with tls-verify=yes and ytdl=no before its
  first loadfile, and fails construction if it cannot be.

Linux video (DR-235 phase 1):
- native_video::enabled() is unconditional on Linux; the
  JELLYTAU_NATIVE_VIDEO opt-in is retired. No platform reports a
  webview video fallback, so the Settings switch no longer appears.
  Windows keeps the webview element until mpv reaches it (DR-237).
- The Linux device profile is unchanged (still h264, DR-234), so this
  ships the configuration that was tested under the env var.
This commit is contained in:
dtourolle
2026-09-24 20:38:32 -04:00
parent fd1277746d
commit 9d9d81bef3
13 changed files with 298 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/requirements.md
+13 -6
View File
@@ -437,7 +437,7 @@ Internal architecture, components, and application logic.
| DR-232 | The mpv render context's lifetime is bound to the GL context it draws into: created on `realize`, freed on `unrealize`, on the same thread, with the update callback unregistered *before* the free so a callback cannot land on a freed context. This is DR-184 on Android restated — a surface outliving its player — and it is a requirement in its own right rather than a fix for a specific crash. The spike observed one SIGSEGV in a decoder thread that three targeted soaks failed to reproduce; what is not in doubt is that the spike never called `mpv_render_context_free` and never tore down on `unrealize`, so nothing defended against the GL context being recreated underneath. Removing the likeliest cause is worth doing whether or not it was the cause | Playback | UR-080 | Proposed |
| DR-233 | Frame pacing goes through mpv's update callback, with `mpv_render_context_report_swap` after each render. Recorded as a requirement because the failure mode misleads: driving the widget's frame clock every tick without reporting the swap leaves mpv with nothing to time against, which looks fine in a window and **judders at fullscreen** — reading as a compositing or GPU limit and being neither | Playback | UR-080 | Proposed |
| DR-234 | The device profile is derived from the **renderer that will decode the stream**, not from a compile-time platform constant. `video_codecs` was `#[cfg(target_os)]`, which is correct only while a build has one video renderer; once mpv and the webview element coexist it must be runtime state. This is the change that converts the measured 7% desktop direct-play rate toward the 85% the Android profile achieves on the same library, because the two differ by nothing except which component decodes. It looks like configuration and is not — it is the input that decides whether the server re-encodes, and getting it wrong fails silently, a claimed codec the renderer cannot decode being a black picture or silence (DR-148, and DR-227's audio override). The webview's narrower *audio* set stops applying to the video path once mpv decodes it, while the multichannel bound still does, since a 5.1 track direct-played into a two-channel sink is silence or inaudible dialogue | Repository | UR-080, UR-070 | In Progress |
| DR-235 | The webview video path is deleted, not merely bypassed. Staged, because a path cannot be removed while a shipped platform still needs it: Linux moves to mpv first, Windows follows, and only then do `hls.js`, `html5Adapter.ts`, `videoLoaderFor` and the `<video>` element go. The staging is the point — a Linux-only version would leave the fork alive permanently, taking video from three renderers to four and giving every seek strategy, track switch and lifecycle bug one more place to be got right. Android keeps ExoPlayer and keeps the webview as its documented opt-out; the background-audio `<audio>` path is untouched. With no HTML5 fallback left, a failed mpv init emits `backend-init-failed` and surfaces a real error rather than silently degrading to the transcode this work exists to stop paying for | Playback | UR-080 | Proposed |
| DR-235 | The webview video path is deleted, not merely bypassed. Staged, because a path cannot be removed while a shipped platform still needs it: Linux moves to mpv first, Windows follows, and only then do `hls.js`, `html5Adapter.ts`, `videoLoaderFor` and the `<video>` element go. The staging is the point — a Linux-only version would leave the fork alive permanently, taking video from three renderers to four and giving every seek strategy, track switch and lifecycle bug one more place to be got right. Android keeps ExoPlayer and keeps the webview as its documented opt-out; the background-audio `<audio>` path is untouched. With no HTML5 fallback left, a failed mpv init emits `backend-init-failed` and surfaces a real error rather than silently degrading to the transcode this work exists to stop paying for | Playback | UR-080 | In Progress |
| DR-236 | Hardware-decode policy is decided from what mpv reports it **selected** (`hwdec-current`), never from what it was asked for. The spike established that hardware decode works through the render API at all — the load-bearing result, since it means direct play is not bought with software decoding — but also that `auto` reached for the discrete GPU in copy-back mode on a hybrid Intel+NVIDIA laptop, the least efficient hardware path, and that `vaapi` fell back to software silently because the libva driver was absent. So zero-copy VA-API on the integrated GPU is preferred where the driver is present, `auto` is a fallback rather than the default, and a missing driver is detected and logged rather than mistaken for a compositing limit | Playback | UR-080 | Proposed |
| DR-237 | Windows reaches the same mpv path, reusing everything except the surface. The surface is genuinely different code — a native child window beneath a transparent WebView2, not GTK — but the render context, lifetime discipline, frame pacing, device profile and hwdec policy are shared, which is why none of them may be guarded on `cfg!(target_os = "linux")`. The cost is mostly build, not video: `libmpv` is currently a Linux-only dependency while Windows is cross-compiled from Linux via `x86_64-pc-windows-msvc` + `cargo-xwin`, so a Windows libmpv must reach that cross-build and its DLL must ship in the NSIS bundle, carrying the LGPL obligations DR-216 already records — dynamic linkage, licence text shipped alongside. Windows gains a native audio decoder as a side effect, which is what the long-blocked Windows audio work wants and cannot otherwise have | Playback | UR-080 | Proposed |
| DR-238 | A transcoded seek re-negotiates the stream on every renderer, not just the webview. Jellyfin produces a transcode *from* `StartTimeTicks`, so where a seek lands is a property of the request rather than of the stream in hand. `determine_video_seek_strategy` treated `is_hls` as a proxy for "seekable in place", which held only because hls.js was always the HLS renderer — it seeks within the VOD playlist it is handed and lets the server catch up. mpv's HLS demuxer cannot make the server transcode from a new offset, so with native video on, every transcoded seek became a backend seek that silently did nothing and presented as "resume does not work". The rule is now written on `needs_transcoding` with hls.js as the stated exception; all four webview cells are unchanged | Player | UR-040 | Done |
@@ -499,6 +499,8 @@ Internal architecture, components, and application logic.
| DR-295 | A series page lists its episodes with one concurrent season fan-out. "More info" on Frasier took ~10 s: the page asked Rust for the episodes and for the current episode as two commands, each of which walked every season, and each walk fetched the eleven seasons one after another — so the wait was the sum of twenty-two listings, each a cache read slowed by whatever the database was writing (the catalog sync at launch measured it at ~4 s per walk). The seasons are now fetched together (`gather_season_episodes`, so the wait is the slowest season), and `repository_get_series_view` returns the episodes and the current episode from one walk, with Next Up and resume fetched alongside it | Repository | UR-062 | Done |
| DR-296 | Returning from background audio resumes the item the native player is actually on, not the one the video page was mounted with. An episode that ends while backgrounded advances in the backend (`advance_to_next_episode_audio_only`), but `player_exit_background_audio` returned only a position, so the webview reloaded the *previous* episode at the new episode's timestamp. The command now returns `BackgroundAudioResume { itemId, positionSeconds }` (`PlayerController::background_audio_resume`); `planHandoffReturn` yields `other-item` when the id differs from the mounted one, and the player page navigates to that episode with `resumeAt=<seconds>`, recording the outgoing episode as watched and suppressing the stale unmount stop report | Playback | UR-040, UR-023 | Done (pending device verification) |
| DR-297 | A background refresh never blanks a library detail page that is on screen, and every load that succeeds clears the page's error. Resuming the app after a few minutes in the background replaced the Frasier series page with "Failed to load item": the resume reload (reconnect / offline-filter change) is a refresh of content the cache had already answered, but any throw in it replaced the page with an error that no later successful reload of the same item cleared — and the catch logged nothing and turned backend errors (plain strings) into the generic text. A failed refresh now keeps the page and logs the thrown value; only failing to open an item shows an error, with the backend's own message | UI | UR-062 | Done |
| DR-298 | mpv receives commands as an argument vector, never as a command string. The pinned `libmpv` crate's `Mpv::command` joins its arguments with spaces and calls `mpv_command_string`, which parses input.conf syntax — whitespace splits, `;` chains a second command, `#` comments out the rest — so a stream URL carrying a server-controlled item id or `TranscodingUrl`, or a download's `file://` path built from its track title, could run any mpv command, `run` included: a crafted title tag was arbitrary code execution on the Linux desktop. The same parse broke every downloaded title containing a space, `Song.mp3` landing in `loadfile`'s flags slot. Every call goes through `mpv_command::command`, which builds a NUL-terminated `argv` for `mpv_command` so each argument reaches mpv as one opaque string, and refuses an argument containing NUL rather than loading a truncated URL | Playback | UR-003, UR-004 | Done |
| DR-299 | Every libmpv handle verifies TLS and never hands a URL to youtube-dl. mpv's `tls-verify` defaults to *no*, and the stream URLs it loads carry the account's `ApiKey`, so anyone able to present a certificate for the server's host — a hostile network, a spoofed DNS answer — read a long-lived token from the one HTTP path in the app that did not check. libmpv also loads its ytdl hook by default, which passes a URL that failed to open, token included, to an external `yt-dlp`. `mpv_command::harden` sets `tls-verify=yes` and `ytdl=no` on each handle before its first `loadfile`, and a handle that cannot be hardened fails construction instead of playing unverified | Playback | UR-012 | Done |
---
@@ -510,8 +512,8 @@ Internal architecture, components, and application logic.
|----------|-------------------------|-------------------------|
| UR-001 | IR-001, IR-002 | - |
| UR-002 | IR-013 | DR-003, DR-012, DR-013, DR-014, DR-294 |
| UR-003 | IR-003, IR-004, IR-011 | DR-002, DR-004, DR-010, DR-182, DR-183, DR-184, DR-185, DR-186, DR-187, DR-188, DR-190, DR-191, DR-192, DR-193, DR-194, DR-195, DR-196, DR-291 |
| UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006, DR-129, DR-171, DR-176, DR-177, DR-181, DR-182, DR-183, DR-185, DR-188, DR-203, DR-265, DR-293 |
| UR-003 | IR-003, IR-004, IR-011 | DR-002, DR-004, DR-010, DR-182, DR-183, DR-184, DR-185, DR-186, DR-187, DR-188, DR-190, DR-191, DR-192, DR-193, DR-194, DR-195, DR-196, DR-291, DR-298 |
| UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006, DR-129, DR-171, DR-176, DR-177, DR-181, DR-182, DR-183, DR-185, DR-188, DR-203, DR-265, DR-293, DR-298 |
| UR-005 | - | DR-001, DR-005, DR-009, DR-178, DR-179, DR-186, DR-193, DR-195 |
| UR-006 | IR-005, IR-006, IR-007, IR-008 | DR-200, DR-201 |
| UR-007 | IR-010 | DR-007, DR-008, DR-016, DR-257, DR-262, DR-277, DR-278 |
@@ -519,7 +521,7 @@ Internal architecture, components, and application logic.
| UR-009 | IR-009, IR-010, IR-011 | - |
| UR-010 | IR-012, IR-021 | DR-037, DR-059 |
| UR-011 | IR-013 | DR-003, DR-015, DR-018 |
| UR-012 | IR-009, IR-014 | DR-198 |
| UR-012 | IR-009, IR-014 | DR-198, DR-299 |
| UR-013 | IR-013 | DR-017 |
| UR-014 | IR-010 | DR-014, DR-019 |
| UR-015 | - | DR-005, DR-020 |
@@ -809,7 +811,7 @@ Internal architecture, components, and application logic.
| UT-213 | The direct-play negotiation, one test per branch, against `PlaybackInfo` fixtures whose shapes were all observed on a live server: a supported source direct-plays; a remuxable one direct-streams and reports itself as *not* transcoding; an unsupported codec transcodes; undecodable audio overrides the server's direct-play offer (silent picture is worse than a transcode); a pinned audio track forces a transcode; a ceiling below the source bitrate transcodes even though the codec is fine, and the ladder agrees that rung constrains it; direct play wins over direct stream when both are offered. Plus the ceiling: a per-playback override governs the stream being opened without disturbing the durable default the Settings screen shows, and dropping it returns to that default | DR-225, DR-227 | Done |
| UT-214 | The loader comes from the transport, never the URL. hls.js is attached for `hls` when available and the element's own loader when not; progressive and local files load directly; the element's `src` is emptied only when hls.js drives it. The two cases that fail against a substring check, and the reason the field exists: a `progressive` stream whose URL contains `.m3u8` is *not* given an HLS loader, and an `hls` stream whose URL contains no `.m3u8` *is*. Both failed against the pre-DR-225 implementation before the fix landed | DR-224 | Done |
| UT-215 | Waiting for the repository rather than racing it: it resolves immediately when the session is already restored, resolves when the session arrives later (the race the player page lost on mount), still rejects when there genuinely is no session, unsubscribes once settled so a later store change cannot re-settle it, and leaves no armed timer to reject an already-resolved promise | DR-013 | Done |
| UT-216 | The native-video opt-in is read from one place and only explicit truthy values enable it: absent, empty, `0`, `no`, `false` and anything unrecognised all mean off, because a half-set variable that half-enabled the renderer would configure mpv for video with nothing drawing it — audio over a black rectangle | DR-231 | Done |
| UT-216 | The native-video opt-in is read from one place and only explicit truthy values enable it: absent, empty, `0`, `no`, `false` and anything unrecognised all mean off, because a half-set variable that half-enabled the renderer would configure mpv for video with nothing drawing it — audio over a black rectangle | DR-231 | Superseded by UT-271 |
| UT-217 | A transcoded HLS stream on the native backend re-negotiates rather than seeking in place, while the same stream under hls.js still seeks in place — the cell that native video made reachable for the first time | DR-238 | Done |
| UT-218 | Every property name matched by the mpv event loop also appears in an `observe_property` call, asserted against the source because the registration cannot be observed at runtime without a live mpv | DR-239 | Done |
| UT-219 | A fullscreen toggle moves the document only when an in-document `<video>` renders, and moves the OS window as well when a native surface does | DR-240 | Done |
@@ -852,7 +854,7 @@ Internal architecture, components, and application logic.
| UT-255 | The offline banner shows while offline on ordinary routes and never on `/player/*`, and stays off while connected or signed out | DR-291 | Done |
| UT-257 | The server-only rule: true only offline with the reveal on and nothing on the device; never for a library tile; and not for a container whose children are downloaded (the greyed-album regression) | DR-292 | Done |
| UT-258 | The list view greys a server-only row, makes it inert to tap, offers the queue button (and the Queued badge once pending), and leaves downloaded rows and containers with device content alone | DR-292 | Done |
| UT-259 | The user may send video to the webview only beside mpv native video on Linux: never on Android, where ExoPlayer is the only video renderer, and not where the webview is the only renderer | DR-293 | Done |
| UT-259 | The user may send video to the webview only beside mpv native video on Linux: never on Android, where ExoPlayer is the only video renderer, and not where the webview is the only renderer | DR-293 | Superseded by UT-272 |
| UT-260 | A downloaded item gets playback info with the server unreachable — immediately, from its download row (local path, direct play, item id as media source) — while an unfinished download, another user's, or an item never downloaded is left to the server | DR-294 | Done |
| UT-261 | Next Up answers from the cache, rather than failing, when the server is unreachable | DR-294 | Done |
| UT-262 | The Android webview fallback is neither offered in Settings nor honoured by the player unless Rust reports it, so a stored "native video off" cannot route video to a renderer that plays the original file silent | DR-293 | Done |
@@ -861,6 +863,11 @@ Internal architecture, components, and application logic.
| UT-265 | `planHandoffReturn` switches to the item the backend advanced to while backgrounded, and reloads in place when the backend is still on the mounted item or reports none | DR-296 | Done |
| UT-266 | After a background-audio episode advance, the controller's resume point names the new episode and carries no base from the previous one | DR-296 | Done |
| UT-267 | A failed refresh of a detail page already on screen shows no error, a successful load clears any error, a failure to open an item shows its message, and a backend error's plain-string text is shown rather than a generic fallback | DR-297 | Done |
| UT-268 | Against a real libmpv, a URL containing `;set volume 13;#` is loaded as one URL and the volume is unchanged, and an argument containing a space stays one argument | DR-298 | Done |
| UT-269 | Neither mpv player calls the string-joining `Mpv::command`; every command goes through `mpv_command::command` | DR-298 | Done |
| UT-270 | A hardened handle reads back `tls-verify=yes` and `ytdl=no`, and both mpv players harden the handle they create | DR-299 | Done |
| UT-271 | Native video is on for Linux whatever `JELLYTAU_NATIVE_VIDEO` says, including unset and explicit "off" values, and off where mpv is not the video renderer | DR-235 | Done |
| UT-272 | No platform reports a webview video fallback, Linux reports native video, and the player status on Linux never sends video to the `<video>` element | DR-235 | Done |
### Integration Tests
| Test ID | Test Description | Traces To | Status |