player: handle ERROR_S from OPENING_S

Note: this patch might need a matching player test case for the Init() failure scenario.

As of this patch, the state machine from the input is the following:

                     |INIT_S|

via input.c:Init()     ⬇   via input.c:Init()
 in case of error
        ⬇      ⬅   |OPENING_S|

        ⬇               ⬇  via input.c:Init()

     |ERROR_S|  ⬅  |PLAYING_S|  ⬅ ⮕  |PAUSE_S| --- ⮕  |ERROR_S|
   via Demux() error             via ControlPause/Unpause
        ⬇               ⬇
    if Init()  ⮕  via input.c:End()
    succeeded        |END_S|

Legend:

• |STATE_S|: a state from the input_thread_t (see input_ChangeState)
• via foobar: the internal function triggering the state transition

The transition from OPENING_S to ERROR_S was not handled correctly by the vlc_player code.

After validation of hypothesis from Thomas, when a media reaches ERROR_S from OPENING_S, because Init() from the file src/input/input.c failed, END_S was never reached.

When vlc_player_SetCurrentMedia or vlc_player_Stop is called, the input thread was added to the list of inputs to be stopped if the item was started, or to the list of inputs to be joined if it wasn't started.

If the input wasn't started at all, no problem would arise, since we don't need to wait for the END_S state.

However, when the input was started, and Init() failed, ie. we follow the transition from OPENING_S to ERROR_S, the END_S state will never be reached, and more precisely INPUT_EVENT_DEAD will be emitted without END_S state being reached.

In the src/player/input.c code, INPUT_EVENT_DEAD means that the input thread has reached the end of the thread function and is now joinable without transitively waiting on the input thread waiting for a state change. When this event is emitted, the input thread is added to the list of input to be joined.

The assertion failing here signals that the input thread is scheduled for joining although it has not been stopped yet.

   Assertion failed: (!vlc_list_HasInput(&player->destructor.inputs, input)),
   function vlc_player_destructor_AddInput, file player.c, line 165.

Three possible ways of solving the issue has been explored for the fix:

1/ Handle the transition in the destructor, allowing inputs to go from the list of inputs to be stopped directly to the list of inputs to be joined. This is by far the simplest method since it mostly consists of removing from the list of inputs if it exists:

diff --git a/src/player/player.c b/src/player/player.c
+++ b/src/player/player.c
@@ -184,6 +184,9 @@ vlc_player_destructor_AddJoinableInput(vlc_player_t *player,
     if (vlc_list_HasInput(&player->destructor.stopping_inputs, input))
         vlc_list_remove(&input->node);

+    if (vlc_list_HasInput(&player->destructor.inputs, input))
+        vlc_list_remove(&input->node);

2/ Handle the transition in the input code, by explicitely moving from the ERROR_S state to the END_S state to match the existing semantics everywhere.

diff --git a/src/input/input.c b/src/input/input.c
+++ b/src/input/input.c
@@ -482,6 +482,10 @@ static void *Run( void *data )
         /* Clean up */
         End( p_input );
     }
+    else
+    {
+        input_ChangeState( p_input, END_S, VLC_TICK_INVALID );
+    }

3/ Handle the transition in the player code, by explicitely handling the state transition from OPENING_S to ERROR_S. Since there are multiple paths leading to ERROR_S, the PLAYING_S state is used as an indication on whether this is a failure from OPENING_S and it can be considered as STOPPED already. This is the version implemented in the current path.

The 3/ solution has been chosen mainly because:

• In 1/, the state change in the destructor can be viewed as a defensive approach, and might allow previously invalid cases to happen while leading to multiple destruction in those cases.

• In 2/, the state change from the input_thread would make sense to unify everywhere, but since there are different runloops for thumbnailer, preparser and playback, it would either lead to inconsistency or a lot of behaviour to check. In addition, though the input_thread exposes an ERROR_S state, the player handles the errors aside from the state and the input thread might change regarding that.

In both approaches, the vlc_player code cannot provide different next media behaviour depending on whether there is an error from the opening, not being able to differenciate between an invalid media and a media running into an error during the playback.

The first approach has no context leading to that, and the second approach would need more state tracking for that, which is exactly what this patch provides without the changes in the input.

Finally, since the vlc_player is the most modern interface, addressing the error state matching with the current design of the vlc_player is what made the most sense to us.

Fixes #26344 (closed)

Co-authored-by: Thomas Guillem thomas@gllm.fr