[vlc-devel] [PATCH v2 8/9] libvlc: player: make stop synchronous

[Romain Vimont]

On Wed, May 15, 2019 at 01:31:14PM +0300, Rémi Denis-Courmont wrote:
> Hi,
> 
> This looks like it'll dead lock if more than one thread calls the stop. Not sure if that worked in the current code or not.

Yes, this is incorrect in that case.

But since the callbacks are called with player locked, we necessarily
need to unlock the player one way or another to wait for the "stopped"
event.

An alternative is to use a condition variable with the player mutex:

    vlc_player_Lock(player);
    bool stop_async = vlc_player_Stop(player);
    if (stop_async)
         vlc_player_CondWait(player, &p_mi->cond_stop);
    vlc_player_Unlock(player);

But this is still not correct, because others vlc_player_Start() and
vlc_player_Stop() can be executed during vlc_player_CondWait().

The problem is that we can't associate a specific stop request with the
right "player state change to stopped", so we would wake up all threads
waiting for stop on the first "state change to stopped".

Since we don't want synchronous stop anymore (it was just for backward
compatibility), I suggest we just make libvlc_media_player_stop()
asynchronous.

And we document breaking changes from libvlc 3 to libvlc 4:
- libvlc_media_player_will_play() is removed (it is meaningless with the
  new player)
- libvlc_media_player callbacks are called with the same (recursive)
  mutex as libvlc_media_player functions
- libvlc_media_player_stop is now asynchronous (it was the only blocking
  function, it does not block anymore)

> Le 15 mai 2019 12:53:16 GMT+03:00, Romain Vimont <rom1v at videolabs.io> a écrit :
> >To preserve the old behavior, make libvlc_media_player_stop()
> >synchronous.
> >---
> > lib/media_player.c          | 13 ++++++++++++-
> > lib/media_player_internal.h |  3 +++
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> >diff --git a/lib/media_player.c b/lib/media_player.c
> >index 17d675e93b..e5f6fc32a7 100644
> >--- a/lib/media_player.c
> >+++ b/lib/media_player.c
> >@@ -108,6 +108,8 @@ on_state_changed(vlc_player_t *player, enum
> >vlc_player_state new_state,
> >     switch (new_state) {
> >         case VLC_PLAYER_STATE_STOPPED:
> >             event.type = libvlc_MediaPlayerStopped;
> >+            if (mp->stop_requested)
> >+                vlc_sem_post(&mp->sem_stop);
> >             break;
> >         case VLC_PLAYER_STATE_STOPPING:
> >             event.type = libvlc_MediaPlayerEndReached;
> >@@ -715,6 +717,9 @@ libvlc_media_player_new( libvlc_instance_t
> >*instance )
> > 
> >     vlc_player_Unlock(mp->player);
> > 
> >+    vlc_sem_init(&mp->sem_stop, 0);
> >+    mp->stop_requested = false;
> >+
> >     mp->i_refcount = 1;
> >     libvlc_event_manager_init(&mp->event_manager, mp);
> > 
> >@@ -796,6 +801,8 @@ static void libvlc_media_player_destroy(
> >libvlc_media_player_t *p_mi )
> >     libvlc_event_manager_destroy(&p_mi->event_manager);
> >     libvlc_media_release( p_mi->p_md );
> > 
> >+    vlc_sem_destroy(&p_mi->sem_stop);
> >+
> >vlc_http_cookie_jar_t *cookies = var_GetAddress( p_mi, "http-cookies"
> >);
> >     if ( cookies )
> >     {
> >@@ -972,9 +979,13 @@ void libvlc_media_player_stop(
> >libvlc_media_player_t *p_mi )
> >     vlc_player_t *player = p_mi->player;
> >     vlc_player_Lock(player);
> > 
> >-    vlc_player_Stop(player);
> >+    bool stop_async = vlc_player_Stop(player);
> >+    p_mi->stop_requested = stop_async;
> > 
> >     vlc_player_Unlock(player);
> >+
> >+    if (stop_async)
> >+        vlc_sem_wait(&p_mi->sem_stop);
> > }
> > 
> > int libvlc_media_player_set_renderer( libvlc_media_player_t *p_mi,
> >diff --git a/lib/media_player_internal.h b/lib/media_player_internal.h
> >index c2d5348e22..a0c712780d 100644
> >--- a/lib/media_player_internal.h
> >+++ b/lib/media_player_internal.h
> >@@ -49,6 +49,9 @@ struct libvlc_media_player_t
> >    struct libvlc_instance_t * p_libvlc_instance; /* Parent instance */
> >     libvlc_media_t * p_md; /* current media descriptor */
> >     libvlc_event_manager_t event_manager;
> >+
> >+    vlc_sem_t sem_stop;
> >+    bool stop_requested;
> > };
> > 
> > libvlc_track_description_t * libvlc_get_track_description(
> >-- 
> >2.20.1
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel