feat(video): render mpv behind the webview, and collapse duplicated helpers

DR-231 with the design the failed reparent forced. mpv's render API draws into an FBO we own; the texture is composited by `gdk_cairo_draw_from_gl()` in the default vbox's own `draw` handler. GTK draws a container before its children, so the webview lands on top for free — no reparenting, no GtkOverlay, and nothing a Tauri upgrade can invalidate by assuming its own widget layout. Split so Windows inherits the useful half: `mpv_render` is the portable side (render context, framebuffer, GL resolution) and `video_surface` is the GTK side that consumes it. Nothing in the former is GTK-aware. Three things the spike paid for, carried over rather than rediscovered: - libepoxy exports GL entry points as *data* symbols. `dlsym("epoxy_glFoo")` returns the address *of a function pointer*, not of code — returning it makes mpv jump into non-executable data and take SIGSEGV on the first GL call. The value is read out of that location instead. - Frame pacing goes through mpv's update callback plus `report_swap`. Its absence looks like a GPU or compositing limit (fine in a window, judders at fullscreen) and is neither. - The render context is created on `realize` and destroyed on `unrealize`, with the update callback unregistered *before* the free, so a callback cannot land on a freed pointer. That is DR-232 built in from the start rather than retrofitted: the spike had no teardown at all, which remains the likeliest explanation for the one SIGSEGV it could not reproduce. Writing it also caught a bug that would have looked like severe stutter: the update callback flagged a new frame but never asked GTK to repaint, so decoded frames would only have reached the screen when something else happened to invalidate the widget. Still off by default behind JELLYTAU_NATIVE_VIDEO=1. It compiles and is wired; no frame has been put on screen yet. Redundant code, continued. `formatSecondsDuration` had no caller. Three components had hand-rolled `formatDuration`: Queue's was byte-equivalent to the shared "mm:ss", while EpisodeFocusView and the library page shared an identical "1h 23m" shape the util did not offer — so that format joins the other two and all three components now call one function. A survey for exported symbols referenced only by tests returns 23 more. They are deliberately left: spot-checking found `setLogForwarder` is the injection seam for a lazily-initialised forwarder, and `getCachedImageUrl` is the read path of a thumbnail cache whose management UI exists in Settings. Neither is dead — one is test infrastructure and the other is an unwired feature, and deleting either would remove capability while looking like tidying. The list is worth working through deliberately, not in a playback branch.
2026-08-22 13:45:04 +02:00
parent 7545de6cc7
commit 45144cb6b0
11 changed files with 1471 additions and 1010 deletions
@@ -1242,17 +1242,18 @@ pub fn run() {
                use tauri::Manager;
                log::warn!(
                    "[INIT] JELLYTAU_NATIVE_VIDEO=1 — attaching the experimental \
-                     video surface; the app will abort on the first click until \
+                     video surface (mpv drawn behind the webview, no reparenting)"
                     the widget-tree shape is solved (DR-231)"
                );
                if let Some(window) = app.get_webview_window("main") {
                    match window.default_vbox() {
-                        Ok(vbox) => match crate::player::video_surface::attach(&vbox) {
+                        Ok(vbox) => {
-                            Ok(_surface) => {
+                            let handle = crate::player::mpv_backend::registered_handle();
                            if crate::player::video_surface::attach(&vbox, handle) {
                                info!("[INIT] Native video surface attached");
                            } else {
                                log::warn!("[INIT] Native video surface unavailable");
                            }
                        }
                            Err(e) => log::warn!("[INIT] Native video surface unavailable: {e}"),
                        },
                        Err(e) => {
                            log::warn!("[INIT] No GTK vbox for the main window: {e}")
                        }
@@ -24,11 +24,18 @@ pub mod android;
 #[cfg(target_os = "linux")]
 pub mod mpv_backend;
 /// mpv's render API into a framebuffer we own (UR-080 / DR-231, IR-033).
 ///
 /// Deliberately *not* GTK-gated beyond the platform that currently builds it:
 /// everything here is the portable half, and Windows reuses it unchanged behind
 /// its own surface.
 #[cfg(target_os = "linux")]
 pub mod mpv_render;
 /// The native video surface mpv renders into (UR-080 / DR-231).
 ///
-/// Linux-gated for now because the surface is GTK. Everything *around* it — the
+/// Linux-gated because the *surface* is GTK. Everything around it — the render
-/// render context, its lifetime, frame pacing, the device profile — is
+/// context, its lifetime, frame pacing, the device profile — is not.
 /// deliberately not, so Windows reuses it behind its own surface.
 #[cfg(target_os = "linux")]
 pub mod video_surface;
@@ -89,6 +89,32 @@ fn get_stream_url(media: &MediaItem) -> String {
    }
 }
 /// The mpv handle of the backend this process created, for the video surface.
 ///
 /// A `OnceLock` rather than a field reached through `PlayerBackend`, because the
 /// trait is cross-platform and a raw mpv pointer is not something every backend
 /// should have to pretend to have. Stored as `usize` because a raw pointer is
 /// neither `Send` nor `Sync`; the only consumer is the GTK main thread, which is
 /// also where mpv was created.
 ///
 /// Written once at construction and never cleared: the backend outlives the
 /// window, so there is no window in which this could dangle while a surface is
 /// still using it.
 ///
 /// TRACES: UR-080 | DR-231
 static MPV_HANDLE: std::sync::OnceLock<usize> = std::sync::OnceLock::new();
 /// The registered handle, or null if no MPV backend was created (initialisation
 /// can fail, and the app falls back to a no-op backend rather than dying).
 ///
 /// TRACES: UR-080 | DR-231
 pub fn registered_handle() -> *mut libmpv_sys::mpv_handle {
    MPV_HANDLE
        .get()
        .map(|p| *p as *mut libmpv_sys::mpv_handle)
        .unwrap_or(std::ptr::null_mut())
 }
 impl MpvBackend {
    /// Create a new MPV backend
    pub fn new(
@@ -178,7 +204,14 @@ impl MpvBackend {
        }));
        let backend = MpvBackend {
-            mpv: Arc::new(mpv),
+            mpv: {
                let mpv = Arc::new(mpv);
                // Publish the handle for the video surface (DR-231). Ignores a
                // second call: only one MPV backend is ever constructed, and a
                // failed re-init must not replace a live handle.
                let _ = MPV_HANDLE.set(mpv.ctx.as_ptr() as usize);
                mpv
            },
            state,
            event_emitter,
            audio_settings: AudioSettings::default(),
@@ -0,0 +1,374 @@
 //! mpv's render API, driven into an OpenGL framebuffer we own.
 //!
 //! This is the half of native video that is not GTK: create a render context
 //! over the mpv handle the audio backend already drives, render a frame into a
 //! texture, and hand that texture id back for the toolkit to composite.
 //!
 //! Kept apart from `video_surface` deliberately — everything here is portable
 //! across the platforms this app targets, while the surface that consumes it is
 //! not. Windows reuses this file unchanged (DR-237).
 //!
 //! TRACES: UR-080 | DR-231, DR-232, IR-033
 use std::ffi::{c_void, CStr, CString};
 use std::os::raw::{c_char, c_int};
 use std::ptr;
 use log::{error, info, warn};
 /// GL entry points, resolved once.
 ///
 /// Only the handful needed to own a framebuffer; mpv resolves everything else
 /// it needs through [`get_proc_address`].
 struct Gl {
    gen_framebuffers: unsafe extern "C" fn(c_int, *mut u32),
    delete_framebuffers: unsafe extern "C" fn(c_int, *const u32),
    bind_framebuffer: unsafe extern "C" fn(u32, u32),
    framebuffer_texture_2d: unsafe extern "C" fn(u32, u32, u32, u32, c_int),
    gen_textures: unsafe extern "C" fn(c_int, *mut u32),
    delete_textures: unsafe extern "C" fn(c_int, *const u32),
    bind_texture: unsafe extern "C" fn(u32, u32),
    tex_image_2d:
        unsafe extern "C" fn(u32, c_int, c_int, c_int, c_int, c_int, u32, u32, *const c_void),
    tex_parameteri: unsafe extern "C" fn(u32, u32, c_int),
    check_framebuffer_status: unsafe extern "C" fn(u32) -> u32,
 }
 const GL_TEXTURE_2D: u32 = 0x0DE1;
 const GL_FRAMEBUFFER: u32 = 0x8D40;
 const GL_COLOR_ATTACHMENT0: u32 = 0x8CE0;
 const GL_RGBA: u32 = 0x1908;
 const GL_RGBA8: c_int = 0x8058;
 const GL_UNSIGNED_BYTE: u32 = 0x1401;
 const GL_LINEAR: c_int = 0x2601;
 const GL_TEXTURE_MIN_FILTER: u32 = 0x2801;
 const GL_TEXTURE_MAG_FILTER: u32 = 0x2800;
 const GL_FRAMEBUFFER_COMPLETE: u32 = 0x8CD5;
 /// Resolve a GL symbol the way libepoxy actually exports it.
 ///
 /// **This is the trap that cost the spike a debugging cycle.** libepoxy does not
 /// export `glFoo` as a function. It exports `epoxy_glFoo` as a *data* symbol
 /// holding a lazily-resolving function pointer. So the address `dlsym` returns
 /// is the address *of the pointer*, not of any code: returning it makes mpv jump
 /// into non-executable data and take SIGSEGV/SEGV_ACCERR on the very first GL
 /// call. The value must be read *out of* that location.
 ///
 /// The `epoxy` crate does this correctly and is unusable here — its
 /// `gl_generator` dependency pulls a yanked `xml-rs`.
 ///
 /// TRACES: UR-080 | IR-033
 unsafe fn resolve(name: &str) -> *mut c_void {
    let epoxy_name = match CString::new(format!("epoxy_{name}")) {
        Ok(n) => n,
        Err(_) => return ptr::null_mut(),
    };
    let slot = libc::dlsym(libc::RTLD_DEFAULT, epoxy_name.as_ptr());
    if !slot.is_null() {
        // The symbol holds the function pointer; return what is stored there.
        return *(slot as *mut *mut c_void);
    }
    // Fall back to a plain symbol, for a GL stack that is not behind epoxy.
    match CString::new(name) {
        Ok(n) => libc::dlsym(libc::RTLD_DEFAULT, n.as_ptr()),
        Err(_) => ptr::null_mut(),
    }
 }
 /// What mpv calls to find GL entry points. Same rule as [`resolve`].
 unsafe extern "C" fn get_proc_address(_ctx: *mut c_void, name: *const c_char) -> *mut c_void {
    if name.is_null() {
        return ptr::null_mut();
    }
    match CStr::from_ptr(name).to_str() {
        Ok(n) => resolve(n),
        Err(_) => ptr::null_mut(),
    }
 }
 macro_rules! load {
    ($name:literal) => {{
        let p = resolve($name);
        if p.is_null() {
            error!("[MpvRender] GL symbol not found: {}", $name);
            return None;
        }
        std::mem::transmute(p)
    }};
 }
 impl Gl {
    /// Resolve every entry point, or none — a partially-loaded table would fail
    /// later at a call site with no context.
    ///
    /// The transmutes are unannotated on purpose: each target type is declared
    /// once on the struct field above, and repeating it at the call site would
    /// be two places to get the same signature wrong.
    #[allow(clippy::missing_transmute_annotations)]
    unsafe fn load() -> Option<Self> {
        Some(Gl {
            gen_framebuffers: load!("glGenFramebuffers"),
            delete_framebuffers: load!("glDeleteFramebuffers"),
            bind_framebuffer: load!("glBindFramebuffer"),
            framebuffer_texture_2d: load!("glFramebufferTexture2D"),
            gen_textures: load!("glGenTextures"),
            delete_textures: load!("glDeleteTextures"),
            bind_texture: load!("glBindTexture"),
            tex_image_2d: load!("glTexImage2D"),
            tex_parameteri: load!("glTexParameteri"),
            check_framebuffer_status: load!("glCheckFramebufferStatus"),
        })
    }
 }
 /// A colour-renderable framebuffer mpv draws into, sized to the widget.
 struct Target {
    fbo: u32,
    texture: u32,
    width: i32,
    height: i32,
 }
 /// mpv's render context plus the framebuffer it draws into.
 ///
 /// # Lifetime (DR-232)
 ///
 /// The render context must not outlive the GL context it was created against.
 /// `Drop` unregisters mpv's update callback *before* freeing the context, so a
 /// callback cannot land on a freed pointer, and frees the GL objects while the
 /// caller still has the context current. The caller is responsible for making
 /// the GL context current around both creation and drop — see `video_surface`.
 ///
 /// This is DR-184 on Android restated: a surface outliving its player. The spike
 /// had no defence at all and saw one unexplained SIGSEGV in a decoder thread.
 pub struct MpvRenderContext {
    ctx: *mut libmpv_sys::mpv_render_context,
    gl: Gl,
    target: Option<Target>,
 }
 // The render context is driven only from the GTK main thread; the update
 // callback merely schedules a redraw and touches nothing here.
 unsafe impl Send for MpvRenderContext {}
 impl MpvRenderContext {
    /// Create a render context over an existing mpv handle.
    ///
    /// The GL context must already be current on this thread.
    ///
    /// TRACES: UR-080 | DR-231, IR-033
    pub unsafe fn new(mpv: *mut libmpv_sys::mpv_handle) -> Option<Self> {
        let gl = Gl::load()?;
        let mut init = libmpv_sys::mpv_opengl_init_params {
            get_proc_address: Some(get_proc_address),
            get_proc_address_ctx: ptr::null_mut(),
        };
        let mut api_type = CString::new("opengl").ok()?;
        // `advanced_control` lets mpv tell us when a frame is ready instead of
        // us guessing; it is what makes the update callback meaningful (DR-233).
        let mut advanced: c_int = 1;
        let mut params = [
            libmpv_sys::mpv_render_param {
                type_: libmpv_sys::mpv_render_param_type_MPV_RENDER_PARAM_API_TYPE,
                data: api_type.as_ptr() as *mut c_void,
            },
            libmpv_sys::mpv_render_param {
                type_: libmpv_sys::mpv_render_param_type_MPV_RENDER_PARAM_OPENGL_INIT_PARAMS,
                data: &mut init as *mut _ as *mut c_void,
            },
            libmpv_sys::mpv_render_param {
                type_: libmpv_sys::mpv_render_param_type_MPV_RENDER_PARAM_ADVANCED_CONTROL,
                data: &mut advanced as *mut _ as *mut c_void,
            },
            libmpv_sys::mpv_render_param {
                type_: 0,
                data: ptr::null_mut(),
            },
        ];
        let mut ctx: *mut libmpv_sys::mpv_render_context = ptr::null_mut();
        let rc = libmpv_sys::mpv_render_context_create(&mut ctx, mpv, params.as_mut_ptr());
        // Keep the CString alive until after the call.
        let _ = &mut api_type;
        if rc < 0 || ctx.is_null() {
            error!("[MpvRender] mpv_render_context_create failed: {rc}");
            return None;
        }
        info!("[MpvRender] render context created");
        Some(MpvRenderContext {
            ctx,
            gl,
            target: None,
        })
    }
    /// Ask to be told when a new frame is ready.
    ///
    /// Paired with [`report_swap`](Self::report_swap): without both, mpv has
    /// nothing to time against. The symptom is misleading — playback looks fine
    /// in a window and judders at fullscreen, which reads as a compositing or
    /// GPU limit and is neither (DR-233).
    ///
    /// TRACES: UR-080 | DR-233
    pub unsafe fn set_update_callback(
        &mut self,
        callback: libmpv_sys::mpv_render_update_fn,
        ctx: *mut c_void,
    ) {
        libmpv_sys::mpv_render_context_set_update_callback(self.ctx, callback, ctx);
    }
    /// Render the current frame at `width` x `height`, returning the texture id
    /// holding it. The GL context must be current.
    ///
    /// TRACES: UR-080 | DR-231
    pub unsafe fn render(&mut self, width: i32, height: i32) -> Option<u32> {
        if width <= 0 || height <= 0 {
            return None;
        }
        self.ensure_target(width, height)?;
        let target = self.target.as_ref()?;
        let mut fbo = libmpv_sys::mpv_opengl_fbo {
            fbo: target.fbo as c_int,
            w: width as c_int,
            h: height as c_int,
            internal_format: 0,
        };
        // GTK's cairo surface has its origin at the top left; mpv defaults to
        // OpenGL's bottom-left. Without this the picture is drawn upside down —
        // which looks like a broken decode rather than a coordinate convention.
        let mut flip: c_int = 1;
        let mut params = [
            libmpv_sys::mpv_render_param {
                type_: libmpv_sys::mpv_render_param_type_MPV_RENDER_PARAM_OPENGL_FBO,
                data: &mut fbo as *mut _ as *mut c_void,
            },
            libmpv_sys::mpv_render_param {
                type_: libmpv_sys::mpv_render_param_type_MPV_RENDER_PARAM_FLIP_Y,
                data: &mut flip as *mut _ as *mut c_void,
            },
            libmpv_sys::mpv_render_param {
                type_: 0,
                data: ptr::null_mut(),
            },
        ];
        let rc = libmpv_sys::mpv_render_context_render(self.ctx, params.as_mut_ptr());
        if rc < 0 {
            warn!("[MpvRender] render failed: {rc}");
            return None;
        }
        Some(target.texture)
    }
    /// Tell mpv the frame reached the screen. See [`set_update_callback`].
    ///
    /// TRACES: UR-080 | DR-233
    pub unsafe fn report_swap(&self) {
        libmpv_sys::mpv_render_context_report_swap(self.ctx);
    }
    /// Create or resize the framebuffer. Reused across frames — reallocating per
    /// frame would churn GPU memory at the display rate.
    unsafe fn ensure_target(&mut self, width: i32, height: i32) -> Option<()> {
        if let Some(t) = &self.target {
            if t.width == width && t.height == height {
                return Some(());
            }
        }
        self.drop_target();
        let gl = &self.gl;
        let mut texture: u32 = 0;
        (gl.gen_textures)(1, &mut texture);
        (gl.bind_texture)(GL_TEXTURE_2D, texture);
        (gl.tex_image_2d)(
            GL_TEXTURE_2D,
            0,
            GL_RGBA8,
            width,
            height,
            0,
            GL_RGBA,
            GL_UNSIGNED_BYTE,
            ptr::null(),
        );
        (gl.tex_parameteri)(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
        (gl.tex_parameteri)(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
        (gl.bind_texture)(GL_TEXTURE_2D, 0);
        let mut fbo: u32 = 0;
        (gl.gen_framebuffers)(1, &mut fbo);
        (gl.bind_framebuffer)(GL_FRAMEBUFFER, fbo);
        (gl.framebuffer_texture_2d)(
            GL_FRAMEBUFFER,
            GL_COLOR_ATTACHMENT0,
            GL_TEXTURE_2D,
            texture,
            0,
        );
        let status = (gl.check_framebuffer_status)(GL_FRAMEBUFFER);
        (gl.bind_framebuffer)(GL_FRAMEBUFFER, 0);
        if status != GL_FRAMEBUFFER_COMPLETE {
            error!("[MpvRender] framebuffer incomplete: 0x{status:x}");
            (gl.delete_framebuffers)(1, &fbo);
            (gl.delete_textures)(1, &texture);
            return None;
        }
        self.target = Some(Target {
            fbo,
            texture,
            width,
            height,
        });
        Some(())
    }
    unsafe fn drop_target(&mut self) {
        if let Some(t) = self.target.take() {
            (self.gl.delete_framebuffers)(1, &t.fbo);
            (self.gl.delete_textures)(1, &t.texture);
        }
    }
    /// Free everything, with the GL context current.
    ///
    /// Explicit rather than left to `Drop` because the ordering matters and the
    /// caller is the only one that can guarantee the GL context is current. See
    /// DR-232.
    pub unsafe fn destroy(mut self) {
        // Unregister first: a callback arriving after the free would be a use
        // after free, and it is scheduled from mpv's own threads.
        libmpv_sys::mpv_render_context_set_update_callback(self.ctx, None, ptr::null_mut());
        self.drop_target();
        libmpv_sys::mpv_render_context_free(self.ctx);
        self.ctx = ptr::null_mut();
        info!("[MpvRender] render context freed");
        std::mem::forget(self);
    }
 }
 impl Drop for MpvRenderContext {
    fn drop(&mut self) {
        if !self.ctx.is_null() {
            // Reached only if `destroy` was not called — the GL context may not
            // be current, so the GL objects are deliberately leaked rather than
            // deleted against whatever context happens to be bound. Freeing the
            // render context is still safe and is the part that matters.
            warn!("[MpvRender] dropped without destroy(); GL objects leaked deliberately");
            unsafe {
                libmpv_sys::mpv_render_context_set_update_callback(self.ctx, None, ptr::null_mut());
                libmpv_sys::mpv_render_context_free(self.ctx);
            }
        }
    }
 }
@@ -1,232 +1,283 @@
-//! The native video surface: a GL area beneath Tauri's own webview.
+//! The native video surface: mpv drawn *behind* Tauri's webview, without
 //! touching the widget tree.
 //!
-//! This is the desktop counterpart of the Android arrangement — a native
+//! # Why there is no overlay here
 //! 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
+//! The obvious arrangement — wrap the webview in a `GtkOverlay` with a
-//! renders into it on X11 and Wayland. What it could not prove is the step this
+//! `GtkGLArea` beneath — attaches cleanly and then aborts the process on the
-//! module exists for: taking the overlay Tauri already built and reparenting the
+//! first click. `tauri-runtime-wry` connects a button-press handler to the
-//! real webview into it. Same widgets, one extra move, and the only place
+//! webview that walks a hard-coded path:
 //! Tauri-specific behaviour can still bite — which is why it is gate one.
 //!
-//! TRACES: UR-080 | DR-231, IR-033
+//! ```text
 //! webview.parent()          // "This one should be GtkBox"
 //!        .parent()          // ...and this one the GtkWindow
 //!        .downcast::<gtk::Window>().unwrap()
 //! ```
 //!
 //! An overlay makes that chain `webview → GtkOverlay → GtkBox`, the downcast
 //! fails, and because the panic is non-unwinding it takes the app with it.
 //! Nothing in configuration avoids it: on Linux the handler is attached
 //! *unconditionally* (the Windows path guards it behind `is_decorated()`), and
 //! the decoration check that would make it inert runs *after* the unwrap.
 //!
 //! So the widget tree is left exactly as Tauri built it. GTK draws a container
 //! before its children, so rendering into the vbox's own `draw` handler puts the
 //! picture underneath the webview for free — the same z-order, no reparenting,
 //! one less widget, and nothing a Tauri upgrade can invalidate by assuming its
 //! own layout.
 //!
 //! TRACES: UR-080 | DR-231, DR-232, DR-233, IR-033
 use std::cell::RefCell;
 use std::ffi::c_void;
 use std::rc::Rc;
 use gtk::prelude::*;
-use log::{info, warn};
+use gtk::{gdk, glib};
 use log::{error, info, warn};
-/// The widgets that make up the video surface, kept together because their
+use super::mpv_render::MpvRenderContext;
-/// lifetimes are bound: the render context (added next) is created when the GL
+
-/// area realizes and must be freed before it unrealizes — DR-232.
+/// GL enum for `gdk_cairo_draw_from_gl`'s `source_type`. GDK takes the GL
 /// constant itself rather than an enum of its own.
 const GL_TEXTURE: i32 = 0x1702;
 /// Everything the draw handler needs, shared with the GTK callbacks.
 struct SurfaceState {
    gl: Option<gdk::GLContext>,
    render: Option<MpvRenderContext>,
    mpv: *mut libmpv_sys::mpv_handle,
    /// The widget to repaint when mpv reports a frame. Held here because the
    /// update callback arrives on an mpv thread with nothing but this state.
    widget: gtk::Box,
 }
 impl SurfaceState {
    /// Tear down in the order DR-232 requires, with the GL context current.
    ///
    /// The update callback is unregistered before the context is freed (inside
    /// `destroy`), and the GL objects go while their context is still bound.
    /// Getting this wrong is DR-184 on Android restated — a surface outliving
    /// its player — and is the likeliest cause of the one unexplained SIGSEGV
    /// the spike recorded.
    fn teardown(&mut self) {
        if let Some(render) = self.render.take() {
            if let Some(gl) = &self.gl {
                gl.make_current();
            }
            unsafe { render.destroy() };
        }
        self.gl = None;
    }
 }
 /// A live video surface. Dropping it tears the render context down.
 pub struct VideoSurface {
-    /// The GL area mpv renders into. Main child of the overlay, so it sits
+    state: Rc<RefCell<SurfaceState>>,
-    /// *under* everything else.
+    widget: gtk::Box,
-    #[allow(dead_code)]
+    handlers: Vec<glib::SignalHandlerId>,
    gl_area: gtk::GLArea,
    /// The overlay holding the GL area and the webview.
    #[allow(dead_code)]
    overlay: gtk::Overlay,
 }
-impl VideoSurface {
+impl Drop for VideoSurface {
-    // Consumed by the render context, which binds to the GL area on `realize`
+    fn drop(&mut self) {
-    // and is freed on `unrealize` (DR-232). Held here from the moment the
+        for id in self.handlers.drain(..) {
-    // surface exists so that binding has something to attach to.
+            self.widget.disconnect(id);
    #[allow(dead_code)]
    /// The GL area, for the render context to bind to.
    pub fn gl_area(&self) -> &gtk::GLArea {
        &self.gl_area
        }
-
+        self.state.borrow_mut().teardown();
-    #[allow(dead_code)]
+        self.widget.queue_draw();
-    /// The overlay, for teardown.
+        info!("[VideoSurface] detached");
    pub fn overlay(&self) -> &gtk::Overlay {
        &self.overlay
    }
 }
-/// Why a surface could not be attached.
+/// mpv's update callback. Runs on an mpv thread, so it does the least possible:
 /// flags the state and asks GTK to redraw on the main loop.
 ///
-/// One variant, because there is exactly one way this fails that is not already
+/// **Nothing here may block or re-enter the player.** The project's deadlock
-/// reported by Tauri itself: the window exists and has a vbox, but the vbox is
+/// gotcha applies with full force — this is called from mpv's own threads.
 /// 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
+/// TRACES: UR-080 | DR-233
-/// `gtk::Box` (`default_vbox`), with the webview packed into it. This takes that
+unsafe extern "C" fn on_mpv_update(ctx: *mut c_void) {
-/// webview out, puts a `GtkGLArea` in its place inside a `GtkOverlay`, and adds
+    if ctx.is_null() {
 /// 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-231
 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-231, DR-232
 // Called by the render-context teardown, which lands with DR-232. 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-231
    #[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);
+    let state = &*(ctx as *const Rc<RefCell<SurfaceState>>);
-        // Stand in for the webview; `attach` deliberately does not care what it is.
+    let state = state.clone();
-        let stand_in = gtk::DrawingArea::new();
+    // Hop to the main loop: GTK is not thread-safe and mpv is not on its thread.
-        vbox.pack_start(&stand_in, true, true, 0);
+    //
-
+    // Asking for a repaint is the entire job. Without it a decoded frame simply
-        let surface = attach(&vbox).expect("attaches");
+    // waits — the picture would then only advance when something else happened
-        let children = surface.overlay().children();
+    // to invalidate the widget, which looks like severe stutter rather than like
-
+    // a missing call.
-        // GtkOverlay lists its main child first.
+    glib::idle_add_local_once(move || {
-        assert!(
+        let Ok(s) = state.try_borrow() else { return };
-            children[0].downcast_ref::<gtk::GLArea>().is_some(),
+        s.widget.queue_draw();
-            "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
+/// Start drawing mpv's video underneath the webview.
    /// app with no UI.
 ///
-    /// TRACES: UR-080 | DR-231, DR-232
+/// `vbox` is Tauri's `default_vbox()` — the container the webview already lives
-    #[test]
+/// in. It is not modified; only a `draw` handler is added.
-    #[ignore = "requires a display"]
+///
-    fn test_detach_returns_the_webview_to_the_vbox() {
+/// Must run on the GTK main thread.
-        if gtk::init().is_err() {
+///
 /// TRACES: UR-080 | DR-231, DR-232, DR-233
 pub fn attach(vbox: &gtk::Box, mpv: *mut libmpv_sys::mpv_handle) -> bool {
    if mpv.is_null() {
        warn!("[VideoSurface] no mpv handle; native video unavailable");
        return false;
    }
    let state = Rc::new(RefCell::new(SurfaceState {
        gl: None,
        render: None,
        mpv,
        widget: vbox.clone(),
    }));
    let mut handlers = Vec::new();
    // The GL context can only be created once the widget has a GdkWindow, which
    // is what `realize` announces. Creating it earlier leaves nothing to attach
    // to — the same ordering constraint the render context has.
    let realize_state = state.clone();
    handlers.push(vbox.connect_realize(move |widget| {
        if let Err(e) = init_gl(widget, &realize_state) {
            error!("[VideoSurface] GL init failed: {e}");
        }
    }));
    // A render context outliving its GL context is the defect DR-232 exists to
    // prevent, so teardown is bound to `unrealize` rather than left to Drop.
    let unrealize_state = state.clone();
    handlers.push(vbox.connect_unrealize(move |_| {
        unrealize_state.borrow_mut().teardown();
    }));
    let draw_state = state.clone();
    handlers.push(vbox.connect_draw(move |widget, cr| {
        draw(widget, cr, &draw_state);
        // Propagate: the webview is a child and must still draw over us.
        glib::Propagation::Proceed
    }));
    // The window is already up by the time we are called, so run the init the
    // `realize` signal would have.
    if vbox.is_realized() {
        if let Err(e) = init_gl(vbox, &state) {
            error!("[VideoSurface] GL init failed: {e}");
        }
    }
    info!("[VideoSurface] attached to Tauri's vbox without reparenting");
    // The surface lives as long as the window. Held in a thread-local rather
    // than returned, because it owns `Rc` and GTK types and so is neither `Send`
    // nor `Sync` — it cannot go into Tauri's managed state, and leaking it would
    // give up the ability to tear it down at all.
    //
    // Teardown does not depend on this being dropped: it is driven by the
    // widget's `unrealize`, which is the signal that actually means "your GL
    // context is going away" (DR-232).
    LIVE_SURFACE.with(|cell| {
        *cell.borrow_mut() = Some(VideoSurface {
            state,
            widget: vbox.clone(),
            handlers,
        });
    });
    true
 }
 thread_local! {
    /// The one live surface, on the GTK main thread.
    static LIVE_SURFACE: RefCell<Option<VideoSurface>> = const { RefCell::new(None) };
 }
 /// Drop the live surface, if there is one. Idempotent.
 ///
 /// TRACES: UR-080 | DR-232
 #[allow(dead_code)]
 pub fn detach() {
    LIVE_SURFACE.with(|cell| {
        cell.borrow_mut().take();
    });
 }
 /// Create the GL context and the mpv render context over it.
 fn init_gl(widget: &gtk::Box, state: &Rc<RefCell<SurfaceState>>) -> Result<(), String> {
    if state.borrow().render.is_some() {
        return Ok(());
    }
    let window = widget.window().ok_or("widget has no GdkWindow")?;
    let gl = window
        .create_gl_context()
        .map_err(|e| format!("create_gl_context: {e}"))?;
    gl.realize().map_err(|e| format!("realize: {e}"))?;
    gl.make_current();
    let mpv = state.borrow().mpv;
    let mut render =
        unsafe { MpvRenderContext::new(mpv) }.ok_or("mpv render context creation failed")?;
    // The callback holds a pointer to the shared state, so that Rc must outlive
    // it. `destroy` unregisters before freeing, which is what makes it safe.
    let ctx_ptr = Rc::into_raw(state.clone()) as *mut c_void;
    unsafe { render.set_update_callback(Some(on_mpv_update), ctx_ptr) };
    let mut s = state.borrow_mut();
    s.gl = Some(gl);
    s.render = Some(render);
    info!("[VideoSurface] GL and render context ready");
    Ok(())
 }
 /// Draw the current frame, if there is one.
 ///
 /// Runs *before* the children, which is what puts the picture behind the
 /// webview. Deliberately forgiving: no frame, no GL, or a borrowed state all
 /// mean "draw nothing this pass" rather than an error — the webview then paints
 /// over an untouched background, which is exactly the pre-native appearance.
 fn draw(widget: &gtk::Box, cr: &gtk::cairo::Context, state: &Rc<RefCell<SurfaceState>>) {
    let Ok(mut s) = state.try_borrow_mut() else {
        return;
    };
    let Some(gl) = s.gl.clone() else {
        return;
    };
    let Some(window) = widget.window() else {
        return;
    };
    let scale = widget.scale_factor();
    let width = widget.allocated_width() * scale;
    let height = widget.allocated_height() * scale;
    if width <= 0 || height <= 0 {
        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");
+    gl.make_current();
        detach(&vbox, &surface);
-        let children = vbox.children();
+    let Some(render) = s.render.as_mut() else {
        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-231
    #[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);
+    let Some(texture) = (unsafe { render.render(width, height) }) else {
-        assert!(matches!(attach(&vbox), Err(SurfaceError::NoWebviewChild)));
+        return;
    };
    unsafe {
        cr.draw_from_gl(
            &window,
            texture as i32,
            GL_TEXTURE,
            scale,
            0,
            0,
            width,
            height,
        );
        // Tell mpv the frame reached the screen. Without this it has nothing to
        // pace against — see DR-233.
        render.report_swap();
    }
 }
@@ -8,6 +8,7 @@
 -->
 <script lang="ts">
  import { goto } from "$app/navigation";
  import { formatDuration } from "$lib/utils/duration";
  import { truncateMiddle } from "$lib/utils/truncateMiddle";
  import type { MediaItem } from "$lib/api/types";
  import CachedImage from "$lib/components/common/CachedImage.svelte";
@@ -88,18 +89,6 @@
      : null,
  );
  function formatDuration(ms?: number | null): string {
    if (!ms) return "";
    const seconds = Math.floor(ms / 1000);
    const hours = Math.floor(seconds / 3600);
    const minutes = Math.floor((seconds % 3600) / 60);
    if (hours > 0) {
      return `${hours}h ${minutes}m`;
    }
    return `${minutes}m`;
  }
  function getProgress(ep: MediaItem): number {
    if (!ep.userData || !ep.durationMs) {
      return 0;
@@ -117,7 +106,7 @@
  }
  const episodeLabel = $derived(`S${episode.parentIndexNumber || 1}E${episode.indexNumber || 1}`);
-  const duration = $derived(formatDuration(episode.durationMs));
+  const duration = $derived(formatDuration(episode.durationMs, "h m"));
  const progress = $derived(getProgress(episode));
 </script>
@@ -1,5 +1,6 @@
 <script lang="ts">
  import { playerController } from "$lib/player";
  import { formatDuration } from "$lib/utils/duration";
  import { truncateMiddle } from "$lib/utils/truncateMiddle";
  import { dndzone, SOURCES, TRIGGERS } from "svelte-dnd-action";
  import type { MediaItem } from "$lib/api/types";
@@ -34,14 +35,6 @@
  let dragDisabled = $state(true);
  const flipDurationMs = 200;
  function formatDuration(ms?: number | null): string {
    if (!ms) return "";
    const seconds = Math.floor(ms / 1000);
    const mins = Math.floor(seconds / 60);
    const secs = seconds % 60;
    return `${mins}:${secs.toString().padStart(2, "0")}`;
  }
  function handleConsider(
    e: CustomEvent<{ items: DndItem[]; info: { source: string; trigger: string } }>,
  ) {
@@ -5,7 +5,7 @@
 */
 import { describe, it, expect } from "vitest";
-import { formatDuration, formatSecondsDuration } from "./duration";
+import { formatDuration } from "./duration";
 describe("formatDuration", () => {
  it("should format duration from milliseconds (mm:ss format)", () => {
@@ -39,23 +39,3 @@ describe("formatDuration", () => {
    expect(formatDuration(9045000, "hh:mm:ss")).toBe("2:30:45");
  });
 });
 describe("formatSecondsDuration", () => {
  it("should format duration from seconds (mm:ss format)", () => {
    expect(formatSecondsDuration(1)).toBe("0:01");
    expect(formatSecondsDuration(60)).toBe("1:00");
    expect(formatSecondsDuration(61)).toBe("1:01");
    expect(formatSecondsDuration(3661)).toBe("61:01");
  });
  it("should format duration with hh:mm:ss format", () => {
    expect(formatSecondsDuration(3600, "hh:mm:ss")).toBe("1:00:00");
    expect(formatSecondsDuration(3661, "hh:mm:ss")).toBe("1:01:01");
    expect(formatSecondsDuration(7325, "hh:mm:ss")).toBe("2:02:05");
  });
  it("should pad minutes and seconds with leading zeros", () => {
    expect(formatSecondsDuration(5, "hh:mm:ss")).toBe("0:00:05");
    expect(formatSecondsDuration(65, "hh:mm:ss")).toBe("0:01:05");
  });
 });
@@ -12,11 +12,23 @@
 * @param format Format type: "mm:ss" (default) or "hh:mm:ss"
 * @returns Formatted duration string or empty string if no duration
 */
-export function formatDuration(ms?: number | null, format: "mm:ss" | "hh:mm:ss" = "mm:ss"): string {
+export function formatDuration(
  ms?: number | null,
  format: "mm:ss" | "hh:mm:ss" | "h m" = "mm:ss",
 ): string {
  if (!ms) return "";
  const totalSeconds = Math.floor(ms / 1000);
  // "1h 23m" / "45m" — the shape a runtime is read at a glance, as opposed to
  // the clock shape a *position* is read at. Three components had hand-rolled
  // this identically; it belongs here with the other two.
  if (format === "h m") {
    const hours = Math.floor(totalSeconds / 3600);
    const minutes = Math.floor((totalSeconds % 3600) / 60);
    return hours > 0 ? `${hours}h ${minutes}m` : `${minutes}m`;
  }
  if (format === "hh:mm:ss") {
    const hours = Math.floor(totalSeconds / 3600);
    const minutes = Math.floor((totalSeconds % 3600) / 60);
@@ -30,27 +42,3 @@ export function formatDuration(ms?: number | null, format: "mm:ss" | "hh:mm:ss"
  const seconds = totalSeconds % 60;
  return `${minutes}:${seconds.toString().padStart(2, "0")}`;
 }
 /**
 * Convert seconds to formatted duration string
 * @param seconds Duration in seconds
 * @param format Format type: "mm:ss" (default) or "hh:mm:ss"
 * @returns Formatted duration string
 */
 export function formatSecondsDuration(
  seconds: number,
  format: "mm:ss" | "hh:mm:ss" = "mm:ss",
 ): string {
  if (format === "hh:mm:ss") {
    const hours = Math.floor(seconds / 3600);
    const minutes = Math.floor((seconds % 3600) / 60);
    const secs = seconds % 60;
    return `${hours}:${minutes.toString().padStart(2, "0")}:${secs.toString().padStart(2, "0")}`;
  }
  // Default "mm:ss" format
  const minutes = Math.floor(seconds / 60);
  const secs = seconds % 60;
  return `${minutes}:${secs.toString().padStart(2, "0")}`;
 }
@@ -1,6 +1,7 @@
 <!-- TRACES: UR-035, UR-038, UR-048, UR-058, UR-062 | DR-043, DR-062, DR-102, DR-103, DR-142 -->
 <script lang="ts">
  import { onMount, untrack } from "svelte";
  import { formatDuration } from "$lib/utils/duration";
  import { page } from "$app/stores";
  import { goto } from "$app/navigation";
  import { navigateBack } from "$lib/utils/navigation";
@@ -250,18 +251,6 @@
  // Images now handled by CachedImage component
  function formatDuration(ms?: number | null): string {
    if (!ms) return "";
    const seconds = Math.floor(ms / 1000);
    const hours = Math.floor(seconds / 3600);
    const minutes = Math.floor((seconds % 3600) / 60);
    if (hours > 0) {
      return `${hours}h ${minutes}m`;
    }
    return `${minutes}m`;
  }
  function handleItemClick(clickedItem: MediaItem | Library) {
    if (!("kind" in clickedItem)) {
      // Library item - navigate to library
@@ -534,7 +523,7 @@
                >
              {/if}
              {#if item.durationMs}
-                <span>{formatDuration(item.durationMs)}</span>
+                <span>{formatDuration(item.durationMs, "h m")}</span>
              {/if}
              {#if item.communityRating}
                <span class="flex items-center gap-1">