[vlc-commits] commit: Qt4: do not crash if the video window is released after the interface ( =?UTF-8?Q?R=C3=A9mi=20Denis=2DCourmont=20?=)
git at videolan.org
git at videolan.org
Sun Nov 21 12:53:40 CET 2010
vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Sun Nov 21 13:45:19 2010 +0200| [841eb240885cbea14245dfd832c9f1ab31d12bf4] | committer: Rémi Denis-Courmont
Qt4: do not crash if the video window is released after the interface
This should fix #3359.
> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=841eb240885cbea14245dfd832c9f1ab31d12bf4
---
modules/gui/qt4/main_interface.cpp | 10 ++++--
modules/gui/qt4/qt4.cpp | 66 +++++++++++++++++++++++------------
2 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/modules/gui/qt4/main_interface.cpp b/modules/gui/qt4/main_interface.cpp
index 5554aca..ddf568d 100644
--- 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 168670b..136995c 100644
--- 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();
}
-
More information about the vlc-commits
mailing list