Merge pull request #3480 from ybnd/bs1770gain-v0.6.0-parsing

Update Bs1770gainBackend to handle bs1770gain v0.6.0 XML output

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
 
@@ -66,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.
 
@@ -147,8 +153,12 @@ class Bs1770gainBackend(Backend):
 
         cmd = 'bs1770gain'
         try:
-            call([cmd, "--help"])
+            version_out = call([cmd, '--version'])
             self.command = cmd
+            self.version = re.search(
+                'bs1770gain ([0-9]+.[0-9]+.[0-9]+), ',
+                version_out.stdout.decode('utf-8')
+            ).group(1)
         except OSError:
             raise FatalReplayGainError(
                 u'Is bs1770gain installed?'
@@ -241,17 +251,23 @@ 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]
         cmd += ["--" + method]
         cmd += ['--xml', '-p']
+        if after_version(self.version, '0.6.0'):
+            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
@@ -286,6 +302,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 +311,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,6 +333,17 @@ 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
 

docs/changelog.rst

@@ -153,6 +153,8 @@ Fixes:
   :bug:`3437`
 * :doc:`/plugins/lyrics`: Fix a corner-case with Genius lowercase artist names
   :bug:`3446`
+* :doc:`/plugins/replaygain`: Support ``bs1770gain`` v0.6.0 and up
+  :bug:`3480`
 
 For plugin developers:
 

test/test_replaygain.py

@@ -58,6 +58,7 @@ def reset_replaygain(item):
 
 
 class ReplayGainCliTestBase(TestHelper):
+
     def setUp(self):
         self.setup_beets()
         self.config['replaygain']['backend'] = self.backend
@@ -150,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":
@@ -227,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:
@@ -249,7 +254,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"""
             <album>
                 <track total="1" number="1" file="&">
                     <integrated lufs="0" lu="0" />