[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#984885: marked as done (unblock: vlc/3.0.12-3)

Your message dated Tue, 09 Mar 2021 18:52:46 +0000
with message-id <E1lJhTK-0004MB-Kc@respighi.debian.org>
and subject line unblock vlc
has caused the Debian Bug report #984885,
regarding unblock: vlc/3.0.12-3
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
984885: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=984885
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
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


--- End Message ---
--- Begin Message --- 
Unblocked vlc.

--- End Message ---
Reply to: