[vlc-devel] [PATCH 5/5] libvlc: player: use vlc_player

[Romain Vimont]

On Thu, May 09, 2019 at 07:48:56PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 9. toukokuuta 2019, 18.56.58 EEST Romain Vimont a écrit :
> > @@ -340,16 +332,13 @@ int libvlc_audio_set_volume( libvlc_media_player_t
> > *mp, int volume )
> > ***************************************************************************
> > **/ int libvlc_audio_get_track_count( libvlc_media_player_t *p_mi )
> >  {
> > -    input_thread_t *p_input_thread = libvlc_get_input_thread( p_mi );
> > -    int i_track_count;
> > +    vlc_player_t *player = p_mi->player;
> > +    vlc_player_Lock(player);
> > 
> > -    if( !p_input_thread )
> > -        return -1;
> > +    int ret = vlc_player_GetTrackCount(p_mi->player, AUDIO_ES);
> > 
> > -    i_track_count = var_CountChoices( p_input_thread, "audio-es" );
> > -
> > -    input_Release(p_input_thread);
> > -    return i_track_count;
> > +    vlc_player_Unlock(player);
> > +    return ret;
> >  }
> 
> Application should (still) be allowed to call this, and most* other functions 
> further down the patch, from within event callbacks. But this leads to 
> deadlock or abort depending on build type.

Yes.

But we want to keep the libvlc API (which does not expose the lock to
the client) unchanged, and we need to lock the player. So the lock must
be recursive in some way:
 1. always make the core vlc_player lock recursive;
 2. make the vlc_player lock recursive on demand (a flag on creation);
 3. use a wrapper in libvlc to expose a recursive lock from the
    non-recursive vlc_player lock (using a thread-local storage bool).

I suggest 3.

What do you think?