From a2f0d6960ff0e1f02d15f201cb37133b7aadc83a Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Wed, 17 Feb 2016 22:54:22 +0100 Subject: [PATCH 1/4] Avoid endless loop if all tracks are unplayable. Limit the number of tries for changing to the nest track. The limit is 2 * tracklist length get all tracks in a shuffled playlist. --- mopidy/core/playback.py | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index d6c470f2..0d945b1c 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -252,22 +252,27 @@ class PlaybackController(object): return pending = self.core.tracklist.eot_track(self._current_tl_track) - while pending: - # TODO: Avoid infinite loops if all tracks are unplayable. - backend = self._get_backend(pending) - if not backend: - continue + # avoid endless loop if 'repeat' is 'true' and no track is playable + # * 2 -> second run to get all playable track in a shuffled playlist + count = self.core.tracklist.get_length() * 2 - try: - if backend.playback.change_track(pending.track).get(): - self._pending_tl_track = pending - break - except Exception: - logger.exception('%s backend caused an exception.', - backend.actor_ref.actor_class.__name__) + while pending: + backend = self._get_backend(pending) + if backend: + try: + if backend.playback.change_track(pending.track).get(): + self._pending_tl_track = pending + break + except Exception: + logger.exception('%s backend caused an exception.', + backend.actor_ref.actor_class.__name__) self.core.tracklist._mark_unplayable(pending) pending = self.core.tracklist.eot_track(pending) + count -= 1 + if not count: + logger.info('No playable track in the list.') + break def _on_tracklist_change(self): """ @@ -352,6 +357,9 @@ class PlaybackController(object): current = self._pending_tl_track or self._current_tl_track pending = tl_track or current or self.core.tracklist.next_track(None) + # avoid endless loop if 'repeat' is 'true' and no track is playable + # * 2 -> second run to get all playable track in a shuffled playlist + count = self.core.tracklist.get_length() * 2 while pending: if self._change(pending, PlaybackState.PLAYING): @@ -360,6 +368,10 @@ class PlaybackController(object): self.core.tracklist._mark_unplayable(pending) current = pending pending = self.core.tracklist.next_track(current) + count -= 1 + if not count: + logger.info('No playable track in the list.') + break # TODO return result? From 6116705c1be9c3d39281991f39b3bfb417d9ed36 Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Wed, 17 Feb 2016 23:41:56 +0100 Subject: [PATCH 2/4] Avoid endless loop in 'next' and 'previous'. --- mopidy/core/playback.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index 0d945b1c..ab96171e 100644 --- a/mopidy/core/playback.py +++ b/mopidy/core/playback.py @@ -295,6 +295,9 @@ class PlaybackController(object): """ state = self.get_state() current = self._pending_tl_track or self._current_tl_track + # avoid endless loop if 'repeat' is 'true' and no track is playable + # * 2 -> second run to get all playable track in a shuffled playlist + count = self.core.tracklist.get_length() * 2 while current: pending = self.core.tracklist.next_track(current) @@ -306,6 +309,10 @@ class PlaybackController(object): # if current == pending: # break current = pending + count -= 1 + if not count: + logger.info('No playable track in the list.') + break # TODO return result? @@ -428,6 +435,9 @@ class PlaybackController(object): self._previous = True state = self.get_state() current = self._pending_tl_track or self._current_tl_track + # avoid endless loop if 'repeat' is 'true' and no track is playable + # * 2 -> second run to get all playable track in a shuffled playlist + count = self.core.tracklist.get_length() * 2 while current: pending = self.core.tracklist.previous_track(current) @@ -439,6 +449,10 @@ class PlaybackController(object): # if current == pending: # break current = pending + count -= 1 + if not count: + logger.info('No playable track in the list.') + break # TODO: no return value? From bf4da7a62700d623757a6e9de94b28cc2ed6c7cc Mon Sep 17 00:00:00 2001 From: Jens Luetjen Date: Thu, 18 Feb 2016 20:30:28 +0100 Subject: [PATCH 3/4] Add tests to ensure that play, next will not busy-loop Test PlaybackController functions play(), next(), previous() and _on_about_to_finish() that they will not loop forever if all tracks are unplayable. --- tests/core/test_playback.py | 105 ++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/tests/core/test_playback.py b/tests/core/test_playback.py index cfd58793..3572800c 100644 --- a/tests/core/test_playback.py +++ b/tests/core/test_playback.py @@ -14,11 +14,42 @@ from tests import dummy_audio class TestPlaybackProvider(backend.PlaybackProvider): + + def __init__(self, audio, backend): + super(TestPlaybackProvider, self).__init__(audio, backend) + self._call_limit = 10 + self._call_count = 0 + self._call_onetime = False + + def reset_call_limit(self): + self._call_count = 0 + self._call_onetime = False + + def is_call_limit_reached(self): + return self._call_count > self._call_limit + + def _translate_uri_call_limit(self, uri): + self._call_count += 1 + if self._call_count > self._call_limit: + # return any url (not 'None') to stop the endless loop + return 'assert: call limit reached' + if 'limit_never' in uri: + # unplayable + return None + elif 'limit_one' in uri: + # one time playable + if self._call_onetime: + return None + self._call_onetime = True + return uri + def translate_uri(self, uri): if 'error' in uri: raise Exception(uri) elif 'unplayable' in uri: return None + elif 'limit' in uri: + return self._translate_uri_call_limit(uri) else: return uri @@ -1125,3 +1156,77 @@ class TestBug1352Regression(BaseTest): self.core.history._add_track.assert_called_once_with(self.tracks[1]) self.core.tracklist._mark_playing.assert_called_once_with(tl_tracks[1]) + + +class TestEndlessLoop(BaseTest): + + tracks_play = [ + Track(uri='dummy:limit_never:a'), + Track(uri='dummy:limit_never:b') + ] + + tracks_other = [ + Track(uri='dummy:limit_never:a'), + Track(uri='dummy:limit_one'), + Track(uri='dummy:limit_never:b') + ] + + def test_play(self): + self.core.tracklist.clear() + self.core.tracklist.add(self.tracks_play) + + self.backend.playback.reset_call_limit().get() + self.core.tracklist.set_repeat(True) + + tl_tracks = self.core.tracklist.get_tl_tracks() + self.core.playback.play(tl_tracks[0]) + self.replay_events() + + self.assertFalse(self.backend.playback.is_call_limit_reached().get()) + + def test_next(self): + self.core.tracklist.clear() + self.core.tracklist.add(self.tracks_other) + + self.backend.playback.reset_call_limit().get() + self.core.tracklist.set_repeat(True) + + tl_tracks = self.core.tracklist.get_tl_tracks() + self.core.playback.play(tl_tracks[1]) + self.replay_events() + + self.core.playback.next() + self.replay_events() + + self.assertFalse(self.backend.playback.is_call_limit_reached().get()) + + def test_previous(self): + self.core.tracklist.clear() + self.core.tracklist.add(self.tracks_other) + + self.backend.playback.reset_call_limit().get() + self.core.tracklist.set_repeat(True) + + tl_tracks = self.core.tracklist.get_tl_tracks() + self.core.playback.play(tl_tracks[1]) + self.replay_events() + + self.core.playback.previous() + self.replay_events() + + self.assertFalse(self.backend.playback.is_call_limit_reached().get()) + + def test_on_about_to_finish(self): + self.core.tracklist.clear() + self.core.tracklist.add(self.tracks_other) + + self.backend.playback.reset_call_limit().get() + self.core.tracklist.set_repeat(True) + + tl_tracks = self.core.tracklist.get_tl_tracks() + self.core.playback.play(tl_tracks[1]) + self.replay_events() + + self.trigger_about_to_finish() + + self.assertFalse(self.backend.playback.is_call_limit_reached().get()) From aafcf8559d0a56db9f9a3d384535c6ba1272f20e Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 26 Mar 2016 09:51:14 +0100 Subject: [PATCH 4/4] docs: Add PR #1455 to changelog --- docs/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2e9ca56a..ee2ddf8c 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,9 @@ Bug fix release. (Fixes: :issue:`935`, :issue:`1453`, :issue:`1474` and :issue:`1480` PR: :issue:`1487`) +- Core: Avoid endless loop if all tracks in the tracklist are unplayable and + consume mode is off. (Fixes: :issue:`1221`, :issue:`1454`, PR: :issue:`1455`) + v2.0.0 (2016-02-15) ===================