From b0698d2e0ad306bbf1bd5259d9b8abd547d7eb29 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 28 Dec 2011 02:38:07 +0100 Subject: [PATCH] Add slice() method to CurrentPlaylistController to reduce copying of the playlist --- docs/changes.rst | 7 ++++--- mopidy/backends/base/current_playlist.py | 13 +++++++++++++ mopidy/frontends/mpd/protocol/current_playlist.py | 5 +++-- mopidy/frontends/mpd/protocol/playback.py | 7 ++++--- tests/backends/base/current_playlist.py | 12 ++++++++++++ 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 5bebd49e..95541797 100644 --- a/docs/changes.rst +++ b/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) diff --git a/mopidy/backends/base/current_playlist.py b/mopidy/backends/base/current_playlist.py index 4e439931..d7e6c331 100644 --- a/mopidy/backends/base/current_playlist.py +++ b/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') diff --git a/mopidy/frontends/mpd/protocol/current_playlist.py b/mopidy/frontends/mpd/protocol/current_playlist.py index 91ef7811..0d61c887 100644 --- a/mopidy/frontends/mpd/protocol/current_playlist.py +++ b/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') diff --git a/mopidy/frontends/mpd/protocol/playback.py b/mopidy/frontends/mpd/protocol/playback.py index 63cfe649..948083a8 100644 --- a/mopidy/frontends/mpd/protocol/playback.py +++ b/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 diff --git a/tests/backends/base/current_playlist.py b/tests/backends/base/current_playlist.py index 2dab625e..e99cd56c 100644 --- a/tests/backends/base/current_playlist.py +++ b/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([])