[vlc-devel] [PATCH 3/4] playlist: add multi-playlist support

[Romain Vimont]

On Mon, Aug 24, 2020 at 08:20:21AM +0200, Steve Lhomme wrote:
> 
> On 2020-08-20 17:44, Romain Vimont wrote:
> > Add the possibility to manage several playlists, identified by a
> > string, in the playlist repository.
> > ---
> >   include/vlc_playlist.h  |  97 ++++++++++++++++++++++++++-
> >   src/libvlccore.sym      |   7 ++
> >   src/playlist/playlist.c |   8 +++
> >   src/playlist/playlist.h |   2 +
> >   src/playlist/repo.c     | 142 ++++++++++++++++++++++++++++++++++++++--
> >   5 files changed, 249 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h
> > index 9a1c66e3dc..d773b20ce8 100644
> > --- a/include/vlc_playlist.h
> > +++ b/include/vlc_playlist.h
> > @@ -371,6 +371,15 @@ vlc_playlist_Lock(vlc_playlist_t *);
> >   VLC_API void
> >   vlc_playlist_Unlock(vlc_playlist_t *);
> > +/**
> > + * Get the playlist id.
> > + *
> > + * If the playlist is added to the playlist repo, this is the id the playlist
> > + * is associated to. Otherwise, id is NULL.
> > + */
> > +VLC_API const char *
> > +vlc_playlist_GetId(vlc_playlist_t *);
> > +
> >   /**
> >    * Add a playlist listener.
> >    *
> > @@ -886,6 +895,8 @@ VLC_API int
> >   vlc_playlist_Export(vlc_playlist_t *playlist, const char *filename,
> >                       const char *type);
> > +#define VLC_MAIN_PLAYLIST_ID "main"
> > +
> >   /** Playlist repository (opaque) */
> >   typedef struct vlc_playlist_repo vlc_playlist_repo_t;
> > @@ -898,13 +909,95 @@ typedef struct vlc_playlist_repo vlc_playlist_repo_t;
> >   VLC_API vlc_playlist_repo_t *
> >   libvlc_GetPlaylistRepo(libvlc_int_t *libvlc);
> > +/**
> > + * Lock the playlist repo.
> > + *
> > + * All playlist repo functions must be called with lock held (check their
> > + * description).
> > +
> > + * \param repo the playlist repo, unlocked
> > + */
> > +VLC_API void
> > +vlc_playlist_repo_Lock(vlc_playlist_repo_t *repo);
> > +
> > +/**
> > + * Unlock the playlist repo.
> > + *
> > + * \param repo the playlist repo, locked
> > + */
> > +VLC_API void
> > +vlc_playlist_repo_Unlock(vlc_playlist_repo_t *repo);
> > +
> > +/**
> > + * Create a new playlist in the repository.
> > + *
> > + * A playlist with the same id must not already exist in this repo (check with
> > + * vlc_playlist_repo_Get() beforehand if necessary).
> > + *
> > + * \param repo the playlist repo, locked
> > + * \param id the id of the playlist to create
> > + * \return the new playlist, NULL on error
> > + */
> > +VLC_API vlc_playlist_t *
> > +vlc_playlist_repo_Create(vlc_playlist_repo_t *repo, const char *id);
> > +
> > +/**
> > + * Remove a playlist from the repository.
> > + *
> > + * A playlist having this id must exist in this repo (check with
> > + * vlc_playlist_repo_Get() beforehand if necessary).
> > + *
> > + * It is not possible to remove the main playlist (with id
> > + * VLC_MAIN_PLAYLIST_ID).
> > + *
> > + * \param repo the playlist repo, locked
> > + * \param id the id of the playlist to remove
> > + */
> > +VLC_API void
> > +vlc_playlist_repo_Remove(vlc_playlist_repo_t *repo, const char *id);
> > +
> > +/**
> > + * Retrieve a playlist from its id.
> > + *
> > + * If no playlist having is this exist in this repo, this function return NULL.
> > + *
> > + * \param repo the playlist repo, locked
> > + * \param id the id of the playlist to get
> > + * \return the associated playlist, NULL if not exist
> > + */
> > +VLC_API vlc_playlist_t *
> > +vlc_playlist_repo_Get(vlc_playlist_repo_t *repo, const char *id);
> > +
> > +/**
> > + * Get a list of all playlists in the repo.
> > + *
> > + * This returns an allocated array of pointers to the playlists. The number of
> > + * playlists is written to the output parameter out_len.
> > + *
> > + * The array must then be released by the caller via free().
> > + *
> > + * It contains pointers to existing playlists, including the main playlist. The
> > + * order is undefined, and may change between successive calls. The id of each
> > + * playlist may be retrieved by vlc_playlist_GetId().
> > + *
> > + * The refcount of the playlists is not incremented. Therefore, they are only
> > + * valid until the call vlc_playlist_repo_Unlock(). To keep a reference longer,
> > + * it is possible to call vlc_playlist_Hold() before unlocking the repo.
> 
> This documentation is confusing.
> I would assume the list of playlists you receive has each of them "locked"
> so it's valid as long as you want. Otherwise the playlist(s) may not be
> valid anymore until you receive them.
> 
> Also is it a lock or a hold on the pointer ? What would a lock lock against
> ?

The playlist repo is locked, but the refcount of the playlists is not
incremented.

While the playlist repo is locked, it is guaranteed that someone else
may not remove a playlist from the repo (by calling
vlc_playlist_repo_Remove()), so the playlists are guaranteed not to be
deleted (more generally, the repo content won't be changed by someone
else).

As soon as the playlist repo is unlocked, it is not guaranteed anymore,
and you must call vlc_playlist_Hold() on playlists you want to keep
before unlocking the repo.