[vlc-devel] [PATCH] decoder: merge conditions and locks
Thomas Guillem
thomas at gllm.fr
Wed Apr 1 19:19:20 CEST 2015
On Wed, Apr 1, 2015, at 19:16, Rémi Denis-Courmont wrote:
> 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.
OK, I'll try to do another patch that keeps the FIFO lock.
>
> > @@ -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/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list