Add slice() method to CurrentPlaylistController to reduce copying of the playlist

docs/changes.rst

@@ -21,10 +21,11 @@ v0.7.0 (in development)
 - The MPD command ``playlistinfo`` is now faster, thanks to John Bäckstrand.
 
 - Added the method
-  :meth:`mopidy.backends.base.CurrentPlaylistController.length()` and
-  :meth:`mopidy.backends.base.CurrentPlaylistController.index()` to reduce the
+  :meth:`mopidy.backends.base.CurrentPlaylistController.length()`,
+  :meth:`mopidy.backends.base.CurrentPlaylistController.index()`, and
+  :meth:`mopidy.backends.base.CurrentPlaylistController.slice()` to reduce the
   need for copying the entire current playlist from one thread to another.
-  Thanks to John Bäckstrand.
+  Thanks to John Bäckstrand for pinpointing the issue.
 
 
 v0.6.0 (2011-10-09)

mopidy/backends/base/current_playlist.py

@@ -223,6 +223,19 @@ class CurrentPlaylistController(object):
         self._cp_tracks = before + shuffled + after
         self.version += 1
 
+    def slice(self, start, end):
+        """
+        Returns a slice of the current playlist, limited by the given
+        start and end positions.
+
+        :param start: position of first track to include in slice
+        :type start: int
+        :param end: position after last track to include in slice
+        :type end: int
+        :rtype: two-tuple of (CPID integer, :class:`mopidy.models.Track`)
+        """
+        return [copy(cp_track) for cp_track in self._cp_tracks[start:end]]
+
     def _trigger_playlist_changed(self):
         logger.debug(u'Triggering playlist changed event')
         BackendListener.send('playlist_changed')

mopidy/frontends/mpd/protocol/current_playlist.py

@@ -76,7 +76,7 @@ def delete_range(context, start, end=None):
         end = int(end)
     else:
         end = context.backend.current_playlist.length.get()
-    cp_tracks = context.backend.current_playlist.cp_tracks.get()[start:end]
+    cp_tracks = context.backend.current_playlist.slice(start, end).get()
     if not cp_tracks:
         raise MpdArgError(u'Bad song index', command=u'delete')
     for (cpid, _) in cp_tracks:
@@ -87,7 +87,8 @@ def delete_songpos(context, songpos):
     """See :meth:`delete_range`"""
     try:
         songpos = int(songpos)
-        (cpid, _) = context.backend.current_playlist.cp_tracks.get()[songpos]
+        (cpid, _) = context.backend.current_playlist.slice(
+            songpos, songpos + 1).get()[0]
         context.backend.current_playlist.remove(cpid=cpid)
     except IndexError:
         raise MpdArgError(u'Bad song index', command=u'delete')

mopidy/frontends/mpd/protocol/playback.py

@@ -178,7 +178,8 @@ def playpos(context, songpos):
     if songpos == -1:
         return _play_minus_one(context)
     try:
-        cp_track = context.backend.current_playlist.cp_tracks.get()[songpos]
+        cp_track = context.backend.current_playlist.slice(
+            songpos, songpos + 1).get()[0]
         return context.backend.playback.play(cp_track).get()
     except IndexError:
         raise MpdArgError(u'Bad song index', command=u'play')
@@ -191,8 +192,8 @@ def _play_minus_one(context):
     elif context.backend.playback.current_cp_track.get() is not None:
         cp_track = context.backend.playback.current_cp_track.get()
         return context.backend.playback.play(cp_track).get()
-    elif context.backend.current_playlist.cp_tracks.get():
-        cp_track = context.backend.current_playlist.cp_tracks.get()[0]
+    elif context.backend.current_playlist.slice(0, 1).get():
+        cp_track = context.backend.current_playlist.slice(0, 1).get()[0]
         return context.backend.playback.play(cp_track).get()
     else:
         return # Fail silently

tests/backends/base/current_playlist.py

@@ -260,6 +260,18 @@ class CurrentPlaylistControllerTest(object):
         self.assertEqual(self.tracks[0], shuffled_tracks[0])
         self.assertEqual(set(self.tracks), set(shuffled_tracks))
 
+    @populate_playlist
+    def test_slice_returns_a_subset_of_tracks(self):
+        track_slice = self.controller.slice(1, 3)
+        self.assertEqual(2, len(track_slice))
+        self.assertEqual(self.tracks[1], track_slice[0].track)
+        self.assertEqual(self.tracks[2], track_slice[1].track)
+
+    @populate_playlist
+    def test_slice_returns_empty_list_if_indexes_outside_tracks_list(self):
+        self.assertEqual(0, len(self.controller.slice(7, 8)))
+        self.assertEqual(0, len(self.controller.slice(-1, 1)))
+
     def test_version_does_not_change_when_appending_nothing(self):
         version = self.controller.version
         self.controller.append([])