[gnome-music/wip/jfelder/mpris-truly-unique-ids: 2/3] mpris: Make the trackid truly unique

[Jean Felder <jfelder src gnome org>]

commit ba26a49a3a4b029c945957bfd1b1d1da24b5c5aa
Author: Jean Felder <jfelder src gnome org>
Date:   Wed Sep 5 01:22:52 2018 +0200

    mpris: Make the trackid truly unique
    
    Playlists can have the same song several times. Therefore, the song id
    is not enough to have a unique identifer.
    
    Use the song position as a suffix to trackid to handle duplicate
    songs.
    A current_song_index property is introduced to have the equivalent of
    the current_song property.
    
    Related: #100

 gnomemusic/mpris.py  | 38 ++++++++++++++++++++++++--------------
 gnomemusic/player.py | 20 +++++++++++++++-----
 2 files changed, 39 insertions(+), 19 deletions(-)
---
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index 38b787b2..f2ce7dcb 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -259,13 +259,16 @@ class MediaPlayer2Service(Server):
             return 'Playlist'
 
     @log
-    def _get_metadata(self, media=None):
+    def _get_metadata(self, media=None, index=None):
+        song_dbus_path = self._get_song_dbus_path(media, index)
+        if not self.player.props.current_song:
+            return {
+                'mpris:trackid': GLib.Variant('o', song_dbus_path)
+            }
+
         if not media:
             media = self.player.props.current_song
-        if not media:
-            return {}
 
-        song_dbus_path = self._get_song_dbus_path(media)
         metadata = {
             'mpris:trackid': GLib.Variant('o', song_dbus_path),
             'xesam:url': GLib.Variant('s', media.get_url())
@@ -338,20 +341,29 @@ class MediaPlayer2Service(Server):
         return metadata
 
     @log
-    def _get_song_dbus_path(self, media):
+    def _get_song_dbus_path(self, media=None, index=None):
         """Convert a Grilo media to a D-Bus path
 
         The hex encoding is used to remove any possible invalid character.
+        Use player index to make the path truly unique in case the same song
+        is present multiple times in a playlist.
+        If media is None, it means that the current song path is requested.
 
         :param Grl.Media media: The media object
+        :param int index: The media position in the current playlist
         :return: a D-Bus id to uniquely identify the song
         :rtype: str
         """
-        if not media:
+        if not self.player.props.current_song:
             return "/org/mpris/MediaPlayer2/TrackList/NoTrack"
 
+        if not media:
+            media = self.player.props.current_song
+            index = self.player.props.current_song_index
+
         id_hex = media.get_id().encode('ascii').hex()
-        path = "/org/gnome/GnomeMusic/TrackList/{}".format(id_hex)
+        path = "/org/gnome/GnomeMusic/TrackList/{}_{}".format(
+            id_hex, index)
         return path
 
     @log
@@ -359,9 +371,9 @@ class MediaPlayer2Service(Server):
         previous_path_list = self._path_list
         self._path_list = []
         self._metadata_list = []
-        for song in self.player.get_mpris_playlist():
-            path = self._get_song_dbus_path(song)
-            metadata = self._get_metadata(song)
+        for index, song in self.player.get_mpris_playlist():
+            path = self._get_song_dbus_path(song, index)
+            metadata = self._get_metadata(song, index)
             self._path_list.append(path)
             self._metadata_list.append(metadata)
 
@@ -369,8 +381,7 @@ class MediaPlayer2Service(Server):
         if (not previous_path_list
                 or previous_path_list[0] != self._path_list[0]
                 or previous_path_list[-1] != self._path_list[-1]):
-            current_song_path = self._get_song_dbus_path(
-                self.player.props.current_song)
+            current_song_path = self._get_song_dbus_path()
             self.TrackListReplaced(self._path_list, current_song_path)
             self.PropertiesChanged(
                 MediaPlayer2Service.MEDIA_PLAYER2_TRACKLIST_IFACE,
@@ -610,8 +621,7 @@ class MediaPlayer2Service(Server):
         pass
 
     def GoTo(self, path):
-        current_song_path = self._get_song_dbus_path(
-            self.player.props.current_song)
+        current_song_path = self._get_song_dbus_path()
         current_song_index = self._path_list.index(current_song_path)
         goto_index = self._path_list.index(path)
         song_offset = goto_index - current_song_index
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index 757bca4e..aef78604 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -492,7 +492,7 @@ class PlayerPlaylist(GObject.GObject):
         This method is used by mpris to expose a TrackList.
 
         :returns: current playlist
-        :rtype: list of Grl.Media
+        :rtype: list of index and Grl.Media
         """
         if not self.props.current_song:
             return []
@@ -530,7 +530,8 @@ class PlayerPlaylist(GObject.GObject):
                 range(nb_songs - offset_inf, nb_songs), indexes,
                 range(offset_sup))
 
-        songs = [self._songs[index][PlayerField.SONG] for index in indexes]
+        songs = [[index, self._songs[index][PlayerField.SONG]]
+                 for index in indexes]
         return songs
 
 
@@ -622,7 +623,7 @@ class Player(GObject.GObject):
 
         self._gst_player.props.url = song.get_url()
 
-        self.emit('song-changed', self._playlist.props.current_song_index)
+        self.emit('song-changed', self.props.current_song_index)
 
     @log
     def _on_eos(self, klass):
@@ -729,7 +730,7 @@ class Player(GObject.GObject):
 
         :param int song_index: position of the song to remove
         """
-        if self._playlist.props.current_song_index == song_index:
+        if self.props.current_song_index == song_index:
             if self.props.has_next:
                 self.next()
             elif self.props.has_previous:
@@ -815,6 +816,15 @@ class Player(GObject.GObject):
         self._repeat = mode
         self._settings.set_enum('repeat', mode)
 
+    @GObject.Property(type=int, default=0, flags=GObject.ParamFlags.READABLE)
+    def current_song_index(self):
+        """Gets current song index.
+
+        :returns: position of the current song in the playlist.
+        :rtype: int
+        """
+        return self._playlist.props.current_song_index
+
     @GObject.Property(
         type=Grl.Media, default=None, flags=GObject.ParamFlags.READABLE)
     def current_song(self):
@@ -879,6 +889,6 @@ class Player(GObject.GObject):
         This method is used by mpris to expose a TrackList.
 
         :returns: current playlist
-        :rtype: list of Grl.Media
+        :rtype: list of index and Grl.Media
         """
         return self._playlist.get_mpris_playlist()