[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