[vlc-devel] commit: Qt4: fix race in requestVideo and simplify ( Rémi Denis-Courmont )

git version control git at videolan.org
Thu Aug 13 17:23:32 CEST 2009


vlc | branch: 1.0-bugfix | Rémi Denis-Courmont <remi at remlab.net> | Thu Aug 13 18:22:41 2009 +0300| [61c5a86ae9c9d33f3ab43d059620c05fe9038ac8] | committer: Rémi Denis-Courmont 

Qt4: fix race in requestVideo and simplify

We were accessing some main interface variables from random threads
without serialization. This simply moves the blocking connection signal
from sole size setting to the whole video widget request call. In the
process, we can remove two signals. We still need a blocking connection
signal. It does not add any new deadlock case, since there was already
blocking connection signal in the same call and in the same direction.
(cherry picked from commit 7c6ef80c3e80d00e7c2000733b1feee0b537267e)

Conflicts:

	modules/gui/qt4/main_interface.cpp
	modules/gui/qt4/main_interface.hpp
	modules/gui/qt4/qt4.cpp

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=61c5a86ae9c9d33f3ab43d059620c05fe9038ac8
---

 modules/gui/qt4/main_interface.cpp |   57 ++++++++++++++++++++++--------------
 modules/gui/qt4/main_interface.hpp |   12 ++++----
 modules/gui/qt4/qt4.cpp            |    8 ++--
 3 files changed, 45 insertions(+), 32 deletions(-)

diff --git a/modules/gui/qt4/main_interface.cpp b/modules/gui/qt4/main_interface.cpp
index 31b990a..921758d 100644
--- a/modules/gui/qt4/main_interface.cpp
+++ b/modules/gui/qt4/main_interface.cpp
@@ -215,18 +215,18 @@ MainInterface::MainInterface( intf_thread_t *_p_intf ) : QVLCMW( _p_intf )
     var_AddCallback( p_intf->p_libvlc, "intf-popupmenu", PopupMenuCB, p_intf );
 
 
-    /* VideoWidget connects to avoid different threads speaking to each other */
+    /* VideoWidget connects for asynchronous calls */
+    connect( this, SIGNAL(askGetVideo(WId*,vout_thread_t*,int*,int*,unsigned*,unsigned *)),
+             this, SLOT(getVideoSlot(WId*,vout_thread_t*,int*,int*,unsigned*,unsigned*)),
+             Qt::BlockingQueuedConnection );
     connect( this, SIGNAL(askReleaseVideo( void )),
-             this, SLOT(releaseVideoSlot( void )), Qt::BlockingQueuedConnection );
+             this, SLOT(releaseVideoSlot( void )),
+             Qt::BlockingQueuedConnection );
 
     if( videoWidget )
     {
         CONNECT( this, askVideoToResize( unsigned int, unsigned int ),
                  videoWidget, SetSizing( unsigned int, unsigned int ) );
-
-        connect( this, SIGNAL(askVideoToShow( unsigned int, unsigned int)),
-             videoWidget, SLOT(SetSizing(unsigned int, unsigned int )),
-             Qt::BlockingQueuedConnection );
     }
 
     CONNECT( this, askUpdate(), this, doComponentsUpdate() );
@@ -372,7 +372,6 @@ void MainInterface::createMainWidget( QSettings *settings )
     bgWidget->resize(
             settings->value( "backgroundSize", QSize( 300, 200 ) ).toSize() );
     bgWidget->updateGeometry();
-    CONNECT( this, askBgWidgetToToggle(), bgWidget, toggle() );
 
     if( i_visualmode != QT_ALWAYS_VIDEO_MODE &&
         i_visualmode != QT_MINIMAL_MODE )
@@ -702,41 +701,55 @@ private:
 };
 
 /**
- * README
- * Thou shall not call/resize/hide widgets from on another thread.
- * This is wrong, and this is THE reason to emit signals on those Video Functions
- **/
-WId MainInterface::requestVideo( vout_thread_t *p_nvout, int *pi_x,
-                                 int *pi_y, unsigned int *pi_width,
-                                 unsigned int *pi_height )
+ * NOTE:
+ * You must note change the state of this object or other Qt4 UI objects,
+ * from the video output thread - only from the Qt4 UI main loop thread.
+ * All window provider queries must be handled through signals or events.
+ * That's why we have all those emit statements...
+ */
+WId MainInterface::getVideo( vout_thread_t *p_nvout, int *pi_x, int *pi_y,
+                             unsigned int *pi_width, unsigned int *pi_height )
+{
+    if( !videoWidget )
+        return 0;
+
+    /* This is a blocking call signal. Results are returned through pointers.
+     * Beware of deadlocks! */
+    WId id;
+    emit askGetVideo( &id, p_nvout, pi_x, pi_y, pi_width, pi_height );
+    return id;
+}
+
+void MainInterface::getVideoSlot( WId *p_id, vout_thread_t *p_nvout,
+                                  int *pi_x, int *pi_y,
+                                  unsigned *pi_width, unsigned *pi_height )
 {
     /* Request the videoWidget */
-    if( !videoWidget ) return 0;
-    WId ret = videoWidget->request( p_nvout,pi_x, pi_y,
+    WId ret = videoWidget->request( p_nvout, pi_x, pi_y,
                                     pi_width, pi_height, b_keep_size );
+    *p_id = ret;
     if( ret ) /* The videoWidget is available */
     {
         /* Did we have a bg ? Hide it! */
         if( VISIBLE( bgWidget) )
         {
             bgWasVisible = true;
-            emit askBgWidgetToToggle();
+            bgWidget->toggle();
         }
         else
             bgWasVisible = false;
 
         /* ask videoWidget to show */
-        emit askVideoToShow( *pi_width, *pi_height );
+        videoWidget->SetSizing( *pi_width, *pi_height );
 
         /* Consider the video active now */
         videoIsActive = true;
 
         emit askUpdate();
     }
-    return ret;
 }
 
-/* Call from the WindowClose function */
+/* Asynchronous call from the WindowClose function */
 void MainInterface::releaseVideo( void )
 {
     emit askReleaseVideo( );
@@ -760,7 +773,7 @@ void MainInterface::releaseVideoSlot( void )
     if( !isFullScreen() ) doComponentsUpdate();
 }
 
-/* Call from WindowControl function */
+/* Asynchronous call from WindowControl function */
 int MainInterface::controlVideo( int i_query, va_list args )
 {
     int i_ret = VLC_SUCCESS;
@@ -850,7 +863,7 @@ void MainInterface::toggleMinimalView( bool b_switch )
     { /* NORMAL MODE then */
         if( !videoWidget || videoWidget->isHidden() )
         {
-            emit askBgWidgetToToggle();
+            bgWidget->toggle();
         }
         else
         {
diff --git a/modules/gui/qt4/main_interface.hpp b/modules/gui/qt4/main_interface.hpp
index 292a18c..d352fae 100644
--- a/modules/gui/qt4/main_interface.hpp
+++ b/modules/gui/qt4/main_interface.hpp
@@ -74,9 +74,8 @@ public:
     virtual ~MainInterface();
 
     /* Video requests from core */
-    WId requestVideo( vout_thread_t *p_nvout, int *pi_x,
-                      int *pi_y, unsigned int *pi_width,
-                      unsigned int *pi_height );
+    WId getVideo( vout_thread_t *p_nvout, int *pi_x, int *pi_y,
+                  unsigned int *pi_width, unsigned int *pi_height );
     void releaseVideo( void  );
     int controlVideo( int i_query, va_list args );
 
@@ -160,6 +159,8 @@ public slots:
     void popupMenu( const QPoint& );
 
     /* Manage the Video Functions from the vout threads */
+    void getVideoSlot( WId *p_id, vout_thread_t *, int *pi_x, int *pi_y,
+                       unsigned *pi_width, unsigned *pi_height );
     void releaseVideoSlot( void );
 
 private slots:
@@ -178,11 +179,10 @@ private slots:
 
     void showCryptedLabel( bool );
 signals:
+    void askGetVideo( WId *p_id, vout_thread_t *, int *pi_x, int *pi_y,
+                      unsigned int *pi_width, unsigned int *pi_height );
     void askReleaseVideo( );
     void askVideoToResize( unsigned int, unsigned int );
-    void askVideoToShow( unsigned int, unsigned int );
-    void askVideoToToggle();
-    void askBgWidgetToToggle();
     void askUpdate();
     void minimalViewToggled( bool );
     void fullscreenInterfaceToggled( bool );
diff --git a/modules/gui/qt4/qt4.cpp b/modules/gui/qt4/qt4.cpp
index 9629921..49e7732 100644
--- a/modules/gui/qt4/qt4.cpp
+++ b/modules/gui/qt4/qt4.cpp
@@ -550,14 +550,14 @@ static int WindowOpen (vlc_object_t *obj)
     msg_Dbg (obj, "requesting video...");
 
 #if defined (Q_WS_X11)
-    wnd->handle.xid = p_mi->requestVideo (wnd->vout, &wnd->pos_x, &wnd->pos_y,
-                                          &wnd->width, &wnd->height);
+    wnd->handle.xid = p_mi->getVideo (wnd->vout, &wnd->pos_x, &wnd->pos_y,
+                                      &wnd->width, &wnd->height);
     if (!wnd->handle.xid)
         return VLC_EGENERIC;
 
 #elif defined (WIN32)
-    wnd->handle.hwnd = p_mi->requestVideo (wnd->vout, &wnd->pos_x, &wnd->pos_y,
-                                           &wnd->width, &wnd->height);
+    wnd->handle.hwnd = p_mi->getVideo (wnd->vout, &wnd->pos_x, &wnd->pos_y,
+                                       &wnd->width, &wnd->height);
     if (!wnd->handle.hwnd)
         return VLC_EGENERIC;
 




More information about the vlc-devel mailing list