From 9871d999bb911c81d1772747cf155baefad4dd49 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 18 Apr 2015 23:04:48 +0200 Subject: [PATCH 1/5] mpd: Replace filterwarnings with deprecation helper --- mopidy/mpd/protocol/music_db.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mopidy/mpd/protocol/music_db.py b/mopidy/mpd/protocol/music_db.py index 0e59706f..0350fc21 100644 --- a/mopidy/mpd/protocol/music_db.py +++ b/mopidy/mpd/protocol/music_db.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, unicode_literals import functools import itertools -import warnings from mopidy.models import Track from mopidy.mpd import exceptions, protocol, translator @@ -174,10 +173,9 @@ def findadd(context, *args): results = context.core.library.search(query=query, exact=True).get() - with warnings.catch_warnings(): + with deprecation.ignore('core.tracklist.add:tracks_arg'): # TODO: for now just use tracks as other wise we have to lookup the # tracks we just got from the search. - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks" argument.*') context.core.tracklist.add(tracks=_get_tracks(results)).get() @@ -452,10 +450,9 @@ def searchadd(context, *args): results = context.core.library.search(query).get() - with warnings.catch_warnings(): + with deprecation.ignore('core.tracklist.add:tracks_arg'): # TODO: for now just use tracks as other wise we have to lookup the # tracks we just got from the search. - warnings.filterwarnings('ignore', 'tracklist.add.*"tracks".*') context.core.tracklist.add(_get_tracks(results)).get() From 7af570418f3966b9678e6c1b0444a3f9097a943b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 18 Apr 2015 23:05:50 +0200 Subject: [PATCH 2/5] mpd: Stop using properties to get values --- mopidy/mpd/protocol/playback.py | 8 ++++---- mopidy/mpd/protocol/status.py | 25 +++++++++++++------------ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index a537819c..26890d10 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -16,7 +16,7 @@ def consume(context, state): 1. When consume is activated, each song played is removed from playlist. """ - context.core.tracklist.consume = state + context.core.tracklist.set_consume(state) @protocol.commands.add('crossfade', seconds=protocol.UINT) @@ -279,7 +279,7 @@ def random(context, state): Sets random state to ``STATE``, ``STATE`` should be 0 or 1. """ - context.core.tracklist.random = state + context.core.tracklist.set_random(state) @protocol.commands.add('repeat', state=protocol.BOOL) @@ -291,7 +291,7 @@ def repeat(context, state): Sets repeat state to ``STATE``, ``STATE`` should be 0 or 1. """ - context.core.tracklist.repeat = state + context.core.tracklist.set_repeat(state) @protocol.commands.add('replay_gain_mode') @@ -409,7 +409,7 @@ def single(context, state): single is activated, playback is stopped after current song, or song is repeated if the ``repeat`` mode is enabled. """ - context.core.tracklist.single = state + context.core.tracklist.set_single(state) @protocol.commands.add('stop') diff --git a/mopidy/mpd/protocol/status.py b/mopidy/mpd/protocol/status.py index aa78b387..9e7c5180 100644 --- a/mopidy/mpd/protocol/status.py +++ b/mopidy/mpd/protocol/status.py @@ -172,20 +172,20 @@ def status(context): - ``elapsed``: Higher resolution means time in seconds with three decimal places for millisecond precision. """ + tl_track = context.core.playback.get_current_tl_track() + futures = { - 'tracklist.length': context.core.tracklist.length, - 'tracklist.version': context.core.tracklist.version, + 'tracklist.length': context.core.tracklist.get_length(), + 'tracklist.version': context.core.tracklist.get_version(), 'mixer.volume': context.core.mixer.get_volume(), - 'tracklist.consume': context.core.tracklist.consume, - 'tracklist.random': context.core.tracklist.random, - 'tracklist.repeat': context.core.tracklist.repeat, - 'tracklist.single': context.core.tracklist.single, - 'playback.state': context.core.playback.state, - 'playback.current_tl_track': context.core.playback.current_tl_track, - 'tracklist.index': ( - context.core.tracklist.index( - context.core.playback.current_tl_track.get())), - 'playback.time_position': context.core.playback.time_position, + 'tracklist.consume': context.core.tracklist.get_consume(), + 'tracklist.random': context.core.tracklist.get_random(), + 'tracklist.repeat': context.core.tracklist.get_repeat(), + 'tracklist.single': context.core.tracklist.get_single(), + 'playback.state': context.core.playback.get_state(), + 'playback.current_tl_track': tl_track, + 'tracklist.index': context.core.tracklist.index(tl_track.get()), + 'playback.time_position': context.core.playback.get_time_position(), } pykka.get_all(futures.values()) result = [ @@ -199,6 +199,7 @@ def status(context): ('xfade', _status_xfade(futures)), ('state', _status_state(futures)), ] + # TODO: add nextsong and nextsongid if futures['playback.current_tl_track'].get() is not None: result.append(('song', _status_songpos(futures))) result.append(('songid', _status_songid(futures))) From 14910730bd70d9135e291a0796d17cedbef5dc3a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 18 Apr 2015 23:13:03 +0200 Subject: [PATCH 3/5] mpd: Cleanup confusing argument names, fixes #1134 Makes sure not to use tlid when we mean songpos and also be a bit more consistent about what we call things across functions. --- mopidy/mpd/protocol/audio_output.py | 4 ++-- mopidy/mpd/protocol/current_playlist.py | 24 ++++++++++++------------ mopidy/mpd/protocol/playback.py | 18 +++++++++--------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/mopidy/mpd/protocol/audio_output.py b/mopidy/mpd/protocol/audio_output.py index 565ea3d0..059c505d 100644 --- a/mopidy/mpd/protocol/audio_output.py +++ b/mopidy/mpd/protocol/audio_output.py @@ -8,7 +8,7 @@ def disableoutput(context, outputid): """ *musicpd.org, audio output section:* - ``disableoutput`` + ``disableoutput {ID}`` Turns an output off. """ @@ -25,7 +25,7 @@ def enableoutput(context, outputid): """ *musicpd.org, audio output section:* - ``enableoutput`` + ``enableoutput {ID}`` Turns an output on. """ diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index e406f2ca..823c2e57 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -73,8 +73,8 @@ def addid(context, uri, songpos=None): return ('Id', tl_tracks[0].tlid) -@protocol.commands.add('delete', position=protocol.RANGE) -def delete(context, position): +@protocol.commands.add('delete', songrange=protocol.RANGE) +def delete(context, songrange): """ *musicpd.org, current playlist section:* @@ -82,8 +82,8 @@ def delete(context, position): Deletes a song from the playlist. """ - start = position.start - end = position.stop + start = songrange.start + end = songrange.stop if end is None: end = context.core.tracklist.length.get() tl_tracks = context.core.tracklist.slice(start, end).get() @@ -119,8 +119,8 @@ def clear(context): context.core.tracklist.clear() -@protocol.commands.add('move', position=protocol.RANGE, to=protocol.UINT) -def move_range(context, position, to): +@protocol.commands.add('move', songrange=protocol.RANGE, to=protocol.UINT) +def move_range(context, songrange, to): """ *musicpd.org, current playlist section:* @@ -129,8 +129,8 @@ def move_range(context, position, to): Moves the song at ``FROM`` or range of songs at ``START:END`` to ``TO`` in the playlist. """ - start = position.start - end = position.stop + start = songrange.start + end = songrange.stop if end is None: end = context.core.tracklist.length.get() context.core.tracklist.move(start, end, to) @@ -320,8 +320,8 @@ def plchangesposid(context, version): return result -@protocol.commands.add('shuffle', position=protocol.RANGE) -def shuffle(context, position=None): +@protocol.commands.add('shuffle', songrange=protocol.RANGE) +def shuffle(context, songrange=None): """ *musicpd.org, current playlist section:* @@ -330,10 +330,10 @@ def shuffle(context, position=None): Shuffles the current playlist. ``START:END`` is optional and specifies a range of songs. """ - if position is None: + if songrange is None: start, end = None, None else: - start, end = position.start, position.stop + start, end = songrange.start, songrange.stop context.core.tracklist.shuffle(start, end) diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 26890d10..21597ee3 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -145,8 +145,8 @@ def pause(context, state=None): context.core.playback.resume() -@protocol.commands.add('play', tlid=protocol.INT) -def play(context, tlid=None): +@protocol.commands.add('play', songpos=protocol.INT) +def play(context, songpos=None): """ *musicpd.org, playback section:* @@ -170,13 +170,13 @@ def play(context, tlid=None): - issues ``play 6`` without quotes around the argument. """ - if tlid is None: + if songpos is None: return context.core.playback.play().get() - elif tlid == -1: + elif songpos == -1: return _play_minus_one(context) try: - tl_track = context.core.tracklist.slice(tlid, tlid + 1).get()[0] + tl_track = context.core.tracklist.slice(songpos, songpos + 1).get()[0] return context.core.playback.play(tl_track).get() except IndexError: raise exceptions.MpdArgError('Bad song index') @@ -324,8 +324,8 @@ def replay_gain_status(context): return 'off' # TODO -@protocol.commands.add('seek', tlid=protocol.UINT, seconds=protocol.UINT) -def seek(context, tlid, seconds): +@protocol.commands.add('seek', songpos=protocol.UINT, seconds=protocol.UINT) +def seek(context, songpos, seconds): """ *musicpd.org, playback section:* @@ -339,8 +339,8 @@ def seek(context, tlid, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ tl_track = context.core.playback.current_tl_track.get() - if context.core.tracklist.index(tl_track).get() != tlid: - play(context, tlid) + if context.core.tracklist.index(tl_track).get() != songpos: + play(context, songpos) context.core.playback.seek(seconds * 1000).get() From b631f1111b3660ff5509934b44b4bd8f1c45f9aa Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 18 Apr 2015 23:18:32 +0200 Subject: [PATCH 4/5] mpd: Reduce thread switches by reusing values from core --- mopidy/mpd/protocol/playback.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 21597ee3..2087ea97 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -183,18 +183,21 @@ def play(context, songpos=None): def _play_minus_one(context): - if (context.core.playback.state.get() == PlaybackState.PLAYING): + playback_state = context.core.playback.get_state().get() + if playback_state == PlaybackState.PLAYING: return # Nothing to do - elif (context.core.playback.state.get() == PlaybackState.PAUSED): + elif playback_state == PlaybackState.PAUSED: return context.core.playback.resume().get() - elif context.core.playback.current_tl_track.get() is not None: - tl_track = context.core.playback.current_tl_track.get() - return context.core.playback.play(tl_track).get() - elif context.core.tracklist.slice(0, 1).get(): - tl_track = context.core.tracklist.slice(0, 1).get()[0] - return context.core.playback.play(tl_track).get() - else: - return # Fail silently + + current_tl_track = context.core.playback.get_current_tl_track().get() + if current_tl_track is not None: + return context.core.playback.play(current_tl_track).get() + + tl_tracks = context.core.tracklist.slice(0, 1).get() + if tl_tracks: + return context.core.playback.play(tl_tracks[0]).get() + + return # Fail silently @protocol.commands.add('playid', tlid=protocol.INT) From 84546488c1d7d5381f016e47d37d32d8034b4a69 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 18 Apr 2015 23:32:02 +0200 Subject: [PATCH 5/5] mpd: Remove core attribute usage --- mopidy/mpd/protocol/current_playlist.py | 26 ++++++++++++++----------- mopidy/mpd/protocol/playback.py | 11 ++++++----- mopidy/mpd/protocol/reflection.py | 2 +- mopidy/mpd/protocol/status.py | 2 +- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/mopidy/mpd/protocol/current_playlist.py b/mopidy/mpd/protocol/current_playlist.py index 823c2e57..f93722ee 100644 --- a/mopidy/mpd/protocol/current_playlist.py +++ b/mopidy/mpd/protocol/current_playlist.py @@ -64,10 +64,14 @@ def addid(context, uri, songpos=None): """ if not uri: raise exceptions.MpdNoExistError('No such song') - if songpos is not None and songpos > context.core.tracklist.length.get(): + + length = context.core.tracklist.get_length() + if songpos is not None and songpos > length.get(): raise exceptions.MpdArgError('Bad song index') + tl_tracks = context.core.tracklist.add( uris=[uri], at_position=songpos).get() + if not tl_tracks: raise exceptions.MpdNoExistError('No such song') return ('Id', tl_tracks[0].tlid) @@ -85,7 +89,7 @@ def delete(context, songrange): start = songrange.start end = songrange.stop if end is None: - end = context.core.tracklist.length.get() + end = context.core.tracklist.get_length().get() tl_tracks = context.core.tracklist.slice(start, end).get() if not tl_tracks: raise exceptions.MpdArgError('Bad song index', command='delete') @@ -132,7 +136,7 @@ def move_range(context, songrange, to): start = songrange.start end = songrange.stop if end is None: - end = context.core.tracklist.length.get() + end = context.core.tracklist.get_length().get() context.core.tracklist.move(start, end, to) @@ -211,7 +215,7 @@ def playlistid(context, tlid=None): return translator.track_to_mpd_format(tl_tracks[0], position=position) else: return translator.tracks_to_mpd_format( - context.core.tracklist.tl_tracks.get()) + context.core.tracklist.get_tl_tracks().get()) @protocol.commands.add('playlistinfo') @@ -236,7 +240,7 @@ def playlistinfo(context, parameter=None): tracklist_slice = protocol.RANGE(parameter) start, end = tracklist_slice.start, tracklist_slice.stop - tl_tracks = context.core.tracklist.tl_tracks.get() + tl_tracks = context.core.tracklist.get_tl_tracks().get() if start and start > len(tl_tracks): raise exceptions.MpdArgError('Bad song index') if end and end > len(tl_tracks): @@ -279,10 +283,10 @@ def plchanges(context, version): - Calls ``plchanges "-1"`` two times per second to get the entire playlist. """ # XXX Naive implementation that returns all tracks as changed - tracklist_version = context.core.tracklist.version.get() + tracklist_version = context.core.tracklist.get_version().get() if version < tracklist_version: return translator.tracks_to_mpd_format( - context.core.tracklist.tl_tracks.get()) + context.core.tracklist.get_tl_tracks().get()) elif version == tracklist_version: # A version match could indicate this is just a metadata update, so # check for a stream ref and let the client know about the change. @@ -290,7 +294,7 @@ def plchanges(context, version): if stream_title is None: return None - tl_track = context.core.playback.current_tl_track.get() + tl_track = context.core.playback.get_current_tl_track().get() position = context.core.tracklist.index(tl_track).get() return translator.track_to_mpd_format( tl_track, position=position, stream_title=stream_title) @@ -311,10 +315,10 @@ def plchangesposid(context, version): ``playlistlength`` returned by status command. """ # XXX Naive implementation that returns all tracks as changed - if int(version) != context.core.tracklist.version.get(): + if int(version) != context.core.tracklist.get_version().get(): result = [] for (position, (tlid, _)) in enumerate( - context.core.tracklist.tl_tracks.get()): + context.core.tracklist.get_tl_tracks().get()): result.append(('cpos', position)) result.append(('Id', tlid)) return result @@ -346,7 +350,7 @@ def swap(context, songpos1, songpos2): Swaps the positions of ``SONG1`` and ``SONG2``. """ - tracks = context.core.tracklist.tracks.get() + tracks = context.core.tracklist.get_tracks().get() song1 = tracks[songpos1] song2 = tracks[songpos2] del tracks[songpos1] diff --git a/mopidy/mpd/protocol/playback.py b/mopidy/mpd/protocol/playback.py index 2087ea97..ce3174d7 100644 --- a/mopidy/mpd/protocol/playback.py +++ b/mopidy/mpd/protocol/playback.py @@ -135,9 +135,10 @@ def pause(context, state=None): if state is None: deprecation.warn('mpd.protocol.playback.pause:state_arg') - if (context.core.playback.state.get() == PlaybackState.PLAYING): + playback_state = context.core.playback.get_state().get() + if (playback_state == PlaybackState.PLAYING): context.core.playback.pause() - elif (context.core.playback.state.get() == PlaybackState.PAUSED): + elif (playback_state == PlaybackState.PAUSED): context.core.playback.resume() elif state: context.core.playback.pause() @@ -341,7 +342,7 @@ def seek(context, songpos, seconds): - issues ``seek 1 120`` without quotes around the arguments. """ - tl_track = context.core.playback.current_tl_track.get() + tl_track = context.core.playback.get_current_tl_track().get() if context.core.tracklist.index(tl_track).get() != songpos: play(context, songpos) context.core.playback.seek(seconds * 1000).get() @@ -356,7 +357,7 @@ def seekid(context, tlid, seconds): Seeks to the position ``TIME`` (in seconds) of song ``SONGID``. """ - tl_track = context.core.playback.current_tl_track.get() + tl_track = context.core.playback.get_current_tl_track().get() if not tl_track or tl_track.tlid != tlid: playid(context, tlid) context.core.playback.seek(seconds * 1000).get() @@ -373,7 +374,7 @@ def seekcur(context, time): '+' or '-', then the time is relative to the current playing position. """ if time.startswith(('+', '-')): - position = context.core.playback.time_position.get() + position = context.core.playback.get_time_position().get() position += protocol.INT(time) * 1000 context.core.playback.seek(position).get() else: diff --git a/mopidy/mpd/protocol/reflection.py b/mopidy/mpd/protocol/reflection.py index 7feccca1..a3608a96 100644 --- a/mopidy/mpd/protocol/reflection.py +++ b/mopidy/mpd/protocol/reflection.py @@ -107,4 +107,4 @@ def urlhandlers(context): """ return [ ('handler', uri_scheme) - for uri_scheme in context.core.uri_schemes.get()] + for uri_scheme in context.core.get_uri_schemes().get()] diff --git a/mopidy/mpd/protocol/status.py b/mopidy/mpd/protocol/status.py index 9e7c5180..16e9d013 100644 --- a/mopidy/mpd/protocol/status.py +++ b/mopidy/mpd/protocol/status.py @@ -34,7 +34,7 @@ def currentsong(context): Displays the song info of the current song (same song that is identified in status). """ - tl_track = context.core.playback.current_tl_track.get() + tl_track = context.core.playback.get_current_tl_track().get() stream_title = context.core.playback.get_stream_title().get() if tl_track is not None: position = context.core.tracklist.index(tl_track).get()