[vlmc-devel] commit: ClipWorkflow: Reworked thread safety. (Hugo Beauzee-Luyssen )

[git at videolan.org]

vlmc | branch: master | Hugo Beauzee-Luyssen <beauze.h at gmail.com> | Sat Mar 27 17:56:15 2010 +0100| [4293db3d74bd61a7eb0d87bf09b2767be400f1f3] | committer: Hugo Beauzee-Luyssen 

ClipWorkflow: Reworked thread safety.

This removes 3 useless mutex, and avoid a race condition.

> http://git.videolan.org/gitweb.cgi/vlmc.git/?a=commit;h=4293db3d74bd61a7eb0d87bf09b2767be400f1f3
---

 src/Workflow/AudioClipWorkflow.cpp |    9 ++-------
 src/Workflow/ClipWorkflow.cpp      |    6 +-----
 src/Workflow/ClipWorkflow.h        |    7 +++----
 src/Workflow/VideoClipWorkflow.cpp |   28 +++++-----------------------
 4 files changed, 11 insertions(+), 39 deletions(-)

diff --git a/src/Workflow/AudioClipWorkflow.cpp b/src/Workflow/AudioClipWorkflow.cpp
index 32644b6..9ebd148 100644
--- a/src/Workflow/AudioClipWorkflow.cpp
+++ b/src/Workflow/AudioClipWorkflow.cpp
@@ -81,7 +81,6 @@ void*
 AudioClipWorkflow::getOutput( ClipWorkflow::GetMode mode )
 {
     QMutexLocker    lock( m_renderLock );
-    QMutexLocker    lock2( m_computedBuffersMutex );
 
     if ( preGetOutput() == false )
         return NULL;
@@ -138,9 +137,7 @@ AudioClipWorkflow::createBuffer( size_t size )
 void
 AudioClipWorkflow::lock( AudioClipWorkflow *cw, quint8 **pcm_buffer , quint32 size )
 {
-    QMutexLocker    lock( cw->m_availableBuffersMutex );
     cw->m_renderLock->lock();
-    cw->m_computedBuffersMutex->lock();
 
     AudioSample     *as = NULL;
     if ( cw->m_availableBuffers.isEmpty() == true )
@@ -192,7 +189,6 @@ AudioClipWorkflow::unlock( AudioClipWorkflow *cw, quint8 *pcm_buffer,
     }
     cw->commonUnlock();
     cw->m_renderLock->unlock();
-    cw->m_computedBuffersMutex->unlock();
 }
 
 void
@@ -229,15 +225,14 @@ AudioClipWorkflow::getMaxComputedBuffers() const
 void
 AudioClipWorkflow::releaseBuffer( AudioSample *sample )
 {
-    QMutexLocker    lock( m_availableBuffersMutex );
+    QMutexLocker    lock( m_renderLock );
     m_availableBuffers.enqueue( sample );
 }
 
 void
 AudioClipWorkflow::flushComputedBuffers()
 {
-    QMutexLocker    lock( m_availableBuffersMutex );
-    QMutexLocker    lock2( m_computedBuffersMutex );
+    QMutexLocker        lock( m_renderLock );
 
     while ( m_computedBuffers.isEmpty() == false )
     {
diff --git a/src/Workflow/ClipWorkflow.cpp b/src/Workflow/ClipWorkflow.cpp
index ef4e2ff..050ce99 100644
--- a/src/Workflow/ClipWorkflow.cpp
+++ b/src/Workflow/ClipWorkflow.cpp
@@ -43,16 +43,12 @@ ClipWorkflow::ClipWorkflow( Clip::Clip* clip ) :
     m_initWaitCond = new WaitCondition;
     m_pausingStateWaitCond = new WaitCondition;
     m_renderLock = new QMutex;
-    m_availableBuffersMutex = new QMutex;
-    m_computedBuffersMutex = new QMutex;
-    m_renderWaitCond = new WaitCondition;
+    m_renderWaitCond = new QWaitCondition;
 }
 
 ClipWorkflow::~ClipWorkflow()
 {
     delete m_renderWaitCond;
-    delete m_computedBuffersMutex;
-    delete m_availableBuffersMutex;
     delete m_renderLock;
     delete m_pausingStateWaitCond;
     delete m_initWaitCond;
diff --git a/src/Workflow/ClipWorkflow.h b/src/Workflow/ClipWorkflow.h
index c2dd17d..150067f 100644
--- a/src/Workflow/ClipWorkflow.h
+++ b/src/Workflow/ClipWorkflow.h
@@ -27,8 +27,9 @@
 
 #include <QObject>
 
-class   QReadWriteLock;
 class   QMutex;
+class   QReadWriteLock;
+class   QWaitCondition;
 
 class   Clip;
 class   WaitCondition;
@@ -241,12 +242,10 @@ class   ClipWorkflow : public QObject
          *          the clipworkflow hasn't generate a frame yet,
          *          while the renderer asks for one.
          */
-        QMutex*                 m_computedBuffersMutex;
-        QMutex*                 m_availableBuffersMutex;
+        QWaitCondition          *m_renderWaitCond;
         qint64                  m_beginPausePts;
         qint64                  m_pauseDuration;
         bool                    m_fullSpeedRender;
-        WaitCondition           *m_renderWaitCond;
 
     private slots:
         void                    loadingComplete();
diff --git a/src/Workflow/VideoClipWorkflow.cpp b/src/Workflow/VideoClipWorkflow.cpp
index 9d0774e..908d3fc 100644
--- a/src/Workflow/VideoClipWorkflow.cpp
+++ b/src/Workflow/VideoClipWorkflow.cpp
@@ -111,25 +111,11 @@ void*
 VideoClipWorkflow::getOutput( ClipWorkflow::GetMode mode )
 {
     QMutexLocker    lock( m_renderLock );
-    QMutexLocker    lock2( m_computedBuffersMutex );
 
     if ( isEndReached() == true )
         return NULL;
     if ( preGetOutput() == false )
-    {
-        QMutexLocker    waitLock( m_renderWaitCond->getMutex() );
-        m_computedBuffersMutex->unlock();
-        m_renderLock->unlock();
-
-        m_renderWaitCond->waitLocked();
-
-        m_renderLock->lock();
-        m_computedBuffersMutex->lock();
-    }
-    //FIXME
-    //TODO: This is a nasty work arround. The race condition needs to be fixed !
-    if ( m_computedBuffers.isEmpty() == true )
-        return NULL;
+        m_renderWaitCond->wait( m_renderLock );
     ::StackedBuffer<LightVideoFrame*>* buff;
     if ( mode == ClipWorkflow::Pop )
         buff = new StackedBuffer( m_computedBuffers.dequeue(), this, true );
@@ -143,11 +129,9 @@ void
 VideoClipWorkflow::lock( VideoClipWorkflow *cw, void **pp_ret, int size )
 {
     Q_UNUSED( size );
-    QMutexLocker        lock( cw->m_availableBuffersMutex );
     LightVideoFrame*    lvf = NULL;
 
     cw->m_renderLock->lock();
-    cw->m_computedBuffersMutex->lock();
     if ( cw->m_availableBuffers.isEmpty() == true )
     {
         lvf = new LightVideoFrame( cw->m_width, cw->m_height );
@@ -172,10 +156,8 @@ VideoClipWorkflow::unlock( VideoClipWorkflow *cw, void *buffer, int width,
     LightVideoFrame     *lvf = cw->m_computedBuffers.last();
     (*(lvf))->ptsDiff = cw->m_currentPts - cw->m_previousPts;
     cw->commonUnlock();
+    cw->m_renderWaitCond->wakeAll();
     cw->m_renderLock->unlock();
-    cw->m_computedBuffersMutex->unlock();
-    QMutexLocker    lock( cw->m_renderWaitCond->getMutex() );
-    cw->m_renderWaitCond->wake();
 }
 
 uint32_t
@@ -193,15 +175,15 @@ VideoClipWorkflow::getMaxComputedBuffers() const
 void
 VideoClipWorkflow::releaseBuffer( LightVideoFrame *lvf )
 {
-    QMutexLocker    lock( m_availableBuffersMutex );
+    QMutexLocker    lock( m_renderLock );
+
     m_availableBuffers.enqueue( lvf );
 }
 
 void
 VideoClipWorkflow::flushComputedBuffers()
 {
-    QMutexLocker    lock( m_computedBuffersMutex );
-    QMutexLocker    lock2( m_availableBuffersMutex );
+    QMutexLocker    lock( m_renderLock );
 
     while ( m_computedBuffers.isEmpty() == false )
         m_availableBuffers.enqueue( m_computedBuffers.dequeue() );