[gnome-music/wip/jfelder/fix-play-and-load-logic: 4/4] player: Correctly handle load and play logic

[Jean Felder <jfelder src gnome org>]

commit 3db22df4db62e85c716b545a77022adeefaad84c
Author: Jean Felder <jfelder src gnome org>
Date:   Mon May 6 17:23:07 2019 +0200

    player: Correctly handle load and play logic
    
    The play method can be called in 4 cases:
    * At the beginning of a playlist to start the player (set_playlist +
      play calls)
    * To resume a song when in pause (play_pause)
    * To go to next or previous song (next and previous calls)
    * At the end of song to play the next song (via _on_eos which itself
      relies on a next call)
    
    The main difficulty inside the play method is to be able to detect if
    the current song has changed or if the player is just paused. In the
    first case, the _load method needs to be called in order to update the
    GstPlayer url.
    Initially, the test to distinguish these two cases was to check if the
    Player was on pause. This worked reliably expect in one case: it was
    impossible to change the current song while an other song was on
    pause (see #256).
    This issue was solved by checking if the song url had changed
    instead. This solved the previous issue but created an other one: the
    repeat-song mode did not work anymore. Indeed, when a song is
    repeated, the playlist url does not change at the end of the song
    (see #278).
    So, an additionnal check was added to test the repeat mode of the
    player. It also fixed the previous indeed the previous issue but
    introduced a new one. In repeat-song mode, the song restarts every
    time the player has been paused (see #280).
    
    All these issues can be solved by adding a song_changed bool parameter
    to the play method with a default value of True. The only case where
    it needs to be set to False is in the play_pause method.
    
    Related: #256, #278
    Closes: #280

 gnomemusic/mpris.py  |  3 ++-
 gnomemusic/player.py | 14 ++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)
---
diff --git a/gnomemusic/mpris.py b/gnomemusic/mpris.py
index b935e316..38b787b2 100644
--- a/gnomemusic/mpris.py
+++ b/gnomemusic/mpris.py
@@ -614,7 +614,8 @@ class MediaPlayer2Service(Server):
             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)
+        song_offset = goto_index - current_song_index
+        self.player.play(song_offset=song_offset)
         return
 
     def TrackListReplaced(self, tracks, current_song):
diff --git a/gnomemusic/player.py b/gnomemusic/player.py
index a7a2656c..e28bfe96 100644
--- a/gnomemusic/player.py
+++ b/gnomemusic/player.py
@@ -632,12 +632,13 @@ class Player(GObject.GObject):
             self.stop()
 
     @log
-    def play(self, song_offset=None):
+    def play(self, song_changed=True, song_offset=None):
         """Play a song.
 
-        If song_offset is None, load and play current song. Otherwise, load a
-        new song and play it.
+        Load a new song or resume playback depending on song_changed
+        value. If song_offset is defined, set a new song and play it.
 
+        :param bool song_changed: indicate if a new song must be loaded
         :param int song_offset: position relative to current song
         """
         if self.props.current_song is None:
@@ -647,10 +648,7 @@ class Player(GObject.GObject):
                 and not self._playlist.set_song(song_offset)):
             return False
 
-        url = self._playlist.props.current_song.get_url()
-        loop_modes = [RepeatMode.SONG, RepeatMode.ALL]
-        if (url != self._gst_player.props.url
-                or self.props.repeat_mode in loop_modes):
+        if song_changed is True:
             self._load(self._playlist.props.current_song)
 
         self._gst_player.props.state = Playback.PLAYING
@@ -694,7 +692,7 @@ class Player(GObject.GObject):
         if self.props.state == Playback.PLAYING:
             self.pause()
         else:
-            self.play()
+            self.play(False)
 
     @log
     def set_playlist(self, playlist_type, playlist_id, model, iter_=None):