From 7ecc6e5af7626bfe39bfa24af61dc04468ba840b Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 26 Oct 2016 22:41:37 +0200 Subject: [PATCH 1/5] audio: Revert MMS related changes. This reverts commit 74cf673171d8ed2748089eecbaeba9f940ed253d. "audio: Wait for state change instead of async done in scanner." This reverts commit 4adea37e970a4e5082366b6d2b1861e2d1006245. "audio: Give up getting duration for MMS (Fixes: #1553)" --- docs/changelog.rst | 2 +- mopidy/audio/scan.py | 39 ++++++++++++++++----------------------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 03bca901..c0d60b54 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -40,7 +40,7 @@ Feature release. - Audio: Fix handling of MMS (and any other streams) that can't switch to playing ``ASYNC``. Previously we would time out trying to get a duration from - these. (Fixes: :issue:`1553`, PR :issue:`1575`, :issue:`1576`) + these. (Fixes: :issue:`1553`, PR :issue:`1575`) v2.0.1 (2016-08-16) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index dde4b26a..e448fe14 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -194,14 +194,13 @@ def _process(pipeline, timeout_ms): missing_message = None duration = None - # TODO: Turn this into a callback table to cleanup code. types = ( - Gst.MessageType.APPLICATION | - Gst.MessageType.DURATION_CHANGED | Gst.MessageType.ELEMENT | - Gst.MessageType.EOS | + Gst.MessageType.APPLICATION | Gst.MessageType.ERROR | - Gst.MessageType.STATE_CHANGED | + Gst.MessageType.EOS | + Gst.MessageType.ASYNC_DONE | + Gst.MessageType.DURATION_CHANGED | Gst.MessageType.TAG ) @@ -236,25 +235,19 @@ def _process(pipeline, timeout_ms): raise exceptions.ScannerError(error) elif msg.type == Gst.MessageType.EOS: return tags, mime, have_audio, duration - elif msg.type == Gst.MessageType.STATE_CHANGED and msg.src == pipeline: - old_state, new_state, pending = msg.parse_state_changed() + elif msg.type == Gst.MessageType.ASYNC_DONE: + success, duration = _query_duration(pipeline) + if tags and success: + return tags, mime, have_audio, duration - if pending == Gst.State.VOID_PENDING: - success, duration = _query_duration(pipeline) - if tags and success: - return tags, mime, have_audio, duration - - if new_state == Gst.State.PAUSED: - # 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 - logger.debug('Using workaround for duration missing.') - result = pipeline.set_state(Gst.State.PLAYING) - if result == Gst.StateChangeReturn.FAILURE: - return tags, mime, have_audio, duration - else: - 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 + logger.debug('Using workaround for duration missing before play.') + result = pipeline.set_state(Gst.State.PLAYING) + if result == Gst.StateChangeReturn.FAILURE: + return tags, mime, have_audio, duration elif msg.type == Gst.MessageType.DURATION_CHANGED: # duration will be read after ASYNC_DONE received; for now From 26e50882e6593ef2571d587275da33fa44949b46 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 26 Oct 2016 22:50:16 +0200 Subject: [PATCH 2/5] audio: Place upper limit on length of log lines for scanner --- mopidy/audio/scan.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index e448fe14..a48b0c9d 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -212,8 +212,10 @@ def _process(pipeline, timeout_ms): break if logger.isEnabledFor(log.TRACE_LOG_LEVEL) and msg.get_structure(): - _trace('element %s: %s', msg.src.get_name(), - msg.get_structure().to_string()) + debug_text = msg.get_structure().to_string() + if len(debug_text) > 77: + debug_text = debug_text[:77] + '...' + _trace('element %s: %s', msg.src.get_name(), debug_text) if msg.type == Gst.MessageType.ELEMENT: if GstPbutils.is_missing_plugin_message(msg): @@ -287,6 +289,9 @@ if __name__ == '__main__': print('%-20s %s' % (key, getattr(result, key))) print('tags') for tag, value in result.tags.items(): - print('%-20s %s' % (tag, value)) + line = '%-20s %s' % (tag, value) + if len(line) > 77: + line = line[:77] + '...' + print(line) except exceptions.ScannerError as error: print('%s: %s' % (uri, error)) From 58d1e0fbeee30f223653a990e9a74d6028843a2c Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 26 Oct 2016 23:02:39 +0200 Subject: [PATCH 3/5] audio: Make missing duration workaround more robust. --- mopidy/audio/scan.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index a48b0c9d..cc182e8d 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -251,10 +251,13 @@ def _process(pipeline, timeout_ms): if result == Gst.StateChangeReturn.FAILURE: return tags, mime, have_audio, duration - elif msg.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 msg.type == Gst.MessageType.DURATION_CHANGED and tags: + # VBR formats sometimes seem to not have a duration by the time we + # go back to paused. So just try to get it right away. + success, duration = _query_duration(pipeline) + pipeline.set_state(Gst.State.PAUSED) + if success: + return tags, mime, have_audio, duration elif msg.type == Gst.MessageType.TAG: taglist = msg.parse_tag() # Note that this will only keep the last tag. @@ -262,11 +265,6 @@ def _process(pipeline, timeout_ms): 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) From 3f46311291ca44c5fa6bef54aab8a0a263bb06c6 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 26 Oct 2016 23:11:48 +0200 Subject: [PATCH 4/5] audio: Do not try duration workaround for non-seekable sources --- mopidy/audio/scan.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index cc182e8d..72c4bee6 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -242,6 +242,10 @@ def _process(pipeline, timeout_ms): if tags and success: return tags, mime, have_audio, duration + # Don't try workaround for non-seekable sources such as mmssrc: + if not _query_seekable(pipeline): + 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. From 8c548250a8490d5c4fd738b52a1b2fe73e160b61 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Wed, 26 Oct 2016 23:48:57 +0200 Subject: [PATCH 5/5] docs: Update changelog with scanner duration changes --- docs/changelog.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index c0d60b54..7cbaa890 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -38,9 +38,9 @@ Feature release. - MPD: Fix inconsistent playlist state after playlist is emptied with repeat and consume mode turned on. (Fixes: :issue:`1512`, PR: :issue:`1549`) -- Audio: Fix handling of MMS (and any other streams) that can't switch to - playing ``ASYNC``. Previously we would time out trying to get a duration from - these. (Fixes: :issue:`1553`, PR :issue:`1575`) +- Audio: Improve handling of duration in scanning. VBR tracks should fail less + frequently and MMS works again. (Fixes: :issue:`1553`, PR :issue:`1575`, + :issue:`1576`, :issue:`1577`) v2.0.1 (2016-08-16)