From 87d71abc289bd7189fae9c7e80115feacb6a1991 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Wed, 6 Mar 2013 18:21:42 -0800
Subject: [PATCH] changelog/cleanup/fixes for #209

The major functional change here is how files move around when in keep_new
mode. Now, files are first moved to the destination directory and then
copied/transcoded back into the library.

This avoids problems where naming conflicts could occur when transcoding from
MP3 to MP3 (and thus not changing the filename).
---
 beetsplug/convert.py     | 77 ++++++++++++++++++++--------------------
 docs/changelog.rst       |  3 ++
 docs/plugins/convert.rst | 10 ++++--
 3 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/beetsplug/convert.py b/beetsplug/convert.py
index 11c23223f..40b4dc4c4 100644
--- a/beetsplug/convert.py
+++ b/beetsplug/convert.py
@@ -22,7 +22,6 @@ from subprocess import Popen
 from beets.plugins import BeetsPlugin
 from beets import ui, util
 from beetsplug.embedart import _embed
-from beets import library
 from beets import config
 
 log = logging.getLogger('beets')
@@ -30,25 +29,19 @@ DEVNULL = open(os.devnull, 'wb')
 _fs_lock = threading.Lock()
 
 
-def _dest_out(lib, dest_dir, item, keep_new):
-    """Path to the files outside the directory"""
-
+def _destination(lib, dest_dir, item, keep_new):
+    """Return the path under `dest_dir` where the file should be placed
+    (possibly after conversion).
+    """
+    dest = lib.destination(item, basedir=dest_dir)
     if keep_new:
-        return os.path.join(dest_dir, lib.destination(item, fragment=True))
-
-    dest = os.path.join(dest_dir, lib.destination(item, fragment=True))
-    return os.path.splitext(dest)[0] + '.mp3'
-
-
-def _dest_converted(lib, dest_dir, item, keep_new):
-    """Path to the newly converted files"""
-
-    if keep_new:
-        dest = lib.destination(item)
+        # When we're keeping the converted file, no extension munging
+        # occurs.
+        return dest
+    else:
+        # Otherwise, replace the extension with .mp3.
         return os.path.splitext(dest)[0] + '.mp3'
 
-    return _dest_out(lib, dest_dir, item, keep_new)
-
 
 def encode(source, dest):
     log.info(u'Started encoding {0}'.format(util.displayable_path(source)))
@@ -71,11 +64,9 @@ def encode(source, dest):
 def convert_item(lib, dest_dir, keep_new):
     while True:
         item = yield
+        dest = _destination(lib, dest_dir, item, keep_new)
 
-        dest_converted = _dest_converted(lib, dest_dir, item, keep_new)
-        dest_out = _dest_out(lib, dest_dir, item, keep_new)
-
-        if os.path.exists(util.syspath(dest_out)):
+        if os.path.exists(util.syspath(dest)):
             log.info(u'Skipping {0} (target file exists)'.format(
                 util.displayable_path(item.path)
             ))
@@ -85,21 +76,36 @@ def convert_item(lib, dest_dir, keep_new):
         # time. (The existence check is not atomic with the directory
         # creation inside this function.)
         with _fs_lock:
-            util.mkdirall(dest_out)
+            util.mkdirall(dest)
+
+        # When keeping the new file in the library, we first move the
+        # current (pristine) file to the destination. We'll then copy it
+        # back to its old path or transcode it to a new path.
+        if keep_new:
+            log.info(u'Moving to {0}'.
+                     format(util.displayable_path(dest)))
+            util.move(item.path, dest)
 
         maxbr = config['convert']['max_bitrate'].get(int)
         if item.format == 'MP3' and item.bitrate < 1000 * maxbr:
+            # No transcoding necessary.
             log.info(u'Copying {0}'.format(util.displayable_path(item.path)))
-            util.copy(item.path, dest_out)
-        else:
-            encode(item.path, dest_converted)
-
             if keep_new:
-                log.info(u'Moving to destination {0}'.
-                         format(util.displayable_path(dest_out)))
-                util.move(item.path, dest_out)
+                util.copy(dest, item.path)
+            else:
+                util.copy(item.path, dest)
 
-        item.path = dest_converted
+        else:
+            if keep_new:
+                item.path = os.path.splitext(item.path)[0] + '.mp3'
+                encode(dest, item.path)
+                lib.store(item)
+            else:
+                encode(item.path, dest)
+
+        # Write tags from the database to the converted file.
+        if not keep_new:
+            item.path = dest
         item.write()
 
         if config['convert']['embed']:
@@ -109,21 +115,14 @@ def convert_item(lib, dest_dir, keep_new):
                 if artpath:
                     _embed(artpath, [item])
 
-        if keep_new:
-            item.read()
-            log.info(u'Updating new format {0}'.format(item.format))
-            item.write()
-            lib.store(item)
-
 
 def convert_func(lib, opts, args):
     dest = opts.dest if opts.dest is not None else \
-        config['convert']['dest'].get()
+            config['convert']['dest'].get()
     if not dest:
         raise ui.UserError('no convert destination set')
     threads = opts.threads if opts.threads is not None else \
-        config['convert']['threads'].get(int)
-
+            config['convert']['threads'].get(int)
     keep_new = opts.keep_new
 
     ui.commands.list_items(lib, ui.decargs(args), opts.album, None)
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 49ce4edfa..797c930da 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -22,6 +22,9 @@ Other stuff:
   track in MusicBrainz and updates your library to reflect it. This can help
   you easily correct errors that have been fixed in the MB database. Thanks to
   Jakob Schnitzer.
+* :doc:`/plugins/convert`: A new ``--keep-new`` option lets you store
+  transcoded files in your library while backing up the originals (instead of
+  vice-versa). Thanks to Lucas Duailibe.
 * :doc:`/plugins/echonest_tempo`: API errors now issue a warning instead of
   exiting with an exception. We also avoid an error when track metadata
   contains newlines.
diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst
index fd45265a6..d88fc40f6 100644
--- a/docs/plugins/convert.rst
+++ b/docs/plugins/convert.rst
@@ -26,13 +26,17 @@ Usage
 To convert a part of your collection, run ``beet convert QUERY``. This
 will display all items matching ``QUERY`` and ask you for confirmation before
 starting the conversion. The ``-a`` (or ``--album``) option causes the command
-to match albums instead of tracks. The ``-k`` (or ``--keep-new``) allows you to
-keep the new, converted, files in your library and move the origin files to the
-destination directory.
+to match albums instead of tracks.
 
 The ``-t`` (``--threads``) and ``-d`` (``--dest``) options allow you to specify
 or overwrite the respective configuration options.
 
+By default, the command places converted files into the destination directory
+and leaves your library pristine. To instead back up your original files into
+the destination directory and keep converted files in your library, use the
+``-k`` (or ``--keep-new``) option.
+
+
 Configuration
 -------------