[vlc-devel] [RFC] core: player: make the lock optionally reentrant

[Rémi Denis-Courmont]

Hi,

Recursive locking only addresses backward compatibility in the corner case than the call is from the same thread. Since that was never a requirement before, that's not backward compatible.

Le 13 mai 2019 16:49:22 GMT+03:00, Romain Vimont <rom1v at videolabs.io> a écrit :
>On Mon, May 13, 2019 at 04:25:46PM +0300, Rémi Denis-Courmont wrote:
>> Hi,
>> 
>> I don't see the point in adding a parameter here. If you want to use
>libpulse-like semantics, just make the lock recursive.
>
>The idea here is to use a recursive lock only for libvlc, and keep the
>"normal" player lock otherwise.
>
>> But in any case, this won't provide backward compatibility.
>
>This will allow to call functions like libvlc_audio_get_track_count()
>from a libvlc player callback like before.
>
>> 
>> Le 13 mai 2019 15:47:36 GMT+03:00, Romain Vimont <rom1v at videolabs.io>
>a écrit :
>> >This will allow to implement the old libvlc API using the new
>player.
>> >
>>
>><https://mailman.videolan.org/pipermail/vlc-devel/2019-May/124461.html>
>> >---
>> > include/vlc_player.h  | 25 ++++++++++++++++++++++++-
>> > src/input/player.c    | 12 ++++++++----
>> > src/playlist/player.c |  3 ++-
>> > 3 files changed, 34 insertions(+), 6 deletions(-)
>> >
>> >diff --git a/include/vlc_player.h b/include/vlc_player.h
>> >index 1aa5f8e687..d4c49fa89d 100644
>> >--- a/include/vlc_player.h
>> >+++ b/include/vlc_player.h
>> >@@ -331,6 +331,29 @@ enum vlc_player_subtitle_sync
>> >     VLC_PLAYER_SUBTITLE_SYNC_APPLY,
>> > };
>> > 
>> >+/**
>> >+ * Player lock type (normal or reentrant)
>> >+ */
>> >+enum vlc_player_lock_type
>> >+{
>> >+    /**
>> >+     * Normal lock
>> >+     *
>> >+     * If the player is already locked, subsequent calls to
>> >vlc_player_Lock()
>> >+     * will deadlock.
>> >+     */
>> >+    VLC_PLAYER_LOCK_NORMAL,
>> >+
>> >+    /**
>> >+     * Reentrant lock
>> >+     *
>> >+     * If the player is already locked, subsequent calls to
>> >vlc_player_Lock()
>> >+     * will still succeed. To unlock the player, one call to
>> >+     * vlc_player_Unlock() per vlc_player_Lock() is necessary.
>> >+     */
>> >+    VLC_PLAYER_LOCK_REENTRANT,
>> >+};
>> >+
>> > /** Player capability: can seek */
>> > #define VLC_PLAYER_CAP_SEEK (1<<0)
>> > /** Player capability: can pause */
>> >@@ -1005,7 +1028,7 @@
>vlc_player_title_list_GetAt(vlc_player_title_list
>> >*titles, size_t idx);
>> >* @return a pointer to a valid player instance or NULL in case of
>error
>> >  */
>> > VLC_API vlc_player_t *
>> >-vlc_player_New(vlc_object_t *parent,
>> >+vlc_player_New(vlc_object_t *parent, enum vlc_player_lock_type
>> >lock_type,
>> >                const struct vlc_player_media_provider
>*media_provider,
>> >                void *media_provider_data);
>> > 
>> >diff --git a/src/input/player.c b/src/input/player.c
>> >index da02375a66..e01701ba65 100644
>> >--- a/src/input/player.c
>> >+++ b/src/input/player.c
>> >@@ -3367,9 +3367,13 @@ vlc_player_vout_Snapshot(vlc_player_t
>*player)
>> > }
>> > 
>> > static void
>> >-vlc_player_InitLocks(vlc_player_t *player)
>> >+vlc_player_InitLocks(vlc_player_t *player, enum
>vlc_player_lock_type
>> >lock_type)
>> > {
>> >-    vlc_mutex_init(&player->lock);
>> >+    if (lock_type == VLC_PLAYER_LOCK_REENTRANT)
>> >+        vlc_mutex_init_recursive(&player->lock);
>> >+    else
>> >+        vlc_mutex_init(&player->lock);
>> >+
>> >     vlc_mutex_init(&player->vout_listeners_lock);
>> >     vlc_mutex_init(&player->aout_listeners_lock);
>> >     vlc_cond_init(&player->start_delay_cond);
>> >@@ -3426,7 +3430,7 @@ vlc_player_Delete(vlc_player_t *player)
>> > }
>> > 
>> > vlc_player_t *
>> >-vlc_player_New(vlc_object_t *parent,
>> >+vlc_player_New(vlc_object_t *parent, enum vlc_player_lock_type
>> >lock_type,
>> >                const struct vlc_player_media_provider
>*media_provider,
>> >                void *media_provider_data)
>> > {
>> >@@ -3514,7 +3518,7 @@ vlc_player_New(vlc_object_t *parent,
>> >     }
>> > 
>> >     player->deleting = false;
>> >-    vlc_player_InitLocks(player);
>> >+    vlc_player_InitLocks(player, lock_type);
>> > 
>> >if (vlc_clone(&player->destructor.thread,
>vlc_player_destructor_Thread,
>> >                   player, VLC_THREAD_PRIORITY_LOW) != 0)
>> >diff --git a/src/playlist/player.c b/src/playlist/player.c
>> >index bbdc39ba0e..8e3d94e2e8 100644
>> >--- a/src/playlist/player.c
>> >+++ b/src/playlist/player.c
>> >@@ -123,7 +123,8 @@ static const struct vlc_player_cbs
>player_callbacks
>> >= {
>> > bool
>> >vlc_playlist_PlayerInit(vlc_playlist_t *playlist, vlc_object_t
>*parent)
>> > {
>> >-    playlist->player = vlc_player_New(parent,
>&player_media_provider,
>> >playlist);
>> >+    playlist->player = vlc_player_New(parent,
>VLC_PLAYER_LOCK_NORMAL,
>> >+                                      &player_media_provider,
>> >playlist);
>> >     if (unlikely(!playlist->player))
>> >         return false;
>> > 
>> >-- 
>> >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
>
>_______________________________________________
>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é.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190513/4d011ee1/attachment.html>