Merge PR #1454 from dublok/fix/1454-endless-loop-if-no-playable-track

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

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):
         """
@@ -290,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)
@@ -301,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?
 
@@ -352,6 +364,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 +375,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?
 
@@ -416,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)
@@ -427,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?
 

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