Revert "Revert "Solved multiple race condition when pausing""

This reverts commit c277adec.

src/LibVLCpp/VLCMediaPlayer.cpp

@@ -73,12 +73,13 @@ void                            MediaPlayer::callbacks( const libvlc_event_t* ev
     MediaPlayer* self = reinterpret_cast<MediaPlayer*>( ptr );
     switch ( event->type )
     {
+        qDebug() << "Event received" << event->type;
     case libvlc_MediaPlayerPlaying:
 //        qDebug() << "Media player playing";
         self->emit playing();
         break;
     case libvlc_MediaPlayerPaused:
-//        qDebug() << "Media player paused";
+        qDebug() << "Media player paused";
         self->emit paused();
         break;
     case libvlc_MediaPlayerStopped:

src/Workflow/ClipWorkflow.cpp

@@ -38,7 +38,8 @@ ClipWorkflow::ClipWorkflow( Clip::Clip* clip ) :
 //    m_backBuffer = new unsigned char[VIDEOHEIGHT * VIDEOWIDTH * 4];
     m_stateLock = new QReadWriteLock;
     m_requiredStateLock = new QMutex;
-    m_waitCond = new WaitCondition;
+    m_waitCond = new QWaitCondition;
+    m_condMutex = new QMutex;
     m_initWaitCond = new WaitCondition;
     m_renderWaitCond = new WaitCondition;
     m_pausingStateWaitCond = new WaitCondition;
@@ -51,6 +52,7 @@ ClipWorkflow::~ClipWorkflow()
 //    delete m_backBufferLock;
     delete m_pausingStateWaitCond;
     delete m_initWaitCond;
+    delete m_condMutex;
     delete m_waitCond;
     delete m_requiredStateLock;
     delete m_stateLock;
@@ -71,11 +73,13 @@ void    ClipWorkflow::checkStateChange()
     QWriteLocker    lock2( m_stateLock );
     if ( m_requiredState != ClipWorkflow::None )
     {
-//        qDebug() << "Changed state from" << m_state << "to state" << m_requiredState;
+        qDebug() << "Changed state from" << m_state << "to state" << m_requiredState;
         m_state = m_requiredState;
         m_requiredState = ClipWorkflow::None;
         checkSynchronisation( m_state );
     }
+    else
+        qDebug() << "No state required";
 }
 
 void    ClipWorkflow::lock( ClipWorkflow* cw, void** pp_ret )
@@ -108,13 +112,15 @@ void    ClipWorkflow::unlock( ClipWorkflow* cw )
 //        qDebug() << "One frame only mode is OFF :(";
     if ( cw->m_state == Rendering )
     {
+        QMutexLocker    lock( cw->m_condMutex );
+
         cw->m_state = Sleeping;
         cw->m_stateLock->unlock();
         cw->m_renderWaitCond->wake();
 
-//            qDebug() << "Entering condwait";
-        cw->m_waitCond->wait();
-//            qDebug() << "Leaved condwait";
+        qDebug() << "Entering condwait";
+        cw->m_waitCond->wait( cw->m_condMutex );
+        qDebug() << "Leaved condwait";
         cw->m_stateLock->lockForWrite();
         cw->m_state = Rendering;
 //        {
@@ -125,9 +131,9 @@ void    ClipWorkflow::unlock( ClipWorkflow* cw )
     }
     else if ( cw->m_state == Paused )
     {
-//        qDebug() << "Forcing pause by pausing thread";
+        qDebug() << "Forcing pause inside of unlock";
         cw->m_stateLock->unlock();
-        cw->m_waitCond->wait();
+        cw->m_waitCond->wait( cw->m_condMutex );
     }
     else
         cw->m_stateLock->unlock();
@@ -197,6 +203,12 @@ bool    ClipWorkflow::isReady() const
     return m_state == ClipWorkflow::Ready;
 }
 
+bool    ClipWorkflow::isPausing() const
+{
+    QReadLocker lock( m_stateLock );
+    return m_state == ClipWorkflow::Pausing;
+}
+
 bool    ClipWorkflow::isEndReached() const
 {
     QReadLocker lock( m_stateLock );
@@ -269,6 +281,7 @@ void            ClipWorkflow::checkSynchronisation( State newState )
             m_initWaitCond->wake();
             break ;
         case Pausing:
+            qDebug() << "Waking m_pausingStateWaitCond";
             m_pausingStateWaitCond->wake();
             break ;
         default:
@@ -287,13 +300,15 @@ void            ClipWorkflow::setState( State state )
 
 void            ClipWorkflow::queryStateChange( State newState )
 {
+    qDebug() << "Querying state change to" << newState;
     QMutexLocker    lock( m_requiredStateLock );
     m_requiredState = newState;
 }
 
 void            ClipWorkflow::wake()
 {
-    m_waitCond->wake();
+    m_waitCond->wakeAll();
+    qDebug() << "Awaked thread";
 }
 
 QReadWriteLock* ClipWorkflow::getStateLock()
@@ -313,6 +328,7 @@ void            ClipWorkflow::pause()
     setState( Paused );
     m_mediaPlayer->pause();
     QMutexLocker    lock( m_requiredStateLock );
+    qDebug() << "ClipWorkflow::pause(); Reseting required state";
     m_requiredState = ClipWorkflow::None;
 }
 
@@ -347,6 +363,11 @@ void        ClipWorkflow::waitForCompleteInit()
 
 void        ClipWorkflow::waitForPausingState()
 {
-    if ( getState() != Pausing )
+    if ( isPausing() == false )
         m_pausingStateWaitCond->wait();
 }
+
+QMutex*     ClipWorkflow::getSleepMutex()
+{
+    return m_condMutex;
+}

src/Workflow/ClipWorkflow.h

@@ -91,6 +91,8 @@ class   ClipWorkflow : public QObject
          */
         bool                    isRendering() const;
 
+        bool                    isPausing() const;
+
         /**
          *  Returns the current workflow state.
          *  Be carrefull, as this function is NOT thread safe, and return the
@@ -150,6 +152,7 @@ class   ClipWorkflow : public QObject
         void                    waitForCompleteInit();
         void                    waitForCompleteRender();
         void                    waitForPausingState();
+        QMutex*                 getSleepMutex();
 
 //        void                    activateOneFrameOnly();
 
@@ -188,7 +191,7 @@ class   ClipWorkflow : public QObject
         LibVLCpp::MediaPlayer*  m_mediaPlayer;
 
         QMutex*                 m_condMutex;
-        WaitCondition*          m_waitCond;
+        QWaitCondition*         m_waitCond;
 
         State                   m_state;
         QReadWriteLock*         m_stateLock;

src/Workflow/MainWorkflow.cpp

@@ -47,6 +47,7 @@ MainWorkflow::MainWorkflow( int trackCount ) :
         connect( m_tracks[i], SIGNAL( trackEndReached( unsigned int ) ), this, SLOT( trackEndReached(unsigned int) ) );
     }
     m_renderStartedLock = new QReadWriteLock;
+    m_renderMutex = new QMutex;
 }
 
 MainWorkflow::~MainWorkflow()
@@ -98,6 +99,7 @@ void    MainWorkflow::startRender()
 unsigned char*    MainWorkflow::getOutput()
 {
     QReadLocker     lock( m_renderStartedLock );
+    QMutexLocker    lock2( m_renderMutex );
 
     if ( m_renderStarted == true )
     {
@@ -122,11 +124,15 @@ unsigned char*    MainWorkflow::getOutput()
 
 void        MainWorkflow::pause()
 {
+    QMutexLocker    lock( m_renderMutex );
+
+    qDebug() << "Pausing.......................";
     for ( unsigned int i = 0; i < m_trackCount; ++i )
     {
         if ( m_tracks[i].activated() == true )
             m_tracks[i]->pause();
     }
+    qDebug() << "Pausing completed <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<\n\n";
 }
 
 void        MainWorkflow::nextFrame()

src/Workflow/MainWorkflow.h

@@ -26,6 +26,7 @@
 
 #include <QObject>
 #include <QReadWriteLock>
+#include <QMutex>
 
 #include "tools/Toggleable.hpp"
 #include "TrackWorkflow.h"
@@ -96,6 +97,8 @@ class   MainWorkflow : public QObject, public Singleton<MainWorkflow>
         bool                            m_renderStarted;
         QReadWriteLock*                 m_renderStartedLock;
 
+        QMutex*                         m_renderMutex;
+
     public slots:
         void                            clipMoved( QUuid, int, int, qint64 );
 

src/Workflow/TrackWorkflow.cpp

@@ -128,9 +128,8 @@ unsigned char*      TrackWorkflow::renderClip( ClipWorkflow* cw, qint64 currentF
 //            qDebug() << "Querying state back to pause after render";
             cw->queryStateChange( ClipWorkflow::Paused );
         }
+        qDebug() << ">>> Awaking ClipWorkflow thread for render";
         cw->wake();
-        //FIXME: sometimes, the renderer isn't awake soon enough, and we can
-        //pass though this function many times before the frame is actually rendered.
     }
     else if ( cw->getState() == ClipWorkflow::Stopped )
     {
@@ -265,7 +264,8 @@ unsigned char*      TrackWorkflow::getOutput( qint64 currentFrame )
     bool                                        needRepositioning;
     bool                                        oneFrameOnlyFlag = false;
 
-//    qDebug() << "Checking flag...";
+qDebug() << "Asking for track output";
+    //    qDebug() << "Checking flag...";
     if ( m_oneFrameOnly == 1 )
     {
 //        qDebug() << "...Flag is activated";
@@ -341,20 +341,29 @@ void            TrackWorkflow::pauseClipWorkflow( ClipWorkflow* cw )
          cw->getState() == ClipWorkflow::Ready ||
          cw->getState() == ClipWorkflow::EndReached )
     {
+        qDebug() << "Pausing a sleeping, ready or EndReached ClipWorkflow";
         cw->getStateLock()->unlock();
         cw->queryStateChange( ClipWorkflow::Pausing );
         cw->wake();
     }
     else if ( cw->getState() == ClipWorkflow::Rendering )
     {
+        qDebug() << "Pausing a rendering clip workflow";
         cw->getStateLock()->unlock();
+        qDebug() << "Waiting for render complete";
         cw->waitForCompleteRender();
+        qDebug() << "Waiting complete";
+        qDebug() << "Waiting for sleep mutex";
+        QMutexLocker    lock( cw->getSleepMutex() );
+        qDebug() << "Got sleep mutex";
         cw->queryStateChange( ClipWorkflow::Pausing );
         cw->wake();
     }
     else if ( cw->getState() == ClipWorkflow::Initializing )
     {
+        qDebug() << "Pausing a Initializing ClipWorkflow";
         cw->getStateLock()->unlock();
+        //TODO: since a Initializing clipworkflow will pause itself at the end, shouldn't we do nothing ?
         cw->waitForCompleteInit();
     }
     else
@@ -362,8 +371,11 @@ void            TrackWorkflow::pauseClipWorkflow( ClipWorkflow* cw )
         qDebug() << "Unexpected ClipWorkflow::State when pausing:" << cw->getState();
         cw->getStateLock()->unlock();
     }
+    qDebug() << "Waiting for pausing state";
     cw->waitForPausingState();
+    qDebug() << "Waiting ok. Pausing now... ";
     cw->pause();
+    qDebug() << "Clip paused.";
 }
 
 void                TrackWorkflow::pause()
@@ -374,6 +386,7 @@ void                TrackWorkflow::pause()
     QMap<qint64, ClipWorkflow*>::iterator       end = m_clips.end();
 
     //FIXME: it's probably bad to iterate over every clip workflows.
+    qDebug() << "Started track pause loop";
     for ( ; it != end; ++it )
     {
         ClipWorkflow*   cw = it.value();
@@ -393,9 +406,11 @@ void                TrackWorkflow::pause()
         {
             //This should never be used.
             //TODO: remove this in a few revision (wrote on July 16 2009 )
+            qDebug() << "State before crash is:" << cw->getState();
             Q_ASSERT( false );
         }
     }
+    qDebug() << "End of loop";
     m_paused = !m_paused;
 }
 

src/renderer/WorkflowRenderer.cpp

@@ -85,9 +85,10 @@ void*   WorkflowRenderer::lock( void* datas )
 
     if ( self->m_oneFrameOnly < 2 )
     {
-//        qDebug() << "\nQuerying new picture";
+        qDebug() << "\nQuerying new picture";
         void* ret = self->m_mainWorkflow->getOutput();
         self->m_lastFrame = static_cast<unsigned char*>( ret );
+        qDebug() << "Got picture";
         return ret;
     }
     else
@@ -151,6 +152,13 @@ void        WorkflowRenderer::previousFrame()
 
 }
 
+void        WorkflowRenderer::pauseMainWorkflow()
+{
+    qDebug() << "In pause callback";
+    m_mainWorkflow->pause();
+    m_paused = true;
+}
+
 void        WorkflowRenderer::togglePlayPause( bool forcePause )
 {
     //If force pause is true, we just ensure that this render is paused... no need to start it.
@@ -160,9 +168,8 @@ void        WorkflowRenderer::togglePlayPause( bool forcePause )
     {
         if ( m_paused == true && forcePause == false )
         {
+            //This will automaticly unpause the ClipWorkflow... no worries
             m_mediaPlayer->play();
-            //This will automaticly unpause... no worries
-//            m_mainWorkflow->pause();
             m_paused = false;
         }
         else
@@ -172,8 +179,7 @@ void        WorkflowRenderer::togglePlayPause( bool forcePause )
             if ( m_paused == false )
             {
                 m_mediaPlayer->pause();
-                m_mainWorkflow->pause();
-                m_paused = true;
+                qDebug() << "Waiting for paused media player";
             }
         }
     }
@@ -213,6 +219,7 @@ void        WorkflowRenderer::__videoPaused()
     {
         m_oneFrameOnly = 0;
     }
+    pauseMainWorkflow();
     emit paused();
 }
 

src/renderer/WorkflowRenderer.h

@@ -59,6 +59,7 @@ class   WorkflowRenderer : public GenericRenderer
         static void         unlock( void* datas );
 
     private:
+        void                pauseMainWorkflow();
         virtual void        startPreview();
 
     private:

src/tools/WaitCondition.hpp

@@ -48,6 +48,10 @@ class   WaitCondition
         {
             m_waitCond->wakeAll();
         }
+        QMutex*         getMutex()
+        {
+            return m_mutex;
+        }
     private:
         QMutex*         m_mutex;
         QWaitCondition* m_waitCond;