Stop the player freeing a render context libmpv still uses
The segfault: Qt destroys and recreates a widget's QOpenGLContext whenever it moves to another top-level window, then calls initializeGL() again. libmpv permits one render context per handle, so the second creation failed with "There is already a mpv_render_context set" -- and the except branch assigned self._render_ctx = None, dropping the last Python reference to the first context, which libmpv was still holding a function pointer into. python-mpv's MpvRenderContext has no __del__ and free() does not unregister the callback, so the ctypes trampoline was collected while registered and the next frame notification jumped into freed memory. Two invariants fix it. initializeGL() now tears down any existing render context first, so a second call is an ordinary recreation. Teardown clears update_cb, calls free() with the GL context current, and only then drops the reference -- and it is connected to QOpenGLContext.aboutToBeDestroyed, so it runs before the GL context dies instead of never. The local reference during teardown is load-bearing: it is what keeps the trampoline alive until free() returns. Three things were destroying that context. Fullscreen reparented the panel into a new top-level window (twice per toggle) and put it back at the end of the splitter, losing the pane layout; it now fullscreens the main window and hides the chrome, reparenting nothing. The tab cross-fade and the dialog blur both grab() the widget tree, which on an OpenGL surface forces a framebuffer readback and returns black -- the fade is skipped for pages holding the video, and dialogs dim rather than blur. Verified on a real Wayland GL context: ten forced context destroy/recreate cycles re-establish the render context every time, and the full app survives tab switching, six fullscreen toggles and resizes with no render-context error and a clean exit. Before this, the same startup dumped core. Also here because they are one-line consequences of touching _create_mpv: an explicit per-platform hwdec list ending in software decoding, and the restored volume actually reaching mpv -- the slider set its value before connecting its signal, so playback always started at 100. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+257
-49
@@ -16,9 +16,11 @@ callback on its own threads. Nothing in those callbacks may touch Qt
|
||||
widgets - they only emit queued Qt signals.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import threading
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
from PySide6.QtCore import Qt, QTimer, Signal, Slot
|
||||
from PySide6.QtCore import QCoreApplication, Qt, QTimer, Signal, Slot
|
||||
from PySide6.QtOpenGLWidgets import QOpenGLWidget
|
||||
from PySide6.QtGui import QOpenGLContext
|
||||
from PySide6.QtWidgets import (
|
||||
@@ -78,8 +80,61 @@ def _build_ytdl_raw_options() -> str:
|
||||
return ",".join(opts)
|
||||
|
||||
|
||||
def _default_hwdec() -> str:
|
||||
"""
|
||||
A platform-appropriate hwdec list.
|
||||
|
||||
Left unset, mpv's decoder choice varies with how libmpv was built, which
|
||||
makes playback problems unreproducible between machines. Naming the
|
||||
candidates makes it deterministic, and every list ends in `no` so a
|
||||
machine whose GPU interop is broken falls back to software decoding
|
||||
rather than showing a black frame.
|
||||
|
||||
Note this does *not* silence `Cannot load libcuda.so.1`: that comes from
|
||||
the GL/driver stack below mpv and appears with `hwdec=no` too. It is
|
||||
harmless stderr noise, not a decoder mpv chose.
|
||||
"""
|
||||
if sys.platform.startswith("win"):
|
||||
return "d3d11va,dxva2-copy,no"
|
||||
if sys.platform == "darwin":
|
||||
return "videotoolbox,no"
|
||||
return "vaapi,vaapi-copy,no"
|
||||
|
||||
|
||||
class MpvRenderWidget(QOpenGLWidget):
|
||||
"""QOpenGLWidget hosting a libmpv render context."""
|
||||
"""
|
||||
QOpenGLWidget hosting a libmpv render context.
|
||||
|
||||
Lifetime rule, and the reason this class is shaped the way it is
|
||||
-------------------------------------------------------------------
|
||||
The **mpv handle** is independent of OpenGL and owns playback state, so it
|
||||
lives as long as the widget. The **render context** belongs to exactly one
|
||||
QOpenGLContext and must not outlive it.
|
||||
|
||||
Qt destroys and recreates a widget's QOpenGLContext whenever the widget
|
||||
moves to another top-level window, and calls initializeGL() again on the
|
||||
new one. libmpv permits only one render context per handle, so the second
|
||||
creation fails -- and the previous code responded by assigning
|
||||
`self._render_ctx = None`, dropping the last Python reference to a context
|
||||
libmpv was still holding a function pointer into. python-mpv's
|
||||
MpvRenderContext has no __del__ and free() does not unregister the
|
||||
callback, so the ctypes trampoline was collected under libmpv's feet and
|
||||
the next frame notification jumped into freed memory. That was the
|
||||
segfault.
|
||||
|
||||
Two invariants prevent it:
|
||||
|
||||
1. initializeGL() tears down any existing render context first, so a
|
||||
second call is a clean recreation rather than an error.
|
||||
2. Teardown clears update_cb, calls free() with the GL context current,
|
||||
and only then drops the reference -- and it is wired to
|
||||
QOpenGLContext.aboutToBeDestroyed, so it runs *before* the GL context
|
||||
goes away rather than never.
|
||||
|
||||
Threading: libmpv fires property observers and the render-update callback
|
||||
on its own threads. Nothing in those callbacks touches Qt widgets; they
|
||||
only emit queued signals.
|
||||
"""
|
||||
|
||||
# Emitted from mpv threads; connected queued to GUI-thread slots
|
||||
mpvPositionChanged = Signal(float)
|
||||
@@ -95,14 +150,30 @@ class MpvRenderWidget(QOpenGLWidget):
|
||||
self.setMinimumHeight(240)
|
||||
self._mpv = None
|
||||
self._render_ctx = None
|
||||
self._renderUpdateRequested.connect(self.update, Qt.ConnectionType.QueuedConnection)
|
||||
self._gl_ctx = None
|
||||
self._proc_addr_fn = None
|
||||
self._shutting_down = False
|
||||
# Gates the mpv-thread render callback. Cleared before free() so a
|
||||
# notification arriving mid-teardown becomes a no-op.
|
||||
self._render_alive = threading.Event()
|
||||
self._renderUpdateRequested.connect(self._request_repaint, Qt.ConnectionType.QueuedConnection)
|
||||
self.frameSwapped.connect(self._on_frame_swapped)
|
||||
self._create_mpv()
|
||||
|
||||
# closeEvent is not the only way this process ends. Without this, an
|
||||
# exception during teardown or any exit that bypasses the main
|
||||
# window's closeEvent leaves libmpv running against a dying
|
||||
# interpreter.
|
||||
app = QCoreApplication.instance()
|
||||
if app is not None:
|
||||
app.aboutToQuit.connect(self.shutdown)
|
||||
|
||||
# ------------------------------------------------------------------ mpv
|
||||
|
||||
def _create_mpv(self) -> None:
|
||||
import mpv
|
||||
|
||||
hwdec = ConfigManager.get("player.hwdec") or "auto"
|
||||
kwargs: Dict[str, Any] = {
|
||||
"vo": "libmpv",
|
||||
"ytdl": True,
|
||||
@@ -110,6 +181,11 @@ class MpvRenderWidget(QOpenGLWidget):
|
||||
"idle": "yes",
|
||||
"osc": False,
|
||||
"input_default_bindings": False,
|
||||
"hwdec": _default_hwdec() if hwdec == "auto" else str(hwdec),
|
||||
# Applied here rather than only through the slider: the slider
|
||||
# sets its restored value before its valueChanged is connected,
|
||||
# so mpv never saw it and playback always started at 100.
|
||||
"volume": max(0, min(100, int(ConfigManager.get("player.volume") or 100))),
|
||||
}
|
||||
ytdlp_path = get_yt_dlp_path()
|
||||
if str(ytdlp_path) != "yt-dlp":
|
||||
@@ -156,9 +232,45 @@ class MpvRenderWidget(QOpenGLWidget):
|
||||
|
||||
# --------------------------------------------------------------- OpenGL
|
||||
|
||||
def _on_render_update(self) -> None:
|
||||
"""
|
||||
libmpv render thread. Must not touch Qt widgets.
|
||||
|
||||
The alive gate matters: between clearing it and free() returning,
|
||||
libmpv may still invoke this, and by then the widget may be on its way
|
||||
out.
|
||||
"""
|
||||
if self._render_alive.is_set():
|
||||
self._renderUpdateRequested.emit()
|
||||
|
||||
@Slot()
|
||||
def _request_repaint(self) -> None:
|
||||
if self._render_ctx is not None and not self._shutting_down:
|
||||
self.update()
|
||||
|
||||
@Slot()
|
||||
def _on_frame_swapped(self) -> None:
|
||||
"""Let libmpv time its display-sync against real buffer swaps."""
|
||||
ctx = self._render_ctx
|
||||
if ctx is None or self._shutting_down:
|
||||
return
|
||||
try:
|
||||
ctx.report_swap()
|
||||
except Exception as e:
|
||||
logger.debug(f"mpv report_swap failed: {e}")
|
||||
|
||||
def initializeGL(self) -> None:
|
||||
from mpv import MpvGlGetProcAddressFn, MpvRenderContext
|
||||
|
||||
# A previous QOpenGLContext may still own a render context: Qt calls
|
||||
# initializeGL again after a context loss, and libmpv allows exactly
|
||||
# one per handle. Tearing down first is what turns "There is already a
|
||||
# mpv_render_context set" into an ordinary recreation.
|
||||
self._teardown_render_context()
|
||||
|
||||
if self._mpv is None or self._shutting_down:
|
||||
return
|
||||
|
||||
def get_proc_address(_ctx, name):
|
||||
glctx = QOpenGLContext.currentContext()
|
||||
if glctx is None:
|
||||
@@ -166,45 +278,141 @@ class MpvRenderWidget(QOpenGLWidget):
|
||||
address = glctx.getProcAddress(name if isinstance(name, bytes) else name.encode("utf-8"))
|
||||
return int(address) if address else 0
|
||||
|
||||
self._get_proc_address = MpvGlGetProcAddressFn(get_proc_address)
|
||||
# Held on the instance: libmpv keeps the raw pointer and some drivers
|
||||
# resolve symbols lazily, well after creation returns.
|
||||
proc_addr_fn = MpvGlGetProcAddressFn(get_proc_address)
|
||||
try:
|
||||
self._render_ctx = MpvRenderContext(
|
||||
render_ctx = MpvRenderContext(
|
||||
self._mpv,
|
||||
"opengl",
|
||||
opengl_init_params={"get_proc_address": self._get_proc_address},
|
||||
opengl_init_params={"get_proc_address": proc_addr_fn},
|
||||
)
|
||||
self._render_ctx.update_cb = self._renderUpdateRequested.emit # mpv thread
|
||||
except Exception as e:
|
||||
# Leave the widget black but keep the app alive (e.g. software GL
|
||||
# contexts that libmpv rejects)
|
||||
logger.error(f"Failed to create mpv render context: {e}")
|
||||
self._render_ctx = None
|
||||
self.mpvError.emit(f"Video output initialization failed: {e}")
|
||||
return
|
||||
|
||||
self._proc_addr_fn = proc_addr_fn
|
||||
self._render_ctx = render_ctx
|
||||
self._render_alive.set()
|
||||
render_ctx.update_cb = self._on_render_update # mpv thread
|
||||
|
||||
gl_ctx = QOpenGLContext.currentContext()
|
||||
self._gl_ctx = gl_ctx
|
||||
if gl_ctx is not None:
|
||||
# The one hook the old code was missing. Without it the render
|
||||
# context outlives the GL context that owns it -- or is never
|
||||
# freed at all.
|
||||
gl_ctx.aboutToBeDestroyed.connect(
|
||||
self._on_gl_context_about_to_be_destroyed,
|
||||
Qt.ConnectionType.DirectConnection,
|
||||
)
|
||||
|
||||
@Slot()
|
||||
def _on_gl_context_about_to_be_destroyed(self) -> None:
|
||||
"""GUI thread. free() requires its GL context to be current."""
|
||||
try:
|
||||
self.makeCurrent()
|
||||
self._teardown_render_context()
|
||||
finally:
|
||||
try:
|
||||
self.doneCurrent()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def _teardown_render_context(self) -> None:
|
||||
"""
|
||||
Release the render context, in the only order that is safe.
|
||||
|
||||
The local reference is load-bearing: it keeps the object -- and the
|
||||
ctypes trampoline libmpv points at -- alive until free() has returned.
|
||||
Dropping it earlier is precisely the use-after-free this class exists
|
||||
to avoid.
|
||||
"""
|
||||
render_ctx, self._render_ctx = self._render_ctx, None
|
||||
gl_ctx, self._gl_ctx = self._gl_ctx, None
|
||||
self._render_alive.clear()
|
||||
|
||||
if gl_ctx is not None:
|
||||
try:
|
||||
gl_ctx.aboutToBeDestroyed.disconnect(self._on_gl_context_about_to_be_destroyed)
|
||||
except (RuntimeError, TypeError):
|
||||
# Already disconnected, or the C++ object is gone.
|
||||
pass
|
||||
|
||||
if render_ctx is None:
|
||||
self._proc_addr_fn = None
|
||||
return
|
||||
|
||||
try:
|
||||
# Note this does *not* unregister: python-mpv installs a no-op
|
||||
# wrapper instead. It only guarantees that a callback arriving
|
||||
# before free() does nothing. free() is what actually unsets the
|
||||
# callback and blocks until in-flight invocations return.
|
||||
render_ctx.update_cb = None
|
||||
except Exception as e:
|
||||
logger.debug(f"Clearing mpv update callback failed: {e}")
|
||||
|
||||
try:
|
||||
render_ctx.free()
|
||||
except Exception as e:
|
||||
logger.debug(f"Error freeing mpv render context: {e}")
|
||||
finally:
|
||||
self._proc_addr_fn = None
|
||||
|
||||
def paintGL(self) -> None:
|
||||
if self._render_ctx is None:
|
||||
# Alias first: self._render_ctx can be cleared by a teardown between
|
||||
# the check and the render.
|
||||
render_ctx = self._render_ctx
|
||||
if render_ctx is None or self._shutting_down:
|
||||
return
|
||||
ratio = self.devicePixelRatioF()
|
||||
w = int(self.width() * ratio)
|
||||
h = int(self.height() * ratio)
|
||||
self._render_ctx.render(
|
||||
flip_y=True,
|
||||
opengl_fbo={"fbo": self.defaultFramebufferObject(), "w": w, "h": h},
|
||||
)
|
||||
if w <= 0 or h <= 0:
|
||||
return
|
||||
try:
|
||||
render_ctx.render(
|
||||
flip_y=True,
|
||||
opengl_fbo={"fbo": self.defaultFramebufferObject(), "w": w, "h": h},
|
||||
)
|
||||
except Exception as e:
|
||||
logger.debug(f"mpv render failed: {e}")
|
||||
|
||||
def resizeGL(self, w: int, h: int) -> None:
|
||||
# The FBO size travels with every render call, so nothing needs
|
||||
# resizing here -- but a resize while paused must still repaint, or
|
||||
# the last frame stays stretched to the old geometry.
|
||||
self.update()
|
||||
|
||||
def shutdown(self) -> None:
|
||||
"""Idempotent: reachable from closeEvent, aboutToQuit and WatchPage."""
|
||||
if self._shutting_down:
|
||||
return
|
||||
self._shutting_down = True
|
||||
|
||||
# Render context before the handle: terminating mpv while a render
|
||||
# context is registered is undefined.
|
||||
try:
|
||||
if self._render_ctx is not None:
|
||||
self._render_ctx.free()
|
||||
self._render_ctx = None
|
||||
if self.context() is not None:
|
||||
self.makeCurrent()
|
||||
try:
|
||||
self._teardown_render_context()
|
||||
finally:
|
||||
self.doneCurrent()
|
||||
else:
|
||||
self._teardown_render_context()
|
||||
except Exception as e:
|
||||
logger.debug(f"Error freeing mpv render context: {e}")
|
||||
try:
|
||||
if self._mpv is not None:
|
||||
self._mpv.terminate()
|
||||
self._mpv = None
|
||||
except Exception as e:
|
||||
logger.debug(f"Error terminating mpv: {e}")
|
||||
logger.debug(f"Error tearing down mpv render context: {e}")
|
||||
|
||||
mpv_inst, self._mpv = self._mpv, None
|
||||
if mpv_inst is not None:
|
||||
try:
|
||||
mpv_inst.terminate()
|
||||
except Exception as e:
|
||||
logger.debug(f"Error terminating mpv: {e}")
|
||||
|
||||
# ------------------------------------------------------------- controls
|
||||
|
||||
@@ -227,7 +435,7 @@ class PlayerPanel(QWidget):
|
||||
self._current_entry: Dict[str, Any] = {}
|
||||
self._duration: float = 0.0
|
||||
self._slider_down = False
|
||||
self._fullscreen_holder: Optional[QWidget] = None
|
||||
self._fullscreen_controller = None
|
||||
|
||||
layout = QVBoxLayout(self)
|
||||
layout.setContentsMargins(0, 0, 0, 0)
|
||||
@@ -472,32 +680,32 @@ class PlayerPanel(QWidget):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def toggle_fullscreen(self) -> None:
|
||||
if self._fullscreen_holder is None:
|
||||
self._fullscreen_parent_layout = self.parentWidget().layout() if self.parentWidget() else None
|
||||
self._fullscreen_holder = self.parentWidget()
|
||||
self.setParent(None)
|
||||
self.setWindowFlags(Qt.WindowType.Window)
|
||||
self.showFullScreen()
|
||||
else:
|
||||
self.setWindowFlags(Qt.WindowType.Widget)
|
||||
if self._fullscreen_parent_layout is not None:
|
||||
self._fullscreen_parent_layout.addWidget(self)
|
||||
else:
|
||||
self.setParent(self._fullscreen_holder)
|
||||
self.showNormal()
|
||||
self.show()
|
||||
self._fullscreen_holder = None
|
||||
def set_fullscreen_controller(self, controller) -> None:
|
||||
"""
|
||||
Injected by the main window, which owns the chrome being hidden.
|
||||
|
||||
def keyPressEvent(self, event) -> None:
|
||||
if event.key() == Qt.Key.Key_Escape and self._fullscreen_holder is not None:
|
||||
self.toggle_fullscreen()
|
||||
elif event.key() == Qt.Key.Key_Space:
|
||||
self.toggle_pause()
|
||||
elif event.key() == Qt.Key.Key_F:
|
||||
self.toggle_fullscreen()
|
||||
else:
|
||||
super().keyPressEvent(event)
|
||||
The panel deliberately does not implement fullscreen itself: the old
|
||||
version reparented itself into a top-level window, which destroyed the
|
||||
QOpenGLContext underneath it twice per toggle and was the most direct
|
||||
route to the render-context crash.
|
||||
"""
|
||||
self._fullscreen_controller = controller
|
||||
|
||||
def is_fullscreen(self) -> bool:
|
||||
controller = self._fullscreen_controller
|
||||
return bool(controller is not None and controller.is_active())
|
||||
|
||||
def toggle_fullscreen(self) -> None:
|
||||
controller = self._fullscreen_controller
|
||||
if controller is None:
|
||||
logger.debug("Fullscreen requested but no controller is attached.")
|
||||
return
|
||||
controller.toggle()
|
||||
|
||||
def exit_fullscreen(self) -> None:
|
||||
controller = self._fullscreen_controller
|
||||
if controller is not None and controller.is_active():
|
||||
controller.exit()
|
||||
|
||||
|
||||
class PlayerUnavailablePanel(QWidget):
|
||||
|
||||
Reference in New Issue
Block a user