Package: release.debian.org Severity: normal User: release.debian.org@packages.debian.org Usertags: unblock X-Debbugs-Cc: sramacher@debian.org Please unblock package vlc/3.0.12-3. [ Reason ] vlc 3.0.x suffers from a long standing issue that causes vlc to freeze on exit when running with a mesa GPU driver. A proper fix would also require changes to mesa (cf https://gitlab.freedesktop.org/mesa/mesa/-/issues/116 for the mesa bug), but attempts to fix mesa caused other regressions, so this fix was reverted. vlc upstream now added a workaround to no longer trigger the condition that leads to the freeze. [ Impact ] Users with affected drivers can reenable hardware accelerated video decoding. [ Tests ] No automated test coverage, but manually tested. [ Risks ] Even if the fix was incomplete, users can continue to disable hardware acceleration or kill the stuck vlc process. vlc is a key package, so requires an unblock. [ Checklist ] [x] all changes are documented in the d/changelog [x] I reviewed all changes and I approve them [x] attach debdiff against the package in testing unblock vlc/3.0.12-3 -- Sebastian Ramacher
diff --git a/debian/changelog b/debian/changelog
index b96fc96a8..1b3237d27 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+vlc (3.0.12-3) unstable; urgency=medium
+
+ * debian/patches: Apply upstream patches to prevent process freeze on exit
+ (Closes: #916595) (LP: #1819543)
+
+ -- Sebastian Ramacher <sramacher@debian.org> Tue, 09 Mar 2021 17:42:00 +0100
+
vlc (3.0.12-2) unstable; urgency=medium
* debian/: Disable live555 plugin due to #981439
diff --git a/debian/patches/0004-qt-add-a-private-structure-for-window-provider.patch b/debian/patches/0004-qt-add-a-private-structure-for-window-provider.patch
new file mode 100644
index 000000000..7788dd33b
--- /dev/null
+++ b/debian/patches/0004-qt-add-a-private-structure-for-window-provider.patch
@@ -0,0 +1,88 @@
+From: =?utf-8?q?R=C3=A9mi_Denis-Courmont?= <remi@remlab.net>
+Date: Sat, 6 Feb 2021 15:00:02 +0200
+Subject: qt: add a private structure for window provider
+
+---
+ modules/gui/qt/qt.cpp | 33 ++++++++++++++++++++++-----------
+ 1 file changed, 22 insertions(+), 11 deletions(-)
+
+diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
+index ab912fd..d5a22d9 100644
+--- a/modules/gui/qt/qt.cpp
++++ b/modules/gui/qt/qt.cpp
+@@ -708,6 +708,10 @@ static void ShowDialog( intf_thread_t *p_intf, int i_dialog_event, int i_arg,
+ */
+ static int WindowControl( vout_window_t *, int i_query, va_list );
+
++typedef struct {
++ MainInterface *mi;
++} vout_window_qt_t;
++
+ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+ {
+ if( cfg->is_standalone )
+@@ -737,21 +741,26 @@ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+ if (unlikely(!active))
+ return VLC_EGENERIC;
+
+- MainInterface *p_mi = p_intf->p_sys->p_mi;
++ vout_window_qt_t *sys = new vout_window_qt_t;
++
++ sys->mi = p_intf->p_sys->p_mi;
+ msg_Dbg( p_wnd, "requesting video window..." );
+
+- if( !p_mi->getVideo( p_wnd, cfg->width, cfg->height, cfg->is_fullscreen ) )
++ if (!sys->mi->getVideo(p_wnd, cfg->width, cfg->height, cfg->is_fullscreen))
++ {
++ delete sys;
+ return VLC_EGENERIC;
++ }
+
+ p_wnd->info.has_double_click = true;
+ p_wnd->control = WindowControl;
+- p_wnd->sys = (vout_window_sys_t*)p_mi;
++ p_wnd->sys = (vout_window_sys_t *)sys;
+ return VLC_SUCCESS;
+ }
+
+ static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
+ {
+- MainInterface *p_mi = (MainInterface *)p_wnd->sys;
++ vout_window_qt_t *sys = (vout_window_qt_t *)p_wnd->sys;
+ QMutexLocker locker (&lock);
+
+ if (unlikely(!active))
+@@ -759,12 +768,12 @@ static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
+ msg_Warn (p_wnd, "video already released before control");
+ return VLC_EGENERIC;
+ }
+- return p_mi->controlVideo( i_query, args );
++ return sys->mi->controlVideo(i_query, args);
+ }
+
+ static void WindowClose( vout_window_t *p_wnd )
+ {
+- MainInterface *p_mi = (MainInterface *)p_wnd->sys;
++ vout_window_qt_t *sys = (vout_window_qt_t *)p_wnd->sys;
+ QMutexLocker locker (&lock);
+
+ /* Normally, the interface terminates after the video. In the contrary, the
+@@ -776,11 +785,13 @@ static void WindowClose( vout_window_t *p_wnd )
+ * That assumes the video output will behave sanely if it window is
+ * destroyed asynchronously.
+ * XCB and Xlib-XCB are fine with that. Plain Xlib wouldn't, */
+- if (unlikely(!active))
++ if (likely(active))
+ {
+- msg_Warn (p_wnd, "video already released");
+- return;
++ msg_Dbg(p_wnd, "releasing video...");
++ sys->mi->releaseVideo();
+ }
+- msg_Dbg (p_wnd, "releasing video...");
+- p_mi->releaseVideo();
++ else
++ msg_Warn (p_wnd, "video already released");
++
++ delete sys;
+ }
diff --git a/debian/patches/0005-qt-create-another-indirection-X11-window.patch b/debian/patches/0005-qt-create-another-indirection-X11-window.patch
new file mode 100644
index 000000000..d48e4bf23
--- /dev/null
+++ b/debian/patches/0005-qt-create-another-indirection-X11-window.patch
@@ -0,0 +1,148 @@
+From: =?utf-8?q?R=C3=A9mi_Denis-Courmont?= <remi@remlab.net>
+Date: Fri, 5 Feb 2021 19:25:48 +0200
+Subject: qt: create another indirection X11 window
+
+The main window may be destroyed before the video window. This notably
+occurs if the user requests to close the main UI via window decorations.
+While Qt allows those requests to be rejected, doing so would
+reintroduce obnoxious bug #4606.
+
+The Qt-X11 display connection will be closed as well as it belongs to
+the QApplication instance.
+
+This creates a separate window belonging to a separate display
+connection, and which is not tied to the QApplication and QMainWindow
+instances. Unfortunately, this adds yet another connection to the X11
+display server in the VLC process in addition to QApplication's and the
+video display's. And that connection won't process events.
+
+Refs #21875.
+---
+ modules/gui/qt/components/interface_widgets.cpp | 4 +-
+ modules/gui/qt/qt.cpp | 59 ++++++++++++++++++++++++-
+ 2 files changed, 61 insertions(+), 2 deletions(-)
+
+diff --git a/modules/gui/qt/components/interface_widgets.cpp b/modules/gui/qt/components/interface_widgets.cpp
+index bcf65d2..0dbefd1 100644
+--- a/modules/gui/qt/components/interface_widgets.cpp
++++ b/modules/gui/qt/components/interface_widgets.cpp
+@@ -227,13 +227,15 @@ QSize VideoWidget::physicalSize() const
+ return current_size;
+ }
+
++void WindowResized(vout_window_t *, const QSize&);
++
+ void VideoWidget::reportSize()
+ {
+ if( !p_window )
+ return;
+
+ QSize size = physicalSize();
+- vout_window_ReportSize( p_window, size.width(), size.height() );
++ WindowResized(p_window, size);
+ }
+
+ /* Set the Widget to the correct Size */
+diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
+index d5a22d9..b900e74 100644
+--- a/modules/gui/qt/qt.cpp
++++ b/modules/gui/qt/qt.cpp
+@@ -361,6 +361,7 @@ static void Abort( void *obj )
+
+ #if defined (QT5_HAS_X11)
+ # include <vlc_xlib.h>
++# include <QX11Info>
+
+ static void *ThreadXCB( void *data )
+ {
+@@ -710,6 +711,9 @@ static int WindowControl( vout_window_t *, int i_query, va_list );
+
+ typedef struct {
+ MainInterface *mi;
++#ifdef QT5_HAS_X11
++ Display *dpy;
++#endif
+ } vout_window_qt_t;
+
+ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+@@ -744,20 +748,69 @@ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+ vout_window_qt_t *sys = new vout_window_qt_t;
+
+ sys->mi = p_intf->p_sys->p_mi;
++ p_wnd->sys = (vout_window_sys_t *)sys;
+ msg_Dbg( p_wnd, "requesting video window..." );
+
++#ifdef QT5_HAS_X11
++ Window xid;
++
++ if (QX11Info::isPlatformX11())
++ {
++ sys->dpy = XOpenDisplay(NULL);
++ if (unlikely(sys->dpy == NULL))
++ {
++ delete sys;
++ return VLC_EGENERIC;
++ }
++
++ int snum = DefaultScreen(sys->dpy);
++ unsigned long black = BlackPixel(sys->dpy, snum);
++
++ xid = XCreateSimpleWindow(sys->dpy, RootWindow(sys->dpy, snum),
++ 0, 0, cfg->width, cfg->height,
++ 0, black, black);
++ }
++#endif
++
+ if (!sys->mi->getVideo(p_wnd, cfg->width, cfg->height, cfg->is_fullscreen))
+ {
++#ifdef QT5_HAS_X11
++ if (QX11Info::isPlatformX11())
++ XCloseDisplay(sys->dpy);
++#endif
+ delete sys;
+ return VLC_EGENERIC;
+ }
+
++#ifdef QT5_HAS_X11
++ if (QX11Info::isPlatformX11())
++ {
++ XReparentWindow(sys->dpy, xid, p_wnd->handle.xid, 0, 0);
++ XMapWindow(sys->dpy, xid);
++ XSync(sys->dpy, True);
++ p_wnd->handle.xid = xid;
++ }
++#endif
+ p_wnd->info.has_double_click = true;
+ p_wnd->control = WindowControl;
+- p_wnd->sys = (vout_window_sys_t *)sys;
+ return VLC_SUCCESS;
+ }
+
++void WindowResized(vout_window_t *wnd, const QSize& size)
++{
++#ifdef QT5_HAS_X11
++ vout_window_qt_t *sys = (vout_window_qt_t *)wnd->sys;
++
++ if (QX11Info::isPlatformX11())
++ {
++ XResizeWindow(sys->dpy, wnd->handle.xid, size.width(), size.height());
++ XClearWindow(sys->dpy, wnd->handle.xid);
++ XSync(sys->dpy, True);
++ }
++#endif
++ vout_window_ReportSize(wnd, size.width(), size.height());
++}
++
+ static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
+ {
+ vout_window_qt_t *sys = (vout_window_qt_t *)p_wnd->sys;
+@@ -793,5 +846,9 @@ static void WindowClose( vout_window_t *p_wnd )
+ else
+ msg_Warn (p_wnd, "video already released");
+
++#if defined (QT5_HAS_X11)
++ if (QX11Info::isPlatformX11())
++ XCloseDisplay(sys->dpy);
++#endif
+ delete sys;
+ }
diff --git a/debian/patches/0006-qt-reparent-video-window-to-root-whence-UI-closes.patch b/debian/patches/0006-qt-reparent-video-window-to-root-whence-UI-closes.patch
new file mode 100644
index 000000000..08b2d4461
--- /dev/null
+++ b/debian/patches/0006-qt-reparent-video-window-to-root-whence-UI-closes.patch
@@ -0,0 +1,106 @@
+From: =?utf-8?q?R=C3=A9mi_Denis-Courmont?= <remi@remlab.net>
+Date: Fri, 5 Feb 2021 19:46:15 +0200
+Subject: qt: reparent video window to root whence UI closes
+
+The video window has to exist until it is closed by its owner, i.e.
+WindowClose() is called. If it stayed as a child of the main UI window,
+it would be destroyed with the main UI window.
+
+This reparents it (back) to the root window before the main UI window
+gets destroyed. This works around #21875.
+---
+ modules/gui/qt/components/interface_widgets.cpp | 2 ++
+ modules/gui/qt/main_interface.cpp | 2 ++
+ modules/gui/qt/qt.cpp | 25 +++++++++++++++++++++++++
+ 3 files changed, 29 insertions(+)
+
+diff --git a/modules/gui/qt/components/interface_widgets.cpp b/modules/gui/qt/components/interface_widgets.cpp
+index 0dbefd1..cfebe61 100644
+--- a/modules/gui/qt/components/interface_widgets.cpp
++++ b/modules/gui/qt/components/interface_widgets.cpp
+@@ -228,6 +228,7 @@ QSize VideoWidget::physicalSize() const
+ }
+
+ void WindowResized(vout_window_t *, const QSize&);
++void WindowReleased(vout_window_t *);
+
+ void VideoWidget::reportSize()
+ {
+@@ -377,6 +378,7 @@ void VideoWidget::release( void )
+
+ if( stable )
+ {
++ WindowReleased(p_window);
+ layout->removeWidget( stable );
+ stable->deleteLater();
+ stable = NULL;
+diff --git a/modules/gui/qt/main_interface.cpp b/modules/gui/qt/main_interface.cpp
+index bb5dad8..1f29f5e 100644
+--- a/modules/gui/qt/main_interface.cpp
++++ b/modules/gui/qt/main_interface.cpp
+@@ -1664,6 +1664,8 @@ void MainInterface::closeEvent( QCloseEvent *e )
+ // hide();
+ if ( b_minimalView )
+ setMinimalView( false );
++ if( videoWidget )
++ releaseVideoSlot();
+ emit askToQuit(); /* ask THEDP to quit, so we have a unique method */
+ /* Accept session quit. Otherwise we break the desktop mamager. */
+ e->accept();
+diff --git a/modules/gui/qt/qt.cpp b/modules/gui/qt/qt.cpp
+index b900e74..7f4c550 100644
+--- a/modules/gui/qt/qt.cpp
++++ b/modules/gui/qt/qt.cpp
+@@ -714,6 +714,8 @@ typedef struct {
+ #ifdef QT5_HAS_X11
+ Display *dpy;
+ #endif
++ bool orphaned;
++ QMutex lock;
+ } vout_window_qt_t;
+
+ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+@@ -748,6 +750,7 @@ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+ vout_window_qt_t *sys = new vout_window_qt_t;
+
+ sys->mi = p_intf->p_sys->p_mi;
++ sys->orphaned = false;
+ p_wnd->sys = (vout_window_sys_t *)sys;
+ msg_Dbg( p_wnd, "requesting video window..." );
+
+@@ -785,6 +788,8 @@ static int WindowOpen( vout_window_t *p_wnd, const vout_window_cfg_t *cfg )
+ #ifdef QT5_HAS_X11
+ if (QX11Info::isPlatformX11())
+ {
++ QMutexLocker locker2(&sys->lock);
++
+ XReparentWindow(sys->dpy, xid, p_wnd->handle.xid, 0, 0);
+ XMapWindow(sys->dpy, xid);
+ XSync(sys->dpy, True);
+@@ -824,6 +829,26 @@ static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
+ return sys->mi->controlVideo(i_query, args);
+ }
+
++void WindowReleased(vout_window_t *wnd)
++{
++ vout_window_qt_t *sys = (vout_window_qt_t *)wnd->sys;
++ QMutexLocker locker(&sys->lock);
++
++ msg_Warn(wnd, "orphaned video window");
++ sys->orphaned = true;
++#if defined (QT5_HAS_X11)
++ if (QX11Info::isPlatformX11())
++ { /* In the unlikely event that WindowOpen() has not yet reparented the
++ * window, WindowOpen() will skip reparenting. Then this call will be
++ * a no-op.
++ */
++ XReparentWindow(sys->dpy, wnd->handle.xid,
++ RootWindow(sys->dpy, DefaultScreen(sys->dpy)), 0, 0);
++ XSync(sys->dpy, True);
++ }
++#endif
++}
++
+ static void WindowClose( vout_window_t *p_wnd )
+ {
+ vout_window_qt_t *sys = (vout_window_qt_t *)p_wnd->sys;
diff --git a/debian/patches/series b/debian/patches/series
index 4ac56b9c1..c923bedff 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,6 @@
0001-configure-fix-linking-on-RISC-V-ISA.patch
0002-Revert-configure-Require-libmodplug-0.8.9.patch
0003-Do-not-generate-cache-during-build.patch
+0004-qt-add-a-private-structure-for-window-provider.patch
+0005-qt-create-another-indirection-X11-window.patch
+0006-qt-reparent-video-window-to-root-whence-UI-closes.patch
Attachment:
signature.asc
Description: PGP signature