[gnome-music/wip/jfelder/mpris-limit-get-songs: 13/14] player: Handle repeat and limit songs in get_songs

[Jean Felder <jfelder src gnome org>]

commit 9ac555f7012556911c9db4bf86114ed089af9c82
Author: Jean Felder <jfelder src gnome org>
Date:   Fri Sep 14 00:16:17 2018 +0200

    player: Handle repeat and limit songs in get_songs
    
    get_songs method is used by MPRIS to expose a playlist to MPRIS
    clients. Returning the whole playlist (for example, all the songs) can
    create instabilities and crashes.
    Besides, according to MPRIS specification, TrackList should only be
    a "short list of tracks which were recently played or will be played
    shortly."
    
    Limit the number of returned to avoid those instabilities. Order the
    returned list following the repeat mode to reflect the current state
    of the player playlist.
    With this change, GoTo method from MPRIS needs to deal with song
    offsets (position relative to the current song). So, update it
    accordingly.
    
    Related: #100

 gnomemusic/mpris.py  | 28 ++++++++++-------
 gnomemusic/player.py | 85 ++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 91 insertions(+), 22 deletions(-)
---
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index 7122bf66..06c8ce77 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -358,6 +358,7 @@ class MediaPlayer2Service(Server):
 
     @log
     def _update_songs_list(self):
+        previous_path_list = self._path_list
         self._path_list = []
         self._metadata_list = []
         for song in self.player.get_songs():
@@ -366,6 +367,17 @@ class MediaPlayer2Service(Server):
             self._path_list.append(path)
             self._metadata_list.append(metadata)
 
+        # current song has changed
+        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)
+            self.TrackListReplaced(self._path_list, current_song_path)
+            self.PropertiesChanged(
+                MediaPlayer2Service.MEDIA_PLAYER2_TRACKLIST_IFACE,
+                {'Tracks': GLib.Variant('ao', self._path_list), }, [])
+
     @log
     def _get_playlist_dbus_path(self, playlist):
         """Convert a playlist to a D-Bus path
@@ -429,6 +441,7 @@ class MediaPlayer2Service(Server):
 
     @log
     def _on_current_song_changed(self, player, position):
+        self._update_songs_list()
         if self.player.props.repeat_mode == RepeatMode.SONG:
             self.Seeked(0)
 
@@ -491,14 +504,6 @@ class MediaPlayer2Service(Server):
     def _on_player_playlist_changed(self, klass):
         self._update_songs_list()
 
-        if self.player.props.current_song:
-            current_song_path = self._get_song_dbus_path(
-                self.player.props.current_song)
-            self.TrackListReplaced(self._path_list, current_song_path)
-            self.PropertiesChanged(
-                MediaPlayer2Service.MEDIA_PLAYER2_TRACKLIST_IFACE,
-                {'Tracks': GLib.Variant('ao', self._path_list), }, [])
-
         if (self.player.get_playlist_type() == PlayerPlaylist.Type.PLAYLIST
                 or self._player_previous_type == PlayerPlaylist.Type.PLAYLIST):
             variant = GLib.Variant('(b(oss))', self._get_active_playlist())
@@ -610,8 +615,11 @@ class MediaPlayer2Service(Server):
         pass
 
     def GoTo(self, path):
-        index = self.path_list.index(path)
-        self.player.play(index)
+        current_song_path = self._get_song_dbus_path(
+            self.player.props.current_song)
+        current_song_index = self._path_list.index(current_song_path)
+        goto_index = self._path_list.index(path)
+        self.player.play(goto_index - current_song_index)
         return
 
     def TrackListReplaced(self, tracks, current_song):
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index b5d8e8ca..10bdaabe 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -24,6 +24,7 @@
 
 from collections import defaultdict
 from enum import IntEnum
+from itertools import chain
 from random import shuffle, randrange
 import logging
 import time
@@ -86,6 +87,8 @@ class PlayerPlaylist(GObject.GObject):
         'song-validated': (GObject.SignalFlags.RUN_FIRST, None, (int, int)),
     }
 
+    _nb_songs_max = 10
+
     def __repr__(self):
         return '<PlayerPlayList>'
 
@@ -160,14 +163,24 @@ class PlayerPlaylist(GObject.GObject):
         return True
 
     @log
-    def set_song(self, song_index):
+    def set_song(self, song_offset):
         """Change playlist index.
 
-        :param int song_index: requested song index
+        :param int song_offset: position relative to current song
         :return: True if the index has changed. False otherwise.
         :rtype: bool
         """
-        if song_index >= len(self._songs):
+        if self._repeat == RepeatMode.SHUFFLE:
+            shuffle = self._shuffle_indexes.index(self._current_index)
+            self._current_index = self._shuffle_indexes[shuffle + song_offset]
+            return True
+
+        song_index = song_offset + self._current_index
+        if self._repeat == RepeatMode.ALL:
+            song_index = song_index % len(self._songs)
+
+        if(song_index >= len(self._songs)
+           or song_index < 0):
             return False
 
         self._current_index = song_index
@@ -313,8 +326,8 @@ class PlayerPlaylist(GObject.GObject):
                 and self._current_index == 0):
             return len(self._songs) - 1
         if self._repeat == RepeatMode.SHUFFLE:
-            index = self._shuffle_indexes.index(self._current_index)
-            return self._shuffle_indexes[index - 1]
+            shuffle_index = self._shuffle_indexes.index(self._current_index)
+            return self._shuffle_indexes[shuffle_index - 1]
         else:
             return self._current_index - 1
 
@@ -478,12 +491,51 @@ class PlayerPlaylist(GObject.GObject):
 
     @log
     def get_songs(self):
-        """Get the current playlist.
+        """Get recent and next songs from the current playlist.
+
+        If the playlist is an album, return all songs.
+        Returned songs are sorted according to the repeat mode.
 
         :returns: current playlist
         :rtype: list of Grl.Media
         """
-        songs = [elt[PlayerField.SONG] for elt in self._songs]
+        if not self.props.current_song:
+            return []
+
+        songs = []
+        nb_songs = len(self._songs)
+        current_index = self._current_index
+        if self._repeat == RepeatMode.SHUFFLE:
+            current_index = self._shuffle_indexes.index(self._current_index)
+
+        index_min = current_index - self._nb_songs_max
+        index_max = current_index + self._nb_songs_max + 1
+        if self._type == PlayerPlaylist.Type.ALBUM:
+            index_min = 0
+            index_max = nb_songs
+
+        first_index = max(index_min, 0)
+        last_index = min(index_max, nb_songs)
+
+        if self._repeat == RepeatMode.SHUFFLE:
+            indexes = self._shuffle_indexes[first_index:last_index]
+        else:
+            indexes = range(first_index, last_index)
+
+        if (self._repeat == RepeatMode.ALL
+                and (last_index - first_index) < (2 * self._nb_songs_max + 1)):
+            offset_sup = min(
+                self._nb_songs_max - last_index + current_index + 1,
+                first_index)
+            offset_inf = min(
+                self._nb_songs_max - current_index + first_index,
+                nb_songs - last_index)
+
+            indexes = chain(
+                range(nb_songs - offset_inf, nb_songs), indexes,
+                range(offset_sup))
+
+        songs = [self._songs[index][PlayerField.SONG] for index in indexes]
         return songs
 
 
@@ -593,13 +645,19 @@ class Player(GObject.GObject):
             self.stop()
 
     @log
-    def play(self, song_index=None):
-        """Play"""
+    def play(self, song_offset=None):
+        """Play a song.
+
+        If song_offset is None, load and play current song. Otherwise, load a
+        new song and play it.
+
+        :param int song_offset: position relative to current song
+        """
         if self.props.current_song is None:
             return
 
-        if (song_index is not None
-                and not self._playlist.set_song(song_index)):
+        if (song_offset is not None
+                and not self._playlist.set_song(song_offset)):
             return False
 
         if self.props.state != Playback.PAUSED:
@@ -835,7 +893,10 @@ class Player(GObject.GObject):
 
     @log
     def get_songs(self):
-        """Get the current playlist.
+        """Get recent and next songs from the current playlist.
+
+        If the playlist is an album, return all songs.
+        Returned songs are sorted according to the repeat mode.
 
         :returns: current playlist
         :rtype: list of Grl.Media