From 48671cbdf1cba0ae7dedf63b037571b49a1465a3 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Mar 2015 10:38:01 -0800 Subject: [PATCH 1/8] Changelog for #1343 --- docs/changelog.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 11a64d3ad..ab9918ae3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,8 @@ Changelog Features: +* :doc:`/plugins/replaygain`: There is a new backend for the `bs1770gain`_ + tool. Thanks to :user:`jmwatte`. :bug:`1343` * There are now multiple levels of verbosity. On the command line, you can make beets somewhat verbose with ``-v`` or very verbose with ``-vv``. :bug:`1244` @@ -129,6 +131,8 @@ For developers: immediately after they are initialized. It's also possible to replace the originally created tasks by returning new ones using this event. +.. _bs1770gain: http://bs1770gain.sourceforge.net + 1.3.10 (January 5, 2015) ------------------------ From 8113c7ba85febeb8c976081adecae6f0e04e5eef Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Tue, 3 Mar 2015 10:40:53 -0800 Subject: [PATCH 2/8] Roll back whitespace changes from #1343 --- beetsplug/replaygain.py | 12 ++---------- docs/plugins/replaygain.rst | 12 ++++++------ 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e9d71619f..c4bebc8dd 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -33,14 +33,12 @@ from beets import config # Utilities. class ReplayGainError(Exception): - """Raised when a local (to a track or an album) error occurs in one of the backends. """ class FatalReplayGainError(Exception): - """Raised when a fatal error occurs in one of the backends. """ @@ -69,7 +67,6 @@ AlbumGain = collections.namedtuple("AlbumGain", "album_gain track_gains") class Backend(object): - """An abstract class representing engine for calculating RG values. """ @@ -90,10 +87,8 @@ class Backend(object): # bsg1770gain backend class Bs1770gainBackend(Backend): - - """bs1770gain is a loudness scanner compliant with ITU-R BS.1770 and its - flavors EBU R128,ATSC A/85 and Replaygain 2.0. It uses a special - designed algorithm to normalize audio to the same level. + """bs1770gain is a loudness scanner compliant with ITU-R BS.1770 and + its flavors EBU R128, ATSC A/85 and Replaygain 2.0. """ def __init__(self, config, log): @@ -576,7 +571,6 @@ class GStreamerBackend(Backend): class AudioToolsBackend(Backend): - """ReplayGain backend that uses `Python Audio Tools `_ and its capabilities to read more file formats and compute ReplayGain values using it replaygain module. @@ -706,7 +700,6 @@ class AudioToolsBackend(Backend): # Main plugin logic. class ReplayGainPlugin(BeetsPlugin): - """Provides ReplayGain analysis. """ @@ -802,7 +795,6 @@ class ReplayGainPlugin(BeetsPlugin): ) self.store_album_gain(album, album_gain.album_gain) - for item, track_gain in itertools.izip(album.items(), album_gain.track_gains): self.store_track_gain(item, track_gain) diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index f08a23953..b8f385df8 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -90,7 +90,8 @@ backend in your configuration file:: replaygain: backend: bs1770gain - + + Configuration ------------- @@ -118,11 +119,10 @@ These options only work with the "command" backend: This option only works with the "bs1770gain" backend: -- **method**: The loudness scanning standard: either 'replaygain' for ReplayGain 2.0, - 'ebu' for EBU R128 or 'atsc' for ATSC A/85. - This dictates the reference level: -18, -23, or -24 LUFS respectively. Default: replaygain - - +- **method**: The loudness scanning standard: either `replaygain` for + ReplayGain 2.0, `ebu` for EBU R128, or `atsc` for ATSC A/85. This dictates + the reference level: -18, -23, or -24 LUFS respectively. Default: + `replaygain` Manual Analysis From 69786b8538caa01c1cea772b1f14f10236f70fd5 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 4 Mar 2015 12:09:40 +0100 Subject: [PATCH 3/8] Fix test.helper.has_program(): encode command subprocess.subprocess.check_call() should receive a byte string command otherwise it fails with a UnicodeDecodeError on systems with non-ascii elements in the system path. Discovered while reviewing PR #1344. --- test/helper.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/helper.py b/test/helper.py index 8d0dbf8a6..bb32c1c87 100644 --- a/test/helper.py +++ b/test/helper.py @@ -51,6 +51,7 @@ from beets.library import Library, Item, Album from beets import importer from beets.autotag.hooks import AlbumInfo, TrackInfo from beets.mediafile import MediaFile, Image +from beets.ui import _encoding # TODO Move AutotagMock here from test import _common @@ -117,9 +118,13 @@ def capture_stdout(): def has_program(cmd, args=['--version']): """Returns `True` if `cmd` can be executed. """ + full_cmd = [cmd] + args + for i, elem in enumerate(full_cmd): + if isinstance(elem, unicode): + full_cmd[i] = elem.encode(_encoding()) try: with open(os.devnull, 'wb') as devnull: - subprocess.check_call([cmd] + args, stderr=devnull, + subprocess.check_call(full_cmd, stderr=devnull, stdout=devnull, stdin=devnull) except OSError: return False From f8e2ca2c944d5dd2c1d7cec46372d0098192ec1b Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 4 Mar 2015 12:15:56 +0100 Subject: [PATCH 4/8] Replaygain tests: more careful plugins unloading When plugins loading as failed plugins unloading may fail in consequence, swallowing the loading error. This fixes it. --- test/test_replaygain.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 64d65b006..2c623725c 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -42,9 +42,18 @@ class ReplayGainCliTestBase(TestHelper): try: self.load_plugins('replaygain') except: - self.teardown_beets() - self.unload_plugins() - raise + import sys + # store exception info so an error in teardown does not swallow it + exc_info = sys.exc_info() + try: + self.teardown_beets() + self.unload_plugins() + except: + # if load_plugins() failed then setup is incomplete and + # teardown operations may fail. In particular # {Item,Album} + # may not have the _original_types attribute in unload_plugins + pass + raise exc_info[1], None, exc_info[2] self.config['replaygain']['backend'] = self.backend album = self.add_album_fixture(2) From 9750351a1b6b98136a32f9d9b2b23f034479f593 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 4 Mar 2015 14:48:17 +0100 Subject: [PATCH 5/8] beets.util.command_output() & related receives bytes - May fail with unicode, esp. will non-ascii system path entry. - Only send it bytes. - Also applies to subprocess.Popen() & co. --- beets/util/__init__.py | 6 +++--- beets/util/artresizer.py | 6 +++--- beetsplug/embedart.py | 6 +++--- beetsplug/replaygain.py | 18 +++++++++--------- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 2d6968e13..53156143f 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -624,7 +624,7 @@ def cpu_count(): num = 0 elif sys.platform == b'darwin': try: - num = int(command_output(['sysctl', '-n', 'hw.ncpu'])) + num = int(command_output([b'sysctl', b'-n', b'hw.ncpu'])) except ValueError: num = 0 else: @@ -641,8 +641,8 @@ def cpu_count(): def command_output(cmd, shell=False): """Runs the command and returns its output after it has exited. - ``cmd`` is a list of arguments starting with the command names. If - ``shell`` is true, ``cmd`` is assumed to be a string and passed to a + ``cmd`` is a list of byte string arguments starting with the command names. + If ``shell`` is true, ``cmd`` is assumed to be a string and passed to a shell to execute. If the process exits with a non-zero return code diff --git a/beets/util/artresizer.py b/beets/util/artresizer.py index b1920d8ac..bce888209 100644 --- a/beets/util/artresizer.py +++ b/beets/util/artresizer.py @@ -91,8 +91,8 @@ def im_resize(maxwidth, path_in, path_out=None): # compatibility. try: util.command_output([ - 'convert', util.syspath(path_in), - '-resize', '{0}x^>'.format(maxwidth), path_out + b'convert', util.syspath(path_in), + b'-resize', b'{0}x^>'.format(maxwidth), path_out ]) except subprocess.CalledProcessError: log.warn(u'artresizer: IM convert failed for {0}', @@ -187,7 +187,7 @@ class ArtResizer(object): # Try invoking ImageMagick's "convert". try: - out = util.command_output(['identify', '--version']) + out = util.command_output([b'identify', b'--version']) if 'imagemagick' in out.lower(): pattern = r".+ (\d+)\.(\d+)\.(\d+).*" diff --git a/beetsplug/embedart.py b/beetsplug/embedart.py index 35feb4d9c..d44095687 100644 --- a/beetsplug/embedart.py +++ b/beetsplug/embedart.py @@ -196,13 +196,13 @@ class EmbedCoverArtPlugin(BeetsPlugin): # Converting images to grayscale tends to minimize the weight # of colors in the diff score. convert_proc = subprocess.Popen( - ['convert', syspath(imagepath), syspath(art), - '-colorspace', 'gray', 'MIFF:-'], + [b'convert', syspath(imagepath), syspath(art), + b'-colorspace', b'gray', b'MIFF:-'], stdout=subprocess.PIPE, close_fds=not is_windows, ) compare_proc = subprocess.Popen( - ['compare', '-metric', 'PHASH', '-', 'null:'], + [b'compare', b'-metric', b'PHASH', b'-', b'null:'], stdin=convert_proc.stdout, stdout=subprocess.PIPE, stderr=subprocess.PIPE, diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index c4bebc8dd..41937d455 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -93,12 +93,12 @@ class Bs1770gainBackend(Backend): def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) - cmd = 'bs1770gain' + cmd = b'bs1770gain' try: - self.method = '--' + config['method'].get(unicode) + self.method = b'--' + config['method'].get(str) except: - self.method = '--replaygain' + self.method = b'--replaygain' try: call([cmd, self.method]) @@ -210,9 +210,9 @@ class CommandBackend(Backend): ) else: # Check whether the program is in $PATH. - for cmd in ('mp3gain', 'aacgain'): + for cmd in (b'mp3gain', b'aacgain'): try: - call([cmd, '-v']) + call([cmd, b'-v']) self.command = cmd except OSError: pass @@ -276,14 +276,14 @@ class CommandBackend(Backend): # tag-writing; this turns the mp3gain/aacgain tool into a gain # calculator rather than a tag manipulator because we take care # of changing tags ourselves. - cmd = [self.command, '-o', '-s', 's'] + cmd = [self.command, b'-o', b'-s', b's'] if self.noclip: # Adjust to avoid clipping. - cmd = cmd + ['-k'] + cmd = cmd + [b'-k'] else: # Disable clipping warning. - cmd = cmd + ['-c'] - cmd = cmd + ['-d', bytes(self.gain_offset)] + cmd = cmd + [b'-c'] + cmd = cmd + [b'-d', bytes(self.gain_offset)] cmd = cmd + [syspath(i.path) for i in items] self._log.debug(u'analyzing {0} files', len(items)) From 5a355201d3de084281c1e3f00f7b0e6650b46557 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Wed, 4 Mar 2015 15:29:19 +0100 Subject: [PATCH 6/8] =?UTF-8?q?test=5Freplaygain:=20fix=20except=20a,=20b:?= =?UTF-8?q?=20=E2=86=92=20except=20(a,=20b):?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/test_replaygain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 2c623725c..fe1bdfea5 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -25,7 +25,7 @@ try: import gi gi.require_version('Gst', '1.0') GST_AVAILABLE = True -except ImportError, ValueError: +except (ImportError, ValueError): GST_AVAILABLE = False if any(has_program(cmd, ['-v']) for cmd in ['mp3gain', 'aacgain']): From d1f6bbaf01269faca6b5868781c03b4e6b1e987a Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Fri, 6 Mar 2015 11:16:20 +0100 Subject: [PATCH 7/8] Discogs plugin: catch client lib ValueErrors When search fails, log the query which caused it and return an empty result. Kind of fixes #1347. --- beetsplug/discogs.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index ac0f516ff..0ea40caee 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -192,7 +192,13 @@ class DiscogsPlugin(BeetsPlugin): # Strip medium information from query, Things like "CD1" and "disk 1" # can also negate an otherwise positive result. query = re.sub(r'(?i)\b(CD|disc)\s*\d+', '', query) - releases = self.discogs_client.search(query, type='release').page(1) + try: + releases = self.discogs_client.search(query, + type='release').page(1) + except ValueError as exc: + self._log.debug("Problem whiile searching for {0!r}: " + "{1}".format(query, exc)) + return [] return [self.get_album_info(release) for release in releases[:5]] def get_album_info(self, result): From bcc591bf978e4966749d72408d0f4c0c7a1b1880 Mon Sep 17 00:00:00 2001 From: Adrian Sampson Date: Fri, 6 Mar 2015 12:01:09 -0800 Subject: [PATCH 8/8] Generalize exception handler for #1347 --- beetsplug/discogs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 0ea40caee..a7e67b922 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -195,8 +195,8 @@ class DiscogsPlugin(BeetsPlugin): try: releases = self.discogs_client.search(query, type='release').page(1) - except ValueError as exc: - self._log.debug("Problem whiile searching for {0!r}: " + except CONNECTION_ERRORS as exc: + self._log.debug("Communication error while searching for {0!r}: " "{1}".format(query, exc)) return [] return [self.get_album_info(release) for release in releases[:5]]