diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 608b8bde..995b76fa 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -240,33 +240,24 @@ class PlaybackController(object): The current playback state will be kept. If it was playing, playing will continue. If it was paused, it will still be paused, etc. """ - original_tl_track = self.get_current_tl_track() - next_tl_track = self.core.tracklist.next_track(original_tl_track) + state = self.get_state() + current = self._pending_tl_track or self._current_tl_track - backend = self._get_backend(next_tl_track) - self._set_current_tl_track(next_tl_track) + # TODO: move to pending track? + self.core.tracklist._mark_played(self._current_tl_track) - if backend: - backend.playback.prepare_change() - backend.playback.change_track(next_tl_track.track) - - if self.get_state() == PlaybackState.PLAYING: - result = backend.playback.play().get() - elif self.get_state() == PlaybackState.PAUSED: - result = backend.playback.pause().get() + while current: + pending = self.core.tracklist.next_track(current) + if self._change(pending, state): + break else: - result = True + self.core.tracklist._mark_unplayable(pending) + # TODO: this could be needed to prevent a loop in rare cases + # if current == pending: + # break + current = pending - if result and self.get_state() != PlaybackState.PAUSED: - self._trigger_track_playback_started() - elif not result: - self.core.tracklist._mark_unplayable(next_tl_track) - # TODO: can cause an endless loop for single track repeat. - self.next() - else: - self.stop() - - self.core.tracklist._mark_played(original_tl_track) + # TODO return result? def pause(self): """Pause playback.""" @@ -301,6 +292,41 @@ class PlaybackController(object): self._play(tl_track=tl_track, tlid=tlid, on_error_step=1) + def _change(self, pending_tl_track, state): + self._pending_tl_track = pending_tl_track + + if not pending_tl_track: + self.stop() + self._on_end_of_stream() # pretend and EOS happend for cleanup + return True + + backend = self._get_backend(pending_tl_track) + if not backend: + return False + + backend.playback.prepare_change() + if not backend.playback.change_track(pending_tl_track.track).get(): + return False # TODO: test for this path + + if state == PlaybackState.PLAYING: + try: + return backend.playback.play().get() + except TypeError: + # TODO: check by binding against underlying play method using + # inspect and otherwise re-raise? + logger.error('%s needs to be updated to work with this ' + 'version of Mopidy.', backend) + return False + elif state == PlaybackState.PAUSED: + return backend.playback.pause().get() + elif state == PlaybackState.STOPPED: + # TODO: emit some event now? + self._current_tl_track = self._pending_tl_track + self._pending_tl_track = None + return True + + raise Exception('Unknown state: %s' % state) + def _play(self, tl_track=None, tlid=None, on_error_step=1): if tl_track is None and tlid is not None: for tl_track in self.core.tracklist.get_tl_tracks(): @@ -352,8 +378,6 @@ class PlaybackController(object): logger.debug('Backend exception', exc_info=True) if success: - self.core.tracklist._mark_playing(tl_track) - self.core.history._add_track(tl_track.track) # TODO: replace with stream-changed self._trigger_track_playback_started() else: diff --git a/mopidy/core/tracklist.py b/mopidy/core/tracklist.py index 13efe322..d141b435 100644 --- a/mopidy/core/tracklist.py +++ b/mopidy/core/tracklist.py @@ -318,10 +318,11 @@ class TracklistController(object): return self._shuffled[0] return None - if tl_track is None: + next_index = self.index(tl_track) + if next_index is None: next_index = 0 else: - next_index = self.index(tl_track) + 1 + next_index += 1 if self.get_repeat(): next_index %= len(self._tl_tracks) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index e3dae7b7..54f3a170 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -23,6 +23,120 @@ class TestBackend(pykka.ThreadingActor, backend.Backend): self.playback = backend.PlaybackProvider(audio=audio, backend=self) +class PlaybackBaseTest(unittest.TestCase): + config = {'core': {'max_tracklist_length': 10000}} + tracks = [Track(uri='dummy:a', length=1234), + Track(uri='dummy:b', length=1234)] + + def setUp(self): # noqa: N802 + self.audio = dummy_audio.DummyAudio.start().proxy() + self.backend = TestBackend.start( + audio=self.audio, config=self.config).proxy() + self.core = core.Core( + audio=self.audio, backends=[self.backend], config=self.config) + self.playback = self.core.playback + + with deprecation.ignore('core.tracklist.add:tracks_arg'): + self.core.tracklist.add(self.tracks) + + self.events = [] + self.patcher = mock.patch('mopidy.audio.listener.AudioListener.send') + self.send_mock = self.patcher.start() + + def send(event, **kwargs): + self.events.append((event, kwargs)) + + self.send_mock.side_effect = send + + def tearDown(self): # noqa: N802 + pykka.ActorRegistry.stop_all() + self.patcher.stop() + + def replay_events(self, until=None): + while self.events: + if self.events[0][0] == until: + break + event, kwargs = self.events.pop(0) + self.core.on_event(event, **kwargs) + + def trigger_about_to_finish(self, replay_until=None): + self.replay_events() + callback = self.audio.get_about_to_finish_callback().get() + callback() + self.replay_events(until=replay_until) + + +class TestNextHandling(PlaybackBaseTest): + + def test_get_current_tl_track_next(self): + self.core.playback.play() + self.replay_events() + + self.core.playback.next() + self.replay_events() + + tl_tracks = self.core.tracklist.get_tl_tracks() + current_tl_track = self.core.playback.get_current_tl_track() + self.assertEqual(current_tl_track, tl_tracks[1]) + + def test_get_pending_tl_track_next(self): + self.core.playback.play() + self.replay_events() + + self.core.playback.next() + + tl_tracks = self.core.tracklist.get_tl_tracks() + self.assertEqual(self.core.playback._pending_tl_track, tl_tracks[1]) + + def test_get_current_track_next(self): + self.core.playback.play() + self.replay_events() + + self.core.playback.next() + self.replay_events() + + current_track = self.core.playback.get_current_track() + self.assertEqual(current_track, self.tracks[1]) + + +class TestPlayUnknownHanlding(PlaybackBaseTest): + + tracks = [Track(uri='unknown:a', length=1234), + Track(uri='dummy:b', length=1234)] + + def test_play_skips_to_next_on_track_without_playback_backend(self): + self.core.playback.play() + + self.replay_events() + + current_track = self.core.playback.get_current_track() + self.assertEqual(current_track, self.tracks[1]) + + +class TestConsumeHandling(PlaybackBaseTest): + + def test_next_in_consume_mode_removes_finished_track(self): + tl_track = self.core.tracklist.get_tl_tracks()[0] + + self.core.playback.play(tl_track) + self.core.tracklist.consume = True + self.replay_events() + + self.core.playback.next() + self.replay_events() + + self.assertNotIn(tl_track, self.core.tracklist.get_tl_tracks()) + + def test_on_about_to_finish_in_consume_mode_removes_finished_track(self): + tl_track = self.core.tracklist.get_tl_tracks()[0] + + self.core.playback.play(tl_track) + self.core.tracklist.consume = True + self.trigger_about_to_finish() + + self.assertNotIn(tl_track, self.core.tracklist.get_tl_tracks()) + + class TestCurrentAndPendingTlTrack(unittest.TestCase): config = {'core': {'max_tracklist_length': 10000}} @@ -172,13 +286,6 @@ class CorePlaybackTest(unittest.TestCase): self.assertEqual( self.core.playback.get_current_tl_track(), self.tl_tracks[0]) - def test_get_current_tl_track_next(self): - self.core.playback.play(self.tl_tracks[0]) - self.core.playback.next() - - self.assertEqual( - self.core.playback.get_current_tl_track(), self.tl_tracks[1]) - def test_get_current_tl_track_prev(self): self.core.playback.play(self.tl_tracks[1]) self.core.playback.previous() @@ -192,13 +299,6 @@ class CorePlaybackTest(unittest.TestCase): self.assertEqual( self.core.playback.get_current_track(), self.tracks[0]) - def test_get_current_track_next(self): - self.core.playback.play(self.tl_tracks[0]) - self.core.playback.next() - - self.assertEqual( - self.core.playback.get_current_track(), self.tracks[1]) - def test_get_current_track_prev(self): self.core.playback.play(self.tl_tracks[1]) self.core.playback.previous() @@ -235,17 +335,6 @@ class CorePlaybackTest(unittest.TestCase): 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_track_without_playback_backend(self): - self.core.playback.play(self.unplayable_tl_track) - - 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( - self.core.playback.current_tl_track, self.tl_tracks[3]) - def test_play_skips_to_next_on_unplayable_track(self): """Checks that we handle backend.change_track failing.""" self.playback2.change_track.return_value.get.return_value = False @@ -458,15 +547,6 @@ class CorePlaybackTest(unittest.TestCase): self.assertIn(tl_track, self.core.tracklist.tl_tracks) - def test_next_in_consume_mode_removes_finished_track(self): - tl_track = self.tl_tracks[0] - self.core.playback.play(tl_track) - self.core.tracklist.consume = True - - self.core.playback.next() - - self.assertNotIn(tl_track, self.core.tracklist.tl_tracks) - @unittest.skip('Currently tests wrong events, and nothing generates them.') @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) @@ -544,15 +624,6 @@ class CorePlaybackTest(unittest.TestCase): self.assertIn(tl_track, self.core.tracklist.tl_tracks) - def test_on_about_to_finish_in_consume_mode_removes_finished_track(self): - tl_track = self.tl_tracks[0] - self.core.playback.play(tl_track) - self.core.tracklist.consume = True - - self.core.playback._on_about_to_finish() # TODO trigger_about_to.. - - self.assertNotIn(tl_track, self.core.tracklist.tl_tracks) - @unittest.skip('Currently tests wrong events, and nothing generates them.') @mock.patch( 'mopidy.core.playback.listener.CoreListener', spec=core.CoreListener) diff --git a/tests/local/test_playback.py b/tests/local/test_playback.py index 1e8f76c7..e149ba49 100644 --- a/tests/local/test_playback.py +++ b/tests/local/test_playback.py @@ -182,8 +182,8 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_play_when_pause_after_next(self): self.playback.play() - self.playback.next() - self.playback.next() + self.playback.next().get() + self.playback.next().get() track = self.playback.get_current_track().get() self.playback.pause() self.playback.play() @@ -203,9 +203,10 @@ 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. - return_values = [True, False] - self.backend.playback.play = lambda: return_values.pop() - self.playback.play() + uri = self.backend.playback.translate_uri(self.tracks[0].uri).get() + self.audio.trigger_fake_playback_failure(uri) + + self.playback.play().get() self.assert_current_track_is_not(self.tracks[0]) self.assert_current_track_is(self.tracks[1]) @@ -225,15 +226,15 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_previous(self): self.playback.play() - self.playback.next() + self.playback.next().get() self.playback.previous() self.assert_current_track_is(self.tracks[0]) @populate_tracklist def test_previous_more(self): self.playback.play() # At track 0 - self.playback.next() # At track 1 - self.playback.next() # At track 2 + self.playback.next().get() # At track 1 + self.playback.next().get() # At track 2 self.playback.previous() # At track 1 self.assert_current_track_is(self.tracks[1]) @@ -280,7 +281,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): old_track = self.playback.get_current_track().get() old_position = self.tracklist.index().get() - self.playback.next() + self.playback.next().get() self.assertEqual(self.tracklist.index().get(), old_position + 1) self.assert_current_track_is_not(old_track) @@ -329,11 +330,12 @@ 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. - return_values = [True, False, True] - self.backend.playback.play = lambda: return_values.pop() + uri = self.backend.playback.translate_uri(self.tracks[1].uri).get() + self.audio.trigger_fake_playback_failure(uri) + self.playback.play() self.assert_current_track_is(self.tracks[0]) - self.playback.next() + self.playback.next().get() self.assert_current_track_is_not(self.tracks[1]) self.assert_current_track_is(self.tracks[2]) @@ -349,7 +351,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_next_track_after_previous(self): self.playback.play() - self.playback.next() + self.playback.next().get() self.playback.previous() self.assert_next_tl_track_is(self.tl_tracks.get()[1]) @@ -360,7 +362,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): def test_next_track_at_end_of_playlist(self): self.playback.play() for _ in self.tl_tracks.get()[1:]: - self.playback.next() + self.playback.next().get() self.assert_next_tl_track_is(None) @populate_tracklist @@ -368,7 +370,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.repeat = True self.playback.play() for _ in self.tracks[1:]: - self.playback.next() + self.playback.next().get() self.assert_next_tl_track_is(self.tl_tracks.get()[0]) @populate_tracklist @@ -392,7 +394,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.repeat = True self.playback.play() self.assert_current_track_is(self.tracks[0]) - self.playback.next() + self.playback.next().get() self.assert_current_track_is(self.tracks[1]) @populate_tracklist @@ -403,7 +405,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.random = True self.playback.play() self.assert_current_track_is(self.tracks[-1]) - self.playback.next() + self.playback.next().get() self.assert_current_track_is(self.tracks[-2]) @populate_tracklist @@ -600,14 +602,14 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_previous_track_after_next(self): self.playback.play() - self.playback.next() + self.playback.next().get() self.assert_previous_tl_track_is(self.tl_tracks.get()[0]) @populate_tracklist def test_previous_track_after_previous(self): self.playback.play() # At track 0 - self.playback.next() # At track 1 - self.playback.next() # At track 2 + self.playback.next().get() # At track 1 + self.playback.next().get() # At track 2 self.playback.previous() # At track 1 self.assert_previous_tl_track_is(self.tl_tracks.get()[0]) @@ -642,7 +644,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_current_track_after_next(self): self.playback.play() - self.playback.next() + self.playback.next().get() self.assert_current_track_is(self.tracks[1]) @populate_tracklist @@ -657,7 +659,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_tracklist_position_after_next(self): self.playback.play() - self.playback.next() + self.playback.next().get() self.assert_current_track_index_is(1) @populate_tracklist @@ -816,7 +818,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): @populate_tracklist def test_seek_beyond_end_of_song_jumps_to_next_song(self): self.playback.play() - self.playback.seek(self.tracks[0].length * 100) + self.playback.seek(self.tracks[0].length * 100).get() self.assert_current_track_is(self.tracks[1]) @populate_tracklist @@ -904,7 +906,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.random = True self.playback.play() - self.playback.next() + self.playback.next().get() current_track = self.playback.get_current_track().get() self.playback.previous() self.assert_current_track_is(current_track) @@ -975,7 +977,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.random = True self.playback.play() for _ in self.tracks[1:]: - self.playback.next() + self.playback.next().get() self.assert_next_tl_track_is(None) @populate_tracklist @@ -992,7 +994,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.tracklist.random = True self.playback.play() for _ in self.tracks: - self.playback.next() + self.playback.next().get() self.assert_next_tl_track_is_not(None) self.assert_state_is(PlaybackState.STOPPED) self.playback.play() @@ -1029,7 +1031,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): track = self.playback.get_current_track().get() self.assertNotIn(track, played) played.append(track) - self.playback.next() + self.playback.next().get() @populate_tracklist @mock.patch('random.shuffle') @@ -1043,7 +1045,7 @@ class LocalPlaybackProviderTest(unittest.TestCase): self.playback.play() self.tracklist.random = True while self.playback.get_state().get() != PlaybackState.STOPPED: - self.playback.next() + self.playback.next().get() actual.append(self.playback.get_current_tl_track().get()) if len(actual) > len(expected): break