[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 13:08:18 CET 2010
vlc/vlc-1.1 | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Sun Nov 21 14:08:14 2010 +0200| [269b9f1bc1a31d5cc7f709f3398a6686cbb7bd45] | committer: Rémi Denis-Courmont
Qt4: do not crash if the video window is released after the interface
This should fix #3359.
(cherry picked from commit 841eb240885cbea14245dfd832c9f1ab31d12bf4)
Conflicts:
modules/gui/qt4/main_interface.cpp
modules/gui/qt4/qt4.cpp
> http://git.videolan.org/gitweb.cgi/vlc/vlc-1.1.git/?a=commit;h=269b9f1bc1a31d5cc7f709f3398a6686cbb7bd45
---
modules/gui/qt4/main_interface.cpp | 11 ++++--
modules/gui/qt4/qt4.cpp | 68 +++++++++++++++++++++++-------------
2 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/modules/gui/qt4/main_interface.cpp b/modules/gui/qt4/main_interface.cpp
index 1bd3e63..a3ad197 100644
--- a/modules/gui/qt4/main_interface.cpp
+++ b/modules/gui/qt4/main_interface.cpp
@@ -264,7 +264,9 @@ MainInterface::~MainInterface()
if( stackCentralOldWidget == videoWidget )
showTab( bgWidget );
- releaseVideoSlot();
+ if( videoWidget )
+ releaseVideoSlot();
+
#ifdef WIN32
if( himl )
ImageList_Destroy( himl );
@@ -586,8 +588,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 5ed7682..98d8aa6 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"
@@ -267,11 +268,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
@@ -299,11 +298,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");
@@ -326,15 +321,13 @@ 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;
}
/* */
vlc_sem_wait (&ready);
vlc_sem_destroy (&ready);
+ busy = active = true;
if( !p_sys->b_isDialogProvider )
{
@@ -379,9 +372,10 @@ static void Close( vlc_object_t *p_this )
x11_display = NULL;
#endif
delete p_sys;
- vlc_mutex_lock (&one.lock);
- one.busy = false;
- vlc_mutex_unlock (&one.lock);
+
+ QMutexLocker locker (&lock);
+ assert (busy);
+ busy = false;
}
static void *Thread( void *obj )
@@ -449,15 +443,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);
/* Last settings */
@@ -478,9 +474,8 @@ static void *Thread( void *obj )
msg_Dbg( p_intf, "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
@@ -555,6 +550,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..." );
@@ -585,6 +584,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 );
}
@@ -592,8 +598,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