From 42fdaf3ff01440445f134d392f6014365f394968 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 13 Nov 2012 14:02:57 +0100 Subject: [PATCH 01/46] audio: Move tests to make room for more audio tests --- tests/audio/__init__.py | 0 tests/{audio_test.py => audio/actor_test.py} | 0 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/audio/__init__.py rename tests/{audio_test.py => audio/actor_test.py} (100%) diff --git a/tests/audio/__init__.py b/tests/audio/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/audio_test.py b/tests/audio/actor_test.py similarity index 100% rename from tests/audio_test.py rename to tests/audio/actor_test.py From 76b1fa8e1bc7efa89533d90f2fbf23a8619a512c Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Tue, 13 Nov 2012 20:31:38 +0100 Subject: [PATCH 02/46] audio: Add state_changed(old_state, new_state) event --- mopidy/audio/listener.py | 15 +++++++++++++++ tests/audio/listener_test.py | 16 ++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 tests/audio/listener_test.py diff --git a/mopidy/audio/listener.py b/mopidy/audio/listener.py index 42c85e1e..da5f7b39 100644 --- a/mopidy/audio/listener.py +++ b/mopidy/audio/listener.py @@ -28,3 +28,18 @@ class AudioListener(object): *MAY* be implemented by actor. """ pass + + def state_changed(self, old_state, new_state): + """ + Called after the playback state have changed. + + Will be called for both immediate and async state changes in GStreamer. + + *MAY* be implemented by actor. + + :param old_state: the state before the change + :type old_state: string from :class:`mopidy.core.PlaybackState` field + :param new_state: the state after the change + :type new_state: string from :class:`mopidy.core.PlaybackState` field + """ + pass diff --git a/tests/audio/listener_test.py b/tests/audio/listener_test.py new file mode 100644 index 00000000..b3274721 --- /dev/null +++ b/tests/audio/listener_test.py @@ -0,0 +1,16 @@ +from __future__ import unicode_literals + +from mopidy import audio + +from tests import unittest + + +class AudioListenerTest(unittest.TestCase): + def setUp(self): + self.listener = audio.AudioListener() + + def test_listener_has_default_impl_for_reached_end_of_stream(self): + self.listener.reached_end_of_stream() + + def test_listener_has_default_impl_for_state_changed(self): + self.listener.state_changed(None, None) From f9bd0d00b3ae187dbe1e441a154508cb718f2c95 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 00:35:17 +0100 Subject: [PATCH 03/46] audio: Move PlaybackState from core to audio so audio can use it --- mopidy/audio/__init__.py | 1 + mopidy/audio/constants.py | 16 ++++++++++++++++ mopidy/core/playback.py | 17 ++--------------- 3 files changed, 19 insertions(+), 15 deletions(-) create mode 100644 mopidy/audio/constants.py diff --git a/mopidy/audio/__init__.py b/mopidy/audio/__init__.py index c3fbc0c9..7cf1dcee 100644 --- a/mopidy/audio/__init__.py +++ b/mopidy/audio/__init__.py @@ -3,3 +3,4 @@ from __future__ import unicode_literals # flake8: noqa from .actor import Audio from .listener import AudioListener +from .constants import PlaybackState diff --git a/mopidy/audio/constants.py b/mopidy/audio/constants.py new file mode 100644 index 00000000..08ad9768 --- /dev/null +++ b/mopidy/audio/constants.py @@ -0,0 +1,16 @@ +from __future__ import unicode_literals + + +class PlaybackState(object): + """ + Enum of playback states. + """ + + #: Constant representing the paused state. + PAUSED = 'paused' + + #: Constant representing the playing state. + PLAYING = 'playing' + + #: Constant representing the stopped state. + STOPPED = 'stopped' diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 54364ec2..273eb68d 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -4,6 +4,8 @@ import logging import random import urlparse +from mopidy.audio import PlaybackState + from . import listener @@ -24,21 +26,6 @@ def option_wrapper(name, default): return property(get_option, set_option) -class PlaybackState(object): - """ - Enum of playback states. - """ - - #: Constant representing the paused state. - PAUSED = 'paused' - - #: Constant representing the playing state. - PLAYING = 'playing' - - #: Constant representing the stopped state. - STOPPED = 'stopped' - - class PlaybackController(object): # pylint: disable = R0902 # Too many instance attributes From 87ce7bbe115aabd107149e4730b7c0a1e83b6b78 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 01:49:05 +0100 Subject: [PATCH 04/46] audio: Maintain state and trigger events based on GStreamer state changes --- mopidy/audio/actor.py | 44 +++++++++++++++++++++++++++++++---- tests/audio/actor_test.py | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index a7b4e8d8..49794c76 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -13,6 +13,7 @@ from mopidy import settings from mopidy.utils import process from . import mixers +from .constants import PlaybackState from .listener import AudioListener logger = logging.getLogger('mopidy.audio') @@ -29,9 +30,11 @@ class Audio(pykka.ThreadingActor): - :attr:`mopidy.settings.OUTPUT` - :attr:`mopidy.settings.MIXER` - :attr:`mopidy.settings.MIXER_TRACK` - """ + #: The GStreamer state mapped to :class:`mopidy.audio.PlaybackState` + state = PlaybackState.STOPPED + def __init__(self): super(Audio, self).__init__() @@ -160,8 +163,12 @@ class Audio(pykka.ThreadingActor): bus.remove_signal_watch() def _on_message(self, bus, message): - if message.type == gst.MESSAGE_EOS: - self._trigger_reached_end_of_stream_event() + if (message.type == gst.MESSAGE_STATE_CHANGED + and message.src == self._playbin): + old_state, new_state, pending_state = message.parse_state_changed() + self._on_playbin_state_changed(old_state, new_state, pending_state) + elif message.type == gst.MESSAGE_EOS: + self._on_end_of_stream() elif message.type == gst.MESSAGE_ERROR: error, debug = message.parse_error() logger.error('%s %s', error, debug) @@ -170,8 +177,35 @@ class Audio(pykka.ThreadingActor): error, debug = message.parse_warning() logger.warning('%s %s', error, debug) - def _trigger_reached_end_of_stream_event(self): - logger.debug('Triggering reached end of stream event') + def _on_playbin_state_changed(self, old_state, new_state, pending_state): + if new_state == gst.STATE_READY and pending_state == gst.STATE_NULL: + # XXX: We're not called on the last state cheng when going down to + # NULL, so we rewrite the second to last call to get the expected + # behavior. + new_state = gst.STATE_NULL + pending_state = gst.STATE_VOID_PENDING + + if pending_state != gst.STATE_VOID_PENDING: + return # Ignore intermediate state changes + + if new_state == gst.STATE_READY: + return # Ignore READY state as it's GStreamer specific + + if new_state == gst.STATE_PLAYING: + new_state = PlaybackState.PLAYING + elif new_state == gst.STATE_PAUSED: + new_state = PlaybackState.PAUSED + elif new_state == gst.STATE_NULL: + new_state = PlaybackState.STOPPED + + old_state, self.state = self.state, new_state + + logger.debug('Triggering state_changed event') + AudioListener.send('state_changed', + old_state=old_state, new_state=new_state) + + def _on_end_of_stream(self): + logger.debug('Triggering reached_end_of_stream event') AudioListener.send('reached_end_of_stream') def set_uri(self, uri): diff --git a/tests/audio/actor_test.py b/tests/audio/actor_test.py index b8b65e83..64666d9d 100644 --- a/tests/audio/actor_test.py +++ b/tests/audio/actor_test.py @@ -1,5 +1,9 @@ from __future__ import unicode_literals +import pygst +pygst.require('0.10') +import gst + from mopidy import audio, settings from mopidy.utils.path import path_to_uri @@ -63,3 +67,48 @@ class AudioTest(unittest.TestCase): @unittest.SkipTest def test_invalid_output_raises_error(self): pass # TODO + + +class AudioStateTest(unittest.TestCase): + def setUp(self): + self.audio = audio.Audio() + + def test_state_starts_as_stopped(self): + self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) + + def test_state_does_not_change_when_in_gst_ready_state(self): + self.audio._on_playbin_state_changed( + gst.STATE_NULL, gst.STATE_READY, gst.STATE_VOID_PENDING) + + self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) + + def test_state_changes_from_stopped_to_playing_on_play(self): + self.audio._on_playbin_state_changed( + gst.STATE_NULL, gst.STATE_READY, gst.STATE_PLAYING) + self.audio._on_playbin_state_changed( + gst.STATE_READY, gst.STATE_PAUSED, gst.STATE_PLAYING) + self.audio._on_playbin_state_changed( + gst.STATE_PAUSED, gst.STATE_PLAYING, gst.STATE_VOID_PENDING) + + self.assertEqual(audio.PlaybackState.PLAYING, self.audio.state) + + def test_state_changes_from_playing_to_paused_on_pause(self): + self.audio.state = audio.PlaybackState.PLAYING + + self.audio._on_playbin_state_changed( + gst.STATE_PLAYING, gst.STATE_PAUSED, gst.STATE_VOID_PENDING) + + self.assertEqual(audio.PlaybackState.PAUSED, self.audio.state) + + def test_state_changes_from_playing_to_stopped_on_stop(self): + self.audio.state = audio.PlaybackState.PLAYING + + self.audio._on_playbin_state_changed( + gst.STATE_PLAYING, gst.STATE_PAUSED, gst.STATE_NULL) + self.audio._on_playbin_state_changed( + gst.STATE_PAUSED, gst.STATE_READY, gst.STATE_NULL) + # We never get the following call, so the logic must work without it + #self.audio._on_playbin_state_changed( + # gst.STATE_READY, gst.STATE_NULL, gst.STATE_VOID_PENDING) + + self.assertEqual(audio.PlaybackState.STOPPED, self.audio.state) From 9168982a6173dd9da0c4f441a5a71d5859956f2b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 01:56:46 +0100 Subject: [PATCH 05/46] core: Pause playback if audio is paused and playback isn't (fixes #232) --- mopidy/core/actor.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 731e5309..858eeaf9 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -4,7 +4,7 @@ import itertools import pykka -from mopidy.audio import AudioListener +from mopidy.audio import AudioListener, PlaybackState from .library import LibraryController from .playback import PlaybackController @@ -55,6 +55,14 @@ class Core(pykka.ThreadingActor, AudioListener): def reached_end_of_stream(self): self.playback.on_end_of_track() + def state_changed(self, old_state, new_state): + # XXX: This is a temporary fix for issue #232 while we wait for a more + # permanent solution with the implementation of issue #234. + if (new_state == PlaybackState.PAUSED + and self.playback.state != PlaybackState.PAUSED): + self.playback.state = new_state + self.playback._trigger_track_playback_paused() + class Backends(list): def __init__(self, backends): From 6b7256a9556cc719173591be3371d4b8fdc3d140 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 10:51:52 +0100 Subject: [PATCH 06/46] audio: Explicitly disconnect signal handles on teardown --- mopidy/audio/actor.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index a7b4e8d8..c2448e1f 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -40,7 +40,8 @@ class Audio(pykka.ThreadingActor): self._mixer_track = None self._software_mixing = False - self._message_processor_set_up = False + self._notify_source_signal_id = None + self._message_signal_id = None def on_start(self): try: @@ -63,7 +64,8 @@ class Audio(pykka.ThreadingActor): fakesink = gst.element_factory_make('fakesink') self._playbin.set_property('video-sink', fakesink) - self._playbin.connect('notify::source', self._on_new_source) + self._notify_source_signal_id = self._playbin.connect( + 'notify::source', self._on_new_source) def _on_new_source(self, element, pad): uri = element.get_property('uri') @@ -79,6 +81,8 @@ class Audio(pykka.ThreadingActor): source.set_property('caps', default_caps) def _teardown_playbin(self): + if self._notify_source_signal_id: + self._playbin.disconnect(self._notify_source_signal_id) self._playbin.set_state(gst.STATE_NULL) def _setup_output(self): @@ -151,12 +155,12 @@ class Audio(pykka.ThreadingActor): def _setup_message_processor(self): bus = self._playbin.get_bus() bus.add_signal_watch() - bus.connect('message', self._on_message) - self._message_processor_set_up = True + self._message_signal_id = bus.connect('message', self._on_message) def _teardown_message_processor(self): - if self._message_processor_set_up: + if self._message_signal_id: bus = self._playbin.get_bus() + bus.disconnect(self._message_signal_id) bus.remove_signal_watch() def _on_message(self, bus, message): From 75f7fc273aea55645143a767e63e190c612bde96 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 10:58:02 +0100 Subject: [PATCH 07/46] spotify: Fix GStreamer warning on seek (fixes #227) Don't call audio.{prepare_change,start_playback}() before/after seek. This does not seem to have any effect on functionality, and avoids Gstreamer failing to disconnect the "notify::source" signal handler, which again causes a warning. --- mopidy/backends/spotify/playback.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mopidy/backends/spotify/playback.py b/mopidy/backends/spotify/playback.py index de82464a..d3585021 100644 --- a/mopidy/backends/spotify/playback.py +++ b/mopidy/backends/spotify/playback.py @@ -52,12 +52,8 @@ class SpotifyPlaybackProvider(base.BasePlaybackProvider): return self.seek(time_position) def seek(self, time_position): - self.audio.prepare_change() self.backend.spotify.session.seek(time_position) - self.audio.start_playback() - self._timer.seek(time_position) - return True def stop(self): From 8a440fff3f0abbb778ae4bc8ba064f6a8247c9d4 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 18:12:44 +0100 Subject: [PATCH 08/46] network: Server send buffer should be bytes, not unicode (#241) --- mopidy/utils/network.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 91831871..3ddfe2ee 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -140,7 +140,7 @@ class Connection(object): self.timeout = timeout self.send_lock = threading.Lock() - self.send_buffer = '' + self.send_buffer = b'' self.stopping = False @@ -193,7 +193,7 @@ class Connection(object): if e.errno in (errno.EWOULDBLOCK, errno.EINTR): return data self.stop('Unexpected client error: %s' % e) - return '' + return b'' def enable_timeout(self): """Reactivate timeout mechanism.""" From 8a292bce58f5c95c42a2c09e59dc4002b7abf830 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 20:45:24 +0100 Subject: [PATCH 09/46] scanner: Move main() function from bin/ to mopidy.scanner --- bin/mopidy-scan | 51 ++++------------------------------------------- mopidy/scanner.py | 43 ++++++++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/bin/mopidy-scan b/bin/mopidy-scan index 001ea372..00f51809 100755 --- a/bin/mopidy-scan +++ b/bin/mopidy-scan @@ -1,48 +1,5 @@ -#!/usr/bin/env python +#! /usr/bin/env python -from __future__ import unicode_literals - -import sys -import logging - -from mopidy import settings -from mopidy.utils.log import setup_console_logging, setup_root_logger -from mopidy.scanner import Scanner, translator -from mopidy.frontends.mpd.translator import tracks_to_tag_cache_format - - -setup_root_logger() -setup_console_logging(2) - - -tracks = [] - - -def store(data): - track = translator(data) - tracks.append(track) - logging.debug('Added %s', track.uri) - - -def debug(uri, error, debug): - logging.error('Failed %s: %s - %s', uri, error, debug) - - -logging.info('Scanning %s', settings.LOCAL_MUSIC_PATH) - - -scanner = Scanner(settings.LOCAL_MUSIC_PATH, store, debug) -try: - scanner.start() -except KeyboardInterrupt: - scanner.stop() - - -logging.info('Done') - - -for a in tracks_to_tag_cache_format(tracks): - if len(a) == 1: - print ('%s' % a).encode('utf-8') - else: - print ('%s: %s' % a).encode('utf-8') +if __name__ == '__main__': + from mopidy.scanner import main + main() diff --git a/mopidy/scanner.py b/mopidy/scanner.py index e5e484e5..c20ef4fb 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -1,5 +1,8 @@ from __future__ import unicode_literals +import logging +import datetime + import gobject gobject.threads_init() @@ -7,10 +10,40 @@ import pygst pygst.require('0.10') import gst -import datetime - -from mopidy.utils.path import path_to_uri, find_files +from mopidy import settings +from mopidy.frontends.mpd import translator as mpd_translator from mopidy.models import Track, Artist, Album +from mopidy.utils import log, path + + +def main(): + log.setup_root_logger() + log.setup_console_logging(2) + + tracks = [] + + def store(data): + track = translator(data) + tracks.append(track) + logging.debug('Added %s', track.uri) + + def debug(uri, error, debug): + logging.error('Failed %s: %s - %s', uri, error, debug) + + logging.info('Scanning %s', settings.LOCAL_MUSIC_PATH) + scanner = Scanner(settings.LOCAL_MUSIC_PATH, store, debug) + try: + scanner.start() + except KeyboardInterrupt: + scanner.stop() + + logging.info('Done') + + for row in mpd_translator.tracks_to_tag_cache_format(tracks): + if len(row) == 1: + print ('%s' % row).encode('utf-8') + else: + print ('%s: %s' % row).encode('utf-8') def translator(data): @@ -56,7 +89,7 @@ def translator(data): class Scanner(object): def __init__(self, folder, data_callback, error_callback=None): - self.files = find_files(folder) + self.files = path.find_files(folder) self.data_callback = data_callback self.error_callback = error_callback self.loop = gobject.MainLoop() @@ -119,7 +152,7 @@ class Scanner(object): def next_uri(self): try: - uri = path_to_uri(self.files.next()) + uri = path.path_to_uri(self.files.next()) except StopIteration: self.stop() return False From bcaeca7acc89324dea4d09f7d986db74d2ac7d04 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 22:51:12 +0100 Subject: [PATCH 10/46] scanner: Support multiple tag sets per track (fixes #236) --- docs/changes.rst | 4 ++++ mopidy/scanner.py | 58 +++++++++++++++++++++++++++++++---------------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index 42478178..c05cda1c 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -122,6 +122,10 @@ backends: - MPD no longer lowercases search queries. This broke e.g. search by URI, where casing may be essential. +- :issue:`236`: The ``mopidy-scan`` command failed to include tags from ALAC + files (Apple lossless) because it didn't support multiple tag messages from + GStreamer per track it scanned. + v0.8.1 (2012-10-30) =================== diff --git a/mopidy/scanner.py b/mopidy/scanner.py index c20ef4fb..d84c262c 100644 --- a/mopidy/scanner.py +++ b/mopidy/scanner.py @@ -89,51 +89,70 @@ def translator(data): class Scanner(object): def __init__(self, folder, data_callback, error_callback=None): + self.data = {} self.files = path.find_files(folder) self.data_callback = data_callback self.error_callback = error_callback self.loop = gobject.MainLoop() - fakesink = gst.element_factory_make('fakesink') + self.fakesink = gst.element_factory_make('fakesink') + self.fakesink.set_property('signal-handoffs', True) + self.fakesink.connect('handoff', self.process_handoff) self.uribin = gst.element_factory_make('uridecodebin') self.uribin.set_property('caps', gst.Caps(b'audio/x-raw-int')) - self.uribin.connect( - 'pad-added', self.process_new_pad, fakesink.get_pad('sink')) + self.uribin.connect('pad-added', self.process_new_pad) self.pipe = gst.element_factory_make('pipeline') self.pipe.add(self.uribin) - self.pipe.add(fakesink) + self.pipe.add(self.fakesink) bus = self.pipe.get_bus() bus.add_signal_watch() + bus.connect('message::application', self.process_application) bus.connect('message::tag', self.process_tags) bus.connect('message::error', self.process_error) - def process_new_pad(self, source, pad, target_pad): - pad.link(target_pad) + def process_handoff(self, fakesink, buffer_, pad): + # When this function is called the first buffer has reached the end of + # the pipeline, and we can continue with the next track. Since we're + # in another thread, we send a message back to the main thread using + # the bus. + structure = gst.Structure('handoff') + message = gst.message_new_application(fakesink, structure) + bus = self.pipe.get_bus() + bus.post(message) + + def process_new_pad(self, source, pad): + pad.link(self.fakesink.get_pad('sink')) + + def process_application(self, bus, message): + if message.src != self.fakesink: + return + + if message.structure.get_name() != 'handoff': + return + + self.data['uri'] = unicode(self.uribin.get_property('uri')) + self.data[gst.TAG_DURATION] = self.get_duration() + + try: + self.data_callback(self.data) + self.next_uri() + except KeyboardInterrupt: + self.stop() def process_tags(self, bus, message): taglist = message.parse_tag() - data = { - 'uri': unicode(self.uribin.get_property('uri')), - gst.TAG_DURATION: self.get_duration(), - } for key in taglist.keys(): # XXX: For some crazy reason some wma files spit out lists here, # not sure if this is due to better data in headers or wma being # stupid. So ugly hack for now :/ if type(taglist[key]) is list: - data[key] = taglist[key][0] + self.data[key] = taglist[key][0] else: - data[key] = taglist[key] - - try: - self.data_callback(data) - self.next_uri() - except KeyboardInterrupt: - self.stop() + self.data[key] = taglist[key] def process_error(self, bus, message): if self.error_callback: @@ -151,6 +170,7 @@ class Scanner(object): return None def next_uri(self): + self.data = {} try: uri = path.path_to_uri(self.files.next()) except StopIteration: @@ -158,7 +178,7 @@ class Scanner(object): return False self.pipe.set_state(gst.STATE_NULL) self.uribin.set_property('uri', uri) - self.pipe.set_state(gst.STATE_PAUSED) + self.pipe.set_state(gst.STATE_PLAYING) return True def start(self): From f0c8c2287fe0741d60cb8291ae6f21ce8d4160a8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 23:37:27 +0100 Subject: [PATCH 11/46] audio: Fix typo --- mopidy/audio/actor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 49794c76..1355cfef 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -179,7 +179,7 @@ class Audio(pykka.ThreadingActor): def _on_playbin_state_changed(self, old_state, new_state, pending_state): if new_state == gst.STATE_READY and pending_state == gst.STATE_NULL: - # XXX: We're not called on the last state cheng when going down to + # XXX: We're not called on the last state change when going down to # NULL, so we rewrite the second to last call to get the expected # behavior. new_state = gst.STATE_NULL From 36a14bd8d60786bb7b392261ad0f2a7a59fe30d6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 23:40:09 +0100 Subject: [PATCH 12/46] audio: Make log message more useful --- mopidy/audio/actor.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 1355cfef..f2208a0a 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -200,7 +200,9 @@ class Audio(pykka.ThreadingActor): old_state, self.state = self.state, new_state - logger.debug('Triggering state_changed event') + logger.debug( + 'Triggering event: state_changed(old_state=%s, new_state=%s)', + old_state, new_state) AudioListener.send('state_changed', old_state=old_state, new_state=new_state) From 326970e5adcb815e3249c0c249bdbfdc1943990d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Wed, 14 Nov 2012 23:42:33 +0100 Subject: [PATCH 13/46] core: Explain reason behind temporary state syncing hack --- mopidy/core/actor.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 858eeaf9..3ebb785b 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -57,7 +57,11 @@ class Core(pykka.ThreadingActor, AudioListener): def state_changed(self, old_state, new_state): # XXX: This is a temporary fix for issue #232 while we wait for a more - # permanent solution with the implementation of issue #234. + # permanent solution with the implementation of issue #234. When the + # Spotify play token is lost, the Spotify backend pauses audio + # playback, but mopidy.core doesn't know this, so we need to update + # mopidy.core's state to match the actual state in mopidy.audio. If we + # don't do this, clients will think that we're still playing. if (new_state == PlaybackState.PAUSED and self.playback.state != PlaybackState.PAUSED): self.playback.state = new_state From b3fd9d8b4071f6483566579fb1d9e0857aa84557 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 15 Nov 2012 08:48:43 +0100 Subject: [PATCH 14/46] deps: Pykka version check doesn't need to work with < 1.0 --- mopidy/utils/deps.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/mopidy/utils/deps.py b/mopidy/utils/deps.py index 41fd513d..3c177036 100644 --- a/mopidy/utils/deps.py +++ b/mopidy/utils/deps.py @@ -135,15 +135,9 @@ def _gstreamer_check_elements(): def pykka_info(): - if hasattr(pykka, '__version__'): - # Pykka >= 0.14 - version = pykka.__version__ - else: - # Pykka < 0.14 - version = pykka.get_version() return { 'name': 'Pykka', - 'version': version, + 'version': pykka.__version__, 'path': pykka.__file__, } From 684586dd184cda31a02fb473a0962eb3f405d3e4 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 15 Nov 2012 09:04:21 +0100 Subject: [PATCH 15/46] mpris: Update for MPRIS 2.2 compliance --- mopidy/frontends/mpris/objects.py | 4 +++- tests/frontends/mpris/root_interface_test.py | 14 +++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index 235dd80a..cf7f71ce 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -30,7 +30,7 @@ PLAYER_IFACE = 'org.mpris.MediaPlayer2.Player' class MprisObject(dbus.service.Object): - """Implements http://www.mpris.org/2.1/spec/""" + """Implements http://www.mpris.org/2.2/spec/""" properties = None @@ -46,6 +46,8 @@ class MprisObject(dbus.service.Object): def _get_root_iface_properties(self): return { 'CanQuit': (True, None), + 'Fullscreen': (False, None), + 'CanSetFullscreen': (False, None), 'CanRaise': (False, None), # NOTE Change if adding optional track list support 'HasTrackList': (False, None), diff --git a/tests/frontends/mpris/root_interface_test.py b/tests/frontends/mpris/root_interface_test.py index 9e16c6bb..722fd2cd 100644 --- a/tests/frontends/mpris/root_interface_test.py +++ b/tests/frontends/mpris/root_interface_test.py @@ -31,6 +31,18 @@ class RootInterfaceTest(unittest.TestCase): def test_constructor_connects_to_dbus(self): self.assert_(self.mpris._connect_to_dbus.called) + def test_fullscreen_returns_false(self): + result = self.mpris.Get(objects.ROOT_IFACE, 'Fullscreen') + self.assertFalse(result) + + def test_setting_fullscreen_fails_and_returns_none(self): + result = self.mpris.Set(objects.ROOT_IFACE, 'Fullscreen', 'True') + self.assertIsNone(result) + + def test_can_set_fullscreen_returns_false(self): + result = self.mpris.Get(objects.ROOT_IFACE, 'CanSetFullscreen') + self.assertFalse(result) + def test_can_raise_returns_false(self): result = self.mpris.Get(objects.ROOT_IFACE, 'CanRaise') self.assertFalse(result) @@ -64,7 +76,7 @@ class RootInterfaceTest(unittest.TestCase): self.assertEquals(result, 'foo') settings.runtime.clear() - def test_supported_uri_schemes_is_empty(self): + def test_supported_uri_schemes_includes_backend_uri_schemes(self): result = self.mpris.Get(objects.ROOT_IFACE, 'SupportedUriSchemes') self.assertEquals(len(result), 1) self.assertEquals(result[0], 'dummy') From 4aa23e3306a534e8222c5ede2f8eaabf863441d9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 15 Nov 2012 09:16:01 +0100 Subject: [PATCH 16/46] docs: A small changelog cleanup --- docs/changes.rst | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index c05cda1c..d2e7d7b5 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -56,23 +56,29 @@ backends: dummy/mocked lower layers easier than with the old variant, where dependencies where looked up in Pykka's actor registry. -- The stored playlists part of the core API has been revised to be more focused - around the playlist URI, and some redundant functionality has been removed: +- Renamed "current playlist" to "tracklist" everywhere, including the core API + used by frontends. - - :attr:`mopidy.core.StoredPlaylistsController.playlists` no longer supports +- Renamed "stored playlists" to "playlists" everywhere, including the core API + used by frontends. + +- The playlists part of the core API has been revised to be more focused around + the playlist URI, and some redundant functionality has been removed: + + - :attr:`mopidy.core.PlaylistsController.playlists` no longer supports assignment to it. The `playlists` property on the backend layer still does, and all functionality is maintained by assigning to the playlists collections at the backend level. - - :meth:`mopidy.core.StoredPlaylistsController.delete` now accepts an URI, - and not a playlist object. + - :meth:`mopidy.core.PlaylistsController.delete` now accepts an URI, and not + a playlist object. - - :meth:`mopidy.core.StoredPlaylistsController.save` now returns the saved + - :meth:`mopidy.core.PlaylistsController.save` now returns the saved playlist. The returned playlist may differ from the saved playlist, and should thus be used instead of the playlist passed to ``save()``. - - :meth:`mopidy.core.StoredPlaylistsController.rename` has been removed, - since renaming can be done with ``save()``. + - :meth:`mopidy.core.PlaylistsController.rename` has been removed, since + renaming can be done with ``save()``. **Changes** @@ -105,12 +111,6 @@ backends: - The Spotify backend now returns the track if you search for the Spotify track URI. (Fixes: :issue:`233`) -- Renamed "current playlist" to "tracklist" everywhere, including the core API - used by frontends. - -- Renamed "stored playlists" to "playlists" everywhere, including the core API - used by frontends. - **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now From 0a96e5dccb3f7177c239f6efefd176ea87850833 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 15 Nov 2012 22:34:20 +0100 Subject: [PATCH 17/46] Update emit_data to take buffers. Simplify emit data method to take Gstreamer buffers. This allows us to more concisely give it buffers with duration, timestamp and other relevant data set. --- mopidy/audio/actor.py | 12 +++--------- mopidy/backends/spotify/session_manager.py | 9 ++++++++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 162e2a05..b422bc67 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -225,22 +225,16 @@ class Audio(pykka.ThreadingActor): """ self._playbin.set_property('uri', uri) - def emit_data(self, capabilities, data): + def emit_data(self, buffer_): """ Call this to deliver raw audio data to be played. Note that the uri must be set to ``appsrc://`` for this to work. - :param capabilities: a GStreamer capabilities string - :type capabilities: string - :param data: raw audio data to be played + :param buffer_: buffer to pass to appsrc + :type buffer_: :class:`gst.Buffer` """ - caps = gst.caps_from_string(capabilities) - buffer_ = gst.Buffer(buffer(data)) - buffer_.set_caps(caps) - source = self._playbin.get_property('source') - source.set_property('caps', caps) source.emit('push-buffer', buffer_) def emit_end_of_stream(self): diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index cd3d97db..8032a289 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -1,5 +1,9 @@ from __future__ import unicode_literals +import pygst +pygst.require('0.10') +import gst + import logging import os import threading @@ -108,7 +112,10 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): 'sample_rate': sample_rate, 'channels': channels, } - self.audio.emit_data(capabilites, bytes(frames)) + buffer_ = gst.Buffer(bytes(frames)) + buffer_.set_caps(gst.caps_from_string(capabilites)) + + self.audio.emit_data(buffer_) return num_frames def play_token_lost(self, session): From f2b975cc37c93cf5633f85497d6e4c5ffa7f572a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 15 Nov 2012 22:38:27 +0100 Subject: [PATCH 18/46] Update emit_data to return true if data was delivered. This is probably not needed, but for the sake of correctnes it doesn't hurt. --- mopidy/audio/actor.py | 5 ++++- mopidy/backends/spotify/session_manager.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index b422bc67..a17033ed 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -231,11 +231,14 @@ class Audio(pykka.ThreadingActor): Note that the uri must be set to ``appsrc://`` for this to work. + Returns true if data was delivered. + :param buffer_: buffer to pass to appsrc :type buffer_: :class:`gst.Buffer` + :rtype: boolean """ source = self._playbin.get_property('source') - source.emit('push-buffer', buffer_) + return source.emit('push-buffer', buffer_) == gst.FLOW_OK def emit_end_of_stream(self): """ diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index 8032a289..998e4d5e 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -115,8 +115,10 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): buffer_ = gst.Buffer(bytes(frames)) buffer_.set_caps(gst.caps_from_string(capabilites)) - self.audio.emit_data(buffer_) - return num_frames + if self.audio.emit_data(buffer_).get(): + return num_frames + else: + return 0 def play_token_lost(self, session): """Callback used by pyspotify""" From d516e9023ab8ee0894341c70c8c6a75a33e9959e Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Thu, 15 Nov 2012 22:49:44 +0100 Subject: [PATCH 19/46] Store active appsrc and refuse data when it is not set. We use the new source flag and the about to finish flags to set and unset the current appsrc. In emit data we now return false if the appsrc is not set. Also note that we need to use b'' for Gstreamer properties as it can't convert unicode to the correct type. I also added the signal disconnect code for about to finish. --- mopidy/audio/actor.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index a17033ed..a23c4c43 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -42,8 +42,10 @@ class Audio(pykka.ThreadingActor): self._mixer = None self._mixer_track = None self._software_mixing = False + self._appsrc = None self._notify_source_signal_id = None + self._about_to_finish_id = None self._message_signal_id = None def on_start(self): @@ -67,9 +69,14 @@ class Audio(pykka.ThreadingActor): fakesink = gst.element_factory_make('fakesink') self._playbin.set_property('video-sink', fakesink) + self._about_to_finish_id = self._playbin.connect( + 'about-to-finish', self._on_about_to_finish) self._notify_source_signal_id = self._playbin.connect( 'notify::source', self._on_new_source) + def _on_about_to_finish(self, element): + self._appsrc = None + def _on_new_source(self, element, pad): uri = element.get_property('uri') if not uri or not uri.startswith('appsrc://'): @@ -82,8 +89,13 @@ class Audio(pykka.ThreadingActor): b'rate=(int)44100') source = element.get_property('source') source.set_property('caps', default_caps) + source.set_property('format', b'time') # Gstreamer does not like unicode + + self._appsrc = source def _teardown_playbin(self): + if self._about_to_finish_id: + self._playbin.disconnect(self._about_to_finish_id) if self._notify_source_signal_id: self._playbin.disconnect(self._notify_source_signal_id) self._playbin.set_state(gst.STATE_NULL) @@ -237,8 +249,9 @@ class Audio(pykka.ThreadingActor): :type buffer_: :class:`gst.Buffer` :rtype: boolean """ - source = self._playbin.get_property('source') - return source.emit('push-buffer', buffer_) == gst.FLOW_OK + if not self._appsrc: + return False + return self._appsrc.emit('push-buffer', buffer_) == gst.FLOW_OK def emit_end_of_stream(self): """ From 02b225eb6e8a32140dfac2f0b9de47780e79160f Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 15 Nov 2012 10:20:58 +0100 Subject: [PATCH 20/46] tests: Update dummy backend's playlists provider implementation --- mopidy/backends/dummy.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/mopidy/backends/dummy.py b/mopidy/backends/dummy.py index af8f7487..d3239b34 100644 --- a/mopidy/backends/dummy.py +++ b/mopidy/backends/dummy.py @@ -82,22 +82,30 @@ class DummyPlaybackProvider(base.BasePlaybackProvider): class DummyPlaylistsProvider(base.BasePlaylistsProvider): def create(self, name): - playlist = Playlist(name=name) + playlist = Playlist(name=name, uri='dummy:%s' % name) self._playlists.append(playlist) return playlist - def delete(self, playlist): - self._playlists.remove(playlist) + def delete(self, uri): + playlist = self.lookup(uri) + if playlist: + self._playlists.remove(playlist) def lookup(self, uri): - return filter(lambda p: p.uri == uri, self._playlists) + for playlist in self._playlists: + if playlist.uri == uri: + return playlist def refresh(self): pass - def rename(self, playlist, new_name): - self._playlists[self._playlists.index(playlist)] = \ - playlist.copy(name=new_name) - def save(self, playlist): - self._playlists.append(playlist) + old_playlist = self.lookup(playlist.uri) + + if old_playlist is not None: + index = self._playlists.index(old_playlist) + self._playlists[index] = playlist + else: + self._playlists.append(playlist) + + return playlist From fff70c46a65855899e252a6e43d427f51e43764b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 10:24:12 +0100 Subject: [PATCH 21/46] core: Make tracklist.append() return the appended TlTracks --- docs/changes.rst | 5 +++++ mopidy/core/tracklist.py | 6 +++++- tests/backends/base/tracklist.py | 9 ++++++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index d2e7d7b5..bf5dd8cb 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -111,6 +111,11 @@ backends: - The Spotify backend now returns the track if you search for the Spotify track URI. (Fixes: :issue:`233`) +- :meth:`mopidy.core.TracklistController.append` now returns a list of the + :class:`mopidy.models.TlTrack` instances that was added to the tracklist. + This makes it easier to start playing one of the tracks that was just + appended to the tracklist. + **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 529d2a7a..5458ee3e 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -92,13 +92,17 @@ class TracklistController(object): :param tracks: tracks to append :type tracks: list of :class:`mopidy.models.Track` + :rtype: list of class:`mopidy.models.TlTrack` """ + tl_tracks = [] for track in tracks: - self.add(track, increase_version=False) + tl_tracks.append(self.add(track, increase_version=False)) if tracks: self.version += 1 + return tl_tracks + def clear(self): """Clear the current playlist.""" self._tl_tracks = [] diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index 64ab10d4..e67a40e4 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -153,10 +153,13 @@ class TracklistControllerTest(object): self.assertEqual(self.playback.state, PlaybackState.STOPPED) self.assertEqual(self.playback.current_track, None) + @populate_playlist + def test_append_returns_the_tl_tracks_that_was_added(self): + tl_tracks = self.controller.append(self.controller.tracks[1:2]) + self.assertEqual(tl_tracks[0][1], self.controller.tracks[1]) + def test_index_returns_index_of_track(self): - tl_tracks = [] - for track in self.tracks: - tl_tracks.append(self.controller.add(track)) + tl_tracks = self.controller.append(self.tracks) self.assertEquals(0, self.controller.index(tl_tracks[0])) self.assertEquals(1, self.controller.index(tl_tracks[1])) self.assertEquals(2, self.controller.index(tl_tracks[2])) From cbc08f398c8de7d2fd0ad316c057ee7ebd5d69ab Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 11:40:09 +0100 Subject: [PATCH 22/46] core: Update tracklist docstrings --- mopidy/core/tracklist.py | 44 +++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 5458ee3e..f8cf819e 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -33,7 +33,7 @@ class TracklistController(object): @property def tracks(self): """ - List of :class:`mopidy.models.Track` in the current playlist. + List of :class:`mopidy.models.Track` in the tracklist. Read-only. """ @@ -42,15 +42,15 @@ class TracklistController(object): @property def length(self): """ - Length of the current playlist. + Length of the tracklist. """ return len(self._tl_tracks) @property def version(self): """ - The current playlist version. Integer which is increased every time the - current playlist is changed. Is not reset before Mopidy is restarted. + The tracklist version. Integer which is increased every time the + tracklist is changed. Is not reset before Mopidy is restarted. """ return self._version @@ -62,20 +62,19 @@ class TracklistController(object): def add(self, track, at_position=None, increase_version=True): """ - Add the track to the end of, or at the given position in the current - playlist. + Add the track to the end of, or at the given position in the tracklist. :param track: track to add :type track: :class:`mopidy.models.Track` - :param at_position: position in current playlist to add track + :param at_position: position in tracklist to add track :type at_position: int or :class:`None` - :param increase_version: if the playlist version should be increased + :param increase_version: if the tracklist version should be increased :type increase_version: :class:`True` or :class:`False` :rtype: two-tuple of (TLID integer, :class:`mopidy.models.Track`) that - was added to the current playlist playlist + was added to the tracklist """ assert at_position <= len(self._tl_tracks), \ - 'at_position can not be greater than playlist length' + 'at_position can not be greater than tracklist length' tl_track = TlTrack(self.tlid, track) if at_position is not None: self._tl_tracks.insert(at_position, tl_track) @@ -88,7 +87,7 @@ class TracklistController(object): def append(self, tracks): """ - Append the given tracks to the current playlist. + Append the given tracks to the tracklist. :param tracks: tracks to append :type tracks: list of :class:`mopidy.models.Track` @@ -104,20 +103,19 @@ class TracklistController(object): return tl_tracks def clear(self): - """Clear the current playlist.""" + """Clear the tracklist.""" self._tl_tracks = [] self.version += 1 def get(self, **criteria): """ - Get track by given criterias from current playlist. + Get track by given criterias from tracklist. Raises :exc:`LookupError` if a unique match is not found. Examples:: - get(tlid=7) # Returns track with TLID 7 - # (current playlist ID) + get(tlid=7) # Returns track with TLID 7 (tracklist ID) get(id=1) # Returns track with ID 1 get(uri='xyz') # Returns track with URI 'xyz' get(id=1, uri='xyz') # Returns track with ID 1 and URI 'xyz' @@ -145,7 +143,7 @@ class TracklistController(object): def index(self, tl_track): """ Get index of the given (TLID integer, :class:`mopidy.models.Track`) - two-tuple in the current playlist. + two-tuple in the tracklist. Raises :exc:`ValueError` if not found. @@ -174,10 +172,10 @@ class TracklistController(object): assert start < end, 'start must be smaller than end' assert start >= 0, 'start must be at least zero' assert end <= len(tl_tracks), \ - 'end can not be larger than playlist length' + 'end can not be larger than tracklist length' assert to_position >= 0, 'to_position must be at least zero' assert to_position <= len(tl_tracks), \ - 'to_position can not be larger than playlist length' + 'to_position can not be larger than tracklist length' new_tl_tracks = tl_tracks[:start] + tl_tracks[end:] for tl_track in tl_tracks[start:end]: @@ -188,7 +186,7 @@ class TracklistController(object): def remove(self, **criteria): """ - Remove the track from the current playlist. + Remove the track from the tracklist. Uses :meth:`get()` to lookup the track to remove. @@ -202,7 +200,7 @@ class TracklistController(object): def shuffle(self, start=None, end=None): """ - Shuffles the entire playlist. If ``start`` and ``end`` is given only + Shuffles the entire tracklist. If ``start`` and ``end`` is given only shuffles the slice ``[start:end]``. :param start: position of first track to shuffle @@ -220,7 +218,7 @@ class TracklistController(object): if end is not None: assert end <= len(tl_tracks), 'end can not be larger than ' + \ - 'playlist length' + 'tracklist length' before = tl_tracks[:start or 0] shuffled = tl_tracks[start:end] @@ -231,8 +229,8 @@ class TracklistController(object): def slice(self, start, end): """ - Returns a slice of the current playlist, limited by the given - start and end positions. + Returns a slice of the tracklist, limited by the given start and end + positions. :param start: position of first track to include in slice :type start: int From 476df7a14debcc9a2c3f867e9fe3716aaf986a75 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 11:40:39 +0100 Subject: [PATCH 23/46] core: Add tracklist_changed() event --- mopidy/core/listener.py | 8 ++++++++ tests/core/listener_test.py | 3 +++ 2 files changed, 11 insertions(+) diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index 9c8bf4bc..2cf49490 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -84,6 +84,14 @@ class CoreListener(object): """ pass + def tracklist_changed(self): + """ + Called whenever the tracklist is changed. + + *MAY* be implemented by actor. + """ + pass + def playlist_changed(self): """ Called whenever a playlist is changed. diff --git a/tests/core/listener_test.py b/tests/core/listener_test.py index 0bc3f8fd..54713916 100644 --- a/tests/core/listener_test.py +++ b/tests/core/listener_test.py @@ -26,6 +26,9 @@ class CoreListenerTest(unittest.TestCase): self.listener.playback_state_changed( PlaybackState.STOPPED, PlaybackState.PLAYING) + def test_listener_has_default_impl_for_tracklist_changed(self): + self.listener.tracklist_changed() + def test_listener_has_default_impl_for_playlist_changed(self): self.listener.playlist_changed() From 6ffc61e9c9b890d31e79ba18c76cfd4d6bbc030d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 11:43:07 +0100 Subject: [PATCH 24/46] core,mpd: Trigger tracklist_changed() instead of playlist_changed() on tracklist change --- docs/changes.rst | 5 +++++ mopidy/core/tracklist.py | 8 ++++---- mopidy/frontends/mpd/actor.py | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index bf5dd8cb..ce1538d8 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -116,6 +116,11 @@ backends: This makes it easier to start playing one of the tracks that was just appended to the tracklist. +- When the tracklist is changed, we now trigger the new + :meth:`mopidy.core.CoreListener.tracklist_changed` event. Previously we + triggered :meth:`mopidy.core.CoreListener.playlist_changed`, which is + intended for stored playlists, not the tracklist. + **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index f8cf819e..4e01ed46 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -58,7 +58,7 @@ class TracklistController(object): def version(self, version): self._version = version self.core.playback.on_tracklist_change() - self._trigger_playlist_changed() + self._trigger_tracklist_changed() def add(self, track, at_position=None, increase_version=True): """ @@ -240,6 +240,6 @@ class TracklistController(object): """ return [copy(tl_track) for tl_track in self._tl_tracks[start:end]] - def _trigger_playlist_changed(self): - logger.debug('Triggering playlist changed event') - listener.CoreListener.send('playlist_changed') + def _trigger_tracklist_changed(self): + logger.debug('Triggering event: tracklist_changed()') + listener.CoreListener.send('tracklist_changed') diff --git a/mopidy/frontends/mpd/actor.py b/mopidy/frontends/mpd/actor.py index 3ba6378c..925b15b7 100644 --- a/mopidy/frontends/mpd/actor.py +++ b/mopidy/frontends/mpd/actor.py @@ -43,7 +43,7 @@ class MpdFrontend(pykka.ThreadingActor, CoreListener): def playback_state_changed(self, old_state, new_state): self.send_idle('player') - def playlist_changed(self): + def tracklist_changed(self): self.send_idle('playlist') def options_changed(self): From d378fd7160f5f715b03dca5560051c0edf8049d9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 11:53:48 +0100 Subject: [PATCH 25/46] tests: Move events tests from tests/backends/ to tests/core --- tests/{backends => core}/events_test.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{backends => core}/events_test.py (100%) diff --git a/tests/backends/events_test.py b/tests/core/events_test.py similarity index 100% rename from tests/backends/events_test.py rename to tests/core/events_test.py From 0e7f867d6793726c3c3e663ca56fe7ea05ce7abb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 13:54:55 +0100 Subject: [PATCH 26/46] core: Test tracklist event trigging --- tests/core/events_test.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/core/events_test.py b/tests/core/events_test.py index 417c5251..2612187c 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -54,3 +54,39 @@ class BackendEventsTest(unittest.TestCase): send.reset_mock() self.core.playback.seek(1000).get() self.assertEqual(send.call_args[0][0], 'seeked') + + def test_tracklist_add_sends_tracklist_changed_event(self, send): + send.reset_mock() + self.core.tracklist.add(Track(uri='dummy:a')).get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + def test_tracklist_append_sends_tracklist_changed_event(self, send): + send.reset_mock() + self.core.tracklist.append([Track(uri='dummy:a')]).get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + def test_tracklist_clear_sends_tracklist_changed_event(self, send): + self.core.tracklist.append([Track(uri='dummy:a')]).get() + send.reset_mock() + self.core.tracklist.clear().get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + def test_tracklist_move_sends_tracklist_changed_event(self, send): + self.core.tracklist.append( + [Track(uri='dummy:a'), Track(uri='dummy:b')]).get() + send.reset_mock() + self.core.tracklist.move(0, 1, 1).get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + def test_tracklist_remove_sends_tracklist_changed_event(self, send): + self.core.tracklist.append([Track(uri='dummy:a')]).get() + send.reset_mock() + self.core.tracklist.remove(uri='dummy:a').get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + def test_tracklist_shuffle_sends_tracklist_changed_event(self, send): + self.core.tracklist.append( + [Track(uri='dummy:a'), Track(uri='dummy:b')]).get() + send.reset_mock() + self.core.tracklist.shuffle().get() + self.assertEqual(send.call_args[0][0], 'tracklist_changed') From 533b46987d49500d13d0843a9bad4f04cfccc017 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 13:56:30 +0100 Subject: [PATCH 27/46] core: Document which methods triggers tracklist_changed() --- mopidy/core/tracklist.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 4e01ed46..f1025e2d 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -64,6 +64,9 @@ class TracklistController(object): """ Add the track to the end of, or at the given position in the tracklist. + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + :param track: track to add :type track: :class:`mopidy.models.Track` :param at_position: position in tracklist to add track @@ -89,6 +92,9 @@ class TracklistController(object): """ Append the given tracks to the tracklist. + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + :param tracks: tracks to append :type tracks: list of :class:`mopidy.models.Track` :rtype: list of class:`mopidy.models.TlTrack` @@ -103,7 +109,12 @@ class TracklistController(object): return tl_tracks def clear(self): - """Clear the tracklist.""" + """ + Clear the tracklist. + + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + """ self._tl_tracks = [] self.version += 1 @@ -157,6 +168,9 @@ class TracklistController(object): """ Move the tracks in the slice ``[start:end]`` to ``to_position``. + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + :param start: position of first track to move :type start: int :param end: position after last track to move @@ -190,6 +204,9 @@ class TracklistController(object): Uses :meth:`get()` to lookup the track to remove. + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + :param criteria: on or more criteria to match by :type criteria: dict """ @@ -203,6 +220,9 @@ class TracklistController(object): Shuffles the entire tracklist. If ``start`` and ``end`` is given only shuffles the slice ``[start:end]``. + Triggers the :method:`mopidy.core.CoreListener.tracklist_changed` + event. + :param start: position of first track to shuffle :type start: int or :class:`None` :param end: position after last track to shuffle From cfac728defdcb46bc9fd7c4c0c97dff613bcc547 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 14:57:25 +0100 Subject: [PATCH 28/46] tests: Don't use indexes into TlTracks --- tests/backends/base/tracklist.py | 12 ++++++------ .../frontends/mpd/protocol/current_playlist_test.py | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/backends/base/tracklist.py b/tests/backends/base/tracklist.py index e67a40e4..65328f60 100644 --- a/tests/backends/base/tracklist.py +++ b/tests/backends/base/tracklist.py @@ -88,7 +88,7 @@ class TracklistControllerTest(object): def test_get_by_uri_returns_unique_match(self): track = Track(uri='a') self.controller.append([Track(uri='z'), track, Track(uri='y')]) - self.assertEqual(track, self.controller.get(uri='a')[1]) + self.assertEqual(track, self.controller.get(uri='a').track) def test_get_by_uri_raises_error_if_multiple_matches(self): track = Track(uri='a') @@ -113,16 +113,16 @@ class TracklistControllerTest(object): track2 = Track(uri='b', name='x') track3 = Track(uri='b', name='y') self.controller.append([track1, track2, track3]) - self.assertEqual(track1, self.controller.get(uri='a', name='x')[1]) - self.assertEqual(track2, self.controller.get(uri='b', name='x')[1]) - self.assertEqual(track3, self.controller.get(uri='b', name='y')[1]) + self.assertEqual(track1, self.controller.get(uri='a', name='x').track) + self.assertEqual(track2, self.controller.get(uri='b', name='x').track) + self.assertEqual(track3, self.controller.get(uri='b', name='y').track) def test_get_by_criteria_that_is_not_present_in_all_elements(self): track1 = Track() track2 = Track(uri='b') track3 = Track() self.controller.append([track1, track2, track3]) - self.assertEqual(track2, self.controller.get(uri='b')[1]) + self.assertEqual(track2, self.controller.get(uri='b').track) def test_append_appends_to_the_tracklist(self): self.controller.append([Track(uri='a'), Track(uri='b')]) @@ -156,7 +156,7 @@ class TracklistControllerTest(object): @populate_playlist def test_append_returns_the_tl_tracks_that_was_added(self): tl_tracks = self.controller.append(self.controller.tracks[1:2]) - self.assertEqual(tl_tracks[0][1], self.controller.tracks[1]) + self.assertEqual(tl_tracks[0].track, self.controller.tracks[1]) def test_index_returns_index_of_track(self): tl_tracks = self.controller.append(self.tracks) diff --git a/tests/frontends/mpd/protocol/current_playlist_test.py b/tests/frontends/mpd/protocol/current_playlist_test.py index 2b6fdbd5..f5f15f81 100644 --- a/tests/frontends/mpd/protocol/current_playlist_test.py +++ b/tests/frontends/mpd/protocol/current_playlist_test.py @@ -41,7 +41,7 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqual(len(self.core.tracklist.tracks.get()), 6) self.assertEqual(self.core.tracklist.tracks.get()[5], needle) self.assertInResponse( - 'Id: %d' % self.core.tracklist.tl_tracks.get()[5][0]) + 'Id: %d' % self.core.tracklist.tl_tracks.get()[5].tlid) self.assertInResponse('OK') def test_addid_with_empty_uri_acks(self): @@ -60,7 +60,7 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqual(len(self.core.tracklist.tracks.get()), 6) self.assertEqual(self.core.tracklist.tracks.get()[3], needle) self.assertInResponse( - 'Id: %d' % self.core.tracklist.tl_tracks.get()[3][0]) + 'Id: %d' % self.core.tracklist.tl_tracks.get()[3].tlid) self.assertInResponse('OK') def test_addid_with_songpos_out_of_bounds_should_ack(self): @@ -94,7 +94,7 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.assertEqual(len(self.core.tracklist.tracks.get()), 5) self.sendRequest( - 'delete "%d"' % self.core.tracklist.tl_tracks.get()[2][0]) + 'delete "%d"' % self.core.tracklist.tl_tracks.get()[2].tlid) self.assertEqual(len(self.core.tracklist.tracks.get()), 4) self.assertInResponse('OK') @@ -424,11 +424,11 @@ class CurrentPlaylistHandlerTest(protocol.BaseTestCase): self.sendRequest('plchangesposid "0"') tl_tracks = self.core.tracklist.tl_tracks.get() self.assertInResponse('cpos: 0') - self.assertInResponse('Id: %d' % tl_tracks[0][0]) + self.assertInResponse('Id: %d' % tl_tracks[0].tlid) self.assertInResponse('cpos: 2') - self.assertInResponse('Id: %d' % tl_tracks[1][0]) + self.assertInResponse('Id: %d' % tl_tracks[1].tlid) self.assertInResponse('cpos: 2') - self.assertInResponse('Id: %d' % tl_tracks[2][0]) + self.assertInResponse('Id: %d' % tl_tracks[2].tlid) self.assertInResponse('OK') def test_shuffle_without_range(self): From 87ba412942340dfc46701b460612a77687193eeb Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 15:01:37 +0100 Subject: [PATCH 29/46] models: Make TlTrack an ImmutableObject --- mopidy/models.py | 43 ++++++++++++++++++++--- tests/models_test.py | 84 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 107 insertions(+), 20 deletions(-) diff --git a/mopidy/models.py b/mopidy/models.py index 511ce847..17616f9d 100644 --- a/mopidy/models.py +++ b/mopidy/models.py @@ -1,7 +1,5 @@ from __future__ import unicode_literals -from collections import namedtuple - class ImmutableObject(object): """ @@ -151,9 +149,6 @@ class Album(ImmutableObject): super(Album, self).__init__(*args, **kwargs) -TlTrack = namedtuple('TlTrack', ['tlid', 'track']) - - class Track(ImmutableObject): """ :param uri: track URI @@ -208,6 +203,44 @@ class Track(ImmutableObject): super(Track, self).__init__(*args, **kwargs) +class TlTrack(ImmutableObject): + """ + A tracklist track. Wraps a regular track and it's tracklist ID. + + The use of :class:`TlTrack` allows the same track to appear multiple times + in the tracklist. + + This class also accepts it's parameters as positional arguments. Both + arguments must be provided, and they must appear in the order they are + listed here. + + This class also supports iteration, so your extract its values like this:: + + (tlid, track) = tl_track + + :param tlid: tracklist ID + :type tlid: int + :param track: the track + :type track: :class:`Track` + """ + + #: The tracklist ID. Read-only. + tlid = None + + #: The track. Read-only. + track = None + + def __init__(self, *args, **kwargs): + if len(args) == 2 and len(kwargs) == 0: + kwargs['tlid'] = args[0] + kwargs['track'] = args[1] + args = [] + super(TlTrack, self).__init__(*args, **kwargs) + + def __iter__(self): + return iter([self.tlid, self.track]) + + class Playlist(ImmutableObject): """ :param uri: playlist URI diff --git a/tests/models_test.py b/tests/models_test.py index 4e3cdabf..d5d58ace 100644 --- a/tests/models_test.py +++ b/tests/models_test.py @@ -314,21 +314,6 @@ class AlbumTest(unittest.TestCase): self.assertNotEqual(hash(album1), hash(album2)) -class TlTrackTest(unittest.TestCase): - def setUp(self): - self.tlid = 123 - self.track = Track() - self.tl_track = TlTrack(self.tlid, self.track) - - def test_tl_track_can_be_accessed_as_a_tuple(self): - self.assertEqual(self.tlid, self.tl_track[0]) - self.assertEqual(self.track, self.tl_track[1]) - - def test_tl_track_can_be_accessed_by_attribute_names(self): - self.assertEqual(self.tlid, self.tl_track.tlid) - self.assertEqual(self.track, self.tl_track.track) - - class TrackTest(unittest.TestCase): def test_uri(self): uri = 'an_uri' @@ -567,6 +552,75 @@ class TrackTest(unittest.TestCase): self.assertNotEqual(hash(track1), hash(track2)) +class TlTrackTest(unittest.TestCase): + def test_tlid(self): + tlid = 123 + tl_track = TlTrack(tlid=tlid) + self.assertEqual(tl_track.tlid, tlid) + self.assertRaises(AttributeError, setattr, tl_track, 'tlid', None) + + def test_track(self): + track = Track() + tl_track = TlTrack(track=track) + self.assertEqual(tl_track.track, track) + self.assertRaises(AttributeError, setattr, tl_track, 'track', None) + + def test_invalid_kwarg(self): + test = lambda: TlTrack(foo='baz') + self.assertRaises(TypeError, test) + + def test_positional_args(self): + tlid = 123 + track = Track() + tl_track = TlTrack(tlid, track) + self.assertEqual(tl_track.tlid, tlid) + self.assertEqual(tl_track.track, track) + + def test_iteration(self): + tlid = 123 + track = Track() + tl_track = TlTrack(tlid, track) + (tlid2, track2) = tl_track + self.assertEqual(tlid2, tlid) + self.assertEqual(track2, track) + + def test_repr(self): + self.assertEquals( + "TlTrack(tlid=123, track=Track(artists=[], uri=u'uri'))", + repr(TlTrack(tlid=123, track=Track(uri='uri')))) + + def test_serialize(self): + self.assertDictEqual( + {'tlid': 123, 'track': {'uri': 'uri', 'name': 'name'}}, + TlTrack(tlid=123, track=Track(uri='uri', name='name')).serialize()) + + def test_eq(self): + tlid = 123 + track = Track() + tl_track1 = TlTrack(tlid=tlid, track=track) + tl_track2 = TlTrack(tlid=tlid, track=track) + self.assertEqual(tl_track1, tl_track2) + self.assertEqual(hash(tl_track1), hash(tl_track2)) + + def test_eq_none(self): + self.assertNotEqual(TlTrack(), None) + + def test_eq_other(self): + self.assertNotEqual(TlTrack(), 'other') + + def test_ne_tlid(self): + tl_track1 = TlTrack(tlid=123) + tl_track2 = TlTrack(tlid=321) + self.assertNotEqual(tl_track1, tl_track2) + self.assertNotEqual(hash(tl_track1), hash(tl_track2)) + + def test_ne_track(self): + tl_track1 = TlTrack(track=Track(uri='a')) + tl_track2 = TlTrack(track=Track(uri='b')) + self.assertNotEqual(tl_track1, tl_track2) + self.assertNotEqual(hash(tl_track1), hash(tl_track2)) + + class PlaylistTest(unittest.TestCase): def test_uri(self): uri = 'an_uri' From 811c508c8029c0fbc6d2850c94dde9765be77be6 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 15:03:05 +0100 Subject: [PATCH 30/46] core: No need to copy immutable TlTrack objects --- mopidy/core/tracklist.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index f1025e2d..4c6dd715 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -1,6 +1,5 @@ from __future__ import unicode_literals -from copy import copy import logging import random @@ -28,7 +27,7 @@ class TracklistController(object): Read-only. """ - return [copy(tl_track) for tl_track in self._tl_tracks] + return self._tl_tracks[:] @property def tracks(self): @@ -258,7 +257,7 @@ class TracklistController(object): :type end: int :rtype: two-tuple of (TLID integer, :class:`mopidy.models.Track`) """ - return [copy(tl_track) for tl_track in self._tl_tracks[start:end]] + return self._tl_tracks[start:end] def _trigger_tracklist_changed(self): logger.debug('Triggering event: tracklist_changed()') From f4cddc0ce58d42c5e461572cdadf043f13cf0725 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 15:46:56 +0100 Subject: [PATCH 31/46] core: Hide internal variables in tracklist --- mopidy/core/tracklist.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 4c6dd715..4a628d81 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -15,8 +15,8 @@ class TracklistController(object): pykka_traversable = True def __init__(self, core): - self.core = core - self.tlid = 0 + self._core = core + self._next_tlid = 0 self._tl_tracks = [] self._version = 0 @@ -56,7 +56,7 @@ class TracklistController(object): @version.setter # noqa def version(self, version): self._version = version - self.core.playback.on_tracklist_change() + self._core.playback.on_tracklist_change() self._trigger_tracklist_changed() def add(self, track, at_position=None, increase_version=True): @@ -77,14 +77,14 @@ class TracklistController(object): """ assert at_position <= len(self._tl_tracks), \ 'at_position can not be greater than tracklist length' - tl_track = TlTrack(self.tlid, track) + tl_track = TlTrack(self._next_tlid, track) if at_position is not None: self._tl_tracks.insert(at_position, tl_track) else: self._tl_tracks.append(tl_track) if increase_version: self.version += 1 - self.tlid += 1 + self._next_tlid += 1 return tl_track def append(self, tracks): From 1d9a2a23b17e7a0460d6deb81a341cef2f6bd1c0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 23:48:59 +0100 Subject: [PATCH 32/46] core: Flake8 1.5 formatting fixes --- mopidy/core/playlists.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 069150e5..63797477 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -20,8 +20,8 @@ class PlaylistsController(object): Read-only. List of :class:`mopidy.models.Playlist`. """ - futures = [b.playlists.playlists - for b in self.backends.with_playlists] + futures = [ + b.playlists.playlists for b in self.backends.with_playlists] results = pykka.get_all(futures) return list(itertools.chain(*results)) @@ -125,8 +125,8 @@ class PlaylistsController(object): :type uri_scheme: string """ if uri_scheme is None: - futures = [b.playlists.refresh() - for b in self.backends.with_playlists] + futures = [ + b.playlists.refresh() for b in self.backends.with_playlists] pykka.get_all(futures) else: backend = self.backends.with_playlists_by_uri_scheme.get( From fd86b7173c2bcf8bfccbe3b3bd06f927ad9efcba Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 23:24:11 +0100 Subject: [PATCH 33/46] core: Add playlist to playlist_changed() event --- docs/changes.rst | 3 +++ mopidy/core/listener.py | 5 ++++- tests/core/listener_test.py | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/changes.rst b/docs/changes.rst index ce1538d8..b0ca8989 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -121,6 +121,9 @@ backends: triggered :meth:`mopidy.core.CoreListener.playlist_changed`, which is intended for stored playlists, not the tracklist. +- The event :meth:`mopidy.core.CoreListener.playlist_changed` has been changed + to include the playlist that was changed. + **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index 2cf49490..df726b77 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -92,11 +92,14 @@ class CoreListener(object): """ pass - def playlist_changed(self): + def playlist_changed(self, playlist): """ Called whenever a playlist is changed. *MAY* be implemented by actor. + + :param playlist: the changed playlist + :type playlist: :class:`mopidy.models.Playlist` """ pass diff --git a/tests/core/listener_test.py b/tests/core/listener_test.py index 54713916..dc3b8964 100644 --- a/tests/core/listener_test.py +++ b/tests/core/listener_test.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals from mopidy.core import CoreListener, PlaybackState -from mopidy.models import Track +from mopidy.models import Playlist, Track from tests import unittest @@ -30,7 +30,7 @@ class CoreListenerTest(unittest.TestCase): self.listener.tracklist_changed() def test_listener_has_default_impl_for_playlist_changed(self): - self.listener.playlist_changed() + self.listener.playlist_changed(Playlist()) def test_listener_has_default_impl_for_options_changed(self): self.listener.options_changed() From 4efff4a5a312833fd1e3fca6a4f4a2c0d1bfdad9 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 16 Nov 2012 23:49:28 +0100 Subject: [PATCH 34/46] core: Trigger playlist_changed() event on create() and save() --- mopidy/core/playlists.py | 10 ++++++++-- tests/core/events_test.py | 23 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 63797477..878b2b1a 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -5,6 +5,8 @@ import urlparse import pykka +from . import listener + class PlaylistsController(object): pykka_traversable = True @@ -47,7 +49,9 @@ class PlaylistsController(object): backend = self.backends.by_uri_scheme[uri_scheme] else: backend = self.backends.with_playlists[0] - return backend.playlists.create(name).get() + playlist = backend.playlists.create(name).get() + listener.CoreListener.send('playlist_changed', playlist=playlist) + return playlist def delete(self, uri): """ @@ -162,4 +166,6 @@ class PlaylistsController(object): backend = self.backends.with_playlists_by_uri_scheme.get( uri_scheme, None) if backend: - return backend.playlists.save(playlist).get() + playlist = backend.playlists.save(playlist).get() + listener.CoreListener.send('playlist_changed', playlist=playlist) + return playlist diff --git a/tests/core/events_test.py b/tests/core/events_test.py index 2612187c..be991e66 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -90,3 +90,26 @@ class BackendEventsTest(unittest.TestCase): send.reset_mock() self.core.tracklist.shuffle().get() self.assertEqual(send.call_args[0][0], 'tracklist_changed') + + @unittest.SkipTest + def test_playlists_load_sends_playlists_loaded_event(self, send): + # TODO Figure out what type of event and how to send events when + # the backend finished loading playlists + pass + + def test_playlists_create_sends_playlist_changed_event(self, send): + send.reset_mock() + self.core.playlists.create('foo').get() + self.assertEqual(send.call_args[0][0], 'playlist_changed') + + @unittest.SkipTest + def test_playlists_delete_sends_playlist_deleted_event(self, send): + # TODO We should probably add a playlist_deleted event + pass + + def test_playlists_save_sends_playlist_changed_event(self, send): + playlist = self.core.playlists.create('foo').get() + send.reset_mock() + playlist = playlist.copy(name='bar') + self.core.playlists.save(playlist).get() + self.assertEqual(send.call_args[0][0], 'playlist_changed') From 5526ee5a957936ae2cd6bf390cff95ead926b607 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:39:19 +0100 Subject: [PATCH 35/46] core: Add CoreListener.playlists_loaded() event --- mopidy/core/listener.py | 8 ++++++++ tests/core/listener_test.py | 3 +++ 2 files changed, 11 insertions(+) diff --git a/mopidy/core/listener.py b/mopidy/core/listener.py index df726b77..dc8bf1d7 100644 --- a/mopidy/core/listener.py +++ b/mopidy/core/listener.py @@ -92,6 +92,14 @@ class CoreListener(object): """ pass + def playlists_loaded(self): + """ + Called when playlists are loaded or refreshed. + + *MAY* be implemented by actor. + """ + pass + def playlist_changed(self, playlist): """ Called whenever a playlist is changed. diff --git a/tests/core/listener_test.py b/tests/core/listener_test.py index dc3b8964..2e121796 100644 --- a/tests/core/listener_test.py +++ b/tests/core/listener_test.py @@ -29,6 +29,9 @@ class CoreListenerTest(unittest.TestCase): def test_listener_has_default_impl_for_tracklist_changed(self): self.listener.tracklist_changed() + def test_listener_has_default_impl_for_playlists_loaded(self): + self.listener.playlists_loaded() + def test_listener_has_default_impl_for_playlist_changed(self): self.listener.playlist_changed(Playlist()) From 426d5aea16de5d0cd3d6e1a0ea8b0cb479dfbce0 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:40:49 +0100 Subject: [PATCH 36/46] core: Trigger playlists_loaded() after playlist refresh --- mopidy/core/playlists.py | 2 ++ tests/core/events_test.py | 14 +++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/mopidy/core/playlists.py b/mopidy/core/playlists.py index 878b2b1a..25ae2bdf 100644 --- a/mopidy/core/playlists.py +++ b/mopidy/core/playlists.py @@ -132,11 +132,13 @@ class PlaylistsController(object): futures = [ b.playlists.refresh() for b in self.backends.with_playlists] pykka.get_all(futures) + listener.CoreListener.send('playlists_loaded') else: backend = self.backends.with_playlists_by_uri_scheme.get( uri_scheme, None) if backend: backend.playlists.refresh().get() + listener.CoreListener.send('playlists_loaded') def save(self, playlist): """ diff --git a/tests/core/events_test.py b/tests/core/events_test.py index be991e66..212f3b5d 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -91,11 +91,15 @@ class BackendEventsTest(unittest.TestCase): self.core.tracklist.shuffle().get() self.assertEqual(send.call_args[0][0], 'tracklist_changed') - @unittest.SkipTest - def test_playlists_load_sends_playlists_loaded_event(self, send): - # TODO Figure out what type of event and how to send events when - # the backend finished loading playlists - pass + def test_playlists_refresh_sends_playlists_loaded_event(self, send): + send.reset_mock() + self.core.playlists.refresh().get() + self.assertEqual(send.call_args[0][0], 'playlists_loaded') + + def test_playlists_refresh_uri_sends_playlists_loaded_event(self, send): + send.reset_mock() + self.core.playlists.refresh(uri_scheme='dummy').get() + self.assertEqual(send.call_args[0][0], 'playlists_loaded') def test_playlists_create_sends_playlist_changed_event(self, send): send.reset_mock() From 9fbb79760760b73675d7c7e83d2f8fd3b41bb6a3 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:52:57 +0100 Subject: [PATCH 37/46] core: flake8 1.5 style fix --- mopidy/core/actor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index fe7cf94b..231fe523 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -78,8 +78,8 @@ class Backends(list): # the X_by_uri_scheme dicts below. self.with_library = [b for b in backends if b.has_library().get()] self.with_playback = [b for b in backends if b.has_playback().get()] - self.with_playlists = [b for b in backends - if b.has_playlists().get()] + self.with_playlists = [ + b for b in backends if b.has_playlists().get()] self.by_uri_scheme = {} for backend in backends: From 0f6c9a1673738b72db21f3ca4b8e856700aa35cc Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:53:45 +0100 Subject: [PATCH 38/46] backends: Add BackendListener interface with playlists_loaded() event --- docs/api/backends.rst | 7 +++++++ mopidy/backends/listener.py | 32 ++++++++++++++++++++++++++++++++ tests/backends/listener_test.py | 13 +++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 mopidy/backends/listener.py create mode 100644 tests/backends/listener_test.py diff --git a/docs/api/backends.rst b/docs/api/backends.rst index 0dc4900d..f0aadd53 100644 --- a/docs/api/backends.rst +++ b/docs/api/backends.rst @@ -33,6 +33,13 @@ Library provider :members: +Backend listener +================ + +.. autoclass:: mopidy.backends.listener.BackendListener + :members: + + .. _backend-implementations: Backend implementations diff --git a/mopidy/backends/listener.py b/mopidy/backends/listener.py new file mode 100644 index 00000000..30b3291d --- /dev/null +++ b/mopidy/backends/listener.py @@ -0,0 +1,32 @@ +from __future__ import unicode_literals + +import pykka + + +class BackendListener(object): + """ + Marker interface for recipients of events sent by the backend actors. + + Any Pykka actor that mixes in this class will receive calls to the methods + defined here when the corresponding events happen in the core actor. This + interface is used both for looking up what actors to notify of the events, + and for providing default implementations for those listeners that are not + interested in all events. + + Normally, only the Core actor should mix in this class. + """ + + @staticmethod + def send(event, **kwargs): + """Helper to allow calling of backend listener events""" + listeners = pykka.ActorRegistry.get_by_class(BackendListener) + for listener in listeners: + getattr(listener.proxy(), event)(**kwargs) + + def playlists_loaded(self): + """ + Called when playlists are loaded or refreshed. + + *MAY* be implemented by actor. + """ + pass diff --git a/tests/backends/listener_test.py b/tests/backends/listener_test.py new file mode 100644 index 00000000..a4df513c --- /dev/null +++ b/tests/backends/listener_test.py @@ -0,0 +1,13 @@ +from __future__ import unicode_literals + +from mopidy.backends.listener import BackendListener + +from tests import unittest + + +class CoreListenerTest(unittest.TestCase): + def setUp(self): + self.listener = BackendListener() + + def test_listener_has_default_impl_for_playlists_loaded(self): + self.listener.playlists_loaded() From 330731a2475333565fae03af8520175a623acf17 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:56:38 +0100 Subject: [PATCH 39/46] core: Forward playlists_loaded() event from backends to frontends --- mopidy/core/actor.py | 8 +++++++- tests/core/events_test.py | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/mopidy/core/actor.py b/mopidy/core/actor.py index 231fe523..a4f184bf 100644 --- a/mopidy/core/actor.py +++ b/mopidy/core/actor.py @@ -5,14 +5,16 @@ import itertools import pykka from mopidy.audio import AudioListener, PlaybackState +from mopidy.backends.listener import BackendListener from .library import LibraryController +from .listener import CoreListener from .playback import PlaybackController from .playlists import PlaylistsController from .tracklist import TracklistController -class Core(pykka.ThreadingActor, AudioListener): +class Core(pykka.ThreadingActor, AudioListener, BackendListener): #: The library controller. An instance of # :class:`mopidy.core.LibraryController`. library = None @@ -67,6 +69,10 @@ class Core(pykka.ThreadingActor, AudioListener): self.playback.state = new_state self.playback._trigger_track_playback_paused() + def playlists_loaded(self): + # Forward event from backend to frontends + CoreListener.send('playlists_loaded') + class Backends(list): def __init__(self, backends): diff --git a/tests/core/events_test.py b/tests/core/events_test.py index 212f3b5d..8f969b0d 100644 --- a/tests/core/events_test.py +++ b/tests/core/events_test.py @@ -20,6 +20,11 @@ class BackendEventsTest(unittest.TestCase): def tearDown(self): pykka.ActorRegistry.stop_all() + def test_backends_playlists_loaded_forwards_event_to_frontends(self, send): + send.reset_mock() + self.core.playlists_loaded().get() + self.assertEqual(send.call_args[0][0], 'playlists_loaded') + def test_pause_sends_track_playback_paused_event(self, send): self.core.tracklist.add(Track(uri='dummy:a')) self.core.playback.play().get() From 4e4c887fb1cb3016944dfd712995386905ca512b Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 00:58:31 +0100 Subject: [PATCH 40/46] spotify: Trigger BackendListener.playlists_loaded() when playlists loads --- mopidy/backends/spotify/session_manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mopidy/backends/spotify/session_manager.py b/mopidy/backends/spotify/session_manager.py index 998e4d5e..b46fd659 100644 --- a/mopidy/backends/spotify/session_manager.py +++ b/mopidy/backends/spotify/session_manager.py @@ -11,6 +11,7 @@ import threading from spotify.manager import SpotifySessionManager as PyspotifySessionManager from mopidy import settings +from mopidy.backends.listener import BackendListener from mopidy.models import Playlist from mopidy.utils import process, versioning @@ -155,6 +156,7 @@ class SpotifySessionManager(process.BaseThread, PyspotifySessionManager): playlists = filter(None, playlists) self.backend.playlists.playlists = playlists logger.info('Loaded %d Spotify playlist(s)', len(playlists)) + BackendListener.send('playlists_loaded') def search(self, query, queue): """Search method used by Mopidy backend""" From a7be82463a099ca235300ce9139258e385e47086 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 01:40:40 +0100 Subject: [PATCH 41/46] spotify: Add playlists.lookup(uri) for looking up loaded playlists --- mopidy/backends/spotify/playlists.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/backends/spotify/playlists.py b/mopidy/backends/spotify/playlists.py index 2c31caa8..bd201179 100644 --- a/mopidy/backends/spotify/playlists.py +++ b/mopidy/backends/spotify/playlists.py @@ -11,7 +11,9 @@ class SpotifyPlaylistsProvider(base.BasePlaylistsProvider): pass # TODO def lookup(self, uri): - pass # TODO + for playlist in self._playlists: + if playlist.uri == uri: + return playlist def refresh(self): pass # TODO From b8c7703c79834d5523b5c3ad3b748fca86157539 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Thu, 15 Nov 2012 22:54:41 +0100 Subject: [PATCH 42/46] mpris: Implement the playlists interface (fixes #229) --- docs/changes.rst | 4 + docs/clients/mpris.rst | 4 +- mopidy/frontends/mpris/actor.py | 39 ++-- mopidy/frontends/mpris/objects.py | 85 ++++++++- tests/frontends/mpris/events_test.py | 18 +- .../mpris/playlists_interface_test.py | 171 ++++++++++++++++++ 6 files changed, 301 insertions(+), 20 deletions(-) create mode 100644 tests/frontends/mpris/playlists_interface_test.py diff --git a/docs/changes.rst b/docs/changes.rst index b0ca8989..4317e4ef 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -124,6 +124,10 @@ backends: - The event :meth:`mopidy.core.CoreListener.playlist_changed` has been changed to include the playlist that was changed. +- The MPRIS playlists interface is now supported by our MPRIS frontend. This + means that you now can select playlists to queue and play from the Ubuntu + Sound Menu. + **Bug fixes** - :issue:`218`: The MPD commands ``listplaylist`` and ``listplaylistinfo`` now diff --git a/docs/clients/mpris.rst b/docs/clients/mpris.rst index 95866089..c782fa26 100644 --- a/docs/clients/mpris.rst +++ b/docs/clients/mpris.rst @@ -9,8 +9,8 @@ Specification. It's a spec that describes a standard D-Bus interface for making media players available to other applications on the same system. Mopidy's :ref:`MPRIS frontend ` currently implements all -required parts of the MPRIS spec, but not the optional playlist interface. For -tracking the development of the playlist interface, see :issue:`229`. +required parts of the MPRIS spec, plus the optional playlist interface. It does +not implement the optional tracklist interface. .. _ubuntu-sound-menu: diff --git a/mopidy/frontends/mpris/actor.py b/mopidy/frontends/mpris/actor.py index 81a44fbb..795b2694 100644 --- a/mopidy/frontends/mpris/actor.py +++ b/mopidy/frontends/mpris/actor.py @@ -57,35 +57,48 @@ class MprisFrontend(pykka.ThreadingActor, CoreListener): self.indicate_server.show() logger.debug('Startup notification sent') - def _emit_properties_changed(self, *changed_properties): + def _emit_properties_changed(self, interface, changed_properties): if self.mpris_object is None: return props_with_new_values = [ - (p, self.mpris_object.Get(objects.PLAYER_IFACE, p)) + (p, self.mpris_object.Get(interface, p)) for p in changed_properties] self.mpris_object.PropertiesChanged( - objects.PLAYER_IFACE, dict(props_with_new_values), []) + interface, dict(props_with_new_values), []) def track_playback_paused(self, track, time_position): - logger.debug('Received track playback paused event') - self._emit_properties_changed('PlaybackStatus') + logger.debug('Received track_playback_paused event') + self._emit_properties_changed(objects.PLAYER_IFACE, ['PlaybackStatus']) def track_playback_resumed(self, track, time_position): - logger.debug('Received track playback resumed event') - self._emit_properties_changed('PlaybackStatus') + logger.debug('Received track_playback_resumed event') + self._emit_properties_changed(objects.PLAYER_IFACE, ['PlaybackStatus']) def track_playback_started(self, track): - logger.debug('Received track playback started event') - self._emit_properties_changed('PlaybackStatus', 'Metadata') + logger.debug('Received track_playback_started event') + self._emit_properties_changed( + objects.PLAYER_IFACE, ['PlaybackStatus', 'Metadata']) def track_playback_ended(self, track, time_position): - logger.debug('Received track playback ended event') - self._emit_properties_changed('PlaybackStatus', 'Metadata') + logger.debug('Received track_playback_ended event') + self._emit_properties_changed( + objects.PLAYER_IFACE, ['PlaybackStatus', 'Metadata']) def volume_changed(self): - logger.debug('Received volume changed event') - self._emit_properties_changed('Volume') + logger.debug('Received volume_changed event') + self._emit_properties_changed(objects.PLAYER_IFACE, ['Volume']) def seeked(self, time_position_in_ms): logger.debug('Received seeked event') self.mpris_object.Seeked(time_position_in_ms * 1000) + + def playlists_loaded(self): + logger.debug('Received playlists_loaded event') + self._emit_properties_changed( + objects.PLAYLISTS_IFACE, ['PlaylistCount']) + + def playlist_changed(self, playlist): + logger.debug('Received playlist_changed event') + playlist_id = self.mpris_object.get_playlist_id(playlist.uri) + playlist = (playlist_id, playlist.name, '') + self.mpris_object.PlaylistChanged(playlist) diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index cf7f71ce..e7a9243e 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +import base64 import logging import os @@ -27,6 +28,7 @@ BUS_NAME = 'org.mpris.MediaPlayer2.mopidy' OBJECT_PATH = '/org/mpris/MediaPlayer2' ROOT_IFACE = 'org.mpris.MediaPlayer2' PLAYER_IFACE = 'org.mpris.MediaPlayer2.Player' +PLAYLISTS_IFACE = 'org.mpris.MediaPlayer2.Playlists' class MprisObject(dbus.service.Object): @@ -39,6 +41,7 @@ class MprisObject(dbus.service.Object): self.properties = { ROOT_IFACE: self._get_root_iface_properties(), PLAYER_IFACE: self._get_player_iface_properties(), + PLAYLISTS_IFACE: self._get_playlists_iface_properties(), } bus_name = self._connect_to_dbus() dbus.service.Object.__init__(self, bus_name, OBJECT_PATH) @@ -78,6 +81,13 @@ class MprisObject(dbus.service.Object): 'CanControl': (self.get_CanControl, None), } + def _get_playlists_iface_properties(self): + return { + 'PlaylistCount': (self.get_PlaylistCount, None), + 'Orderings': (self.get_Orderings, None), + 'ActivePlaylist': (self.get_ActivePlaylist, None), + } + def _connect_to_dbus(self): logger.debug('Connecting to D-Bus...') mainloop = dbus.mainloop.glib.DBusGMainLoop() @@ -86,10 +96,22 @@ class MprisObject(dbus.service.Object): logger.info('Connected to D-Bus') return bus_name - def _get_track_id(self, tl_track): + def get_playlist_id(self, playlist_uri): + # Only A-Za-z0-9_ is allowed, which is 63 chars, so we can't use + # base64. Luckily, D-Bus does not limit the length of object paths. + # Since base32 pads trailing bytes with = chars, we need to replace + # them with the allow _ char. + encoded_uri = base64.b32encode(playlist_uri).replace('=', '_') + return '/com/mopidy/playlist/%s' % encoded_uri + + def get_playlist_uri(self, playlist_id): + encoded_uri = playlist_id.split('/')[-1].replace('_', '=') + return base64.b32decode(encoded_uri) + + def get_track_id(self, tl_track): return '/com/mopidy/track/%d' % tl_track.tlid - def _get_tlid(self, track_id): + def get_track_tlid(self, track_id): assert track_id.startswith('/com/mopidy/track/') return track_id.split('/')[-1] @@ -239,7 +261,7 @@ class MprisObject(dbus.service.Object): current_tl_track = self.core.playback.current_tl_track.get() if current_tl_track is None: return - if track_id != self._get_track_id(current_tl_track): + if track_id != self.get_track_id(current_tl_track): return if position < 0: return @@ -337,7 +359,7 @@ class MprisObject(dbus.service.Object): return {'mpris:trackid': ''} else: (_, track) = current_tl_track - metadata = {'mpris:trackid': self._get_track_id(current_tl_track)} + metadata = {'mpris:trackid': self.get_track_id(current_tl_track)} if track.length: metadata['mpris:length'] = track.length * 1000 if track.uri: @@ -420,3 +442,58 @@ class MprisObject(dbus.service.Object): def get_CanControl(self): # NOTE This could be a setting for the end user to change. return True + + ### Playlists interface methods + + @dbus.service.method(dbus_interface=PLAYLISTS_IFACE) + def ActivatePlaylist(self, playlist_id): + logger.debug( + '%s.ActivatePlaylist(%r) called', PLAYLISTS_IFACE, playlist_id) + playlist_uri = self.get_playlist_uri(playlist_id) + playlist = self.core.playlists.lookup(playlist_uri).get() + if playlist and playlist.tracks: + tl_tracks = self.core.tracklist.append(playlist.tracks).get() + self.core.playback.play(tl_tracks[0]) + + @dbus.service.method(dbus_interface=PLAYLISTS_IFACE) + def GetPlaylists(self, index, max_count, order, reverse): + logger.debug( + '%s.GetPlaylists(%r, %r, %r, %r) called', + PLAYLISTS_IFACE, index, max_count, order, reverse) + playlists = self.core.playlists.playlists.get() + if order == 'Alphabetical': + playlists.sort(key=lambda p: p.name, reverse=reverse) + elif order == 'Modified': + playlists.sort(key=lambda p: p.last_modified, reverse=reverse) + elif order == 'User' and reverse: + playlists.reverse() + slice_end = index + max_count + playlists = playlists[index:slice_end] + results = [ + (self.get_playlist_id(p.uri), p.name, '') + for p in playlists] + return dbus.Array(results, signature='(oss)') + + ### Playlists interface signals + + @dbus.service.signal(dbus_interface=PLAYLISTS_IFACE, signature='(oss)') + def PlaylistChanged(self, playlist): + logger.debug('%s.PlaylistChanged signaled', PLAYLISTS_IFACE) + # Do nothing, as just calling the method is enough to emit the signal. + + ### Playlists interface properties + + def get_PlaylistCount(self): + return len(self.core.playlists.playlists.get()) + + def get_Orderings(self): + return [ + 'Alphabetical', # Order by playlist.name + 'Modified', # Order by playlist.last_modified + 'User', # Don't change order + ] + + def get_ActivePlaylist(self): + playlist_is_valid = False + playlist = ('/', 'None', '') + return (playlist_is_valid, playlist) diff --git a/tests/frontends/mpris/events_test.py b/tests/frontends/mpris/events_test.py index 94f48115..18a9de6f 100644 --- a/tests/frontends/mpris/events_test.py +++ b/tests/frontends/mpris/events_test.py @@ -5,7 +5,7 @@ import sys import mock from mopidy.exceptions import OptionalDependencyError -from mopidy.models import Track +from mopidy.models import Playlist, Track try: from mopidy.frontends.mpris import MprisFrontend, objects @@ -75,3 +75,19 @@ class BackendEventsTest(unittest.TestCase): def test_seeked_event_causes_mpris_seeked_event(self): self.mpris_frontend.seeked(31000) self.mpris_object.Seeked.assert_called_with(31000000) + + def test_playlists_loaded_event_changes_playlist_count(self): + self.mpris_object.Get.return_value = 17 + self.mpris_frontend.playlists_loaded() + self.assertListEqual(self.mpris_object.Get.call_args_list, [ + ((objects.PLAYLISTS_IFACE, 'PlaylistCount'), {}), + ]) + self.mpris_object.PropertiesChanged.assert_called_with( + objects.PLAYLISTS_IFACE, {'PlaylistCount': 17}, []) + + def test_playlist_changed_event_causes_mpris_playlist_changed_event(self): + self.mpris_object.get_playlist_id.return_value = 'id-for-dummy:foo' + playlist = Playlist(uri='dummy:foo', name='foo') + self.mpris_frontend.playlist_changed(playlist) + self.mpris_object.PlaylistChanged.assert_called_with( + ('id-for-dummy:foo', 'foo', '')) diff --git a/tests/frontends/mpris/playlists_interface_test.py b/tests/frontends/mpris/playlists_interface_test.py new file mode 100644 index 00000000..21038d4b --- /dev/null +++ b/tests/frontends/mpris/playlists_interface_test.py @@ -0,0 +1,171 @@ +from __future__ import unicode_literals + +import datetime +import sys + +import mock +import pykka + +from mopidy import core, exceptions +from mopidy.audio import PlaybackState +from mopidy.backends import dummy +from mopidy.models import Track + +try: + from mopidy.frontends.mpris import objects +except exceptions.OptionalDependencyError: + pass + +from tests import unittest + + +@unittest.skipUnless(sys.platform.startswith('linux'), 'requires Linux') +class PlayerInterfaceTest(unittest.TestCase): + def setUp(self): + objects.MprisObject._connect_to_dbus = mock.Mock() + self.backend = dummy.DummyBackend.start(audio=None).proxy() + self.core = core.Core.start(backends=[self.backend]).proxy() + self.mpris = objects.MprisObject(core=self.core) + + foo = self.core.playlists.create('foo').get() + foo = foo.copy(last_modified=datetime.datetime(2012, 3, 1, 6, 0, 0)) + foo = self.core.playlists.save(foo).get() + + bar = self.core.playlists.create('bar').get() + bar = bar.copy(last_modified=datetime.datetime(2012, 2, 1, 6, 0, 0)) + bar = self.core.playlists.save(bar).get() + + baz = self.core.playlists.create('baz').get() + baz = baz.copy(last_modified=datetime.datetime(2012, 1, 1, 6, 0, 0)) + baz = self.core.playlists.save(baz).get() + self.playlist = baz + + def tearDown(self): + pykka.ActorRegistry.stop_all() + + def test_activate_playlist_appends_tracks_to_tracklist(self): + self.core.tracklist.append([ + Track(uri='dummy:old-a'), + Track(uri='dummy:old-b'), + ]) + self.playlist = self.playlist.copy(tracks=[ + Track(uri='dummy:baz-a'), + Track(uri='dummy:baz-b'), + Track(uri='dummy:baz-c'), + ]) + self.playlist = self.core.playlists.save(self.playlist).get() + + self.assertEqual(2, self.core.tracklist.length.get()) + + playlists = self.mpris.GetPlaylists(0, 100, 'User', False) + playlist_id = playlists[2][0] + self.mpris.ActivatePlaylist(playlist_id) + + self.assertEqual(5, self.core.tracklist.length.get()) + self.assertEqual( + PlaybackState.PLAYING, self.core.playback.state.get()) + self.assertEqual( + self.playlist.tracks[0], self.core.playback.current_track.get()) + + def test_activate_empty_playlist_is_harmless(self): + self.assertEqual(0, self.core.tracklist.length.get()) + + playlists = self.mpris.GetPlaylists(0, 100, 'User', False) + playlist_id = playlists[2][0] + self.mpris.ActivatePlaylist(playlist_id) + + self.assertEqual(0, self.core.tracklist.length.get()) + self.assertEqual( + PlaybackState.STOPPED, self.core.playback.state.get()) + self.assertIsNone(self.core.playback.current_track.get()) + + def test_get_playlists_in_alphabetical_order(self): + result = self.mpris.GetPlaylists(0, 100, 'Alphabetical', False) + + self.assertEqual(3, len(result)) + + self.assertEqual('/com/mopidy/playlist/MR2W23LZHJRGC4Q_', result[0][0]) + self.assertEqual('bar', result[0][1]) + + self.assertEqual('/com/mopidy/playlist/MR2W23LZHJRGC6Q_', result[1][0]) + self.assertEqual('baz', result[1][1]) + + self.assertEqual('/com/mopidy/playlist/MR2W23LZHJTG63Y_', result[2][0]) + self.assertEqual('foo', result[2][1]) + + def test_get_playlists_in_reverse_alphabetical_order(self): + result = self.mpris.GetPlaylists(0, 100, 'Alphabetical', True) + + self.assertEqual(3, len(result)) + self.assertEqual('foo', result[0][1]) + self.assertEqual('baz', result[1][1]) + self.assertEqual('bar', result[2][1]) + + def test_get_playlists_in_modified_order(self): + result = self.mpris.GetPlaylists(0, 100, 'Modified', False) + + self.assertEqual(3, len(result)) + self.assertEqual('baz', result[0][1]) + self.assertEqual('bar', result[1][1]) + self.assertEqual('foo', result[2][1]) + + def test_get_playlists_in_reverse_modified_order(self): + result = self.mpris.GetPlaylists(0, 100, 'Modified', True) + + self.assertEqual(3, len(result)) + self.assertEqual('foo', result[0][1]) + self.assertEqual('bar', result[1][1]) + self.assertEqual('baz', result[2][1]) + + def test_get_playlists_in_user_order(self): + result = self.mpris.GetPlaylists(0, 100, 'User', False) + + self.assertEqual(3, len(result)) + self.assertEqual('foo', result[0][1]) + self.assertEqual('bar', result[1][1]) + self.assertEqual('baz', result[2][1]) + + def test_get_playlists_in_reverse_user_order(self): + result = self.mpris.GetPlaylists(0, 100, 'User', True) + + self.assertEqual(3, len(result)) + self.assertEqual('baz', result[0][1]) + self.assertEqual('bar', result[1][1]) + self.assertEqual('foo', result[2][1]) + + def test_get_playlists_slice_on_start_of_list(self): + result = self.mpris.GetPlaylists(0, 2, 'User', False) + + self.assertEqual(2, len(result)) + self.assertEqual('foo', result[0][1]) + self.assertEqual('bar', result[1][1]) + + def test_get_playlists_slice_later_in_list(self): + result = self.mpris.GetPlaylists(2, 2, 'User', False) + + self.assertEqual(1, len(result)) + self.assertEqual('baz', result[0][1]) + + def test_get_playlist_count_returns_number_of_playlists(self): + result = self.mpris.Get(objects.PLAYLISTS_IFACE, 'PlaylistCount') + + self.assertEqual(3, result) + + def test_get_orderings_includes_alpha_modified_and_user(self): + result = self.mpris.Get(objects.PLAYLISTS_IFACE, 'Orderings') + + self.assertIn('Alphabetical', result) + self.assertNotIn('Created', result) + self.assertIn('Modified', result) + self.assertNotIn('Played', result) + self.assertIn('User', result) + + def test_get_active_playlist_does_not_return_a_playlist(self): + result = self.mpris.Get(objects.PLAYLISTS_IFACE, 'ActivePlaylist') + valid, playlist = result + playlist_id, playlist_name, playlist_icon_uri = playlist + + self.assertEqual(False, valid) + self.assertEqual('/', playlist_id) + self.assertEqual('None', playlist_name) + self.assertEqual('', playlist_icon_uri) From 5368c5fade66ff38879f155bb3eb42212fdaae0a Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 16:49:23 +0100 Subject: [PATCH 43/46] mpd: Docstring formatting --- mopidy/frontends/mpd/protocol/music_db.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mopidy/frontends/mpd/protocol/music_db.py b/mopidy/frontends/mpd/protocol/music_db.py index 00559e13..bea57198 100644 --- a/mopidy/frontends/mpd/protocol/music_db.py +++ b/mopidy/frontends/mpd/protocol/music_db.py @@ -112,9 +112,7 @@ def list_(context, field, mpd_query=None): ``artist``, ``date``, or ``genre``. ``ARTIST`` is an optional parameter when type is ``album``, - ``date``, or ``genre``. - - This filters the result list by an artist. + ``date``, or ``genre``. This filters the result list by an artist. *Clarifications:* From 5efce8ac76fba238deedadd93cda5092a0ba7d11 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 17:09:27 +0100 Subject: [PATCH 44/46] local: Trigger playlists_loaded() event on playlist load/refresh --- mopidy/backends/local/playlists.py | 3 ++- tests/backends/base/events.py | 23 +++++++++++++++++++++++ tests/backends/local/events_test.py | 8 ++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/backends/base/events.py create mode 100644 tests/backends/local/events_test.py diff --git a/mopidy/backends/local/playlists.py b/mopidy/backends/local/playlists.py index 05873a98..ea45bcbb 100644 --- a/mopidy/backends/local/playlists.py +++ b/mopidy/backends/local/playlists.py @@ -6,7 +6,7 @@ import os import shutil from mopidy import settings -from mopidy.backends import base +from mopidy.backends import base, listener from mopidy.models import Playlist from mopidy.utils import formatting, path @@ -63,6 +63,7 @@ class LocalPlaylistsProvider(base.BasePlaylistsProvider): playlists.append(playlist) self.playlists = playlists + listener.BackendListener.send('playlists_loaded') def save(self, playlist): assert playlist.uri, 'Cannot save playlist without URI' diff --git a/tests/backends/base/events.py b/tests/backends/base/events.py new file mode 100644 index 00000000..0a2e6722 --- /dev/null +++ b/tests/backends/base/events.py @@ -0,0 +1,23 @@ +from __future__ import unicode_literals + +import mock +import pykka + +from mopidy import core, audio +from mopidy.backends import listener + + +@mock.patch.object(listener.BackendListener, 'send') +class BackendEventsTest(object): + def setUp(self): + self.audio = mock.Mock(spec=audio.Audio) + self.backend = self.backend_class.start(audio=audio).proxy() + self.core = core.Core.start(backends=[self.backend]).proxy() + + def tearDown(self): + pykka.ActorRegistry.stop_all() + + def test_playlists_refresh_sends_playlists_loaded_event(self, send): + send.reset_mock() + self.core.playlists.refresh().get() + self.assertEqual(send.call_args[0][0], 'playlists_loaded') diff --git a/tests/backends/local/events_test.py b/tests/backends/local/events_test.py new file mode 100644 index 00000000..ba61f97a --- /dev/null +++ b/tests/backends/local/events_test.py @@ -0,0 +1,8 @@ +from mopidy.backends.local import LocalBackend + +from tests import unittest +from tests.backends.base import events + + +class LocalBackendEventsTest(events.BackendEventsTest, unittest.TestCase): + backend_class = LocalBackend From 15cb291316b40621f18247cb9e71d56ca86c9dfa Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 17:10:27 +0100 Subject: [PATCH 45/46] tests: Rename test file so that it's executed --- tests/backends/local/{playlists.py => playlists_test.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/backends/local/{playlists.py => playlists_test.py} (100%) diff --git a/tests/backends/local/playlists.py b/tests/backends/local/playlists_test.py similarity index 100% rename from tests/backends/local/playlists.py rename to tests/backends/local/playlists_test.py From 2ff362a7f0cf26baa1ac99ef5b9b04bf72e2287d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 17 Nov 2012 17:14:40 +0100 Subject: [PATCH 46/46] mpris: Fix typo --- mopidy/frontends/mpris/objects.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mopidy/frontends/mpris/objects.py b/mopidy/frontends/mpris/objects.py index e7a9243e..a66abdb5 100644 --- a/mopidy/frontends/mpris/objects.py +++ b/mopidy/frontends/mpris/objects.py @@ -99,8 +99,8 @@ class MprisObject(dbus.service.Object): def get_playlist_id(self, playlist_uri): # Only A-Za-z0-9_ is allowed, which is 63 chars, so we can't use # base64. Luckily, D-Bus does not limit the length of object paths. - # Since base32 pads trailing bytes with = chars, we need to replace - # them with the allow _ char. + # Since base32 pads trailing bytes with "=" chars, we need to replace + # them with an allowed character such as "_". encoded_uri = base64.b32encode(playlist_uri).replace('=', '_') return '/com/mopidy/playlist/%s' % encoded_uri