[vlc-devel] [PATCH] decoder: merge conditions and locks

Rémi Denis-Courmont remi at remlab.net
Wed Apr 1 19:16:28 CEST 2015


Le mercredi 01 avril 2015, 19:10:39 Thomas Guillem a écrit :
>  - Merge wait_fifo condition with wait_acknowledge condition.
>  - Merge Fifo condition with wait_request condition.
>  - Merge Fifo lock with owner lock. The Fifo is not locked by its own lock
>    anymore.
> ---
>  src/input/decoder.c | 61
> ++++++++++++++++++++++++++++------------------------- 1 file changed, 32
> insertions(+), 29 deletions(-)
> 
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 76c4038..c2cf590 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -89,7 +89,6 @@ struct decoder_owner_sys_t
>      vlc_mutex_t lock;
>      vlc_cond_t  wait_request;
>      vlc_cond_t  wait_acknowledge;
> -    vlc_cond_t  wait_fifo; /* TODO: merge with wait_acknowledge */
> 
>      /* -- These variables need locking on write(only) -- */
>      audio_output_t *p_aout;
> @@ -1421,10 +1420,10 @@ static void *DecoderThread( void *p_data )
>      {
>          block_t *p_block;
> 
> -        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 );
> 
> -        vlc_cond_signal( &p_owner->wait_fifo );
> +        vlc_cond_signal( &p_owner->wait_acknowledge );
> 
>          while( vlc_fifo_IsEmpty( p_owner->p_fifo ) )
>          {
> @@ -1436,7 +1435,9 @@ static void *DecoderThread( void *p_data )
>              }
> 
>              p_owner->b_idle = true;
> -            vlc_fifo_Wait( p_owner->p_fifo );
> +            vlc_cond_signal( &p_owner->wait_acknowledge );
> +            vlc_cond_wait( &p_owner->wait_request, &p_owner->lock );
> +
>              /* Make sure there is no cancellation point other than this
> one^^. * If you need one, be sure to push cleanup of p_block. */
> p_owner->b_idle = false;
> @@ -1597,7 +1598,6 @@ static decoder_t * CreateDecoder( vlc_object_t
> *p_parent, vlc_mutex_init( &p_owner->lock );
>      vlc_cond_init( &p_owner->wait_request );
>      vlc_cond_init( &p_owner->wait_acknowledge );
> -    vlc_cond_init( &p_owner->wait_fifo );
> 
>      p_owner->b_fmt_description = false;
>      p_owner->p_description = NULL;
> @@ -1711,7 +1711,6 @@ static void DeleteDecoder( decoder_t * p_dec )
>          vlc_object_release( p_owner->p_packetizer );
>      }
> 
> -    vlc_cond_destroy( &p_owner->wait_fifo );
>      vlc_cond_destroy( &p_owner->wait_acknowledge );
>      vlc_cond_destroy( &p_owner->wait_request );
>      vlc_mutex_destroy( &p_owner->lock );
> @@ -1852,18 +1851,13 @@ void input_DecoderDelete( decoder_t *p_dec )
>      DeleteDecoder( p_dec );
>  }
> 
> -/**
> - * Put a block_t in the decoder's fifo.
> - * Thread-safe w.r.t. the decoder. May be a cancellation point.
> - *
> - * \param p_dec the decoder object
> - * \param p_block the data block
> - */
> -void input_DecoderDecode( decoder_t *p_dec, block_t *p_block, bool
> b_do_pace ) +static void DecoderDecodeLocked( 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_assert_locked( &p_owner->lock );
> +
>      if( !b_do_pace )
>      {
>          /* FIXME: ideally we would check the time amount of data
> @@ -1882,11 +1876,27 @@ void input_DecoderDecode( decoder_t *p_dec, block_t
> *p_block, bool b_do_pace ) * 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 );
> +            vlc_cond_wait( &p_owner->wait_acknowledge, &p_owner->lock );
>      }
> 
>      vlc_fifo_QueueUnlocked( p_owner->p_fifo, p_block );
> -    vlc_fifo_Unlock( p_owner->p_fifo );
> +    vlc_cond_signal( &p_owner->wait_request );
> +}
> +
> +/**
> + * Put a block_t in the decoder's fifo.
> + * Thread-safe w.r.t. the decoder. May be a cancellation point.
> + *
> + * \param p_dec the decoder object
> + * \param p_block the data block
> + */
> +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_mutex_lock( &p_owner->lock );
> +    DecoderDecodeLocked( p_dec, p_block, b_do_pace );
> +    vlc_mutex_unlock( &p_owner->lock );
>  }
> 
>  bool input_DecoderIsEmpty( decoder_t * p_dec )
> @@ -1924,10 +1934,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->wait_request );
> +    vlc_mutex_unlock( &p_owner->lock );
>  }
> 
>  static void DecoderFlush( decoder_t *p_dec )
> @@ -1936,11 +1946,9 @@ static void DecoderFlush( decoder_t *p_dec )
> 
>      vlc_assert_locked( &p_owner->lock );
> 
> -    vlc_fifo_Lock( p_owner->p_fifo );
>      /* Empty the fifo */
>      block_ChainRelease( vlc_fifo_DequeueAllUnlocked( p_owner->p_fifo ) );
>      p_owner->b_draining = false; /* flush supersedes drain */
> -    vlc_fifo_Unlock( p_owner->p_fifo );
> 
>      /* Monitor for flush end */
>      p_owner->b_flushing = true;
> @@ -1950,7 +1958,7 @@ static void DecoderFlush( decoder_t *p_dec )
>      block_t *p_null = DecoderBlockFlushNew();
>      if( !p_null )
>          return;
> -    input_DecoderDecode( p_dec, p_null, false );
> +    DecoderDecodeLocked( p_dec, p_null, false );
> 
>      /* */
>      while( p_owner->b_flushing )
> @@ -2127,7 +2135,6 @@ void input_DecoderWait( decoder_t *p_dec )
>      assert( p_owner->b_waiting );
> 
>      vlc_mutex_lock( &p_owner->lock );
> -    vlc_fifo_Lock( p_owner->p_fifo );
>      while( !p_owner->b_has_data )
>      {
>          if( p_owner->b_idle && vlc_fifo_IsEmpty( p_owner->p_fifo ) )

You cannot do that without the FIFO lock. The whole patch is busted.

> @@ -2135,12 +2142,8 @@ void input_DecoderWait( decoder_t *p_dec )
>              msg_Warn( p_dec, "can't wait without data to decode" );
>              break;
>          }
> -        vlc_fifo_Signal( p_owner->p_fifo );
> -        vlc_fifo_Unlock( p_owner->p_fifo );
>          vlc_cond_wait( &p_owner->wait_acknowledge, &p_owner->lock );
> -        vlc_fifo_Lock( p_owner->p_fifo );
>      }
> -    vlc_fifo_Unlock( p_owner->p_fifo );
>      vlc_mutex_unlock( &p_owner->lock );
>  }

-- 
Rémi Denis-Courmont
http://www.remlab.net/




More information about the vlc-devel mailing list