[vlc-devel] [RFC PATCH 4/5] decoder: fix deadlock when flushing paused

[Rémi Denis-Courmont]

On Friday 06 November 2015 14:48:46 Thomas Guillem wrote:
> Unpause the decoder when flushing, then restore previous paused state

Now there is indeed a deadlock here right now. But:

1) I think flushing should be moved to the decoder thread loop like the rest 
(will finish and test tomorrow) - pending hypothetical asynchronous decoding,
2) resuming to flush might cause annoying audio glitches.

> ---
>  src/input/decoder.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 0d52f2e..ec3d6da 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -112,6 +112,7 @@ struct decoder_owner_sys_t
>      bool b_draining;
>      bool b_drained;
>      bool b_idle;
> +    bool b_pause_after_flush;
> 
>      /* CC */
>      struct
> @@ -1335,7 +1336,16 @@ static void DecoderProcessOnFlush( decoder_t *p_dec )
> p_owner->b_flushing = false;
>          vlc_cond_signal( &p_owner->wait_acknowledge );
>      }
> +    bool b_pause_after_flush = p_owner->b_pause_after_flush;
> +    p_owner->b_pause_after_flush = false;
>      vlc_mutex_unlock( &p_owner->lock );
> +
> +    if( b_pause_after_flush )
> +    {
> +        vlc_fifo_Lock( p_owner->p_fifo );
> +        p_owner->paused = true;
> +        vlc_fifo_Unlock( p_owner->p_fifo );
> +    }
>  }
> 
>  /**
> @@ -1554,6 +1564,7 @@ static decoder_t * CreateDecoder( vlc_object_t
> *p_parent, p_owner->b_draining = false;
>      p_owner->b_drained = false;
>      p_owner->b_idle = false;
> +    p_owner->b_pause_after_flush = p_owner->paused;
> 
>      es_format_Init( &p_owner->fmt, UNKNOWN_ES, 0 );
> 
> @@ -1948,6 +1959,13 @@ static void DecoderFlush( decoder_t *p_dec )
>       && p_owner->frames_countdown == 0 )
>          p_owner->frames_countdown++;
> 
> +    if( p_owner->paused )
> +    {
> +        p_owner->b_pause_after_flush = true;
> +        p_owner->paused = false;
> +        vlc_fifo_Signal( p_owner->p_fifo );
> +    }
> +    vlc_fifo_Signal( p_owner->p_fifo );
>      vlc_fifo_Unlock( p_owner->p_fifo );
> 
>      /* Monitor for flush end */
> @@ -2068,6 +2086,14 @@ void input_DecoderChangePause( decoder_t *p_dec, bool
> b_paused, mtime_t i_date ) * 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 );
> +    if( !b_paused && !p_owner->paused )
> +    {
> +        /* If we are flushing and not paused anymore, cancel pause after
> flush +         * */
> +        vlc_mutex_lock( &p_owner->lock );

Lock inversion?

> +        p_owner->b_pause_after_flush = false;
> +        vlc_mutex_unlock( &p_owner->lock );
> +    }
>      p_owner->paused = b_paused;
>      p_owner->pause_date = i_date;
>      p_owner->frames_countdown = 0;

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