[vlc-devel] [PATCH VLC3] playlist: temporize on errors

[Rémi Denis-Courmont]

Le keskiviikkona 22. toukokuuta 2019, 17.13.28 EEST Romain Vimont a écrit :
> When repeat/loop is enabled, and items fail to play, VLC enters an
> infinite liveloop and flood errors.
> 
> We cannot just stop on errors, because retrying indefinitely may be the
> expected behavior:
> <https://trac.videolan.org/vlc/ticket/18582#comment:3>
> 
> Instead, wait some delay before starting the next item, depending on the
> number of consecutive errors:
>  - 1st error: 100ms
>  - 2nd error: 200ms
>  - 3rd error: 400ms
>  - 4th error: 800ms
>  - 5th error: 1.6s
>  - further errors: 3.2s
> 
> A single successful playback resets the errors counter.
> 
> Fixes #5901 #6245 #6339 #7305 #7798 #8893 #9230 #11066 #11961 #12104
>       #12909 #13878 #14679 #15805 #16697 #17660 #18582 #18942 #19284
>       #19313 #20365 #21564 #21672 #22118

This does not fix #5901. Error hell still crashes. In fact, instead of just 
making VLC barely responsive, the incomplete Qt patch series now takes the 
whole system down into thrashing hell.

> ---
>  src/input/input.c                |  2 ++
>  src/playlist/engine.c            |  1 +
>  src/playlist/playlist_internal.h |  1 +
>  src/playlist/thread.c            | 22 ++++++++++++++++++++--
>  4 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/src/input/input.c b/src/input/input.c
> index 0cb2729a40..ef8d95adac 100644
> --- a/src/input/input.c
> +++ b/src/input/input.c
> @@ -3217,6 +3217,8 @@ static void input_ChangeState( input_thread_t
> *p_input, int i_state ) input_priv(p_input)->i_state = i_state;
>      if( i_state == ERROR_S )
>          input_item_SetErrorWhenReading( input_priv(p_input)->p_item, true
> ); +    else if ( i_state == PLAYING_S )
> +        input_item_SetErrorWhenReading( input_priv(p_input)->p_item, false
> ); input_SendEventState( p_input, i_state );
>  }
> 
> diff --git a/src/playlist/engine.c b/src/playlist/engine.c
> index ede6c9b3ce..b914cd5e4f 100644
> --- a/src/playlist/engine.c
> +++ b/src/playlist/engine.c
> @@ -261,6 +261,7 @@ playlist_t *playlist_Create( vlc_object_t *p_parent )
>      pl_priv(p_playlist)->status.p_item = NULL;
>      pl_priv(p_playlist)->status.p_node = p_playlist->p_playing;
>      pl_priv(p_playlist)->request.b_request = false;
> +    pl_priv(p_playlist)->i_consecutive_errors = 0;
>      p->request.input_dead = false;
> 
>      if (ml != NULL)
> diff --git a/src/playlist/playlist_internal.h
> b/src/playlist/playlist_internal.h index 9537a5f270..990ab402ab 100644
> --- a/src/playlist/playlist_internal.h
> +++ b/src/playlist/playlist_internal.h
> @@ -87,6 +87,7 @@ typedef struct playlist_private_t
> 
>      int      i_last_playlist_id; /**< Last id to an item */
>      bool     b_reset_currently_playing; /** Reset current item array */
> +    int      i_consecutive_errors; /**< Number of consecutive items in
> error */
> 
>      bool     b_tree; /**< Display as a tree */
>      bool     b_preparse; /**< Preparse items */
> diff --git a/src/playlist/thread.c b/src/playlist/thread.c
> index ef96d92d3a..cd85c8088e 100644
> --- a/src/playlist/thread.c
> +++ b/src/playlist/thread.c
> @@ -421,7 +421,7 @@ static playlist_item_t *NextItem( playlist_t *p_playlist
> ) return p_new;
>  }
> 
> -static void LoopInput( playlist_t *p_playlist )
> +static bool LoopInput( playlist_t *p_playlist )
>  {
>      playlist_private_t *p_sys = pl_priv(p_playlist);
>      input_thread_t *p_input = p_sys->p_input;
> @@ -439,6 +439,10 @@ static void LoopInput( playlist_t *p_playlist )
>          vlc_cond_wait( &p_sys->signal, &p_sys->lock );
>      }
> 
> +    input_item_t *item = input_GetItem(p_input);
> +    assert(item);
> +    bool ok = !item->b_error_when_reading;

This is racy.

> +
>      /* This input is dead. Remove it ! */
>      PL_DEBUG( "dead input" );
>      p_sys->p_input = NULL;
> @@ -455,6 +459,8 @@ static void LoopInput( playlist_t *p_playlist )
> 
>      input_Close( p_input );
>      PL_LOCK;
> +
> +    return ok;
>  }
> 
>  static bool Next( playlist_t *p_playlist )
> @@ -492,7 +498,19 @@ static void *Thread ( void *data )
>          /* Playlist in running state */
>          while( !p_sys->killed && Next( p_playlist ) )
>          {
> -            LoopInput( p_playlist );
> +            bool ok = LoopInput( p_playlist );
> +            if (ok)
> +                p_sys->i_consecutive_errors = 0;
> +            else
> +            {
> +                if (p_sys->i_consecutive_errors < 6)
> +                    p_sys->i_consecutive_errors++;
> +
> +                int slowdown = 1 << pys->i_consecutive_errors;
> +                /* 100ms, 200ms, 400ms, 800ms, 1.6s, 3.2s */
> +                mtime_t deadline = mdate() + slowdown * 100000L; /* usecs
> */
> +                vlc_cond_timedwait(&p_sys->signal, &p_sys->lock,
> deadline);
> +            }
>              played = true;
>          }


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