andpp/mopidy
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge branch 'feature/mpd-performance-fixes' into develop

Browse Source
This commit is contained in:
Stein Magnus Jodal 2011-12-28 02:38:17 +01:00
commit fbfced32c3
12 changed files with 155 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
docs/changes.rst
View File

@ -18,6 +18,15 @@ v0.7.0 (in development)
if you use playlist folders, you will no longer get lots of log messages
about bad playlists.
- The MPD command ``playlistinfo`` is now faster, thanks to John Bäckstrand.
- Added the method
: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 for pinpointing the issue.
v0.6.0 (2011-10-09)
===================

42
mopidy/backends/base/current_playlist.py
View File

@ -28,7 +28,7 @@ class CurrentPlaylistController(object):
Read-only.
"""
return [copy(ct) for ct in self._cp_tracks]
return [copy(cp_track) for cp_track in self._cp_tracks]
@property
def tracks(self):
@ -37,7 +37,14 @@ class CurrentPlaylistController(object):
Read-only.
"""
return [ct[1] for ct in self._cp_tracks]
return [cp_track.track for cp_track in self._cp_tracks]
@property
def length(self):
"""
Length of the current playlist.
"""
return len(self._cp_tracks)
@property
def version(self):
@ -116,9 +123,9 @@ class CurrentPlaylistController(object):
matches = self._cp_tracks
for (key, value) in criteria.iteritems():
if key == 'cpid':
matches = filter(lambda ct: ct[0] == value, matches)
matches = filter(lambda ct: ct.cpid == value, matches)
else:
matches = filter(lambda ct: getattr(ct[1], key) == value,
matches = filter(lambda ct: getattr(ct.track, key) == value,
matches)
if len(matches) == 1:
return matches[0]
@ -129,6 +136,19 @@ class CurrentPlaylistController(object):
else:
raise LookupError(u'"%s" match multiple tracks' % criteria_string)
def index(self, cp_track):
"""
Get index of the given (CPID integer, :class:`mopidy.models.Track`)
two-tuple in the current playlist.
Raises :exc:`ValueError` if not found.
:param cp_track: track to find the index of
:type cp_track: two-tuple (CPID integer, :class:`mopidy.models.Track`)
:rtype: int
"""
return self._cp_tracks.index(cp_track)
def move(self, start, end, to_position):
"""
Move the tracks in the slice ``[start:end]`` to ``to_position``.
@ -168,7 +188,6 @@ class CurrentPlaylistController(object):
:param criteria: on or more criteria to match by
:type criteria: dict
:type track: :class:`mopidy.models.Track`
"""
cp_track = self.get(**criteria)
position = self._cp_tracks.index(cp_track)
@ -204,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')

64
mopidy/frontends/mpd/protocol/current_playlist.py
View File

@ -1,7 +1,8 @@
from mopidy.frontends.mpd.exceptions import (MpdArgError, MpdNoExistError,
MpdNotImplemented)
from mopidy.frontends.mpd.protocol import handle_request
from mopidy.frontends.mpd.translator import tracks_to_mpd_format
from mopidy.frontends.mpd.translator import (track_to_mpd_format,
tracks_to_mpd_format)
@handle_request(r'^add "(?P<uri>[^"]*)"$')
def add(context, uri):
@ -74,8 +75,8 @@ def delete_range(context, start, end=None):
if end is not None:
end = int(end)
else:
end = len(context.backend.current_playlist.tracks.get())
cp_tracks = context.backend.current_playlist.cp_tracks.get()[start:end]
end = context.backend.current_playlist.length.get()
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:
@ -86,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')
@ -157,8 +159,7 @@ def moveid(context, cpid, to):
cpid = int(cpid)
to = int(to)
cp_track = context.backend.current_playlist.get(cpid=cpid).get()
position = context.backend.current_playlist.cp_tracks.get().index(
cp_track)
position = context.backend.current_playlist.index(cp_track).get()
context.backend.current_playlist.move(position, position + 1, to)
@handle_request(r'^playlist$')
@ -193,10 +194,8 @@ def playlistfind(context, tag, needle):
if tag == 'filename':
try:
cp_track = context.backend.current_playlist.get(uri=needle).get()
(cpid, track) = cp_track
position = context.backend.current_playlist.cp_tracks.get().index(
cp_track)
return track.mpd_format(cpid=cpid, position=position)
position = context.backend.current_playlist.index(cp_track).get()
return track_to_mpd_format(cp_track, position=position)
except LookupError:
return None
raise MpdNotImplemented # TODO
@ -215,18 +214,16 @@ def playlistid(context, cpid=None):
try:
cpid = int(cpid)
cp_track = context.backend.current_playlist.get(cpid=cpid).get()
position = context.backend.current_playlist.cp_tracks.get().index(
cp_track)
return cp_track.track.mpd_format(position=position, cpid=cpid)
position = context.backend.current_playlist.index(cp_track).get()
return track_to_mpd_format(cp_track, position=position)
except LookupError:
raise MpdNoExistError(u'No such song', command=u'playlistid')
else:
cpids = [ct[0] for ct in
context.backend.current_playlist.cp_tracks.get()]
return tracks_to_mpd_format(
context.backend.current_playlist.tracks.get(), cpids=cpids)
context.backend.current_playlist.cp_tracks.get())
@handle_request(r'^playlistinfo$')
@handle_request(r'^playlistinfo "-1"$')
@handle_request(r'^playlistinfo "(?P<songpos>-?\d+)"$')
@handle_request(r'^playlistinfo "(?P<start>\d+):(?P<end>\d+)*"$')
def playlistinfo(context, songpos=None,
@ -245,36 +242,22 @@ def playlistinfo(context, songpos=None,
- uses negative indexes, like ``playlistinfo "-1"``, to request
the entire playlist
"""
if songpos == "-1":
songpos = None
if songpos is not None:
songpos = int(songpos)
start = songpos
end = songpos + 1
if start == -1:
end = None
cpids = [ct[0] for ct in
context.backend.current_playlist.cp_tracks.get()]
return tracks_to_mpd_format(
context.backend.current_playlist.tracks.get(),
start, end, cpids=cpids)
cp_track = context.backend.current_playlist.get(cpid=songpos).get()
return track_to_mpd_format(cp_track, position=songpos)
else:
if start is None:
start = 0
start = int(start)
if not (0 <= start <= len(
context.backend.current_playlist.tracks.get())):
if not (0 <= start <= context.backend.current_playlist.length.get()):
raise MpdArgError(u'Bad song index', command=u'playlistinfo')
if end is not None:
end = int(end)
if end > len(context.backend.current_playlist.tracks.get()):
if end > context.backend.current_playlist.length.get():
end = None
cpids = [ct[0] for ct in
context.backend.current_playlist.cp_tracks.get()]
return tracks_to_mpd_format(
context.backend.current_playlist.tracks.get(),
start, end, cpids=cpids)
cp_tracks = context.backend.current_playlist.cp_tracks.get()
return tracks_to_mpd_format(cp_tracks, start, end)
@handle_request(r'^playlistsearch "(?P<tag>[^"]+)" "(?P<needle>[^"]+)"$')
@handle_request(r'^playlistsearch (?P<tag>\S+) "(?P<needle>[^"]+)"$')
@ -313,10 +296,8 @@ def plchanges(context, version):
"""
# XXX Naive implementation that returns all tracks as changed
if int(version) < context.backend.current_playlist.version:
cpids = [ct[0] for ct in
context.backend.current_playlist.cp_tracks.get()]
return tracks_to_mpd_format(
context.backend.current_playlist.tracks.get(), cpids=cpids)
context.backend.current_playlist.cp_tracks.get())
@handle_request(r'^plchangesposid "(?P<version>\d+)"$')
def plchangesposid(context, version):
@ -392,7 +373,6 @@ def swapid(context, cpid1, cpid2):
cpid2 = int(cpid2)
cp_track1 = context.backend.current_playlist.get(cpid=cpid1).get()
cp_track2 = context.backend.current_playlist.get(cpid=cpid2).get()
cp_tracks = context.backend.current_playlist.cp_tracks.get()
position1 = cp_tracks.index(cp_track1)
position2 = cp_tracks.index(cp_track2)
position1 = context.backend.current_playlist.index(cp_track1).get()
position2 = context.backend.current_playlist.index(cp_track2).get()
swap(context, position1, position2)

9
mopidy/frontends/mpd/protocol/music_db.py
View File

@ -1,8 +1,9 @@
import re
import shlex
from mopidy.frontends.mpd.protocol import handle_request, stored_playlists
from mopidy.frontends.mpd.exceptions import MpdArgError, MpdNotImplemented
from mopidy.frontends.mpd.protocol import handle_request, stored_playlists
from mopidy.frontends.mpd.translator import playlist_to_mpd_format
def _build_query(mpd_query):
"""
@ -68,7 +69,8 @@ def find(context, mpd_query):
- also uses the search type "date".
"""
query = _build_query(mpd_query)
return context.backend.library.find_exact(**query).get().mpd_format()
return playlist_to_mpd_format(
context.backend.library.find_exact(**query).get())
@handle_request(r'^findadd '
r'(?P<query>("?([Aa]lbum|[Aa]rtist|[Ff]ilename|[Tt]itle|[Aa]ny)"? '
@ -324,7 +326,8 @@ def search(context, mpd_query):
- also uses the search type "date".
"""
query = _build_query(mpd_query)
return context.backend.library.search(**query).get().mpd_format()
return playlist_to_mpd_format(
context.backend.library.search(**query).get())
@handle_request(r'^update( "(?P<uri>[^"]+)")*$')
def update(context, uri=None, rescan_unmodified_files=False):

7
mopidy/frontends/mpd/protocol/playback.py
View File

@ -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

12
mopidy/frontends/mpd/protocol/status.py
View File

@ -1,8 +1,9 @@
import pykka.future
from mopidy.backends.base import PlaybackController
from mopidy.frontends.mpd.protocol import handle_request
from mopidy.frontends.mpd.exceptions import MpdNotImplemented
from mopidy.frontends.mpd.protocol import handle_request
from mopidy.frontends.mpd.translator import track_to_mpd_format
#: Subsystems that can be registered with idle command.
SUBSYSTEMS = ['database', 'mixer', 'options', 'output',
@ -32,9 +33,8 @@ def currentsong(context):
"""
current_cp_track = context.backend.playback.current_cp_track.get()
if current_cp_track is not None:
return current_cp_track.track.mpd_format(
position=context.backend.playback.current_playlist_position.get(),
cpid=current_cp_track.cpid)
position = context.backend.playback.current_playlist_position.get()
return track_to_mpd_format(current_cp_track, position=position)
@handle_request(r'^idle$')
@handle_request(r'^idle (?P<subsystems>.+)$')
@ -166,7 +166,7 @@ def status(context):
decimal places for millisecond precision.
"""
futures = {
'current_playlist.tracks': context.backend.current_playlist.tracks,
'current_playlist.length': context.backend.current_playlist.length,
'current_playlist.version': context.backend.current_playlist.version,
'mixer.volume': context.mixer.volume,
'playback.consume': context.backend.playback.consume,
@ -213,7 +213,7 @@ def _status_consume(futures):
return 0
def _status_playlist_length(futures):
return len(futures['current_playlist.tracks'].get())
return futures['current_playlist.length'].get()
def _status_playlist_version(futures):
return futures['current_playlist.version'].get()

5
mopidy/frontends/mpd/protocol/stored_playlists.py
View File

@ -1,7 +1,8 @@
import datetime as dt
from mopidy.frontends.mpd.protocol import handle_request
from mopidy.frontends.mpd.exceptions import MpdNoExistError, MpdNotImplemented
from mopidy.frontends.mpd.protocol import handle_request
from mopidy.frontends.mpd.translator import playlist_to_mpd_format
@handle_request(r'^listplaylist "(?P<name>[^"]+)"$')
def listplaylist(context, name):
@ -40,7 +41,7 @@ def listplaylistinfo(context, name):
"""
try:
playlist = context.backend.stored_playlists.get(name=name).get()
return playlist.mpd_format()
return playlist_to_mpd_format(playlist)
except LookupError:
raise MpdNoExistError(
u'No such playlist', command=u'listplaylistinfo')

26
mopidy/frontends/mpd/translator.py
View File

@ -2,26 +2,28 @@ import os
import re
from mopidy import settings
from mopidy.utils.path import mtime as get_mtime
from mopidy.frontends.mpd import protocol
from mopidy.utils.path import uri_to_path, split_path
from mopidy.models import CpTrack
from mopidy.utils.path import mtime as get_mtime, uri_to_path, split_path
def track_to_mpd_format(track, position=None, cpid=None):
def track_to_mpd_format(track, position=None):
"""
Format track for output to MPD client.
:param track: the track
:type track: :class:`mopidy.models.Track`
:type track: :class:`mopidy.models.Track` or :class:`mopidy.models.CpTrack`
:param position: track's position in playlist
:type position: integer
:param cpid: track's CPID (current playlist ID)
:type cpid: integer
:param key: if we should set key
:type key: boolean
:param mtime: if we should set mtime
:type mtime: boolean
:rtype: list of two-tuples
"""
if isinstance(track, CpTrack):
(cpid, track) = track
else:
(cpid, track) = (None, track)
result = [
('file', track.uri or ''),
('Time', track.length and (track.length // 1000) or 0),
@ -88,14 +90,15 @@ def artists_to_mpd_format(artists):
artists.sort(key=lambda a: a.name)
return u', '.join([a.name for a in artists if a.name])
def tracks_to_mpd_format(tracks, start=0, end=None, cpids=None):
def tracks_to_mpd_format(tracks, start=0, end=None):
"""
Format list of tracks for output to MPD client.
Optionally limit output to the slice ``[start:end]`` of the list.
:param tracks: the tracks
:type tracks: list of :class:`mopidy.models.Track`
:type tracks: list of :class:`mopidy.models.Track` or
:class:`mopidy.models.CpTrack`
:param start: position of first track to include in output
:type start: int (positive or negative)
:param end: position after last track to include in output
@ -106,11 +109,10 @@ def tracks_to_mpd_format(tracks, start=0, end=None, cpids=None):
end = len(tracks)
tracks = tracks[start:end]
positions = range(start, end)
cpids = cpids and cpids[start:end] or [None for _ in tracks]
assert len(tracks) == len(positions) == len(cpids)
assert len(tracks) == len(positions)
result = []
for track, position, cpid in zip(tracks, positions, cpids):
result.append(track_to_mpd_format(track, position, cpid))
for track, position in zip(tracks, positions):
result.append(track_to_mpd_format(track, position))
return result
def playlist_to_mpd_format(playlist, *args, **kwargs):

8
mopidy/models.py
View File

@ -185,10 +185,6 @@ class Track(ImmutableObject):
self.__dict__['artists'] = frozenset(kwargs.pop('artists', []))
super(Track, self).__init__(*args, **kwargs)
def mpd_format(self, *args, **kwargs):
from mopidy.frontends.mpd import translator
return translator.track_to_mpd_format(self, *args, **kwargs)
class Playlist(ImmutableObject):
"""
@ -224,7 +220,3 @@ class Playlist(ImmutableObject):
def length(self):
"""The number of tracks in the playlist. Read-only."""
return len(self.tracks)
def mpd_format(self, *args, **kwargs):
from mopidy.frontends.mpd import translator
return translator.playlist_to_mpd_format(self, *args, **kwargs)

33
tests/backends/base/current_playlist.py
View File

@ -1,7 +1,7 @@
import mock
import random
from mopidy.models import Playlist, Track
from mopidy.models import CpTrack, Playlist, Track
from mopidy.gstreamer import GStreamer
from tests.backends.base import populate_playlist
@ -18,6 +18,13 @@ class CurrentPlaylistControllerTest(object):
assert len(self.tracks) == 3, 'Need three tracks to run tests.'
def test_length(self):
self.assertEqual(0, len(self.controller.cp_tracks))
self.assertEqual(0, self.controller.length)
self.controller.append(self.tracks)
self.assertEqual(3, len(self.controller.cp_tracks))
self.assertEqual(3, self.controller.length)
def test_add(self):
for track in self.tracks:
cp_track = self.controller.add(track)
@ -136,6 +143,18 @@ class CurrentPlaylistControllerTest(object):
self.assertEqual(self.playback.state, self.playback.STOPPED)
self.assertEqual(self.playback.current_track, None)
def test_index_returns_index_of_track(self):
cp_tracks = []
for track in self.tracks:
cp_tracks.append(self.controller.add(track))
self.assertEquals(0, self.controller.index(cp_tracks[0]))
self.assertEquals(1, self.controller.index(cp_tracks[1]))
self.assertEquals(2, self.controller.index(cp_tracks[2]))
def test_index_raises_value_error_if_item_not_found(self):
test = lambda: self.controller.index(CpTrack(0, Track()))
self.assertRaises(ValueError, test)
@populate_playlist
def test_move_single(self):
self.controller.move(0, 0, 2)
@ -241,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([])

18
tests/frontends/mpd/protocol/current_playlist_test.py
View File

@ -271,11 +271,17 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
self.sendRequest(u'playlistinfo')
self.assertInResponse(u'Title: a')
self.assertInResponse(u'Pos: 0')
self.assertInResponse(u'Title: b')
self.assertInResponse(u'Pos: 1')
self.assertInResponse(u'Title: c')
self.assertInResponse(u'Pos: 2')
self.assertInResponse(u'Title: d')
self.assertInResponse(u'Pos: 3')
self.assertInResponse(u'Title: e')
self.assertInResponse(u'Pos: 4')
self.assertInResponse(u'Title: f')
self.assertInResponse(u'Pos: 5')
self.assertInResponse(u'OK')
def test_playlistinfo_with_songpos(self):
@ -286,11 +292,17 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
self.sendRequest(u'playlistinfo "4"')
self.assertNotInResponse(u'Title: a')
self.assertNotInResponse(u'Pos: 0')
self.assertNotInResponse(u'Title: b')
self.assertNotInResponse(u'Pos: 1')
self.assertNotInResponse(u'Title: c')
self.assertNotInResponse(u'Pos: 2')
self.assertNotInResponse(u'Title: d')
self.assertNotInResponse(u'Pos: 3')
self.assertInResponse(u'Title: e')
self.assertInResponse(u'Pos: 4')
self.assertNotInResponse(u'Title: f')
self.assertNotInResponse(u'Pos: 5')
self.assertInResponse(u'OK')
def test_playlistinfo_with_negative_songpos_same_as_playlistinfo(self):
@ -306,11 +318,17 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase):
self.sendRequest(u'playlistinfo "2:"')
self.assertNotInResponse(u'Title: a')
self.assertNotInResponse(u'Pos: 0')
self.assertNotInResponse(u'Title: b')
self.assertNotInResponse(u'Pos: 1')
self.assertInResponse(u'Title: c')
self.assertInResponse(u'Pos: 2')
self.assertInResponse(u'Title: d')
self.assertInResponse(u'Pos: 3')
self.assertInResponse(u'Title: e')
self.assertInResponse(u'Pos: 4')
self.assertInResponse(u'Title: f')
self.assertInResponse(u'Pos: 5')
self.assertInResponse(u'OK')
def test_playlistinfo_with_closed_range(self):

8
tests/frontends/mpd/serializer_test.py
View File

@ -4,7 +4,7 @@ import os
from mopidy import settings
from mopidy.utils.path import mtime, uri_to_path
from mopidy.frontends.mpd import translator, protocol
from mopidy.models import Album, Artist, Playlist, Track
from mopidy.models import Album, Artist, CpTrack, Playlist, Track
from tests import unittest
@ -45,17 +45,17 @@ class TrackMpdFormatTest(unittest.TestCase):
self.assert_(('Pos', 1) not in result)
def test_track_to_mpd_format_with_cpid(self):
result = translator.track_to_mpd_format(Track(), cpid=1)
result = translator.track_to_mpd_format(CpTrack(1, Track()))
self.assert_(('Id', 1) not in result)
def test_track_to_mpd_format_with_position_and_cpid(self):
result = translator.track_to_mpd_format(Track(), position=1, cpid=2)
result = translator.track_to_mpd_format(CpTrack(2, Track()), position=1)
self.assert_(('Pos', 1) in result)
self.assert_(('Id', 2) in result)
def test_track_to_mpd_format_for_nonempty_track(self):
result = translator.track_to_mpd_format(
self.track, position=9, cpid=122)
CpTrack(122, self.track), position=9)
self.assert_(('file', 'a uri') in result)
self.assert_(('Time', 137) in result)
self.assert_(('Artist', 'an artist') in result)