convert: Add --format option

This option allows the user to specify the format on the command line
instead of editing the configuration.

The commit also includes some refactoring. In particular adding
arguments to functions to avoid dependence on global state.

Doc and Changelog in next commit

beetsplug/convert.py

@@ -37,26 +37,22 @@ ALIASES = {
 }
 
 
-def _destination(dest_dir, item, keep_new, path_formats):
+def replace_ext(path, ext):
-    """Return the path under `dest_dir` where the file should be placed
+    """Return the path with its extension replaced by `ext`.
-    (possibly after conversion).
+
+    The new extension must not contain a leading dot.
     """
-    dest = item.destination(basedir=dest_dir, path_formats=path_formats)
+    return os.path.splitext(path)[0] + '.' + ext
-    if keep_new:
-        # When we're keeping the converted file, no extension munging
-        # occurs.
-        return dest
-    else:
-        # Otherwise, replace the extension.
-        _, ext = get_format()
-        return os.path.splitext(dest)[0] + ext
 
 
-def get_format():
+def get_format(format=None):
-    """Get the currently configured format command and extension.
+    """Return the command tempate and the extension from the config.
     """
-    format = config['convert']['format'].get(unicode).lower()
+    if not format:
+        format = config['convert']['format'].get(unicode).lower()
     format = ALIASES.get(format, format)
+    # TODO extension may default to format so this doesn't have to be a
+    # dictionary
     format_info = config['convert']['formats'][format].get(dict)
 
     # Convenience and backwards-compatibility shortcuts.
@@ -74,7 +70,7 @@ def get_format():
     try:
         return (
             format_info['command'].encode('utf8'),
-            (u'.' + format_info['extension']).encode('utf8'),
+            format_info['extension'].encode('utf8'),
         )
     except KeyError:
         raise ui.UserError(
@@ -83,11 +79,10 @@ def get_format():
         )
 
 
-def encode(source, dest):
+def encode(command, source, dest):
-    """Encode ``source`` to ``dest`` using the command from ``get_format()``.
+    """Encode `source` to `dest` using command template `command`.
 
-    Raises an ``ui.UserError`` if the command was not found and a
+    Raises `subprocess.CalledProcessError` if the command exited with a
-    ``subprocess.CalledProcessError`` if the command exited with a
     non-zero status code.
     """
     quiet = config['convert']['quiet'].get()
@@ -95,7 +90,6 @@ def encode(source, dest):
     if not quiet:
         log.info(u'Started encoding {0}'.format(util.displayable_path(source)))
 
-    command, _ = get_format()
     command = Template(command).safe_substitute({
         'source': pipes.quote(source),
         'dest':   pipes.quote(dest),
@@ -115,7 +109,7 @@ def encode(source, dest):
         raise
     except OSError as exc:
         raise ui.UserError(
-            u'convert: could invoke ffmpeg: {0}'.format(exc)
+            u"convert: could invoke '{0}': {0}".format(command, exc)
         )
 
     if not quiet:
@@ -134,16 +128,21 @@ def should_transcode(item):
         item.bitrate >= 1000 * maxbr
 
 
-def convert_item(dest_dir, keep_new, path_formats):
+def convert_item(dest_dir, keep_new, path_formats, command, ext):
     while True:
         item = yield
-        dest = _destination(dest_dir, item, keep_new, path_formats)
+        dest = item.destination(basedir=dest_dir, path_formats=path_formats)
 
-        if os.path.exists(util.syspath(dest)):
+        # When keeping the new file in the library, we first move the
-            log.info(u'Skipping {0} (target file exists)'.format(
+        # current (pristine) file to the destination. We'll then copy it
-                util.displayable_path(item.path)
+        # back to its old path or transcode it to a new path.
-            ))
+        if keep_new:
-            continue
+            original = dest
+            converted = replace_ext(item.path, ext)
+        else:
+            original = item.path
+            dest = replace_ext(dest, ext)
+            converted = dest
 
         # Ensure that only one thread tries to create directories at a
         # time. (The existence check is not atomic with the directory
@@ -151,19 +150,16 @@ def convert_item(dest_dir, keep_new, path_formats):
         with _fs_lock:
             util.mkdirall(dest)
 
-        # When keeping the new file in the library, we first move the
+        if os.path.exists(util.syspath(dest)):
-        # current (pristine) file to the destination. We'll then copy it
+            log.info(u'Skipping {0} (target file exists)'.format(
-        # back to its old path or transcode it to a new path.
+                util.displayable_path(item.path)
+            ))
+            continue
+
         if keep_new:
             log.info(u'Moving to {0}'.
-                     format(util.displayable_path(dest)))
+                     format(util.displayable_path(original)))
-            util.move(item.path, dest)
+            util.move(item.path, original)
-            original = dest
-            _, ext = get_format()
-            converted = os.path.splitext(item.path)[0] + ext
-        else:
-            original = item.path
-            converted = dest
 
         if not should_transcode(item):
             # No transcoding necessary.
@@ -171,7 +167,7 @@ def convert_item(dest_dir, keep_new, path_formats):
             util.copy(original, converted)
         else:
             try:
-                encode(original, converted)
+                encode(command, original, converted)
             except subprocess.CalledProcessError:
                 continue
 
@@ -198,12 +194,12 @@ def convert_on_import(lib, item):
     library.
     """
     if should_transcode(item):
-        _, ext = get_format()
+        command, ext = get_format()
         fd, dest = tempfile.mkstemp(ext)
         os.close(fd)
         _temp_files.append(dest)  # Delete the transcode later.
         try:
-            encode(item.path, dest)
+            encode(command, item.path, dest)
         except subprocess.CalledProcessError:
             return
         item.path = dest
@@ -213,21 +209,24 @@ def convert_on_import(lib, item):
 
 
 def convert_func(lib, opts, args):
-    dest = opts.dest if opts.dest is not None else \
+    if not opts.dest:
-        config['convert']['dest'].get()
+        opts.dest = config['convert']['dest'].get()
-
+    if not opts.dest:
-    if not dest:
         raise ui.UserError('no convert destination set')
+    opts.dest = util.bytestring_path(opts.dest)
 
-    dest = util.bytestring_path(dest)
+    if not opts.threads:
-    threads = opts.threads if opts.threads is not None else \
+        opts.threads = config['convert']['threads'].get(int)
-        config['convert']['threads'].get(int)
-    keep_new = opts.keep_new
 
-    if not config['convert']['paths']:
+    if config['convert']['paths']:
-        path_formats = ui.get_path_formats()
-    else:
         path_formats = ui.get_path_formats(config['convert']['paths'])
+    else:
+        path_formats = ui.get_path_formats()
+
+    if not opts.format:
+        opts.format = config['convert']['format'].get(unicode).lower()
+
+    command, ext = get_format(opts.format)
 
     ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
 
@@ -238,9 +237,12 @@ def convert_func(lib, opts, args):
         items = (i for a in lib.albums(ui.decargs(args)) for i in a.items())
     else:
         items = iter(lib.items(ui.decargs(args)))
-    convert = [convert_item(dest, keep_new, path_formats)
+    convert_stages = []
-               for i in range(threads)]
+    for i in range(opts.threads):
-    pipe = util.pipeline.Pipeline([items, convert])
+        convert_stages.append(
+            convert_item(opts.dest, opts.keep_new, path_formats, command, ext)
+        )
+    pipe = util.pipeline.Pipeline([items, convert_stages])
     pipe.run_parallel()
 
 
@@ -305,6 +307,8 @@ class ConvertPlugin(BeetsPlugin):
                               and move the old files')
         cmd.parser.add_option('-d', '--dest', action='store',
                               help='set the destination directory')
+        cmd.parser.add_option('-f', '--format', action='store', dest='format',
+                              help='set the destination directory')
         cmd.func = convert_func
         return [cmd]
 

test/test_convert.py

@@ -72,9 +72,21 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
         self.load_plugins('convert')
 
         self.convert_dest = os.path.join(self.temp_dir, 'convert_dest')
-        self.config['convert']['dest'] = str(self.convert_dest)
+        self.config['convert'] = {
-        self.config['convert']['command'] = u'cp $source $dest'
+            'dest': self.convert_dest,
-        self.config['convert']['paths']['default'] = u'converted'
+            'paths': {'default': 'converted'},
+            'format': 'mp3',
+            'formats': {
+                'mp3': {
+                    'command': 'cp $source $dest',
+                    'extension': 'mp3',
+                },
+                'opus': {
+                    'command': 'cp $source $dest',
+                    'extension': 'opus',
+                }
+            }
+        }
 
     def tearDown(self):
         self.unload_plugins()
@@ -95,6 +107,12 @@ class ConvertCliTest(unittest.TestCase, TestHelper):
         self.item.load()
         self.assertEqual(os.path.splitext(self.item.path)[1], '.mp3')
 
+    def test_format_option(self):
+        with control_stdin('y'):
+            self.run_command('convert', '--format', 'opus', self.item.path)
+            converted = os.path.join(self.convert_dest, 'converted.opus')
+        self.assertTrue(os.path.isfile(converted))
+
     def test_embed_album_art(self):
         self.config['convert']['embed'] = True
         image_path = os.path.join(_common.RSRC, 'image-2x3.jpg')