Qt4: do not crash if the video window is released after the interface

This should fix #3359.

diff --git a/modules/gui/qt4/main_interface.cpp b/modules/gui/qt4/main_interface.cpp
index 5554aca7718b8cfb0c12636400a122477ede54f4..ddf568dfc19cf7e5800da6de340649de547bd536 100644 (file)
--- a/modules/gui/qt4/main_interface.cpp
+++ b/modules/gui/qt4/main_interface.cpp
@@ -267,7 +267,8 @@ MainInterface::~MainInterface()
     if( stackCentralOldWidget == videoWidget )
         showTab( bgWidget );
 
-    releaseVideoSlot();
+    if( videoWidget )
+        releaseVideoSlot();
 
 #ifdef WIN32
     if( himl )
@@ -601,8 +602,11 @@ void MainInterface::releaseVideo( void )
 /* Function that is CONNECTED to the previous emit */
 void MainInterface::releaseVideoSlot( void )
 {
-    if( videoWidget )
-        videoWidget->release();
+    /* This function is called when the embedded video window is destroyed,
+     * or in the rare case that the embedded window is still here but the
+     * Qt4 interface exits. */
+    assert( videoWidget );
+    videoWidget->release();
     setVideoOnTop( false );
     setVideoFullScreen( false );
 

diff --git a/modules/gui/qt4/qt4.cpp b/modules/gui/qt4/qt4.cpp
index 168670b56c4c13afb51412d3b1804d79996054fd..136995ced0e7d9cb70d3516904de1c8bc5e65c38 100644 (file)
--- a/modules/gui/qt4/qt4.cpp
+++ b/modules/gui/qt4/qt4.cpp
@@ -28,6 +28,7 @@
 
 #include <QApplication>
 #include <QDate>
+#include <QMutex>
 
 #include "qt4.hpp"
 
@@ -271,11 +272,9 @@ static vlc_sem_t ready;
 #ifdef Q_WS_X11
 static char *x11_display = NULL;
 #endif
-static struct
-{
-    vlc_mutex_t lock;
-    bool busy;
-} one = { VLC_STATIC_MUTEX, false };
+static QMutex lock;
+static bool busy = false;
+static bool active = false;
 
 /*****************************************************************************
  * Module callbacks
@@ -321,11 +320,7 @@ static int Open( vlc_object_t *p_this, bool isDialogProvider )
     char *display = NULL;
 #endif
 
-    bool busy;
-    vlc_mutex_lock (&one.lock);
-    busy = one.busy;
-    one.busy = true;
-    vlc_mutex_unlock (&one.lock);
+    QMutexLocker locker (&lock);
     if (busy)
     {
         msg_Err (p_this, "cannot start Qt4 multiple times");
@@ -352,9 +347,6 @@ static int Open( vlc_object_t *p_this, bool isDialogProvider )
     {
         delete p_sys;
         free (display);
-        vlc_mutex_lock (&one.lock);
-        one.busy = false;
-        vlc_mutex_unlock (&one.lock);
         return VLC_ENOMEM;
     }
 #endif
@@ -364,6 +356,7 @@ static int Open( vlc_object_t *p_this, bool isDialogProvider )
      * an embedded video window. */
     vlc_sem_wait (&ready);
     vlc_sem_destroy (&ready);
+    busy = active = true;
 
 #ifndef Q_WS_MAC
     if( !isDialogProvider )
@@ -416,8 +409,9 @@ static void Close( vlc_object_t *p_this )
 #endif
     delete p_sys;
 
-    vlc_mutex_locker locker (&one.lock);
-    one.busy = false;
+    QMutexLocker locker (&lock);
+    assert (busy);
+    busy = false;
 }
 
 static void *Thread( void *obj )
@@ -487,15 +481,17 @@ static void *Thread( void *obj )
 
     /* Create the normal interface in non-DP mode */
     if( !p_intf->p_sys->b_isDialogProvider )
+    {
         p_mi = new MainInterface( p_intf );
+        p_intf->p_sys->p_mi = p_mi;
+    }
     else
         p_mi = NULL;
-    p_intf->p_sys->p_mi = p_mi;
 
     /* Explain how to show a dialog :D */
     p_intf->pf_show_dialog = ShowDialog;
 
-    /* */
+    /* Tell the main LibVLC thread we are ready */
     vlc_sem_post (&ready);
 
 #ifdef Q_WS_MAC
@@ -525,9 +521,8 @@ static void *Thread( void *obj )
     msg_Dbg( p_intf, "QApp exec() finished" );
     if (p_mi != NULL)
     {
-#warning BUG!
-        /* FIXME: the video window may still be registerd at this point */
-        /* See LP#448082 as an example. */
+        QMutexLocker locker (&lock);
+        active = false;
 
         p_intf->p_sys->p_mi = NULL;
         /* Destroy first the main interface because it is connected to some
@@ -602,6 +597,10 @@ static int WindowOpen( vlc_object_t *p_obj )
         return VLC_EGENERIC;
     }
 
+    QMutexLocker locker (&lock);
+    if (unlikely(!active))
+        return VLC_EGENERIC;
+
     MainInterface *p_mi = p_intf->p_sys->p_mi;
     msg_Dbg( p_obj, "requesting video..." );
 
@@ -637,6 +636,13 @@ static int WindowOpen( vlc_object_t *p_obj )
 static int WindowControl( vout_window_t *p_wnd, int i_query, va_list args )
 {
     MainInterface *p_mi = (MainInterface *)p_wnd->sys;
+    QMutexLocker locker (&lock);
+
+    if (unlikely(!active))
+    {
+        msg_Warn (p_wnd, "video already released before control");
+        return VLC_EGENERIC;
+    }
     return p_mi->controlVideo( i_query, args );
 }
 
@@ -644,8 +650,22 @@ static void WindowClose( vlc_object_t *p_obj )
 {
     vout_window_t *p_wnd = (vout_window_t*)p_obj;
     MainInterface *p_mi = (MainInterface *)p_wnd->sys;
-
-    msg_Dbg( p_obj, "releasing video..." );
+    QMutexLocker locker (&lock);
+
+    /* Normally, the interface terminates after the video. In the contrary, the
+     * Qt4 main loop is gone, so we cannot send any event to the user interface
+     * widgets. Ideally, we would keep the Qt4 main loop running until after
+     * the video window is released. But it is far simpler to just have the Qt4
+     * thread destroy the window early, and to turn this function into a stub.
+     *
+     * 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))
+    {
+        msg_Warn (p_obj, "video already released");
+        return;
+    }
+    msg_Dbg (p_obj, "releasing video...");
     p_mi->releaseVideo();
 }
-