From 0f2a9bdcdc979a5b40b173f6b4a4c94815651272 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 31 Jul 2014 11:09:16 +0200 Subject: [PATCH 1/5] Record singletons for incremental import We still need to implement this for flat imports, archives and toppath singletons. Fixes #860. --- beets/importer.py | 24 ++++++++++++++---------- test/helper.py | 25 ++++++++++++++++++------- test/test_importer.py | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 558dcc03c..12c9c6336 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -638,10 +638,6 @@ class SingletonImportTask(ImportTask): def imported_items(self): return [self.item] - def save_history(self): - # TODO we should also save history for singletons - pass - def apply_metadata(self): autotag.apply_item_metadata(self.item, self.match.info) @@ -894,18 +890,26 @@ def read_tasks(session): continue # When incremental, skip paths in the history. - if session.config['incremental'] \ - and tuple(paths) in history_dirs: - log.debug(u'Skipping previously-imported path: %s' % - displayable_path(paths)) + if session.config['incremental'] and tuple(paths) in history_dirs: + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(paths))) incremental_skipped += 1 continue # Yield all the necessary tasks. if session.config['singletons']: for item in items: - if not (resuming and progress_element(toppath, item.path)): - yield SingletonImportTask(toppath, item) + # TODO Abstract all the progress and incremental + # stuff away! + if resuming and progress_element(toppath, item.path): + continue + if session.config['incremental'] \ + and (item.path,) in history_dirs: + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(paths))) + incremental_skipped += 1 + continue + yield SingletonImportTask(toppath, item) yield SentinelImportTask(toppath, paths) else: yield ImportTask(toppath, paths, items) diff --git a/test/helper.py b/test/helper.py index aa9c92add..4bcab88ad 100644 --- a/test/helper.py +++ b/test/helper.py @@ -180,24 +180,35 @@ class TestHelper(object): beets.plugins._instances = {} def create_importer(self, item_count=1, album_count=1): - """Returns import session with fixtures. + """Create files to import and return corresponding session. Copies the specified number of files to a subdirectory of - ``self.temp_dir`` and creates a ``TestImportSession`` for this - path. + `self.temp_dir` and creates a `TestImportSession` for this path. """ import_dir = os.path.join(self.temp_dir, 'import') if not os.path.isdir(import_dir): os.mkdir(import_dir) - for i in range(album_count): - album = u'album {0}'.format(i) + album_no = 0 + while album_count: + album = u'album {0}'.format(album_no) album_dir = os.path.join(import_dir, album) + if os.path.exists(album_dir): + album_no += 1 + continue os.mkdir(album_dir) - for j in range(item_count): - title = 'track {0}'.format(j) + album_count -= 1 + + track_no = 0 + album_item_count = item_count + while album_item_count: + title = 'track {0}'.format(track_no) src = os.path.join(_common.RSRC, 'full.mp3') dest = os.path.join(album_dir, '{0}.mp3'.format(title)) + if os.path.exists(dest): + track_no += 1 + continue + album_item_count -= 1 shutil.copy(src, dest) mediafile = MediaFile(dest) mediafile.update({ diff --git a/test/test_importer.py b/test/test_importer.py index 4f99e5157..80a94833a 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1159,6 +1159,45 @@ class ResumeImportTest(unittest.TestCase, TestHelper): self.assertIsNotNone(self.lib.items('title:track 1').get()) +class IncrementalImportTest(unittest.TestCase, TestHelper): + + def setUp(self): + self.setup_beets() + self.config['import']['incremental'] = True + + def tearDown(self): + self.teardown_beets() + + def test_incremental_album(self): + importer = self.create_importer(album_count=1) + importer.run() + + # Change album name so the original file would be imported again + # if incremental was off. + album = self.lib.albums().get() + album['album'] = 'edited album' + album.store() + + importer = self.create_importer(album_count=1) + importer.run() + self.assertEqual(len(self.lib.albums()), 2) + + def test_incremental_item(self): + self.config['import']['singletons'] = True + importer = self.create_importer(item_count=1) + importer.run() + + # Change track name so the original file would be imported again + # if incremental was off. + item = self.lib.items().get() + item['artist'] = 'edited artist' + item.store() + + importer = self.create_importer(item_count=1) + importer.run() + self.assertEqual(len(self.lib.items()), 2) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) From 1eb62bcd7287936e1e175fa283f08d75868e0c62 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Thu, 31 Jul 2014 11:48:35 +0200 Subject: [PATCH 2/5] Handle incremental and resumed imports in session --- beets/importer.py | 108 +++++++++++++++++++++++++++++----------------- 1 file changed, 69 insertions(+), 39 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 12c9c6336..060a188c9 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -179,6 +179,7 @@ class ImportSession(object): self.paths = paths self.query = query self.seen_idents = set() + self._is_resuming = dict() # Normalize the paths. if self.paths: @@ -294,6 +295,50 @@ class ImportSession(object): # User aborted operation. Silently stop. pass + # Incremental and resumed imports + + def already_imported(self, toppath, paths): + """Returns true if the files belonging to this task have already + been imported in a previous session. + """ + if self.is_resuming(toppath) \ + and all(map(lambda p: progress_element(toppath, p), paths)): + return True + if self.config['incremental'] \ + and tuple(paths) in self.history_dirs: + return True + + return False + + @property + def history_dirs(self): + if not hasattr(self, '_history_dirs'): + self._history_dirs = history_get() + return self._history_dirs + + def is_resuming(self, toppath): + """Return `True` if user wants to resume import of this path. + + You have to call `ask_resume` first to determine the return value. + """ + return self._is_resuming.get(toppath, False) + + def ask_resume(self, toppath): + """If import of `toppath` was aborted in an earlier session, ask + user if she wants to resume the import. + + Determines the return value of `is_resuming(toppath)`. + """ + if self.want_resume and has_progress(toppath): + # Either accept immediately or prompt for input to decide. + if self.want_resume is True or \ + self.should_resume(toppath): + log.warn('Resuming interrupted import of %s' % toppath) + self._is_resuming[toppath] = True + else: + # Clear progress; we're starting from the top. + progress_reset(toppath) + # The importer task class. @@ -815,24 +860,11 @@ def read_tasks(session): in the user-specified list of paths. In the case of a singleton import, yields single-item tasks instead. """ - # Look for saved incremental directories. - if session.config['incremental']: - incremental_skipped = 0 - history_dirs = history_get() - + skipped = 0 for toppath in session.paths: # Determine if we want to resume import of the toppath - resuming = False - if session.want_resume and has_progress(toppath): - # Either accept immediately or prompt for input to decide. - if session.want_resume is True or \ - session.should_resume(toppath): - log.warn('Resuming interrupted import of %s' % toppath) - resuming = True - else: - # Clear progress; we're starting from the top. - progress_reset(toppath) + session.ask_resume(toppath) # Extract archives. archive_task = None @@ -856,7 +888,13 @@ def read_tasks(session): # Check whether the path is to a file. if not os.path.isdir(syspath(toppath)): - if resuming and progress_element(toppath, toppath): + # FIXME remove duplicate code. We could put the debug + # statement into `session.alread_imported` but I don't feel + # comfortable triggering an action in a query. + if session.already_imported(toppath, toppath): + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(toppath))) + skipped += 1 continue try: @@ -878,40 +916,33 @@ def read_tasks(session): for _, items in autotag.albums_in_dir(toppath): all_items += items if all_items: + if session.already_imported(toppath, [toppath]): + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(toppath))) + skipped += 1 + continue yield ImportTask(toppath, [toppath], all_items) yield SentinelImportTask(toppath) continue # Produce paths under this directory. for paths, items in autotag.albums_in_dir(toppath): - # Skip according to progress. - if resuming \ - and all(map(lambda p: progress_element(toppath, p), paths)): - continue - - # When incremental, skip paths in the history. - if session.config['incremental'] and tuple(paths) in history_dirs: - log.debug(u'Skipping previously-imported path: {0}' - .format(displayable_path(paths))) - incremental_skipped += 1 - continue - - # Yield all the necessary tasks. if session.config['singletons']: for item in items: - # TODO Abstract all the progress and incremental - # stuff away! - if resuming and progress_element(toppath, item.path): - continue - if session.config['incremental'] \ - and (item.path,) in history_dirs: + if session.already_imported(toppath, [item.path]): log.debug(u'Skipping previously-imported path: {0}' .format(displayable_path(paths))) - incremental_skipped += 1 + skipped += 1 continue yield SingletonImportTask(toppath, item) yield SentinelImportTask(toppath, paths) + else: + if session.already_imported(toppath, paths): + log.debug(u'Skipping previously-imported path: {0}' + .format(displayable_path(paths))) + skipped += 1 + continue yield ImportTask(toppath, paths, items) # Indicate the directory is finished. @@ -922,9 +953,8 @@ def read_tasks(session): yield archive_task # Show skipped directories. - if session.config['incremental'] and incremental_skipped: - log.info(u'Incremental import: skipped %i directories.' % - incremental_skipped) + if skipped: + log.info(u'Skipped {0} directories.'.format(skipped)) def query_tasks(session): From b27409684e113d5c8f02b5ac4936f17dcf9174e5 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Tue, 5 Aug 2014 10:45:32 +0200 Subject: [PATCH 3/5] 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 | 118 ++++++++++++++++++++++--------------------- test/test_convert.py | 24 +++++++-- 2 files changed, 82 insertions(+), 60 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 9892cc001..37395db1a 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -37,26 +37,22 @@ ALIASES = { } -def _destination(dest_dir, item, keep_new, path_formats): - """Return the path under `dest_dir` where the file should be placed - (possibly after conversion). +def replace_ext(path, ext): + """Return the path with its extension replaced by `ext`. + + The new extension must not contain a leading dot. """ - dest = item.destination(basedir=dest_dir, path_formats=path_formats) - 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 + return os.path.splitext(path)[0] + '.' + ext -def get_format(): - """Get the currently configured format command and extension. +def get_format(format=None): + """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): - """Encode ``source`` to ``dest`` using the command from ``get_format()``. +def encode(command, source, dest): + """Encode `source` to `dest` using command template `command`. - Raises an ``ui.UserError`` if the command was not found and a - ``subprocess.CalledProcessError`` if the command exited with a + Raises `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)): - log.info(u'Skipping {0} (target file exists)'.format( - util.displayable_path(item.path) - )) - continue + # 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: + 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 - # current (pristine) file to the destination. We'll then copy it - # back to its old path or transcode it to a new path. + if os.path.exists(util.syspath(dest)): + log.info(u'Skipping {0} (target file exists)'.format( + util.displayable_path(item.path) + )) + continue + if keep_new: log.info(u'Moving to {0}'. - format(util.displayable_path(dest))) - util.move(item.path, dest) - original = dest - _, ext = get_format() - converted = os.path.splitext(item.path)[0] + ext - else: - original = item.path - converted = dest + format(util.displayable_path(original))) + util.move(item.path, original) 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 \ - config['convert']['dest'].get() - - if not dest: + if not opts.dest: + opts.dest = config['convert']['dest'].get() + if not opts.dest: raise ui.UserError('no convert destination set') + opts.dest = util.bytestring_path(opts.dest) - dest = util.bytestring_path(dest) - threads = opts.threads if opts.threads is not None else \ - config['convert']['threads'].get(int) - keep_new = opts.keep_new + if not opts.threads: + opts.threads = config['convert']['threads'].get(int) - if not config['convert']['paths']: - path_formats = ui.get_path_formats() - else: + if config['convert']['paths']: 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) - for i in range(threads)] - pipe = util.pipeline.Pipeline([items, convert]) + convert_stages = [] + for i in range(opts.threads): + 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] diff --git a/test/test_convert.py b/test/test_convert.py index 21de94696..ebe30ad8c 100644 --- a/test/test_convert.py +++ b/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']['command'] = u'cp $source $dest' - self.config['convert']['paths']['default'] = u'converted' + self.config['convert'] = { + 'dest': self.convert_dest, + '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') From c2822a5b90d97f0bdbf6a996b2e489b73f29ec73 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Tue, 5 Aug 2014 11:50:06 +0200 Subject: [PATCH 4/5] Documentation and changelog for b2740968 --- docs/changelog.rst | 2 + docs/plugins/convert.rst | 94 ++++++++++++++++++++-------------------- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 26e095cd0..74ce6195b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -67,6 +67,8 @@ Little improvements and fixes: import. * :doc:`/plugins/chroma`: A new ``auto`` configuration option disables fingerprinting on import. Thanks to ddettrittus. +* :doc:`/plugins/convert`: Add ``--format`` option to select the + transoding command from the command-line. 1.3.6 (May 10, 2014) diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index 5eff02f1e..bf5d62ddb 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -4,23 +4,17 @@ Convert Plugin The ``convert`` plugin lets you convert parts of your collection to a directory of your choice, transcoding audio and embedding album art along the way. It can transcode to and from any format using a configurable command -line. It will skip files that are already present in the target directory. -Converted files follow the same path formats as your library. - -.. _FFmpeg: http://ffmpeg.org +line. Installation ------------ -First, enable the ``convert`` plugin (see :doc:`/plugins/index`). +Enable the ``convert`` plugin in your configuration (see +:doc:`/plugins/index`). By default, the plugin depends on `FFmpeg`_ to +transcode the audio, so you might want to install it. -To transcode music, this plugin requires the ``ffmpeg`` command-line -tool. If its executable is in your path, it will be found automatically -by the plugin. Otherwise, configure the plugin to locate the executable:: - - convert: - ffmpeg: /usr/bin/ffmpeg +.. _FFmpeg: http://ffmpeg.org Usage @@ -28,11 +22,22 @@ 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 +starting the conversion. The command will then transcode all the +matching files to the destination directory given by the ``-d`` +(``--dest``) option or the ``dest`` configuration. The path layout +mirrors that of your library, but it may be customized through the +``paths`` configuration. + +The plugin uses a command-line program to transcode the audio. With the +``-f`` (``--format``) option you can choose the transcoding command +and customize the available commands +:ref:`through the configuration `. + +The ``-a`` (or ``--album``) option causes the command to match albums instead of tracks. -The ``-t`` (``--threads``) and ``-d`` (``--dest``) options allow you to specify -or overwrite the respective configuration options. +The ``-t`` (``--threads``) option allows you to specify or overwrite +the respective configuration option. By default, the command places converted files into the destination directory and leaves your library pristine. To instead back up your original files into @@ -48,7 +53,7 @@ The plugin offers several configuration options, all of which live under the * ``dest`` sets the directory the files will be converted (or copied) to. A destination is required---you either have to provide it in the config file - or on the command line using the ``-d`` flag. + or on the command-line using the ``-d`` flag. * ``embed`` indicates whether or not to embed album art in converted items. Default: true. * If you set ``max_bitrate``, all lossy files with a higher bitrate will be @@ -69,39 +74,14 @@ The plugin offers several configuration options, all of which live under the encoding. By default, the plugin will detect the number of processors available and use them all. -These config options control the transcoding process: +.. _convert-format-config: -* ``format`` is the name of the audio file format to transcode to. Files that - are already in the format (and are below the maximum bitrate) will not be - transcoded. The plugin includes default commands for the formats MP3, AAC, - ALAC, FLAC, Opus, Vorbis, and Windows Media; the default is MP3. If you want - to use a different format (or customize the transcoding options), use the - options below. -* ``extension`` is the filename extension to be used for newly transcoded - files. This is implied by the ``format`` option, but you can set it yourself - if you're using a different format. -* ``command`` is the command line to use to transcode audio. A default - command, usually using an FFmpeg invocation, is implied by the ``format`` - option. The tokens ``$source`` and ``$dest`` in the command are replaced - with the paths to the existing and new file. For example, the command - ``ffmpeg -i $source -y -aq 4 $dest`` transcodes to MP3 using FFmpeg at the - V4 quality level. +Configuring the transcoding command +``````````````````````````````````` -Here's an example configuration:: - - convert: - embed: false - format: aac - max_bitrate: 200 - dest: /home/user/MusicForPhone - threads: 4 - paths: - default: $albumartist/$title - -If you have several formats you want to switch between, you can list them -under the ``formats`` key and refer to them using the ``format`` option. Each -key under ``formats`` should contain values for ``command`` and ``extension`` -as described above:: +You can customize the transcoding command through the ``formats`` map +and select a command with the ``--format`` command-line option or the +``format`` configuration.:: convert: format: speex @@ -112,3 +92,25 @@ as described above:: wav: command: ffmpeg -i $source -y -acodec pcm_s16le $dest extension: wav + +In this example ``beet convert`` will use the *speex* command by +default. To convert the audio to `wav`, run ``beet convert -f wav``. + +Each entry in the ``formats`` map consists of a key (the name of the +format) as well as the command and the extension. ``extension`` is the +filename extension to be used for newly transcoded files. +``command`` is the command-line to use to transcode audio. The tokens +``$source`` and ``$dest`` in the command are replaced with the paths to +the existing and new file. + +The plugin in comes with default commands for the most common audio +formats: `mp3`, `alac`, `flac`, `aac`, `opus`, `ogg`, `wmv`. For +details have a look at the output of ``beet config -d``. + +For a one-command-fits-all solution use the ``convert.command`` and +``convert.extension`` options. If these are set the formats are ignored +and the given command is used for all conversions.:: + + convert: + command: ffmpeg -i $source -y -vn -aq 2 $dest + extension: mp3 From 29e4fde57165e7dfc1482d566934d4bd125fa7d3 Mon Sep 17 00:00:00 2001 From: Thomas Scholtes Date: Tue, 5 Aug 2014 12:06:35 +0200 Subject: [PATCH 5/5] convert: Simplify format configuration. We don't have to specify the extension. By default it is the same as the format name. --- beetsplug/convert.py | 71 ++++++++++++++++------------------------ docs/plugins/convert.rst | 16 ++++----- test/test_convert.py | 9 ++--- 3 files changed, 40 insertions(+), 56 deletions(-) diff --git a/beetsplug/convert.py b/beetsplug/convert.py index 37395db1a..8113c86ca 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -25,6 +25,7 @@ import pipes from beets import ui, util, plugins, config from beets.plugins import BeetsPlugin from beetsplug.embedart import embed_item +from beets.util.confit import ConfigTypeError log = logging.getLogger('beets') _fs_lock = threading.Lock() @@ -51,32 +52,33 @@ def get_format(format=None): 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. - keys = config['convert'].keys() - if 'command' in keys: - format_info['command'] = config['convert']['command'].get(unicode) - elif 'opts' in keys: - # Undocumented option for backwards compatibility with < 1.3.1. - format_info['command'] = u'ffmpeg -i $source -y {0} $dest'.format( - config['convert']['opts'].get(unicode) - ) - if 'extension' in keys: - format_info['extension'] = config['convert']['extension'].get(unicode) try: - return ( - format_info['command'].encode('utf8'), - format_info['extension'].encode('utf8'), - ) + format_info = config['convert']['formats'][format].get(dict) + command = format_info['command'] + extension = format_info['extension'] except KeyError: raise ui.UserError( u'convert: format {0} needs "command" and "extension" fields' .format(format) ) + except ConfigTypeError: + command = config['convert']['formats'][format].get(str) + extension = format + + # Convenience and backwards-compatibility shortcuts. + keys = config['convert'].keys() + if 'command' in keys: + command = config['convert']['command'].get(unicode) + elif 'opts' in keys: + # Undocumented option for backwards compatibility with < 1.3.1. + command = u'ffmpeg -i $source -y {0} $dest'.format( + config['convert']['opts'].get(unicode) + ) + if 'extension' in keys: + extension = config['convert']['extension'].get(unicode) + + return (command.encode('utf8'), extension.encode('utf8')) def encode(command, source, dest): @@ -263,29 +265,14 @@ class ConvertPlugin(BeetsPlugin): u'command': u'ffmpeg -i $source -y -vn -acodec alac $dest', u'extension': u'm4a', }, - u'flac': { - u'command': u'ffmpeg -i $source -y -vn -acodec flac $dest', - u'extension': u'flac', - }, - u'mp3': { - u'command': u'ffmpeg -i $source -y -vn -aq 2 $dest', - u'extension': u'mp3', - }, - u'opus': { - u'command': u'ffmpeg -i $source -y -vn -acodec libopus ' - u'-ab 96k $dest', - u'extension': u'opus', - }, - u'ogg': { - u'command': u'ffmpeg -i $source -y -vn -acodec libvorbis ' - u'-aq 2 $dest', - u'extension': u'ogg', - }, - u'windows media': { - u'command': u'ffmpeg -i $source -y -vn -acodec wmav2 ' - u'-vn $dest', - u'extension': u'wma', - }, + u'flac': u'ffmpeg -i $source -y -vn -acodec flac $dest', + u'mp3': u'ffmpeg -i $source -y -vn -aq 2 $dest', + u'opus': + u'ffmpeg -i $source -y -vn -acodec libopus -ab 96k $dest', + u'ogg': + u'ffmpeg -i $source -y -vn -acodec libvorbis -aq 2 $dest', + u'wma': + u'ffmpeg -i $source -y -vn -acodec wmav2 -vn $dest', }, u'max_bitrate': 500, u'auto': False, diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index bf5d62ddb..485c80400 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -89,19 +89,19 @@ and select a command with the ``--format`` command-line option or the speex: command: ffmpeg -i $source -y -acodec speex $dest extension: spx - wav: - command: ffmpeg -i $source -y -acodec pcm_s16le $dest - extension: wav + wav: ffmpeg -i $source -y -acodec pcm_s16le $dest In this example ``beet convert`` will use the *speex* command by default. To convert the audio to `wav`, run ``beet convert -f wav``. +This will also use the format key (`wav`) as the file extension. Each entry in the ``formats`` map consists of a key (the name of the -format) as well as the command and the extension. ``extension`` is the -filename extension to be used for newly transcoded files. -``command`` is the command-line to use to transcode audio. The tokens -``$source`` and ``$dest`` in the command are replaced with the paths to -the existing and new file. +format) as well as the command and the possibly the file extension. +``extension`` is the filename extension to be used for newly transcoded +files. If only the command is given as a string, the file extension +defaults to the format’s name. ``command`` is the command-line to use +to transcode audio. The tokens ``$source`` and ``$dest`` in the command +are replaced with the paths to the existing and new file. The plugin in comes with default commands for the most common audio formats: `mp3`, `alac`, `flac`, `aac`, `opus`, `ogg`, `wmv`. For diff --git a/test/test_convert.py b/test/test_convert.py index ebe30ad8c..24ed44748 100644 --- a/test/test_convert.py +++ b/test/test_convert.py @@ -77,13 +77,10 @@ class ConvertCliTest(unittest.TestCase, TestHelper): 'paths': {'default': 'converted'}, 'format': 'mp3', 'formats': { - 'mp3': { - 'command': 'cp $source $dest', - 'extension': 'mp3', - }, + 'mp3': 'cp $source $dest', 'opus': { 'command': 'cp $source $dest', - 'extension': 'opus', + 'extension': 'ops', } } } @@ -110,7 +107,7 @@ class ConvertCliTest(unittest.TestCase, TestHelper): 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') + converted = os.path.join(self.convert_dest, 'converted.ops') self.assertTrue(os.path.isfile(converted)) def test_embed_album_art(self):