Merge branch 'release-2.0' into develop

docs/changelog.rst

@@ -23,6 +23,17 @@ Bug fix release.
   to only process audio. This should fix the "Volume/mute is not available"
   warning.
 
+- Audio: Fix buffer conversion. This fixes image extraction.
+  (PR: :issue:`1472`)
+
+- Audio: Update scan logic to workaround GStreamer issue where tags and
+  duration might only be available after we start playing.
+  (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/audio/scan.py

@@ -61,8 +61,7 @@ class Scanner(object):
 
         try:
             _start_pipeline(pipeline)
-            tags, mime, have_audio = _process(pipeline, timeout)
+            tags, mime, have_audio, duration = _process(pipeline, timeout)
-            duration = _query_duration(pipeline)
             seekable = _query_seekable(pipeline)
         finally:
             signals.clear()
@@ -136,30 +135,6 @@ def _start_pipeline(pipeline):
         pipeline.set_state(Gst.State.PLAYING)
 
 
-def _query_duration(pipeline, timeout=100):
-    # 1. Try and get a duration, return if success.
-    # 2. Some formats need to play some buffers before duration is found.
-    # 3. Wait for a duration change event.
-    # 4. Try and get a duration again.
-
-    success, duration = pipeline.query_duration(Gst.Format.TIME)
-    if success and duration >= 0:
-        return duration // Gst.MSECOND
-
-    result = pipeline.set_state(Gst.State.PLAYING)
-    if result == Gst.StateChangeReturn.FAILURE:
-        return None
-
-    gst_timeout = timeout * Gst.MSECOND
-    bus = pipeline.get_bus()
-    bus.timed_pop_filtered(gst_timeout, Gst.MessageType.DURATION_CHANGED)
-
-    success, duration = pipeline.query_duration(Gst.Format.TIME)
-    if success and duration >= 0:
-        return duration // Gst.MSECOND
-    return None
-
-
 def _query_seekable(pipeline):
     query = Gst.Query.new_seeking(Gst.Format.TIME)
     pipeline.query(query)
@@ -172,6 +147,7 @@ def _process(pipeline, timeout_ms):
     mime = None
     have_audio = False
     missing_message = None
+    duration = None
 
     types = (
         Gst.MessageType.ELEMENT |
@@ -179,11 +155,12 @@ def _process(pipeline, timeout_ms):
         Gst.MessageType.ERROR |
         Gst.MessageType.EOS |
         Gst.MessageType.ASYNC_DONE |
+        Gst.MessageType.DURATION_CHANGED |
         Gst.MessageType.TAG
     )
 
     timeout = timeout_ms
-    previous = int(time.time() * 1000)
+    start = int(time.time() * 1000)
     while timeout > 0:
         message = bus.timed_pop_filtered(timeout * Gst.MSECOND, types)
 
@@ -197,7 +174,7 @@ def _process(pipeline, timeout_ms):
                 mime = message.get_structure().get_value('caps').get_name()
                 if mime and (
                         mime.startswith('text/') or mime == 'application/xml'):
-                    return tags, mime, have_audio
+                    return tags, mime, have_audio, duration
             elif message.get_structure().get_name() == 'have-audio':
                 have_audio = True
         elif message.type == Gst.MessageType.ERROR:
@@ -205,21 +182,43 @@ def _process(pipeline, timeout_ms):
             if missing_message and not mime:
                 caps = missing_message.get_structure().get_value('detail')
                 mime = caps.get_structure(0).get_name()
-                return tags, mime, have_audio
+                return tags, mime, have_audio, duration
             raise exceptions.ScannerError(error)
         elif message.type == Gst.MessageType.EOS:
-            return tags, mime, have_audio
+            return tags, mime, have_audio, duration
         elif message.type == Gst.MessageType.ASYNC_DONE:
-            if message.src == pipeline:
+            success, duration = pipeline.query_duration(Gst.Format.TIME)
-                return tags, mime, have_audio
+            if success:
+                duration = duration // Gst.MSECOND
+            else:
+                duration = None
+
+            if tags and duration is not None:
+                return tags, mime, have_audio, duration
+
+            # Workaround for upstream bug which causes tags/duration to arrive
+            # after pre-roll. We get around this by starting to play the track
+            # and then waiting for a duration change.
+            # https://bugzilla.gnome.org/show_bug.cgi?id=763553
+            result = pipeline.set_state(Gst.State.PLAYING)
+            if result == Gst.StateChangeReturn.FAILURE:
+                return tags, mime, have_audio, duration
+
+        elif message.type == Gst.MessageType.DURATION_CHANGED:
+            # duration will be read after ASYNC_DONE received; for now
+            # just give it a non-None value to flag that we have a duration:
+            duration = 0
         elif message.type == Gst.MessageType.TAG:
             taglist = message.parse_tag()
             # Note that this will only keep the last tag.
             tags.update(tags_lib.convert_taglist(taglist))
 
-        now = int(time.time() * 1000)
+        timeout = timeout_ms - (int(time.time() * 1000) - start)
-        timeout -= now - previous
+
-        previous = now
+        # workaround for https://bugzilla.gnome.org/show_bug.cgi?id=763553:
+        # if we got what we want then stop playing (and wait for ASYNC_DONE)
+        if tags and duration is not None:
+            pipeline.set_state(Gst.State.PAUSED)
 
     raise exceptions.ScannerError('Timeout after %dms' % timeout_ms)
 

mopidy/audio/tags.py

@@ -53,6 +53,10 @@ gstreamer-GstTagList.html
                 result[tag].append(value.decode('utf-8', 'replace'))
             elif isinstance(value, (compat.text_type, bool, numbers.Number)):
                 result[tag].append(value)
+            elif isinstance(value, Gst.Sample):
+                data = _extract_sample_data(value)
+                if data:
+                    result[tag].append(data)
             else:
                 logger.log(
                     log.TRACE_LOG_LEVEL,
@@ -62,6 +66,13 @@ gstreamer-GstTagList.html
     return result
 
 
+def _extract_sample_data(sample):
+    buf = sample.get_buffer()
+    if not buf:
+        return None
+    return buf.extract_dup(0, buf.get_size())
+
+
 # TODO: split based on "stream" and "track" based conversion? i.e. handle data
 # from radios in it's own helper instead?
 def convert_tags_to_track(tags):

mopidy/core/playback.py

@@ -252,12 +252,13 @@ class PlaybackController(object):
             return
 
         pending = self.core.tracklist.eot_track(self._current_tl_track)
-        while pending:
+        # avoid endless loop if 'repeat' is 'true' and no track is playable
-            # TODO: Avoid infinite loops if all tracks are unplayable.
+        # * 2 -> second run to get all playable track in a shuffled playlist
-            backend = self._get_backend(pending)
+        count = self.core.tracklist.get_length() * 2
-            if not backend:
-                continue
 
+        while pending:
+            backend = self._get_backend(pending)
+            if backend:
                 try:
                     if backend.playback.change_track(pending.track).get():
                         self._pending_tl_track = pending
@@ -268,6 +269,10 @@ class PlaybackController(object):
 
             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())