<html><head></head><body>Hi,<br><br>While RPC or IPC interfaces do need a weak identifier for playlist items, I don't think that reproducing the old playlist item ID is a good idea.<br><br>The past has shown that, if the playlist exposes those ID's, they will be used wrongly. That is to say, they will be used by local interfaces that really should not use them on the one hand. And they will be used with improper locking and/or reference counting. That and the callback deadlocks are The Two Design Bugs in the legacy playlist. I don't want to see them again.<br><br>As such I'd argue that the mapping should be done privately and most suitably by those interfaces that require it, rather than the playlist core.<br><br><div class="gmail_quote">Le 4 mars 2019 12:53:21 GMT+02:00, Romain Vimont <rom1v@videolabs.io> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">Playlist items are intended to be handled by index (with lock held) or<br>by instance (vlc_playlist_item_t *).<br><br>Some modules, like http, require to reference playlist items "remotely".<br>To simplify their implementation, assign an id, unique within a playlist<br>instance, to every items.<hr> include/vlc_playlist.h | 16 ++++++++++++++++<br> src/libvlccore.sym | 2 ++<br> src/playlist/content.c | 27 +++++++++++++++++++++------<br> src/playlist/item.c | 9 ++++++++-<br> src/playlist/item.h | 3 ++-<br> src/playlist/playlist.c | 1 +<br> src/playlist/playlist.h | 1 +<br> src/playlist/preparse.c | 9 ++++++---<br> src/playlist/test.c | 4 ++--<br> 9 files changed, 59 insertions(+), 13 deletions(-)<br><br>diff --git a/include/vlc_playlist.h b/include/vlc_playlist.h<br>index 52edae9957..c282e32859 100644<br>--- a/include/vlc_playlist.h<br>+++ b/include/vlc_playlist.h<br>@@ -317,6 +317,12 @@ vlc_playlist_item_Release(vlc_playlist_item_t *);<br> VLC_API input_item_t *<br> vlc_playlist_item_GetMedia(vlc_playlist_item_t *);<br> <br>+/**<br>+ * Return a unique id for the playlist item instance.<br>+ */<br>+VLC_API uint64_t<br>+vlc_playlist_item_GetId(vlc_playlist_item_t *);<br>+<br> /* Playlist */<br> <br> /**<br>@@ -656,6 +662,16 @@ vlc_playlist_IndexOf(vlc_playlist_t *playlist, const vlc_playlist_item_t *item);<br> VLC_API ssize_t<br> vlc_playlist_IndexOfMedia(vlc_playlist_t *playlist, const input_item_t *media);<br> <br>+/**<br>+ * Return the index of a given item id.<br>+ *<br>+ * \param playlist the playlist, locked<br>+ * \param id the id to locate<br>+ * \return the index of the playlist item having the id (-1 if not found)<br>+ */<br>+VLC_API ssize_t<br>+vlc_playlist_IndexOfId(vlc_playlist_t *playlist, uint64_t id);<br>+<br> /**<br> * Return the playback "repeat" mode.<br> *<br>diff --git a/src/libvlccore.sym b/src/libvlccore.sym<br>index ef9e2d5561..79c1aa1c72 100644<br>--- a/src/libvlccore.sym<br>+++ b/src/libvlccore.sym<br>@@ -928,6 +928,7 @@ vlc_player_vout_Snapshot<br> vlc_playlist_item_Hold<br> vlc_playlist_item_Release<br> vlc_playlist_item_GetMedia<br>+vlc_playlist_item_GetId<br> vlc_playlist_New<br> vlc_playlist_Delete<br> vlc_playlist_Lock<br>@@ -947,6 +948,7 @@ vlc_playlist_Shuffle<br> vlc_playlist_Sort<br> vlc_playlist_IndexOf<br> vlc_playlist_IndexOfMedia<br>+vlc_playlist_IndexOfId<br> vlc_playlist_GetPlaybackRepeat<br> vlc_playlist_GetPlaybackOrder<br> vlc_playlist_SetPlaybackRepeat<br>diff --git a/src/playlist/content.c b/src/playlist/content.c<br>index 43b3800d9f..5f307a9a79 100644<br>--- a/src/playlist/content.c<br>+++ b/src/playlist/content.c<br>@@ -205,6 +205,18 @@ vlc_playlist_IndexOfMedia(vlc_playlist_t *playlist, const input_item_t *media)<br> return -1;<br> }<br> <br>+ssize_t<br>+vlc_playlist_IndexOfId(vlc_playlist_t *playlist, uint64_t id)<br>+{<br>+ vlc_playlist_AssertLocked(playlist);<br>+<br>+ playlist_item_vector_t *items = &playlist->items;<br>+ for (size_t i = 0; i < items->size; ++i)<br>+ if (items->data[i]->id == id)<br>+ return i;<br>+ return -1;<br>+}<br>+<br> void<br> vlc_playlist_Clear(vlc_playlist_t *playlist)<br> {<br>@@ -218,13 +230,15 @@ vlc_playlist_Clear(vlc_playlist_t *playlist)<br> }<br> <br> static int<br>-vlc_playlist_MediaToItems(input_item_t *const media[], size_t count,<br>- vlc_playlist_item_t *items[])<br>+vlc_playlist_MediaToItems(vlc_playlist_t *playlist, input_item_t *const media[],<br>+ size_t count, vlc_playlist_item_t *items[])<br> {<br>+ vlc_playlist_AssertLocked(playlist);<br> size_t i;<br> for (i = 0; i < count; ++i)<br> {<br>- items[i] = vlc_playlist_item_New(media[i]);<br>+ uint64_t id = playlist->idgen++;<br>+ items[i] = vlc_playlist_item_New(media[i], id);<br> if (unlikely(!items[i]))<br> break;<br> }<br>@@ -250,7 +264,7 @@ vlc_playlist_Insert(vlc_playlist_t *playlist, size_t index,<br> return VLC_ENOMEM;<br> <br> /* create playlist items in place */<br>- int ret = vlc_playlist_MediaToItems(media, count,<br>+ int ret = vlc_playlist_MediaToItems(playlist, media, count,<br> &playlist->items.data[index]);<br> if (ret != VLC_SUCCESS)<br> {<br>@@ -307,7 +321,8 @@ vlc_playlist_Replace(vlc_playlist_t *playlist, size_t index,<br> vlc_playlist_AssertLocked(playlist);<br> assert(index < playlist->items.size);<br> <br>- vlc_playlist_item_t *item = vlc_playlist_item_New(media);<br>+ uint64_t id = playlist->idgen++;<br>+ vlc_playlist_item_t *item = vlc_playlist_item_New(media, id);<br> if (!item)<br> return VLC_ENOMEM;<br> <br>@@ -347,7 +362,7 @@ vlc_playlist_Expand(vlc_playlist_t *playlist, size_t index,<br> return VLC_ENOMEM;<br> <br> /* create playlist items in place */<br>- ret = vlc_playlist_MediaToItems(&media[1], count - 1,<br>+ ret = vlc_playlist_MediaToItems(playlist, &media[1], count - 1,<br> &playlist->items.data[index + 1]);<br> if (ret != VLC_SUCCESS)<br> {<br>diff --git a/src/playlist/item.c b/src/playlist/item.c<br>index 3cf06f9b3a..e4a1060e93 100644<br>--- a/src/playlist/item.c<br>+++ b/src/playlist/item.c<br>@@ -28,13 +28,14 @@<br> #include <vlc_input_item.h><br> <br> vlc_playlist_item_t *<br>-vlc_playlist_item_New(input_item_t *media)<br>+vlc_playlist_item_New(input_item_t *media, uint64_t id)<br> {<br> vlc_playlist_item_t *item = malloc(sizeof(*item));<br> if (unlikely(!item))<br> return NULL;<br> <br> vlc_atomic_rc_init(&item->rc);<br>+ item->id = id;<br> item->media = media;<br> input_item_Hold(media);<br> return item;<br>@@ -61,3 +62,9 @@ vlc_playlist_item_GetMedia(vlc_playlist_item_t *item)<br> {<br> return item->media;<br> }<br>+<br>+uint64_t<br>+vlc_playlist_item_GetId(vlc_playlist_item_t *item)<br>+{<br>+ return item->id;<br>+}<br>diff --git a/src/playlist/item.h b/src/playlist/item.h<br>index f55cc83e84..1c4f03add7 100644<br>--- a/src/playlist/item.h<br>+++ b/src/playlist/item.h<br>@@ -29,11 +29,12 @@ typedef struct input_item_t input_item_t;<br> struct vlc_playlist_item<br> {<br> input_item_t *media;<br>+ uint64_t id;<br> vlc_atomic_rc_t rc;<br> };<br> <br> /* _New() is private, it is called when inserting new media in the playlist */<br> vlc_playlist_item_t *<br>-vlc_playlist_item_New(input_item_t *media);<br>+vlc_playlist_item_New(input_item_t *media, uint64_t id);<br> <br> #endif<br>diff --git a/src/playlist/playlist.c b/src/playlist/playlist.c<br>index 618433c9e3..94e33c0e52 100644<br>--- a/src/playlist/playlist.c<br>+++ b/src/playlist/playlist.c<br>@@ -52,6 +52,7 @@ vlc_playlist_New(vlc_object_t *parent)<br> vlc_list_init(&playlist->listeners);<br> playlist->repeat = VLC_PLAYLIST_PLAYBACK_REPEAT_NONE;<br> playlist->order = VLC_PLAYLIST_PLAYBACK_ORDER_NORMAL;<br>+ playlist->idgen = 0;<br> <br> return playlist;<br> }<br>diff --git a/src/playlist/playlist.h b/src/playlist/playlist.h<br>index 1090d2146b..da1e082fae 100644<br>--- a/src/playlist/playlist.h<br>+++ b/src/playlist/playlist.h<br>@@ -57,6 +57,7 @@ struct vlc_playlist<br> struct vlc_list listeners; /**< list of vlc_playlist_listener_id.node */<br> enum vlc_playlist_playback_repeat repeat;<br> enum vlc_playlist_playback_order order;<br>+ uint64_t idgen;<br> };<br> <br> static inline void<br>diff --git a/src/playlist/preparse.c b/src/playlist/preparse.c<br>index c636757ff5..c82ce2b675 100644<br>--- a/src/playlist/preparse.c<br>+++ b/src/playlist/preparse.c<br>@@ -32,14 +32,17 @@<br> typedef struct VLC_VECTOR(input_item_t *) media_vector_t;<br> <br> static void<br>-vlc_playlist_CollectChildren(media_vector_t *dest, input_item_node_t *node)<br>+vlc_playlist_CollectChildren(vlc_playlist_t *playlist,<br>+ media_vector_t *dest,<br>+ input_item_node_t *node)<br> {<br>+ vlc_playlist_AssertLocked(playlist);<br> for (int i = 0; i < node->i_children; ++i)<br> {<br> input_item_node_t *child = node->pp_children[i];<br> input_item_t *media = child->p_item;<br> vlc_vector_push(dest, media);<br>- vlc_playlist_CollectChildren(dest, child);<br>+ vlc_playlist_CollectChildren(playlist, dest, child);<br> }<br> }<br> <br>@@ -50,7 +53,7 @@ vlc_playlist_ExpandItem(vlc_playlist_t *playlist, size_t index,<br> vlc_playlist_AssertLocked(playlist);<br> <br> media_vector_t flatten = VLC_VECTOR_INITIALIZER;<br>- vlc_playlist_CollectChildren(&flatten, node);<br>+ vlc_playlist_CollectChildren(playlist, &flatten, node);<br> <br> int ret = vlc_playlist_Expand(playlist, index, flatten.data, flatten.size);<br> vlc_vector_destroy(&flatten);<br>diff --git a/src/playlist/test.c b/src/playlist/test.c<br>index df0eb6fe0e..dfdaeaf6ed 100644<br>--- a/src/playlist/test.c<br>+++ b/src/playlist/test.c<br>@@ -1502,7 +1502,7 @@ test_request_remove_adapt(void)<br> vlc_playlist_AddListener(playlist, &cbs, &ctx, false);<br> assert(listener);<br> <br>- vlc_playlist_item_t *dummy = vlc_playlist_item_New(media[10]);<br>+ vlc_playlist_item_t *dummy = vlc_playlist_item_New(media[10], 0);<br> assert(dummy);<br> <br> /* remove items in a wrong order at wrong position, as if the playlist had<br>@@ -1700,7 +1700,7 @@ test_request_move_adapt(void)<br> vlc_playlist_AddListener(playlist, &cbs, &ctx, false);<br> assert(listener);<br> <br>- vlc_playlist_item_t *dummy = vlc_playlist_item_New(media[15]);<br>+ vlc_playlist_item_t *dummy = vlc_playlist_item_New(media[15], 0);<br> assert(dummy);<br> <br> /* move items in a wrong order at wrong position, as if the playlist had</pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>