diff --git a/mopidy/backend.py b/mopidy/backend.py index 7e020b77..3852b1d4 100644 --- a/mopidy/backend.py +++ b/mopidy/backend.py @@ -177,26 +177,40 @@ class PlaybackProvider(object): """ return self.audio.pause_playback().get() - def play(self, track): + def play(self): """ - Play given track. + Start playback. *MAY be reimplemented by subclass.* - :param track: the track to play - :type track: :class:`mopidy.models.Track` :rtype: :class:`True` if successful, else :class:`False` """ - self.audio.prepare_change() - self.change_track(track) return self.audio.start_playback().get() + def prepare_change(self): + """ + Indicate that an URI change is about to happen. + + *MAY be reimplemented by subclass.* + + It is extremely unlikely it makes sense for any backends to override + this. For most practical purposes it should be considered an internal + call between backends and core that backend authors should not touch. + """ + self.audio.prepare_change().get() + def change_track(self, track): """ Swith to provided track. *MAY be reimplemented by subclass.* + This is very likely the *only* thing you need to override as a backend + author. Typically this is where you convert any mopidy specific URIs + to real URIs and then return:: + + return super(MyBackend, self).change_track(track.copy(uri=new_uri)) + :param track: the track to play :type track: :class:`mopidy.models.Track` :rtype: :class:`True` if successful, else :class:`False` @@ -232,6 +246,9 @@ class PlaybackProvider(object): *MAY be reimplemented by subclass.* + Should not be used for tracking if tracks have been played / when we + are done playing them. + :rtype: :class:`True` if successful, else :class:`False` """ return self.audio.stop_playback().get() diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 0b714598..4f51f328 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -308,7 +308,12 @@ class PlaybackController(object): self.set_current_tl_track(tl_track) self.set_state(PlaybackState.PLAYING) backend = self._get_backend() - success = backend and backend.playback.play(tl_track.track).get() + success = False + + if backend: + backend.playback.prepare_change() + backend.playback.change_track(tl_track.track) + success = backend.playback.play().get() if success: self.core.tracklist._mark_playing(tl_track) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index e6dc3ce1..e84e7301 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -100,19 +100,25 @@ class CorePlaybackTest(unittest.TestCase): def test_play_selects_dummy1_backend(self): self.core.playback.play(self.tl_tracks[0]) - self.playback1.play.assert_called_once_with(self.tracks[0]) + self.playback1.prepare_change.assert_called_once_with() + self.playback1.change_track.assert_called_once_with(self.tracks[0]) + self.playback1.play.assert_called_once_with() self.assertFalse(self.playback2.play.called) def test_play_selects_dummy2_backend(self): self.core.playback.play(self.tl_tracks[1]) self.assertFalse(self.playback1.play.called) - self.playback2.play.assert_called_once_with(self.tracks[1]) + self.playback2.prepare_change.assert_called_once_with() + self.playback2.change_track.assert_called_once_with(self.tracks[1]) + self.playback2.play.assert_called_once_with() def test_play_skips_to_next_on_unplayable_track(self): self.core.playback.play(self.unplayable_tl_track) - self.playback1.play.assert_called_once_with(self.tracks[3]) + self.playback1.prepare_change.assert_called_once_with() + self.playback1.change_track.assert_called_once_with(self.tracks[3]) + self.playback1.play.assert_called_once_with() self.assertFalse(self.playback2.play.called) self.assertEqual( diff --git a/tests/dummy_backend.py b/tests/dummy_backend.py index d0816096..d4441673 100644 --- a/tests/dummy_backend.py +++ b/tests/dummy_backend.py @@ -62,15 +62,23 @@ class DummyLibraryProvider(backend.LibraryProvider): class DummyPlaybackProvider(backend.PlaybackProvider): def __init__(self, *args, **kwargs): super(DummyPlaybackProvider, self).__init__(*args, **kwargs) + self._uri = None self._time_position = 0 def pause(self): return True - def play(self, track): + def play(self): + return self._uri and self._uri != 'dummy:error' + + def change_track(self, track): """Pass a track with URI 'dummy:error' to force failure""" + self._uri = track.uri self._time_position = 0 - return track.uri != 'dummy:error' + return True + + def prepare_change(self): + pass def resume(self): return True @@ -80,6 +88,7 @@ class DummyPlaybackProvider(backend.PlaybackProvider): return True def stop(self): + self._uri = None return True def get_time_position(self): diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 3ccd8d8f..4c4ded24 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -154,7 +154,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_play_skips_to_next_track_on_failure(self): # If backend's play() returns False, it is a failure. - self.backend.playback.play = lambda track: track != self.tracks[0] + return_values = [True, False] + self.backend.playback.play = lambda: return_values.pop() self.playback.play() self.assertNotEqual(self.playback.current_track, self.tracks[0]) self.assertEqual(self.playback.current_track, self.tracks[1]) @@ -214,7 +215,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_previous_skips_to_previous_track_on_failure(self): # If backend's play() returns False, it is a failure. - self.backend.playback.play = lambda track: track != self.tracks[1] + return_values = [True, False, True] + self.backend.playback.play = lambda: return_values.pop() self.playback.play(self.tracklist.tl_tracks[2]) self.assertEqual(self.playback.current_track, self.tracks[2]) self.playback.previous() @@ -281,7 +283,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_next_skips_to_next_track_on_failure(self): # If backend's play() returns False, it is a failure. - self.backend.playback.play = lambda track: track != self.tracks[1] + return_values = [True, False, True] + self.backend.playback.play = lambda: return_values.pop() self.playback.play() self.assertEqual(self.playback.current_track, self.tracks[0]) self.playback.next() @@ -455,7 +458,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_end_of_track_skips_to_next_track_on_failure(self): # If backend's play() returns False, it is a failure. - self.backend.playback.play = lambda track: track != self.tracks[1] + return_values = [True, False, True] + self.backend.playback.play = lambda: return_values.pop() self.playback.play() self.assertEqual(self.playback.current_track, self.tracks[0]) self.playback.on_end_of_track()