Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: sramac...@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 <sramac...@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?= <r...@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?= <r...@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?= <r...@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

Reply via email to