From 53820c0a9843ce613feeb2d9896739f0d3cbf369 Mon Sep 17 00:00:00 2001 From: ybnd Date: Tue, 28 Jan 2020 09:45:20 +0100 Subject: [PATCH 1/9] Handle bs1770gain v0.6.0 XML output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove `0%\x08\x08` from output (backspace code doesn't resolve; progress percentages get spliced in) * Handle changed attributes/fields: * `sample-peak` attribute `factor` is called `amplitude` instead * Album summary is not included in a `summary` tag now, but in two separate `integrated` and `sample-peak` tags * Handle `lu` attribute * Get bs1770gain version * If v0.6.0 or later, add `--unit=ebu` flag to convert `db` attributes to LUFS * May be useful later on ### Output examples Track: ``` ``` Album: ``` ``` --- beetsplug/replaygain.py | 32 ++++++++++++++++++++++++++++++-- test/test_replaygain.py | 3 +-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 1076ac714..3e284ce62 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -22,6 +22,7 @@ import math import sys import warnings import enum +import re import xml.parsers.expat from six.moves import zip @@ -135,6 +136,11 @@ class Bs1770gainBackend(Backend): -18: "replaygain", } + version = re.search( + 'bs1770gain ([0-9]+.[0-9]+.[0-9]+), ', + call(['bs1770gain', '--version']).stdout.decode('utf-8') + ).group(1) + def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) config.add({ @@ -252,6 +258,8 @@ class Bs1770gainBackend(Backend): cmd = [self.command] cmd += ["--" + method] cmd += ['--xml', '-p'] + if self.version >= '0.6.0': + cmd += ['--unit=ebu'] # set units to LU # Workaround for Windows: the underlying tool fails on paths # with the \\?\ prefix, so we don't use it here. This @@ -286,6 +294,7 @@ class Bs1770gainBackend(Backend): album_gain = {} # mutable variable so it can be set from handlers parser = xml.parsers.expat.ParserCreate(encoding='utf-8') state = {'file': None, 'gain': None, 'peak': None} + album_state = {'gain': None, 'peak': None} def start_element_handler(name, attrs): if name == u'track': @@ -294,9 +303,13 @@ class Bs1770gainBackend(Backend): raise ReplayGainError( u'duplicate filename in bs1770gain output') elif name == u'integrated': - state['gain'] = float(attrs[u'lu']) + if 'lu' in attrs: + state['gain'] = float(attrs[u'lu']) elif name == u'sample-peak': - state['peak'] = float(attrs[u'factor']) + if 'factor' in attrs: + state['peak'] = float(attrs[u'factor']) + elif 'amplitude' in attrs: + state['peak'] = float(attrs[u'amplitude']) def end_element_handler(name): if name == u'track': @@ -312,10 +325,25 @@ class Bs1770gainBackend(Backend): 'the output of bs1770gain') album_gain["album"] = Gain(state['gain'], state['peak']) state['gain'] = state['peak'] = None + elif len(per_file_gain) == len(path_list): + if state['gain'] is not None: + album_state['gain'] = state['gain'] + if state['peak'] is not None: + album_state['peak'] = state['peak'] + if album_state['gain'] is not None \ + and album_state['peak'] is not None: + album_gain["album"] = Gain( + album_state['gain'], album_state['peak']) + state['gain'] = state['peak'] = None + parser.StartElementHandler = start_element_handler parser.EndElementHandler = end_element_handler try: + if type(text) == bytes: + text = text.decode('utf-8') + while '\x08' in text: + text = re.sub('[^\x08]\x08', '', text) parser.Parse(text, True) except xml.parsers.expat.ExpatError: raise ReplayGainError( diff --git a/test/test_replaygain.py b/test/test_replaygain.py index fe0515bee..437b1426a 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -40,7 +40,7 @@ if any(has_program(cmd, ['-v']) for cmd in ['mp3gain', 'aacgain']): else: GAIN_PROG_AVAILABLE = False -if has_program('bs1770gain', ['--replaygain']): +if has_program('bs1770gain'): LOUDNESS_PROG_AVAILABLE = True else: LOUDNESS_PROG_AVAILABLE = False @@ -58,7 +58,6 @@ def reset_replaygain(item): class ReplayGainCliTestBase(TestHelper): - def setUp(self): self.setup_beets() self.config['replaygain']['backend'] = self.backend From c78afb1a97bb41788197ec1f7f3965e2e6a4f61b Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 16:37:56 +0100 Subject: [PATCH 2/9] Don't call bs1770gain outside of try statement --- beetsplug/replaygain.py | 12 +++++++----- test/test_replaygain.py | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 3e284ce62..5b715191e 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -136,11 +136,6 @@ class Bs1770gainBackend(Backend): -18: "replaygain", } - version = re.search( - 'bs1770gain ([0-9]+.[0-9]+.[0-9]+), ', - call(['bs1770gain', '--version']).stdout.decode('utf-8') - ).group(1) - def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) config.add({ @@ -155,6 +150,13 @@ class Bs1770gainBackend(Backend): try: call([cmd, "--help"]) self.command = cmd + try: + self.version = re.search( + '([0-9]+.[0-9]+.[0-9]+), ', + call([cmd, '--version']).stdout.decode('utf-8') + ).group(1) + except AttributeError: + self.version = '0.0.0' except OSError: raise FatalReplayGainError( u'Is bs1770gain installed?' diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 437b1426a..fe0515bee 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -40,7 +40,7 @@ if any(has_program(cmd, ['-v']) for cmd in ['mp3gain', 'aacgain']): else: GAIN_PROG_AVAILABLE = False -if has_program('bs1770gain'): +if has_program('bs1770gain', ['--replaygain']): LOUDNESS_PROG_AVAILABLE = True else: LOUDNESS_PROG_AVAILABLE = False @@ -58,6 +58,7 @@ def reset_replaygain(item): class ReplayGainCliTestBase(TestHelper): + def setUp(self): self.setup_beets() self.config['replaygain']['backend'] = self.backend From c1cb78c908b39d506aa6ea9384c1eb98938217cd Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 17:59:57 +0100 Subject: [PATCH 3/9] Small fixes in `replaygain.Bs1770gainBackend` and test_replaygain.py * Fix unspecified `gain_adjustment` when method defined in config * Fix difference between dB and LUFS values in case of mismatched `target_level`/`method`: ``` db_to_lufs( target_level ) - lufs_to_dB( -23 ) ``` * Ignore single assertion in case of bs1770gain (cherry picked from commit 2395bf224032c44f1ea5d28e0c63af96a92b96df) --- beetsplug/replaygain.py | 7 +++++-- test/test_replaygain.py | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 5b715191e..b86ef4b1e 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -249,12 +249,15 @@ class Bs1770gainBackend(Backend): if self.__method != "": # backward compatibility to `method` option method = self.__method + gain_adjustment = target_level \ + - [k for k, v in self.methods.items() if v == method][0] elif target_level in self.methods: method = self.methods[target_level] gain_adjustment = 0 else: - method = self.methods[-23] - gain_adjustment = target_level - lufs_to_db(-23) + lufs_target = -23 + method = self.methods[lufs_target] + gain_adjustment = target_level - lufs_target # Construct shell command. cmd = [self.command] diff --git a/test/test_replaygain.py b/test/test_replaygain.py index fe0515bee..3f317aeb3 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -151,7 +151,9 @@ class ReplayGainCliTestBase(TestHelper): self.assertEqual(max(gains), min(gains)) self.assertNotEqual(max(gains), 0.0) - self.assertNotEqual(max(peaks), 0.0) + if not self.backend == "bs1770gain": + # Actually produces peaks == 0.0 ~ self.add_album_fixture + self.assertNotEqual(max(peaks), 0.0) def test_cli_writes_only_r128_tags(self): if self.backend == "command": From c3817a4c06ea2e029a9d6a07a5f0e1facd6779b1 Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 18:13:59 +0100 Subject: [PATCH 4/9] Implement review comments * safer version comparison * regex bytes directly * handle b'\x08 ...' case * test_replaygain.py: injected command output should match the type of the actual output --- beetsplug/replaygain.py | 37 ++++++++++++++++++++++++------------- test/test_replaygain.py | 4 ++-- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index b86ef4b1e..661fcc2e3 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -25,6 +25,7 @@ import enum import re import xml.parsers.expat from six.moves import zip +from packaging import version from beets import ui from beets.plugins import BeetsPlugin @@ -148,19 +149,18 @@ class Bs1770gainBackend(Backend): cmd = 'bs1770gain' try: - call([cmd, "--help"]) + version_out = call([cmd, '--version']) self.command = cmd - try: - self.version = re.search( - '([0-9]+.[0-9]+.[0-9]+), ', - call([cmd, '--version']).stdout.decode('utf-8') - ).group(1) - except AttributeError: - self.version = '0.0.0' + self.version = re.search( + '([0-9]+.[0-9]+.[0-9]+), ', + version_out.stdout.decode('utf-8') + ).group(1) except OSError: raise FatalReplayGainError( u'Is bs1770gain installed?' ) + except AttributeError: + self.version = '0.0.0' if not self.command: raise FatalReplayGainError( u'no replaygain command found: install bs1770gain' @@ -263,7 +263,7 @@ class Bs1770gainBackend(Backend): cmd = [self.command] cmd += ["--" + method] cmd += ['--xml', '-p'] - if self.version >= '0.6.0': + if version.parse(self.version) >= version.parse('0.6.0'): cmd += ['--unit=ebu'] # set units to LU # Workaround for Windows: the underlying tool fails on paths @@ -345,10 +345,21 @@ class Bs1770gainBackend(Backend): parser.EndElementHandler = end_element_handler try: - if type(text) == bytes: - text = text.decode('utf-8') - while '\x08' in text: - text = re.sub('[^\x08]\x08', '', text) + # Sometimes, the XML out put of `bs1770gain` gets spliced with + # some progress percentages: b'9%\x08\x0810%\x08\x08' + # that are supposed to be canceled out by appending + # a b'\x08' backspace characters for every character, + # + # For some reason, these backspace characters don't get + # resolved, resulting in mangled XML. + + # While there are backspace characters in the output + while b'\x08' in text: + text = re.sub(b'[^\x08]\x08|^\x08', b'', text) + # Replace every occurence of a non-backspace character + # followed by a backspace character or a backspace character + # at the beginning of the string by an empty byte string b'' + parser.Parse(text, True) except xml.parsers.expat.ExpatError: raise ReplayGainError( diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 3f317aeb3..3ac19f8f9 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -40,7 +40,7 @@ if any(has_program(cmd, ['-v']) for cmd in ['mp3gain', 'aacgain']): else: GAIN_PROG_AVAILABLE = False -if has_program('bs1770gain', ['--replaygain']): +if has_program('bs1770gain'): LOUDNESS_PROG_AVAILABLE = True else: LOUDNESS_PROG_AVAILABLE = False @@ -252,7 +252,7 @@ class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase): @patch('beetsplug.replaygain.call') def test_malformed_output(self, call_patch): # Return malformed XML (the ampersand should be &) - call_patch.return_value = CommandOutput(stdout=""" + call_patch.return_value = CommandOutput(stdout=b""" From 506be0259789c552f346d618d1774847e57ce0cf Mon Sep 17 00:00:00 2001 From: ybnd Date: Thu, 30 Jan 2020 20:11:09 +0100 Subject: [PATCH 5/9] Remove `packaging` dependency --- beetsplug/replaygain.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 661fcc2e3..950969547 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -25,7 +25,6 @@ import enum import re import xml.parsers.expat from six.moves import zip -from packaging import version from beets import ui from beets.plugins import BeetsPlugin @@ -68,6 +67,11 @@ def call(args, **kwargs): raise ReplayGainError(u"argument encoding failed") +def after_version(version_a, version_b): + return tuple(int(s) for s in version_a.split('.')) \ + >= tuple(int(s) for s in version_b.split('.')) + + def db_to_lufs(db): """Convert db to LUFS. @@ -263,7 +267,7 @@ class Bs1770gainBackend(Backend): cmd = [self.command] cmd += ["--" + method] cmd += ['--xml', '-p'] - if version.parse(self.version) >= version.parse('0.6.0'): + if after_version(self.version, '0.6.0'): cmd += ['--unit=ebu'] # set units to LU # Workaround for Windows: the underlying tool fails on paths From bef473c8e85b375a2dc5a40feb82bc971286e550 Mon Sep 17 00:00:00 2001 From: ybnd Date: Fri, 31 Jan 2020 07:42:50 +0100 Subject: [PATCH 6/9] Remove spliced progress regex and add --suppress-progress flag --- beetsplug/replaygain.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 950969547..3c652c7bd 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -268,7 +268,8 @@ class Bs1770gainBackend(Backend): cmd += ["--" + method] cmd += ['--xml', '-p'] if after_version(self.version, '0.6.0'): - cmd += ['--unit=ebu'] # set units to LU + cmd += ['--unit=ebu'] # set units to LU + cmd += ['--suppress-progress'] # don't print % to XML output # Workaround for Windows: the underlying tool fails on paths # with the \\?\ prefix, so we don't use it here. This @@ -349,21 +350,6 @@ class Bs1770gainBackend(Backend): parser.EndElementHandler = end_element_handler try: - # Sometimes, the XML out put of `bs1770gain` gets spliced with - # some progress percentages: b'9%\x08\x0810%\x08\x08' - # that are supposed to be canceled out by appending - # a b'\x08' backspace characters for every character, - # - # For some reason, these backspace characters don't get - # resolved, resulting in mangled XML. - - # While there are backspace characters in the output - while b'\x08' in text: - text = re.sub(b'[^\x08]\x08|^\x08', b'', text) - # Replace every occurence of a non-backspace character - # followed by a backspace character or a backspace character - # at the beginning of the string by an empty byte string b'' - parser.Parse(text, True) except xml.parsers.expat.ExpatError: raise ReplayGainError( From 9465e933ba4d24dd409fe4de894da71808113179 Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 5 Feb 2020 08:36:44 +0100 Subject: [PATCH 7/9] Add to changelog --- docs/changelog.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 95fde6888..ffbcf3b97 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -151,6 +151,8 @@ Fixes: * :doc:`/plugins/bpd`: Fix the transition to next track when in consume mode. Thanks to :user:`aereaux`. :bug:`3437` +* :doc:`/plugins/replaygain`: Support ``bs1770gain`` v0.6.0 and up + :bug:`3480` For plugin developers: From 7005691410549c65c680c66bd852a38ad725c981 Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 5 Feb 2020 08:52:50 +0100 Subject: [PATCH 8/9] Add comment to clarify unexpected AttributeError handling --- beetsplug/replaygain.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 3c652c7bd..f5a4a2c55 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -164,6 +164,9 @@ class Bs1770gainBackend(Backend): u'Is bs1770gain installed?' ) except AttributeError: + # Raised by ReplayGainLdnsCliMalformedTest.test_malformed_output + # in test_replaygain.py; bs1770gain backend runs even though + # the bs1770gain command is not found test.helper.has_program self.version = '0.0.0' if not self.command: raise FatalReplayGainError( From 63ea17365a5dc75a9decbba3899bb4975df3989c Mon Sep 17 00:00:00 2001 From: ybnd Date: Wed, 5 Feb 2020 09:03:45 +0100 Subject: [PATCH 9/9] Modify patched stdout in test_malformed_output --- beetsplug/replaygain.py | 7 +------ test/test_replaygain.py | 4 +++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index f5a4a2c55..646e3acce 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -156,18 +156,13 @@ class Bs1770gainBackend(Backend): version_out = call([cmd, '--version']) self.command = cmd self.version = re.search( - '([0-9]+.[0-9]+.[0-9]+), ', + 'bs1770gain ([0-9]+.[0-9]+.[0-9]+), ', version_out.stdout.decode('utf-8') ).group(1) except OSError: raise FatalReplayGainError( u'Is bs1770gain installed?' ) - except AttributeError: - # Raised by ReplayGainLdnsCliMalformedTest.test_malformed_output - # in test_replaygain.py; bs1770gain backend runs even though - # the bs1770gain command is not found test.helper.has_program - self.version = '0.0.0' if not self.command: raise FatalReplayGainError( u'no replaygain command found: install bs1770gain' diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 3ac19f8f9..969f5c230 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -230,7 +230,9 @@ class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase): # Patch call to return nothing, bypassing the bs1770gain installation # check. - call_patch.return_value = CommandOutput(stdout=b"", stderr=b"") + call_patch.return_value = CommandOutput( + stdout=b'bs1770gain 0.0.0, ', stderr=b'' + ) try: self.load_plugins('replaygain') except Exception: