[vlc-devel] [PATCH 2/3] decoder: merge the decoder lock and the fifo lock

Thomas Guillem thomas at gllm.fr
Tue Oct 10 11:37:35 CEST 2017


Having one lock for the input and the decoder thread make the code easier to
maintain. This commit re-implements vlc_fifo using block_Chain* API. This
allows to use the decoder's own lock and conditions to signal/lock the fifo.
---
 src/input/decoder.c | 197 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 130 insertions(+), 67 deletions(-)

diff --git a/src/input/decoder.c b/src/input/decoder.c
index 39ccd1acf0..c739402ac5 100644
--- a/src/input/decoder.c
+++ b/src/input/decoder.c
@@ -94,8 +94,14 @@ struct decoder_owner_sys_t
     vlc_meta_t     *p_description;
     atomic_int     reload;
 
-    /* fifo */
-    block_fifo_t *p_fifo;
+    struct
+    {
+        vlc_cond_t wait;
+        block_t *p_first;
+        block_t **pp_last;
+        size_t i_size;
+        size_t i_depth;
+    } fifo;
 
     /* Lock for communication with decoder thread */
     vlc_mutex_t lock;
@@ -151,6 +157,76 @@ struct decoder_owner_sys_t
 #define DECODER_SPU_VOUT_WAIT_DURATION ((int)(0.200*CLOCK_FREQ))
 #define BLOCK_FLAG_CORE_PRIVATE_RELOADED (1 << BLOCK_FLAG_CORE_PRIVATE_SHIFT)
 
+static void dec_fifo_Reset( decoder_owner_sys_t *p_owner )
+{
+    p_owner->fifo.p_first = NULL;
+    p_owner->fifo.pp_last = &p_owner->fifo.p_first;
+    p_owner->fifo.i_size = p_owner->fifo.i_depth = 0;
+}
+
+static void dec_fifo_Init( decoder_owner_sys_t *p_owner )
+{
+    vlc_cond_init( &p_owner->fifo.wait );
+    dec_fifo_Reset( p_owner );
+}
+
+static void dec_fifo_Destroy( decoder_owner_sys_t *p_owner )
+{
+    block_ChainRelease( p_owner->fifo.p_first );
+    vlc_cond_destroy( &p_owner->fifo.wait );
+}
+
+static void dec_fifo_ReleaseAll( decoder_owner_sys_t *p_owner )
+{
+    block_ChainRelease( p_owner->fifo.p_first );
+    dec_fifo_Reset( p_owner );
+}
+
+static void dec_fifo_Queue( decoder_owner_sys_t *p_owner, block_t *p_block )
+{
+    block_ChainLastAppend( &p_owner->fifo.pp_last, p_block );
+    p_owner->fifo.i_size += p_block->i_buffer;
+    p_owner->fifo.i_depth++;
+    vlc_cond_signal( &p_owner->fifo.wait );
+}
+
+static void dec_fifo_QueueUnlocked( decoder_owner_sys_t *p_owner, block_t *p_block )
+{
+    vlc_mutex_lock( &p_owner->lock );
+    dec_fifo_Queue( p_owner, p_block );
+    vlc_mutex_unlock( &p_owner->lock );
+}
+
+static block_t *dec_fifo_Dequeue( decoder_owner_sys_t *p_owner )
+{
+    block_t *p_block = p_owner->fifo.p_first;
+    if( !p_block )
+        return NULL;
+    p_owner->fifo.p_first = p_block->p_next;
+    if( !p_owner->fifo.p_first )
+        p_owner->fifo.pp_last = &p_owner->fifo.p_first;
+
+    p_block->p_next = NULL;
+    p_owner->fifo.i_size -= p_block->i_buffer;
+    p_owner->fifo.i_depth--;
+    return p_block;
+}
+
+static size_t dec_fifo_GetCount( decoder_owner_sys_t *p_owner )
+{
+    return p_owner->fifo.i_depth;
+}
+
+static size_t dec_fifo_GetBytes( decoder_owner_sys_t *p_owner )
+{
+    return p_owner->fifo.i_size;
+}
+
+static bool dec_fifo_IsEmpty( decoder_owner_sys_t *p_owner )
+{
+    return p_owner->fifo.p_first == NULL;
+}
+
 /**
  * Load a decoder module
  */
@@ -737,12 +813,12 @@ static int DecoderTimedWait( decoder_t *p_dec, mtime_t deadline )
     if (deadline - mdate() <= 0)
         return VLC_SUCCESS;
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
     while( !p_owner->flushing
-        && vlc_fifo_TimedWaitCond( p_owner->p_fifo, &p_owner->wait_timed,
-                                   deadline ) == 0 );
+        && vlc_cond_timedwait( &p_owner->wait_timed, &p_owner->lock,
+                               deadline ) == 0 );
     int ret = p_owner->flushing ? VLC_EGENERIC : VLC_SUCCESS;
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    vlc_mutex_unlock( &p_owner->lock );
     return ret;
 }
 
@@ -927,8 +1003,8 @@ static void DecoderPlayCc( decoder_t *p_dec, block_t *p_cc,
         if( !p_owner->cc.pp_decoder[i] )
             continue;
 
-        block_FifoPut( p_owner->cc.pp_decoder[i]->p_owner->p_fifo,
-            (i_cc_decoder > 1) ? block_Duplicate(p_cc) : p_cc);
+        dec_fifo_QueueUnlocked( p_owner->cc.pp_decoder[i]->p_owner,
+                                i_cc_decoder > 1 ? block_Duplicate(p_cc) : p_cc );
 
         i_cc_decoder--;
         b_processed = true;
@@ -1032,14 +1108,13 @@ static int DecoderPlayVideo( decoder_t *p_dec, picture_t *p_picture,
     DecoderFixTs( p_dec, &p_picture->date, NULL, NULL,
                   &i_rate, DECODER_BOGUS_VIDEO_DELAY );
 
-    vlc_mutex_unlock( &p_owner->lock );
-
     /* FIXME: The *input* FIFO should not be locked here. This will not work
      * properly if/when pictures are queued asynchronously. */
-    vlc_fifo_Lock( p_owner->p_fifo );
+
     if( unlikely(p_owner->paused) && likely(p_owner->frames_countdown > 0) )
         p_owner->frames_countdown--;
-    vlc_fifo_Unlock( p_owner->p_fifo );
+
+    vlc_mutex_unlock( &p_owner->lock );
 
     /* */
     if( p_vout == NULL )
@@ -1522,8 +1597,8 @@ static void *DecoderThread( void *p_data )
     bool paused = false;
 
     /* The decoder's main loop */
-    vlc_fifo_Lock( p_owner->p_fifo );
-    vlc_fifo_CleanupPush( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
+    mutex_cleanup_push( &p_owner->lock );
 
     for( ;; )
     {
@@ -1532,12 +1607,12 @@ static void *DecoderThread( void *p_data )
              * for the sake of flushing (glitches could otherwise happen). */
             int canc = vlc_savecancel();
 
-            vlc_fifo_Unlock( p_owner->p_fifo );
+            vlc_mutex_unlock( &p_owner->lock );
 
             /* Flush the decoder (and the output) */
             DecoderProcessFlush( p_dec );
 
-            vlc_fifo_Lock( p_owner->p_fifo );
+            vlc_mutex_lock( &p_owner->lock );
             vlc_restorecancel( canc );
 
             /* Reset flushing after DecoderProcess in case input_DecoderFlush
@@ -1554,7 +1629,7 @@ static void *DecoderThread( void *p_data )
             mtime_t date = p_owner->pause_date;
 
             paused = p_owner->paused;
-            vlc_fifo_Unlock( p_owner->p_fifo );
+            vlc_mutex_unlock( &p_owner->lock );
 
             /* NOTE: Only the audio and video outputs care about pause. */
             msg_Dbg( p_dec, "toggling %s", paused ? "resume" : "pause" );
@@ -1564,14 +1639,14 @@ static void *DecoderThread( void *p_data )
                 aout_DecChangePause( p_owner->p_aout, paused, date );
 
             vlc_restorecancel( canc );
-            vlc_fifo_Lock( p_owner->p_fifo );
+            vlc_mutex_lock( &p_owner->lock );
             continue;
         }
 
         if( p_owner->paused && p_owner->frames_countdown == 0 )
         {   /* Wait for resumption from pause */
             p_owner->b_idle = true;
-            vlc_fifo_Wait( p_owner->p_fifo );
+            vlc_cond_wait( &p_owner->fifo.wait, &p_owner->lock );
             p_owner->b_idle = false;
             continue;
         }
@@ -1579,13 +1654,13 @@ static void *DecoderThread( void *p_data )
         vlc_cond_signal( &p_owner->wait_fifo_consumed );
         vlc_testcancel(); /* forced expedited cancellation in case of stop */
 
-        block_t *p_block = vlc_fifo_DequeueUnlocked( p_owner->p_fifo );
+        block_t *p_block = dec_fifo_Dequeue( p_owner );
         if( p_block == NULL )
         {
             if( likely(!p_owner->b_draining) )
             {   /* Wait for a block to decode (or a request to drain) */
                 p_owner->b_idle = true;
-                vlc_fifo_Wait( p_owner->p_fifo );
+                vlc_cond_wait( &p_owner->fifo.wait, &p_owner->lock );
                 p_owner->b_idle = false;
                 continue;
             }
@@ -1593,7 +1668,7 @@ static void *DecoderThread( void *p_data )
              * drain. Pass p_block = NULL to decoder just once. */
         }
 
-        vlc_fifo_Unlock( p_owner->p_fifo );
+        vlc_mutex_unlock( &p_owner->lock );
 
         int canc = vlc_savecancel();
         DecoderProcess( p_dec, p_block );
@@ -1613,9 +1688,7 @@ static void *DecoderThread( void *p_data )
             p_owner->b_draining = false;
             p_owner->drained = true;
         }
-        vlc_fifo_Lock( p_owner->p_fifo );
         vlc_cond_signal( &p_owner->wait_acknowledge );
-        vlc_mutex_unlock( &p_owner->lock );
     }
     vlc_cleanup_pop();
     vlc_assert_unreachable();
@@ -1683,14 +1756,7 @@ static decoder_t * CreateDecoder( vlc_object_t *p_parent,
 
     es_format_Init( &p_owner->fmt, fmt->i_cat, 0 );
 
-    /* decoder fifo */
-    p_owner->p_fifo = block_FifoNew();
-    if( unlikely(p_owner->p_fifo == NULL) )
-    {
-        free( p_owner );
-        vlc_object_release( p_dec );
-        return NULL;
-    }
+    dec_fifo_Init( p_owner );
 
     vlc_mutex_init( &p_owner->lock );
     vlc_cond_init( &p_owner->wait_request );
@@ -1800,7 +1866,7 @@ static void DeleteDecoder( decoder_t * p_dec )
     UnloadDecoder( p_dec );
 
     /* Free all packets still in the decoder fifo. */
-    block_FifoRelease( p_owner->p_fifo );
+    dec_fifo_ReleaseAll( p_owner );
 
     /* Cleanup */
     if( p_owner->p_aout )
@@ -1852,6 +1918,8 @@ static void DeleteDecoder( decoder_t * p_dec )
         vlc_object_release( p_owner->p_packetizer );
     }
 
+    dec_fifo_Destroy( p_owner );
+
     vlc_cond_destroy( &p_owner->wait_timed );
     vlc_cond_destroy( &p_owner->wait_fifo_consumed );
     vlc_cond_destroy( &p_owner->wait_acknowledge );
@@ -1967,14 +2035,12 @@ void input_DecoderDelete( decoder_t *p_dec )
 
     vlc_cancel( p_owner->thread );
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
     /* Signal DecoderTimedWait */
     p_owner->flushing = true;
     vlc_cond_signal( &p_owner->wait_timed );
-    vlc_fifo_Unlock( p_owner->p_fifo );
 
     /* Make sure we aren't waiting/decoding anymore */
-    vlc_mutex_lock( &p_owner->lock );
     p_owner->b_waiting = false;
     vlc_cond_signal( &p_owner->wait_request );
 
@@ -2013,17 +2079,17 @@ void input_DecoderDecode( decoder_t *p_dec, block_t *p_block, bool b_do_pace )
 {
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
     if( !b_do_pace )
     {
         /* FIXME: ideally we would check the time amount of data
          * in the FIFO instead of its size. */
         /* 400 MiB, i.e. ~ 50mb/s for 60s */
-        if( vlc_fifo_GetBytes( p_owner->p_fifo ) > 400*1024*1024 )
+        if( dec_fifo_GetBytes( p_owner ) > 400*1024*1024 )
         {
             msg_Warn( p_dec, "decoder/packetizer fifo full (data not "
                       "consumed quickly enough), resetting fifo!" );
-            block_ChainRelease( vlc_fifo_DequeueAllUnlocked( p_owner->p_fifo ) );
+            dec_fifo_ReleaseAll( p_owner );
         }
     }
     else
@@ -2031,12 +2097,12 @@ void input_DecoderDecode( decoder_t *p_dec, block_t *p_block, bool b_do_pace )
     {   /* The FIFO is not consumed when waiting, so pacing would deadlock VLC.
          * Locking is not necessary as b_waiting is only read, not written by
          * the decoder thread. */
-        while( vlc_fifo_GetCount( p_owner->p_fifo ) >= 10 )
-            vlc_fifo_WaitCond( p_owner->p_fifo, &p_owner->wait_fifo_consumed );
+        while( dec_fifo_GetCount( p_owner ) >= 10 )
+            vlc_cond_wait( &p_owner->wait_fifo_consumed, &p_owner->lock );
     }
 
-    vlc_fifo_QueueUnlocked( p_owner->p_fifo, p_block );
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    dec_fifo_Queue( p_owner, p_block );
+    vlc_mutex_unlock( &p_owner->lock );
 }
 
 bool input_DecoderIsEmpty( decoder_t * p_dec )
@@ -2045,17 +2111,15 @@ bool input_DecoderIsEmpty( decoder_t * p_dec )
 
     assert( !p_owner->b_waiting );
 
-    vlc_fifo_Lock( p_owner->p_fifo );
-    if( !vlc_fifo_IsEmpty( p_dec->p_owner->p_fifo ) || p_owner->b_draining )
+    vlc_mutex_lock( &p_owner->lock );
+    if( !dec_fifo_IsEmpty( p_owner ) || p_owner->b_draining )
     {
-        vlc_fifo_Unlock( p_owner->p_fifo );
+        vlc_mutex_unlock( &p_owner->lock );
         return false;
     }
-    vlc_fifo_Unlock( p_owner->p_fifo );
 
     bool b_empty;
 
-    vlc_mutex_lock( &p_owner->lock );
 #ifdef ENABLE_SOUT
     if( p_owner->p_sout_input != NULL )
         b_empty = sout_InputIsEmpty( p_owner->p_sout_input );
@@ -2084,10 +2148,10 @@ void input_DecoderDrain( decoder_t *p_dec )
 {
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
     p_owner->b_draining = true;
-    vlc_fifo_Signal( p_owner->p_fifo );
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    vlc_cond_signal( &p_owner->fifo.wait );
+    vlc_mutex_unlock( &p_owner->lock );
 }
 
 /**
@@ -2098,10 +2162,10 @@ void input_DecoderFlush( decoder_t *p_dec )
 {
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
 
     /* Empty the fifo */
-    block_ChainRelease( vlc_fifo_DequeueAllUnlocked( p_owner->p_fifo ) );
+    dec_fifo_ReleaseAll( p_owner );
 
     /* Don't need to wait for the DecoderThread to flush. Indeed, if called a
      * second time, this function will clear the FIFO again before anything was
@@ -2115,10 +2179,10 @@ void input_DecoderFlush( decoder_t *p_dec )
      && p_owner->frames_countdown == 0 )
         p_owner->frames_countdown++;
 
-    vlc_fifo_Signal( p_owner->p_fifo );
+    vlc_cond_signal( &p_owner->fifo.wait );
     vlc_cond_signal( &p_owner->wait_timed );
 
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    vlc_mutex_unlock( &p_owner->lock );
 }
 
 void input_DecoderIsCcPresent( decoder_t *p_dec, bool pb_present[4] )
@@ -2206,12 +2270,13 @@ void input_DecoderChangePause( decoder_t *p_dec, bool b_paused, mtime_t i_date )
     /* Normally, p_owner->b_paused != b_paused here. But if a track is added
      * while the input is paused (e.g. add sub file), then b_paused is
      * (incorrectly) false. FIXME: This is a bug in the decoder owner. */
-    vlc_fifo_Lock( p_owner->p_fifo );
+
+    vlc_mutex_lock( &p_owner->lock );
     p_owner->paused = b_paused;
     p_owner->pause_date = i_date;
     p_owner->frames_countdown = 0;
-    vlc_fifo_Signal( p_owner->p_fifo );
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    vlc_cond_signal( &p_owner->fifo.wait );
+    vlc_mutex_unlock( &p_owner->lock );
 }
 
 void input_DecoderChangeDelay( decoder_t *p_dec, mtime_t i_delay )
@@ -2262,14 +2327,11 @@ void input_DecoderWait( decoder_t *p_dec )
          * owner */
         if( p_owner->paused )
             break;
-        vlc_fifo_Lock( p_owner->p_fifo );
-        if( p_owner->b_idle && vlc_fifo_IsEmpty( p_owner->p_fifo ) )
+        if( p_owner->b_idle && dec_fifo_IsEmpty( p_owner ) )
         {
             msg_Err( p_dec, "buffer deadlock prevented" );
-            vlc_fifo_Unlock( p_owner->p_fifo );
             break;
         }
-        vlc_fifo_Unlock( p_owner->p_fifo );
         vlc_cond_wait( &p_owner->wait_acknowledge, &p_owner->lock );
     }
     vlc_mutex_unlock( &p_owner->lock );
@@ -2282,12 +2344,10 @@ void input_DecoderFrameNext( decoder_t *p_dec, mtime_t *pi_duration )
     assert( p_owner->paused );
     *pi_duration = 0;
 
-    vlc_fifo_Lock( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
     p_owner->frames_countdown++;
-    vlc_fifo_Signal( p_owner->p_fifo );
-    vlc_fifo_Unlock( p_owner->p_fifo );
+    vlc_cond_signal( &p_owner->fifo.wait );
 
-    vlc_mutex_lock( &p_owner->lock );
     if( p_owner->fmt.i_cat == VIDEO_ES )
     {
         if( p_owner->p_vout )
@@ -2328,7 +2388,10 @@ size_t input_DecoderGetFifoSize( decoder_t *p_dec )
 {
     decoder_owner_sys_t *p_owner = p_dec->p_owner;
 
-    return block_FifoSize( p_owner->p_fifo );
+    vlc_mutex_lock( &p_owner->lock );
+    size_t i_size = dec_fifo_GetBytes( p_owner );
+    vlc_mutex_unlock( &p_owner->lock );
+    return i_size;
 }
 
 void input_DecoderGetObjects( decoder_t *p_dec,
-- 
2.11.0



More information about the vlc-devel mailing list