gui: picture-in-picture mode may cause segfault on wayland environment

feeluown/gui/uimain/nowplaying_overlay.py

@@ -54,8 +54,8 @@ def __init__(self, app: 'GuiApp', parent=None):
 
  self.artwork_view.mv_btn.clicked.connect(self.play_mv)
  self.ctl_btns.media_btns.toggle_video_btn.clicked.connect(self.enter_video_mode)
- self._app.player.video_format_changed.connect(
- self.on_video_format_changed, aioqueue=True
+ self._app.player.video_channel_changed.connect(
+ self.on_video_channel_changed, aioqueue=True
  )
 
  def setup_ui(self):
@@ -82,7 +82,8 @@ def enter_video_mode(self):
  self._app.watch_mgr.exit_fullwindow_mode()
  video_widget = self._app.ui.mpv_widget
  video_widget.overlay_auto_visible = True
- self.artwork_view.set_body(video_widget)
+ with video_widget.change_parent():
+ self.artwork_view.set_body(video_widget)
  self.ctl_btns.hide()
  self.progress.hide()
  video_widget.ctl_bar.clear_adhoc_btns()
@@ -114,8 +115,11 @@ def showEvent(self, a0) -> None:
  def sizeHint(self):
  return QSize(500, 400)
 
- def on_video_format_changed(self, video_format):
- if video_format is None:
+ def on_video_channel_changed(self, _):
+ if (
+ bool(self._app.player.video_channel) is False
+ and not self._app.ui.mpv_widget.is_changing_parent
+ ):
  self.enter_cover_mode()
 
 

feeluown/gui/watch.py

@@ -67,7 +67,7 @@ def initialize(self):
  self._ui.pc_panel.media_btns.toggle_video_btn.clicked.connect(
  lambda: self.set_mode(Mode.fullwindow))
  self._app.player.media_changed.connect(self.on_media_changed, aioqueue=True)
- self._app.player.video_format_changed.connect(self.on_video_format_changed, aioqueue=True) # noqa
+ self._app.player.video_channel_changed.connect(self.on_video_channel_changed, aioqueue=True) # noqa
 
  self._pip_container.setMinimumSize(200, 130)
  self._pip_container.hide()
@@ -95,7 +95,8 @@ def enter_fullwindow_mode(self, go_back=None):
  video_widget = self._app.ui.mpv_widget
  logger.debug("enter video-show fullwindow mode")
  if video_widget.parent() != self._fullwindow_container:
- self._fullwindow_container.set_body(video_widget)
+ with video_widget.change_parent():
+ self._fullwindow_container.set_body(video_widget)
 
  self._fullwindow_container.show()
  self._fullwindow_container.raise_()
@@ -130,7 +131,8 @@ def enter_pip_mode(self):
  video_widget.overlay_auto_visible = True
 
  if video_widget.parent() != self._pip_container:
- self._pip_container.attach_widget(self._app.ui.mpv_widget)
+ with video_widget.change_parent():
+ self._pip_container.attach_widget(video_widget)
 
  video_widget.ctl_bar.clear_adhoc_btns()
  fullscreen_btn = video_widget.ctl_bar.add_adhoc_btn('全屏')
@@ -167,11 +169,16 @@ def on_media_changed(self, media):
  logger.debug('media is changed to none, hide video-show')
  self.set_mode(Mode.none)
 
- def on_video_format_changed(self, video_format):
- if video_format is None:
- # HELP(cosven): Event if player play a valid video, the video_format
+ def on_video_channel_changed(self, _):
+ if bool(self._app.player.video_channel) is False:
+ # When the mpv widget is changing it's parent, the video_channel may be
+ # changed to empty manully (see mpv_widget.change_parent).
+ if not self._app.ui.mpv_widget.is_changing_parent:
+ return
+
+ # HELP(cosven): Even if player play a valid video, the video_format
  # is changed to none first, and then it is changed to the real value.
- # So remember if the video widget is visible before hide it.
+ # So check if the video widget is visible before hide it.
  self._is_visible_before_auto_set_to_none = self._app.ui.mpv_widget.isVisible() # noqa
  self.set_mode(Mode.none)
  else:

feeluown/gui/widgets/mpv_.py

@@ -1,9 +1,12 @@
+from contextlib import contextmanager
+
 from PyQt5.QtCore import QMetaObject, pyqtSlot, QSize
 from PyQt5.QtOpenGL import QGLContext
 
-from mpv import MpvRenderContext, OpenGlCbGetProcAddrFn
+from mpv import MpvRenderContext, OpenGlCbGetProcAddrFn, _mpv_set_property_string
 
 from feeluown.gui.widgets.video import VideoOpenGLWidget
+from feeluown.gui.helpers import IS_MACOS
 
 
 def get_proc_addr(_, name):
@@ -34,6 +37,7 @@ def __init__(self, app, parent=None):
  self.mpv = self._app.player._mpv # noqa
  self.ctx = None
  self.get_proc_addr_c = OpenGlCbGetProcAddrFn(get_proc_addr)
+ self._is_changing_parent = False
 
  def initializeGL(self):
  params = {'get_proc_address': self.get_proc_addr_c}
@@ -95,6 +99,55 @@ def sizeHint(self):
  return QSize(self.mpv.width, self.mpv.height)
  return super().sizeHint()
 
+ @property
+ def is_changing_parent(self):
+ return self._is_changing_parent
+
+ @contextmanager
+ def change_parent(self):
+ assert self._is_changing_parent is False, 'implementation bug'
+
+ # on macOS, changing mpv widget parent cause no side effects.
+ # on Linux (wayland), it seems changing mpv widget parent may cause segfault,
+ # so do some hack to avoid crash.
+ if not IS_MACOS:
+ self._is_changing_parent = True
+ self._before_change_mpv_widget_parent()
+ try:
+ yield
+ finally:
+ if not IS_MACOS:
+ self._after_change_mpv_widget_parent()
+ self._is_changing_parent = False
+
+ def _before_change_mpv_widget_parent(self):
+ """
+ According to Qt docs, reparenting an OpenGLWidget will destory the GL context.
+ In mpv widget, it calls _mpv_opengl_cb_uninit_gl. After uninit_gl, mpv can't show
+ video anymore because video_out is destroyed.
+
+ See mpv mpv_opengl_cb_uninit_gl implementation for more details.
+ """
+ _mpv_set_property_string(self._app.player._mpv.handle, b'vid', b'no')
+
+ def _after_change_mpv_widget_parent(self):
+ """
+ To recover the video show, we should reinit gl and reinit video. gl is
+ automatically reinited when the mpv_widget do painting. We should
+ manually reinit video.
+
+ NOTE(cosven): After some investigation, I found that the API in mpv to
+ reinit video_out(maybe video is almost same as video_out)
+ is init_best_video_out. Theoretically, sending 'video-reload' command
+ will trigger this API. However, we can't run this command
+ in our case and I don't know why. Another way to trigger
+ the API is to switch track. Changing vid property just switch the track.
+
+ Inpect mpv init_best_video_out caller for more details. You should see
+ mp_switch_track_n is one of the entrypoint.
+ """
+ _mpv_set_property_string(self._app.player._mpv.handle, b'vid', b'1')
+
 
 # TODO: 实现 MpvEmbeddedWidget
 class MpvEmbeddedWidget:

feeluown/player/base_player.py

@@ -46,6 +46,8 @@ def __init__(self, _=None, **kwargs):
 
  self._current_media = None
  self._current_metadata = Metadata()
+ self._video_format = None
+ self._video_channel = False # False, True, 1, int
 
  #: player position changed signal
  self.position_changed = Signal()

feeluown/player/mpvplayer.py

@@ -68,6 +68,7 @@ def __init__(self, _=None, audio_device=b'auto', winid=None,
 
  #: if video_format changes to None, there is no video available
  self.video_format_changed = Signal()
+ self.video_channel_changed = Signal()
 
  self._mpv.observe_property(
  'time-pos',
@@ -271,6 +272,20 @@ def video_format(self, vformat):
  # firstly changed to None, and then changed to the real format.
  self.video_format_changed.emit(vformat)
 
+ @property
+ def video_channel(self):
+ return self._video_channel
+
+ @video_channel.setter
+ def video_channel(self, value):
+ """
+ According to practice:
+ - when playing a audio, the video channel is changed to False.
+ - when playing a video, the video channel is changed to 1.
+ """
+ self._video_channel = value
+ self.video_channel_changed.emit(value)
+
  def _stop_mpv(self):
  # Remove current media.
  self._mpv.play("")
@@ -289,6 +304,9 @@ def _on_duration_changed(self, duration):
  def _on_video_format_changed(self, vformat):
  self.video_format = vformat
 
+ def _on_video_changed(self, video):
+ self.video_channel = video
+
  def _on_event(self, event):
  event_id = event['event_id']
  if event_id == MpvEventID.END_FILE: