diff --git a/docs/changelog.rst b/docs/changelog.rst index 15df5bf9..7657bfd7 100644 --- a/docs/changelog.rst +++ b/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) =================== diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index c63405b0..27888638 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -61,8 +61,7 @@ class Scanner(object): try: _start_pipeline(pipeline) - tags, mime, have_audio = _process(pipeline, timeout) - duration = _query_duration(pipeline) + tags, mime, have_audio, duration = _process(pipeline, timeout) 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: - return tags, mime, have_audio + success, duration = pipeline.query_duration(Gst.Format.TIME) + 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 -= now - previous - previous = now + timeout = timeout_ms - (int(time.time() * 1000) - start) + + # 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) diff --git a/mopidy/audio/tags.py b/mopidy/audio/tags.py index 38a0bac9..5ae86468 100644 --- a/mopidy/audio/tags.py +++ b/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): diff --git a/mopidy/core/playback.py b/mopidy/core/playback.py index d6c470f2..ab96171e 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): """ @@ -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? 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())