core: Refactor next() to use pending_track for state changes

This commit is contained in:
Thomas Adamcik 2015-09-04 16:08:54 +02:00
parent 7201f2cb10
commit 2cd9903a54
4 changed files with 196 additions and 98 deletions

View File

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

View File

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

View File

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

View File

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