From c77b63f4c8dcbd495ec88d24709571b7061d1b62 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 10 Apr 2015 23:28:00 +0200 Subject: [PATCH 1/9] audio: Add main method to scanner for quick testing --- mopidy/audio/scan.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 3880d91a..58793905 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -160,3 +160,28 @@ def _process(pipeline, timeout_ms): timeout -= clock.get_time() - start raise exceptions.ScannerError('Timeout after %dms' % timeout_ms) + + +if __name__ == '__main__': + import os + import sys + + import gobject + + from mopidy.utils import path + + gobject.threads_init() + + scanner = Scanner(5000) + for uri in sys.argv[1:]: + if not gst.uri_is_valid(uri): + uri = path.path_to_uri(os.path.abspath(uri)) + try: + result = scanner.scan(uri) + for key in ('uri', 'mime', 'duration', 'seekable'): + print '%-20s %s' % (key, getattr(result, key)) + print 'tags' + for tag, value in result.tags.items(): + print '%-20s %s' % (tag, value) + except exceptions.ScannerError as error: + print '%s: %s' % (uri, error) From 05c4af017ba2891ee2bcfa56951421fbcfaad76a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Fri, 10 Apr 2015 23:31:05 +0200 Subject: [PATCH 2/9] audio: Create fakesinks on the fly for scanner pads This makes us correctly handle say when someone gives us a movie, or something else that seems to have multiple things that can be encoded internally. --- mopidy/audio/scan.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 58793905..d9e5ae94 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -70,17 +70,16 @@ def _setup_pipeline(uri, proxy_config=None): typefind = gst.element_factory_make('typefind') decodebin = gst.element_factory_make('decodebin2') - sink = gst.element_factory_make('fakesink') pipeline = gst.element_factory_make('pipeline') - pipeline.add_many(src, typefind, decodebin, sink) + pipeline.add_many(src, typefind, decodebin) gst.element_link_many(src, typefind, decodebin) if proxy_config: utils.setup_proxy(src, proxy_config) decodebin.set_property('caps', _RAW_AUDIO) - decodebin.connect('pad-added', _pad_added, sink) + decodebin.connect('pad-added', _pad_added, pipeline) typefind.connect('have-type', _have_type, decodebin) return pipeline @@ -92,8 +91,13 @@ def _have_type(element, probability, caps, decodebin): element.get_bus().post(msg) -def _pad_added(element, pad, sink): - return pad.link(sink.get_pad('sink')) +def _pad_added(element, pad, pipeline): + sink = gst.element_factory_make('fakesink') + sink.set_property('sync', False) + + pipeline.add(sink) + sink.sync_state_with_parent() + pad.link(sink.get_pad('sink')) def _start_pipeline(pipeline): From dfaa3f143391c2d856d9b7d9b69e1841cc0e3f71 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 11 Apr 2015 00:03:20 +0200 Subject: [PATCH 3/9] audio: Have scanner tell us if we found decodeable audio --- mopidy/audio/scan.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index d9e5ae94..822277eb 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -14,7 +14,7 @@ from mopidy.utils import encoding _missing_plugin_desc = gst.pbutils.missing_plugin_message_get_description _Result = collections.namedtuple( - 'Result', ('uri', 'tags', 'duration', 'seekable', 'mime')) + 'Result', ('uri', 'tags', 'duration', 'seekable', 'mime', 'playable')) _RAW_AUDIO = gst.Caps(b'audio/x-raw-int; audio/x-raw-float') @@ -51,14 +51,14 @@ class Scanner(object): try: _start_pipeline(pipeline) - tags, mime = _process(pipeline, self._timeout_ms) + tags, mime, have_audio = _process(pipeline, self._timeout_ms) duration = _query_duration(pipeline) seekable = _query_seekable(pipeline) finally: pipeline.set_state(gst.STATE_NULL) del pipeline - return _Result(uri, tags, duration, seekable, mime) + return _Result(uri, tags, duration, seekable, mime, have_audio) # Turns out it's _much_ faster to just create a new pipeline for every as @@ -87,8 +87,9 @@ def _setup_pipeline(uri, proxy_config=None): def _have_type(element, probability, caps, decodebin): decodebin.set_property('sink-caps', caps) - msg = gst.message_new_application(element, caps.get_structure(0)) - element.get_bus().post(msg) + struct = gst.Structure('have-type') + struct['caps'] = caps.get_structure(0) + element.get_bus().post(gst.message_new_application(element, struct)) def _pad_added(element, pad, pipeline): @@ -99,6 +100,10 @@ def _pad_added(element, pad, pipeline): sink.sync_state_with_parent() pad.link(sink.get_pad('sink')) + if pad.get_caps().is_subset(_RAW_AUDIO): + struct = gst.Structure('have-audio') + element.get_bus().post(gst.message_new_application(element, struct)) + def _start_pipeline(pipeline): if pipeline.set_state(gst.STATE_PAUSED) == gst.STATE_CHANGE_NO_PREROLL: @@ -127,7 +132,7 @@ def _process(pipeline, timeout_ms): clock = pipeline.get_clock() bus = pipeline.get_bus() timeout = timeout_ms * gst.MSECOND - tags, mime, missing_description = {}, None, None + tags, mime, have_audio, missing_description = {}, None, False, None types = (gst.MESSAGE_ELEMENT | gst.MESSAGE_APPLICATION | gst.MESSAGE_ERROR | gst.MESSAGE_EOS | gst.MESSAGE_ASYNC_DONE | gst.MESSAGE_TAG) @@ -143,19 +148,22 @@ def _process(pipeline, timeout_ms): missing_description = encoding.locale_decode( _missing_plugin_desc(message)) elif message.type == gst.MESSAGE_APPLICATION: - mime = message.structure.get_name() - if mime.startswith('text/') or mime == 'application/xml': - return tags, mime + if message.structure.get_name() == 'have-type': + mime = message.structure['caps'].get_name() + if mime.startswith('text/') or mime == 'application/xml': + return tags, mime, have_audio + elif message.structure.get_name() == 'have-audio': + have_audio = True elif message.type == gst.MESSAGE_ERROR: error = encoding.locale_decode(message.parse_error()[0]) if missing_description: error = '%s (%s)' % (missing_description, error) raise exceptions.ScannerError(error) elif message.type == gst.MESSAGE_EOS: - return tags, mime + return tags, mime, have_audio elif message.type == gst.MESSAGE_ASYNC_DONE: if message.src == pipeline: - return tags, mime + return tags, mime, have_audio elif message.type == gst.MESSAGE_TAG: taglist = message.parse_tag() # Note that this will only keep the last tag. @@ -182,7 +190,7 @@ if __name__ == '__main__': uri = path.path_to_uri(os.path.abspath(uri)) try: result = scanner.scan(uri) - for key in ('uri', 'mime', 'duration', 'seekable'): + for key in ('uri', 'mime', 'duration', 'playable', 'seekable'): print '%-20s %s' % (key, getattr(result, key)) print 'tags' for tag, value in result.tags.items(): From 9bc4d8b713bcb22e65544e6bff860ab8432dca3a Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 11 Apr 2015 00:29:07 +0200 Subject: [PATCH 4/9] audio: Make scanner handle all media types. I don't think this makes anything slower, as before we would still decode anything we came across in the hopes that we find raw audio. --- mopidy/audio/scan.py | 3 +-- tests/audio/test_scan.py | 29 +++++++++++++---------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 822277eb..9412b231 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -78,9 +78,8 @@ def _setup_pipeline(uri, proxy_config=None): if proxy_config: utils.setup_proxy(src, proxy_config) - decodebin.set_property('caps', _RAW_AUDIO) - decodebin.connect('pad-added', _pad_added, pipeline) typefind.connect('have-type', _have_type, decodebin) + decodebin.connect('pad-added', _pad_added, pipeline) return pipeline diff --git a/tests/audio/test_scan.py b/tests/audio/test_scan.py index b2937a3f..f58b2202 100644 --- a/tests/audio/test_scan.py +++ b/tests/audio/test_scan.py @@ -16,8 +16,7 @@ from tests import path_to_data_dir class ScannerTest(unittest.TestCase): def setUp(self): # noqa: N802 self.errors = {} - self.tags = {} - self.durations = {} + self.result = {} def find(self, path): media_dir = path_to_data_dir(path) @@ -31,19 +30,17 @@ class ScannerTest(unittest.TestCase): uri = path_lib.path_to_uri(path) key = uri[len('file://'):] try: - result = scanner.scan(uri) - self.tags[key] = result.tags - self.durations[key] = result.duration + self.result[key] = scanner.scan(uri) except exceptions.ScannerError as error: self.errors[key] = error def check(self, name, key, value): name = path_to_data_dir(name) - self.assertEqual(self.tags[name][key], value) + self.assertEqual(self.result[name].tags[key], value) def test_tags_is_set(self): self.scan(self.find('scanner/simple')) - self.assert_(self.tags) + self.assert_(self.result.values()[0].tags) def test_errors_is_not_set(self): self.scan(self.find('scanner/simple')) @@ -52,10 +49,10 @@ class ScannerTest(unittest.TestCase): def test_duration_is_set(self): self.scan(self.find('scanner/simple')) - self.assertEqual( - self.durations[path_to_data_dir('scanner/simple/song1.mp3')], 4680) - self.assertEqual( - self.durations[path_to_data_dir('scanner/simple/song1.ogg')], 4680) + ogg = path_to_data_dir('scanner/simple/song1.ogg') + mp3 = path_to_data_dir('scanner/simple/song1.mp3') + self.assertEqual(self.result[mp3].duration, 4680) + self.assertEqual(self.result[ogg].duration, 4680) def test_artist_is_set(self): self.scan(self.find('scanner/simple')) @@ -78,17 +75,17 @@ class ScannerTest(unittest.TestCase): def test_other_media_is_ignored(self): self.scan(self.find('scanner/image')) - self.assert_(self.errors) + self.assertFalse(self.result.values()[0].playable) def test_log_file_that_gst_thinks_is_mpeg_1_is_ignored(self): self.scan([path_to_data_dir('scanner/example.log')]) - self.assertLess( - self.durations[path_to_data_dir('scanner/example.log')], 100) + log = path_to_data_dir('scanner/example.log') + self.assertLess(self.result[log].duration, 100) def test_empty_wav_file(self): self.scan([path_to_data_dir('scanner/empty.wav')]) - self.assertEqual( - self.durations[path_to_data_dir('scanner/empty.wav')], 0) + wav = path_to_data_dir('scanner/empty.wav') + self.assertEqual(self.result[wav].duration, 0) @unittest.SkipTest def test_song_without_time_is_handeled(self): From 48a461991a764b0bea368f7db3b3640ca80aa816 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sat, 11 Apr 2015 00:38:46 +0200 Subject: [PATCH 5/9] local: Skip unplayable tracks --- mopidy/local/commands.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mopidy/local/commands.py b/mopidy/local/commands.py index af8b0025..4383decb 100644 --- a/mopidy/local/commands.py +++ b/mopidy/local/commands.py @@ -135,7 +135,9 @@ class ScanCommand(commands.Command): file_uri = path.path_to_uri(os.path.join(media_dir, relpath)) result = scanner.scan(file_uri) tags, duration = result.tags, result.duration - if duration < MIN_DURATION_MS: + if not result.playable: + logger.warning('Failed %s: No audio found in file.', uri) + elif duration < MIN_DURATION_MS: logger.warning('Failed %s: Track shorter than %dms', uri, MIN_DURATION_MS) else: From 1a1a0753a47ade4faa01248136acf6062ded5365 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 12 Apr 2015 14:16:35 +0200 Subject: [PATCH 6/9] audio: Use print function in scanner --- mopidy/audio/scan.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index 9412b231..f2a93620 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -1,4 +1,5 @@ -from __future__ import absolute_import, division, unicode_literals +from __future__ import ( + absolute_import, division, print_function, unicode_literals) import collections @@ -190,9 +191,9 @@ if __name__ == '__main__': try: result = scanner.scan(uri) for key in ('uri', 'mime', 'duration', 'playable', 'seekable'): - print '%-20s %s' % (key, getattr(result, key)) - print 'tags' + print('%-20s %s' % (key, getattr(result, key))) + print('tags') for tag, value in result.tags.items(): - print '%-20s %s' % (tag, value) + print('%-20s %s' % (tag, value)) except exceptions.ScannerError as error: - print '%s: %s' % (uri, error) + print('%s: %s' % (uri, error)) From 68c2758009b58ba81419b25baacb3ec6570cc174 Mon Sep 17 00:00:00 2001 From: Thomas Adamcik Date: Sun, 12 Apr 2015 14:24:28 +0200 Subject: [PATCH 7/9] docs: Add scanner improvements to changelog --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ce7be87b..b3a7a459 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -17,6 +17,10 @@ v1.0.1 (UNRELEASED) behavior was confusing for many users and doesn't work well with the plans for multiple outputs. +- Audio: Update scanner to decode all media it finds. This should fix cases + where the scanner hangs on non-audio files like video. The scanner will now + also let us know if we found any decodeable audio. (Fixes: :issue:`726`) + v1.0.0 (2015-03-25) =================== From f85ea2a39d9d87e5da491847a4bd3ad165fbd5ca Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sun, 12 Apr 2015 23:03:46 +0200 Subject: [PATCH 8/9] flake8: Fix new import order warnings (cherry picked from commit 71ab9733c760b68a49fa80f21793e1dbcdc3d86f) --- mopidy/audio/actor.py | 2 +- mopidy/audio/scan.py | 2 +- mopidy/utils/deps.py | 4 ++-- tests/audio/test_actor.py | 4 ++-- tests/stream/test_library.py | 4 ++-- tests/utils/test_deps.py | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/mopidy/audio/actor.py b/mopidy/audio/actor.py index 35bd215f..e0a7892a 100644 --- a/mopidy/audio/actor.py +++ b/mopidy/audio/actor.py @@ -8,7 +8,7 @@ import gobject import pygst pygst.require('0.10') import gst # noqa -import gst.pbutils +import gst.pbutils # noqa import pykka diff --git a/mopidy/audio/scan.py b/mopidy/audio/scan.py index f2a93620..d1e83407 100644 --- a/mopidy/audio/scan.py +++ b/mopidy/audio/scan.py @@ -6,7 +6,7 @@ import collections import pygst pygst.require('0.10') import gst # noqa -import gst.pbutils +import gst.pbutils # noqa from mopidy import exceptions from mopidy.audio import utils diff --git a/mopidy/utils/deps.py b/mopidy/utils/deps.py index bc9f7c2f..aafede9d 100644 --- a/mopidy/utils/deps.py +++ b/mopidy/utils/deps.py @@ -5,12 +5,12 @@ import os import platform import sys +import pkg_resources + import pygst pygst.require('0.10') import gst # noqa -import pkg_resources - from mopidy.utils import formatting diff --git a/tests/audio/test_actor.py b/tests/audio/test_actor.py index fbc440de..b00646bc 100644 --- a/tests/audio/test_actor.py +++ b/tests/audio/test_actor.py @@ -6,12 +6,12 @@ import unittest import gobject gobject.threads_init() +import mock + import pygst pygst.require('0.10') import gst # noqa -import mock - import pykka from mopidy import audio diff --git a/tests/stream/test_library.py b/tests/stream/test_library.py index 93292376..65c59cb6 100644 --- a/tests/stream/test_library.py +++ b/tests/stream/test_library.py @@ -5,12 +5,12 @@ import unittest import gobject gobject.threads_init() +import mock + import pygst pygst.require('0.10') import gst # noqa: pygst magic is needed to import correct gst -import mock - from mopidy.models import Track from mopidy.stream import actor from mopidy.utils.path import path_to_uri diff --git a/tests/utils/test_deps.py b/tests/utils/test_deps.py index 95f5b982..3b06973f 100644 --- a/tests/utils/test_deps.py +++ b/tests/utils/test_deps.py @@ -6,12 +6,12 @@ import unittest import mock +import pkg_resources + import pygst pygst.require('0.10') import gst # noqa -import pkg_resources - from mopidy.utils import deps From c8b348a61deae2c8de2d504df1643dd90ec2de60 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Mon, 13 Apr 2015 08:16:54 +0200 Subject: [PATCH 9/9] docs: Tweak changelog --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index b3a7a459..6cf8ca15 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -8,6 +8,8 @@ This changelog is used to track all major changes to Mopidy. v1.0.1 (UNRELEASED) =================== +Bug fix release. + - Audio: Software volume control has been reworked to greatly reduce the delay between changing the volume and the change taking effect. (Fixes: :issue:`1097`)