Merge pull request #4521 from aereaux/patch-1

Fix recent bug that would not transcode when format was wrong

beetsplug/convert.py

@@ -102,10 +102,9 @@ def should_transcode(item, fmt):
             not (item.format.lower() in LOSSLESS_FORMATS):
         return False
     maxbr = config['convert']['max_bitrate'].get(Optional(int))
-    if maxbr is None:
-        return False
-    return fmt.lower() != item.format.lower() or \
-        item.bitrate >= 1000 * maxbr
+    if maxbr is not None and item.bitrate >= 1000 * maxbr:
+        return True
+    return fmt.lower() != item.format.lower()
 
 
 class ConvertPlugin(BeetsPlugin):

docs/plugins/convert.rst

@@ -88,10 +88,11 @@ file. The available options are:
 - **embed**: Embed album art in converted items. Default: ``yes``.
 - **id3v23**: Can be used to override the global ``id3v23`` option. Default:
   ``inherit``.
-- **max_bitrate**: All lossy files with a higher bitrate will be
-  transcoded and those with a lower bitrate will simply be copied. Note that
-  this does not guarantee that all converted files will have a lower
-  bitrate---that depends on the encoder and its configuration.
+- **max_bitrate**: By default, the plugin does not transcode files that are
+  already in the destination format. This option instead also transcodes files
+  with high bitrates, even if they are already in the same format as the
+  output.  Note that this does not guarantee that all converted files will have
+  a lower bitrate---that depends on the encoder and its configuration.
   Default: none.
 - **no_convert**: Does not transcode items matching provided query string
   (see :doc:`/reference/query`). (i.e. ``format:AAC, format:WMA`` or

test/test_convert.py

@@ -171,12 +171,11 @@ class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand):
         )
         self.config['convert'] = {
             'dest': self.convert_dest,
-            # Enforce running convert
-            'max_bitrate': 1,
             'paths': {'default': 'converted'},
             'format': 'mp3',
             'formats': {
                 'mp3': self.tagged_copy_cmd('mp3'),
+                'ogg': self.tagged_copy_cmd('ogg'),
                 'opus': {
                     'command': self.tagged_copy_cmd('opus'),
                     'extension': 'ops',
@@ -251,12 +250,48 @@ class ConvertCliTest(unittest.TestCase, TestHelper, ConvertCommand):
             self.run_convert('An impossible query')
         self.assertEqual(logs[0], 'convert: Empty query result.')
 
-    def test_no_transcode_when_max_bitrate_set_to_none(self):
-        self.config['convert']['max_bitrate'] = None
+    def test_no_transcode_when_maxbr_set_high_and_different_formats(self):
+        self.config['convert']['max_bitrate'] = 5000
+        with control_stdin('y'):
+            self.run_convert()
+        converted = os.path.join(self.convert_dest, b'converted.mp3')
+        self.assertFileTag(converted, 'mp3')
+
+    def test_transcode_when_maxbr_set_low_and_different_formats(self):
+        self.config['convert']['max_bitrate'] = 5
+        with control_stdin('y'):
+            self.run_convert()
+        converted = os.path.join(self.convert_dest, b'converted.mp3')
+        self.assertFileTag(converted, 'mp3')
+
+    def test_transcode_when_maxbr_set_to_none_and_different_formats(self):
+        with control_stdin('y'):
+            self.run_convert()
+        converted = os.path.join(self.convert_dest, b'converted.mp3')
+        self.assertFileTag(converted, 'mp3')
+
+    def test_no_transcode_when_maxbr_set_high_and_same_formats(self):
+        self.config['convert']['max_bitrate'] = 5000
+        self.config['convert']['format'] = 'ogg'
         with control_stdin('y'):
             self.run_convert()
         converted = os.path.join(self.convert_dest, b'converted.ogg')
-        self.assertNoFileTag(converted, 'mp3')
+        self.assertNoFileTag(converted, 'ogg')
+
+    def test_transcode_when_maxbr_set_low_and_same_formats(self):
+        self.config['convert']['max_bitrate'] = 5
+        self.config['convert']['format'] = 'ogg'
+        with control_stdin('y'):
+            self.run_convert()
+        converted = os.path.join(self.convert_dest, b'converted.ogg')
+        self.assertFileTag(converted, 'ogg')
+
+    def test_transcode_when_maxbr_set_to_none_and_same_formats(self):
+        self.config['convert']['format'] = 'ogg'
+        with control_stdin('y'):
+            self.run_convert()
+        converted = os.path.join(self.convert_dest, b'converted.ogg')
+        self.assertNoFileTag(converted, 'ogg')
 
 
 @_common.slow_test()
@@ -272,8 +307,6 @@ class NeverConvertLossyFilesTest(unittest.TestCase, TestHelper,
         self.convert_dest = os.path.join(self.temp_dir, b'convert_dest')
         self.config['convert'] = {
             'dest': self.convert_dest,
-            # Enforce running convert
-            'max_bitrate': 1,
             'paths': {'default': 'converted'},
             'never_convert_lossy_files': True,
             'format': 'mp3',