From 84cf31b92980a527930cb6ee333c2b2508977757 Mon Sep 17 00:00:00 2001
From: Duncan Tourolle <duncan@tourolle.paris>
Date: Sat, 22 Aug 2026 09:05:06 +0200
Subject: [PATCH] feat(video): build the native video surface, and fix what
 running it exposed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three things, all found by actually running the app rather than by reading it.

The surface (DR-230). A GtkGLArea as the main child of a GtkOverlay with
Tauri's own webview reparented on top — the desktop shape of what Android
already does with ExoPlayer. It attaches cleanly and is then **off by
default**, because the reparent fails the gate the spike said it would.

`tauri-runtime-wry`'s undecorated-resizing handler walks a hard-coded path on
every button press in the webview:

    webview.parent()   // "This one should be GtkBox"
           .parent()   // ...and this one the GtkWindow
           .downcast::<gtk::Window>().unwrap()

Wrapping the webview makes that chain webview -> GtkOverlay -> GtkBox, the
downcast fails, and the panic is non-unwinding so it aborts the process. The
decoration check that would make the handler inert runs *after* the unwrap, so
no window configuration avoids it. The surface attaching successfully is
therefore not the gate — a click is. It lives behind JELLYTAU_NATIVE_VIDEO=1
with the mechanism written down, because the next attempt needs to keep Tauri's
two-hop shape intact and that is the whole design constraint.

Also settles a dependency question the spike left implied: the render API is
reachable from the pinned libmpv revision. Its safe `render` module is an empty
stub, but libmpv-sys carries every render symbol and `Mpv::ctx` is public, so
the context can be built over the handle the audio backend already drives. This
does not need the libmpv2 migration first.

The HLS effect re-ran on object identity. `currentSelection` is a struct, and
every reload replaces it even when the URL and transport are unchanged — so the
effect tore down hls.js and reattached for an unchanged stream, leaving the
element blank until a seek forced another cycle. The pre-DR-224 code read a
plain URL *string*, where re-assigning the same value was a no-op; the codebase
documents relying on that and swapping in a struct broke it silently. The
loader decision now takes a primitive transport tag, so the component cannot
depend on object identity — the bug is unrepresentable rather than merely
fixed.

The device profile contradicted itself. The direct-play profile claimed h264
alone on the webview path while the transcoding profile said "you may transcode
to h264 or hevc" — telling the server "I cannot play hevc, so re-encode it" and
then "re-encoding it to hevc is fine". Streams came back carrying
VideoCodec=h264,hevc with hevc-level/profile/bitdepth set. When the server took
that option the webview got something it could not decode, which presents as
video stuck on its first frame rather than as an error. Transcode targets are
now derived from the same codec list as direct play, capped to the two codecs a
Jellyfin server actually encodes so a wider decode list never asks for an av1
encode.

That is the third defect in one family: a decode capability stated in more than
one place, with the copies disagreeing. DR-233 exists to collapse them into one
renderer-derived source, and this is evidence for it rather than a preference.

Not fixed here, and worth knowing:

- The requested VideoBitrate is sized to the ceiling, not to the source — a
  2.2 Mbps source was being re-encoded at 19.8 Mbps, roughly 9x. Pre-existing,
  but this branch is the first thing that knows the source bitrate and so the
  first that can cap it.
- The `debug` build type produces an APK with the *release* applicationId:
  `applicationIdSuffix = ".debug"` is present in the canonical gradle and absent
  from the generated copy, though the identical line in the `release` block
  survives. Not caused by our sync, which is a plain cp. Independent of this
  work; it is why the side-by-side release build is the one that installs.
---
 src-tauri/Cargo.lock                         |   2 +
 src-tauri/Cargo.toml                         |  19 ++
 src-tauri/src/lib.rs                         |  50 ++++
 src-tauri/src/player/mod.rs                  |   8 +
 src-tauri/src/player/video_surface.rs        | 232 +++++++++++++++++++
 src-tauri/src/repository/online.rs           |  24 +-
 src/lib/components/player/VideoPlayer.svelte |  17 +-
 src/lib/player/streamTransport.ts            |  22 +-
 8 files changed, 370 insertions(+), 4 deletions(-)
 create mode 100644 src-tauri/src/player/video_surface.rs

diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock
index 32751700..608acafd 100644
--- a/src-tauri/Cargo.lock
+++ b/src-tauri/Cargo.lock
@@ -2191,11 +2191,13 @@ dependencies = [
  "env_logger",
  "futures-util",
  "getrandom 0.2.16",
+ "gtk",
  "hostname",
  "jni 0.21.1",
  "keyring",
  "libc",
  "libmpv",
+ "libmpv-sys",
  "log",
  "ndk-context",
  "rand 0.8.7",
diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml
index 553eeca7..7d7d1e5d 100644
--- a/src-tauri/Cargo.toml
+++ b/src-tauri/Cargo.toml
@@ -114,6 +114,25 @@ libc = "0.2"
 # than changing it. To take upstream fixes, bump this deliberately.
 libmpv = { git = "https://github.com/ParadoxSpiral/libmpv-rs.git", rev = "3e6c389b716f52a595cc5e8e3fa1f96cb76b3de7" }
 
+# The raw FFI bindings behind `libmpv`, pinned to the *same* revision so the two
+# can never describe different ABIs.
+#
+# Needed because the safe crate's `render` module is an empty stub at this
+# revision — the render API (`mpv_render_context_create` and friends) exists only
+# in the sys bindings, which do carry all of it. `Mpv::ctx` is public, so the
+# render context can be built over the same handle the safe wrapper drives. This
+# is what makes native video reachable *without* first completing the libmpv2
+# migration, which the spike's use of `libmpv2-sys` had implied was a
+# prerequisite.
+#
+# TRACES: UR-080 | DR-230, IR-033
+libmpv-sys = { git = "https://github.com/ParadoxSpiral/libmpv-rs.git", rev = "3e6c389b716f52a595cc5e8e3fa1f96cb76b3de7" }
+
+# Same major as the one Tauri/wry already resolve, so `gtk_window()` and
+# `default_vbox()` hand back types this crate can name rather than a second,
+# incompatible GTK.
+gtk = "0.18"
+
 # JNI for Android ExoPlayer integration
 [target.'cfg(target_os = "android")'.dependencies]
 jni = "0.21"
diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs
index a51fa8e0..5b8e76ce 100644
--- a/src-tauri/src/lib.rs
+++ b/src-tauri/src/lib.rs
@@ -1210,6 +1210,56 @@ pub fn run() {
             // listened for on the frontend via the generated bindings.
             builder.mount_events(app);
 
+            // Native video surface: put a GL area under Tauri's webview so mpv
+            // can draw beneath the controls (UR-080 / DR-230).
+            //
+            // 🔴 OFF BY DEFAULT — the naive reparent crashes the app on the
+            // first click. `tauri-runtime-wry`'s undecorated-resizing handler
+            // walks a hard-coded two-hop path on every button press in the
+            // webview:
+            //
+            //     webview.parent()          // "This one should be GtkBox"
+            //            .parent()          // ...and this one the GtkWindow
+            //            .downcast::<gtk::Window>().unwrap()
+            //
+            // Wrapping the webview in a GtkOverlay makes that chain
+            // webview → GtkOverlay → GtkBox, the downcast fails, and because the
+            // panic is non-unwinding it aborts the process. The decoration check
+            // that would otherwise make this handler inert runs *after* the
+            // unwrap, so no window configuration avoids it.
+            //
+            // This is the "only place Tauri-specific behaviour could still bite"
+            // that the spike named as the untested half of G1. It bites. The
+            // surface attaches perfectly and then dies on interaction, so
+            // "attached successfully" in the log is not the gate — a click is.
+            //
+            // Kept behind an env var rather than deleted so the next attempt has
+            // something to iterate on:  JELLYTAU_NATIVE_VIDEO=1 bun run tauri dev
+            //
+            // TRACES: UR-080 | DR-230
+            #[cfg(target_os = "linux")]
+            if std::env::var("JELLYTAU_NATIVE_VIDEO").as_deref() == Ok("1") {
+                use tauri::Manager;
+                log::warn!(
+                    "[INIT] JELLYTAU_NATIVE_VIDEO=1 — attaching the experimental \
+                     video surface; the app will abort on the first click until \
+                     the widget-tree shape is solved (DR-230)"
+                );
+                if let Some(window) = app.get_webview_window("main") {
+                    match window.default_vbox() {
+                        Ok(vbox) => match crate::player::video_surface::attach(&vbox) {
+                            Ok(_surface) => {
+                                info!("[INIT] Native video surface attached");
+                            }
+                            Err(e) => log::warn!("[INIT] Native video surface unavailable: {e}"),
+                        },
+                        Err(e) => {
+                            log::warn!("[INIT] No GTK vbox for the main window: {e}")
+                        }
+                    }
+                }
+            }
+
             // In-app update, desktop only.
             //
             // Registered here rather than in the builder chain above because a
diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs
index 1ae2e871..bd11bf57 100644
--- a/src-tauri/src/player/mod.rs
+++ b/src-tauri/src/player/mod.rs
@@ -24,6 +24,14 @@ pub mod android;
 #[cfg(target_os = "linux")]
 pub mod mpv_backend;
 
+/// The native video surface mpv renders into (UR-080 / DR-230).
+///
+/// Linux-gated for now because the surface is GTK. Everything *around* it — the
+/// render context, its lifetime, frame pacing, the device profile — is
+/// deliberately not, so Windows reuses it behind its own surface.
+#[cfg(target_os = "linux")]
+pub mod video_surface;
+
 // Platforms with no native audio backend (e.g. Windows) render audio-only
 // playback through a webview <audio> element, mirroring how all video renders.
 #[cfg(not(any(target_os = "linux", target_os = "android")))]
diff --git a/src-tauri/src/player/video_surface.rs b/src-tauri/src/player/video_surface.rs
new file mode 100644
index 00000000..0abc3ab7
--- /dev/null
+++ b/src-tauri/src/player/video_surface.rs
@@ -0,0 +1,232 @@
+//! The native video surface: a GL area beneath Tauri's own webview.
+//!
+//! This is the desktop counterpart of the Android arrangement — a native
+//! renderer at the bottom of the stack with a transparent webview drawn over it,
+//! so the Svelte controls composite on top of moving video.
+//!
+//! The spike that authorised this built its *own* `GtkOverlay` and proved mpv
+//! renders into it on X11 and Wayland. What it could not prove is the step this
+//! module exists for: taking the overlay Tauri already built and reparenting the
+//! real webview into it. Same widgets, one extra move, and the only place
+//! Tauri-specific behaviour can still bite — which is why it is gate one.
+//!
+//! TRACES: UR-080 | DR-230, IR-033
+
+use gtk::prelude::*;
+use log::{info, warn};
+
+/// The widgets that make up the video surface, kept together because their
+/// lifetimes are bound: the render context (added next) is created when the GL
+/// area realizes and must be freed before it unrealizes — DR-231.
+pub struct VideoSurface {
+    /// The GL area mpv renders into. Main child of the overlay, so it sits
+    /// *under* everything else.
+    #[allow(dead_code)]
+    gl_area: gtk::GLArea,
+    /// The overlay holding the GL area and the webview.
+    #[allow(dead_code)]
+    overlay: gtk::Overlay,
+}
+
+impl VideoSurface {
+    // Consumed by the render context, which binds to the GL area on `realize`
+    // and is freed on `unrealize` (DR-231). Held here from the moment the
+    // surface exists so that binding has something to attach to.
+    #[allow(dead_code)]
+    /// The GL area, for the render context to bind to.
+    pub fn gl_area(&self) -> &gtk::GLArea {
+        &self.gl_area
+    }
+
+    #[allow(dead_code)]
+    /// The overlay, for teardown.
+    pub fn overlay(&self) -> &gtk::Overlay {
+        &self.overlay
+    }
+}
+
+/// Why a surface could not be attached.
+///
+/// One variant, because there is exactly one way this fails that is not already
+/// reported by Tauri itself: the window exists and has a vbox, but the vbox is
+/// not shaped the way Tauri has always shaped it.
+#[derive(Debug)]
+pub enum SurfaceError {
+    /// The vbox held no webview to reparent — Tauri's layout has changed.
+    NoWebviewChild,
+}
+
+impl std::fmt::Display for SurfaceError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            SurfaceError::NoWebviewChild => write!(
+                f,
+                "Tauri's default vbox had no child to reparent — its window layout has changed"
+            ),
+        }
+    }
+}
+
+impl std::error::Error for SurfaceError {}
+
+/// Build the overlay and move Tauri's webview on top of it.
+///
+/// Tauri's Linux window is an `ApplicationWindow` holding a single vertical
+/// `gtk::Box` (`default_vbox`), with the webview packed into it. This takes that
+/// webview out, puts a `GtkGLArea` in its place inside a `GtkOverlay`, and adds
+/// the webview back as the *overlay* child so it draws above.
+///
+/// **Must run on the GTK main thread.** Every GTK call here is main-thread-only,
+/// and the caller reaches it via `run_on_main_thread`.
+///
+/// Ordering matters: the GL area is added as the overlay's main child *before*
+/// the webview goes back, because `GtkOverlay` treats its first `add` as the
+/// bottom of the stack. Adding them the other way round yields a webview with
+/// video painted over it — an easy mistake with an obvious symptom.
+///
+/// TRACES: UR-080 | DR-230
+pub fn attach(vbox: &gtk::Box) -> Result<VideoSurface, SurfaceError> {
+    // Tauri packs exactly one child (the webview) into the default vbox. Take it
+    // rather than assume its type: wry's widget is an implementation detail, and
+    // all this needs is "whatever Tauri put here".
+    let children = vbox.children();
+    let webview = children
+        .into_iter()
+        .next()
+        .ok_or(SurfaceError::NoWebviewChild)?;
+
+    let gl_area = gtk::GLArea::new();
+    // No depth buffer: mpv draws a flat picture into an FBO and nothing here is
+    // 3D. Asking for one costs memory on every resize for nothing.
+    gl_area.set_has_depth_buffer(false);
+    gl_area.set_has_stencil_buffer(false);
+    // Fill the overlay rather than centring at intrinsic size — the same defect
+    // `videoFitClass` had to fix on the webview side, where `max-w-full` only
+    // ever shrank and a 480p source rendered as a small box on a black screen.
+    gl_area.set_hexpand(true);
+    gl_area.set_vexpand(true);
+
+    let overlay = gtk::Overlay::new();
+
+    // Reparent. `remove` drops the container's reference, so hold one across the
+    // move or the widget is destroyed between the two calls.
+    let webview_ref = webview.clone();
+    vbox.remove(&webview);
+
+    overlay.add(&gl_area); // main child — the bottom of the stack
+    overlay.add_overlay(&webview_ref); // drawn above the video
+
+    // The webview must keep receiving input: it *is* the UI. `GtkOverlay` passes
+    // events to overlay children by default, so pass-through stays off — setting
+    // it would send clicks to the GL area, which has no controls on it.
+    overlay.set_overlay_pass_through(&webview_ref, false);
+
+    vbox.pack_start(&overlay, true, true, 0);
+    overlay.show_all();
+
+    info!("[VideoSurface] GL area attached beneath Tauri's webview");
+
+    Ok(VideoSurface { gl_area, overlay })
+}
+
+/// Put Tauri's window back the way it was found.
+///
+/// Not merely tidiness: the webview outlives the video surface, so if the
+/// surface is torn down without returning the webview to the vbox the UI
+/// disappears while the app keeps running. Mirrors [`attach`] exactly.
+///
+/// TRACES: UR-080 | DR-230, DR-231
+// Called by the render-context teardown, which lands with DR-231. Written now,
+// beside `attach`, because a reparent whose inverse is written later is a
+// reparent whose inverse is written wrong.
+#[allow(dead_code)]
+pub fn detach(vbox: &gtk::Box, surface: &VideoSurface) {
+    let children = surface.overlay.children();
+    for child in children {
+        // Everything except the GL area came from the vbox and goes back to it.
+        if child.downcast_ref::<gtk::GLArea>().is_some() {
+            continue;
+        }
+        surface.overlay.remove(&child);
+        vbox.pack_start(&child, true, true, 0);
+    }
+    vbox.remove(&surface.overlay);
+    vbox.show_all();
+    warn!("[VideoSurface] detached; webview returned to Tauri's vbox");
+}
+
+#[cfg(test)]
+mod tests {
+    //! These exercise GTK widget wiring, so they need a display and are ignored
+    //! by default — CI has no X11 or Wayland session. Run locally with
+    //! `cargo test -- --ignored video_surface`.
+
+    use super::*;
+
+    /// The stacking order is the whole point, and getting it backwards produces
+    /// video painted over the controls rather than under them.
+    ///
+    /// TRACES: UR-080 | DR-230
+    #[test]
+    #[ignore = "requires a display"]
+    fn test_gl_area_is_below_the_reparented_webview() {
+        if gtk::init().is_err() {
+            return;
+        }
+        let vbox = gtk::Box::new(gtk::Orientation::Vertical, 0);
+        // Stand in for the webview; `attach` deliberately does not care what it is.
+        let stand_in = gtk::DrawingArea::new();
+        vbox.pack_start(&stand_in, true, true, 0);
+
+        let surface = attach(&vbox).expect("attaches");
+        let children = surface.overlay().children();
+
+        // GtkOverlay lists its main child first.
+        assert!(
+            children[0].downcast_ref::<gtk::GLArea>().is_some(),
+            "the GL area must be the overlay's main child, i.e. underneath"
+        );
+        assert!(
+            children.len() > 1,
+            "the reparented widget must still be present"
+        );
+    }
+
+    /// A surface that tears down without returning the webview leaves a running
+    /// app with no UI.
+    ///
+    /// TRACES: UR-080 | DR-230, DR-231
+    #[test]
+    #[ignore = "requires a display"]
+    fn test_detach_returns_the_webview_to_the_vbox() {
+        if gtk::init().is_err() {
+            return;
+        }
+        let vbox = gtk::Box::new(gtk::Orientation::Vertical, 0);
+        let stand_in = gtk::DrawingArea::new();
+        vbox.pack_start(&stand_in, true, true, 0);
+
+        let surface = attach(&vbox).expect("attaches");
+        detach(&vbox, &surface);
+
+        let children = vbox.children();
+        assert_eq!(children.len(), 1, "exactly the original child comes back");
+        assert!(
+            children[0].downcast_ref::<gtk::DrawingArea>().is_some(),
+            "and it is the webview stand-in, not the overlay"
+        );
+    }
+
+    /// A vbox Tauri has not populated is a changed assumption, not a panic.
+    ///
+    /// TRACES: UR-080 | DR-230
+    #[test]
+    #[ignore = "requires a display"]
+    fn test_an_empty_vbox_is_an_error_not_a_panic() {
+        if gtk::init().is_err() {
+            return;
+        }
+        let vbox = gtk::Box::new(gtk::Orientation::Vertical, 0);
+        assert!(matches!(attach(&vbox), Err(SurfaceError::NoWebviewChild)));
+    }
+}
diff --git a/src-tauri/src/repository/online.rs b/src-tauri/src/repository/online.rs
index 0c4af516..1139b0e9 100644
--- a/src-tauri/src/repository/online.rs
+++ b/src-tauri/src/repository/online.rs
@@ -862,7 +862,29 @@ impl OnlineRepository {
                     context: "Streaming".to_string(),
                     protocol: "hls".to_string(),
                     container: "ts".to_string(),
-                    video_codec: Some("h264,hevc".to_string()),
+                    // The server may only transcode *to* something this renderer
+                    // can decode. This said "h264,hevc" unconditionally while the
+                    // direct-play profile above claims h264 alone on the webview
+                    // path — a straight contradiction: it tells the server "I
+                    // cannot play hevc, so re-encode it" and then "re-encoding it
+                    // to hevc is fine". When the server took that option the
+                    // webview got a stream it could not decode, which presents as
+                    // video stuck on its first frame rather than as an error.
+                    //
+                    // Derived from the same codec list as direct play, so the two
+                    // halves of the profile cannot disagree again. Capped at the
+                    // two codecs a Jellyfin server actually encodes, so widening
+                    // the decode list never asks it for an av1 encode.
+                    //
+                    // TRACES: UR-004, UR-080 | DR-233
+                    video_codec: Some(
+                        if video_codecs.contains("hevc") {
+                            "h264,hevc"
+                        } else {
+                            "h264"
+                        }
+                        .to_string(),
+                    ),
                     audio_codec: "aac,mp3".to_string(),
                     max_audio_channels: max_audio_channels.clone(),
                 },
diff --git a/src/lib/components/player/VideoPlayer.svelte b/src/lib/components/player/VideoPlayer.svelte
index 20ed1adf..406a2d2c 100644
--- a/src/lib/components/player/VideoPlayer.svelte
+++ b/src/lib/components/player/VideoPlayer.svelte
@@ -82,7 +82,7 @@
     type BackgroundAudioState,
   } from "./backgroundAudioHandoff";
   import { createLogger } from "$lib/utils/logger";
-  import { elementSrcFor, videoLoaderFor } from "$lib/player/streamTransport";
+  import { elementSrcFor, loaderForTransport } from "$lib/player/streamTransport";
 
   const log = createLogger("VideoPlayer");
 
@@ -199,6 +199,12 @@
   // TRACES: UR-079 | DR-224
   let currentSelection = $state<StreamSelection>(untrack(() => selection));
   const currentStreamUrl = $derived(currentSelection.url);
+  /**
+   * The transport as a plain string, so effects can depend on its *value*.
+   * A `$derived` primitive only notifies when it actually changes, which is what
+   * keeps the HLS teardown from re-running for an unchanged stream.
+   */
+  const transportKind = $derived(currentSelection.transport.type);
   let hasReportedStart = $state(false);
   let progressInterval: ReturnType<typeof setInterval> | null = null;
   let isMediaReady = $state(false); // Track if media is ready to play (implements Loading state from DR-001)
@@ -641,8 +647,15 @@
     }
 
     // The loader comes from the backend's tagged transport, never from the URL.
+    //
+    // Read through the *primitive* `transportKind`, never `currentSelection`
+    // itself: this effect tears down and rebuilds hls.js, and a selection object
+    // is replaced on every reload — so depending on the object re-ran the whole
+    // teardown for an unchanged stream and left the element showing nothing
+    // until a seek forced another cycle.
+    //
     // TRACES: UR-079 | DR-224 | UT-213
-    const loader = videoLoaderFor(currentSelection, {
+    const loader = loaderForTransport(transportKind, {
       hlsJsSupported: Hls.isSupported(),
       nativeHlsSupported: !!videoElement.canPlayType("application/vnd.apple.mpegurl"),
     });
diff --git a/src/lib/player/streamTransport.ts b/src/lib/player/streamTransport.ts
index cddbe5ac..205ef668 100644
--- a/src/lib/player/streamTransport.ts
+++ b/src/lib/player/streamTransport.ts
@@ -44,7 +44,27 @@ export function videoLoaderFor(
   selection: Pick<StreamSelection, "url" | "transport">,
   capabilities: LoaderCapabilities,
 ): VideoLoader {
-  if (selection.transport.type !== "hls") {
+  return loaderForTransport(selection.transport.type, capabilities);
+}
+
+/**
+ * The same decision, taken from the transport *tag* alone.
+ *
+ * Exists because a Svelte `$effect` that reads the whole selection re-runs
+ * whenever the selection **object** is replaced — even with an identical URL and
+ * transport — and the HLS effect's teardown/rebuild is not idempotent: it
+ * destroys the hls.js instance and reattaches, which leaves the element with no
+ * video until something forces another cycle. The pre-DR-224 code read a plain
+ * URL *string*, so re-assigning the same value was a no-op and the effect stayed
+ * put. Passing primitives restores that.
+ *
+ * TRACES: UR-079 | DR-224 | UT-213
+ */
+export function loaderForTransport(
+  transport: Transport["type"],
+  capabilities: LoaderCapabilities,
+): VideoLoader {
+  if (transport !== "hls") {
     // Progressive and local files are what the element loads natively. No
     // MediaSource, no playlist parsing.
     return "direct";