diff --git a/NEWS b/NEWS index e6646e7bc..763dcec9a 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ it is now possible to specify a log file in .beetsconfig. Also, logs are now appended rather than overwritten and contain timestamps. +* Album art fetching and plugin events are each now run in separate + pipeline stages during imports. This should bring additional + performance when using album art plugins like embedart or + beets-lyrics. * Fix crash when autotagging files with no metadata. 1.0b8 diff --git a/beets/importer.py b/beets/importer.py index 300a056fb..d3670973e 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -282,6 +282,11 @@ class ImportTask(object): return True else: assert False + def should_skip(self): + """After a choice has been made, returns True if this is a + sentinel or it has been marked for skipping. + """ + return self.sentinel or self.choice_flag == action.SKIP # Full-album pipeline stages. @@ -420,17 +425,13 @@ def show_progress(config): def apply_choices(config): """A coroutine for applying changes to albums during the autotag - process. The parameters to the generator control the behavior of - the import. The coroutine accepts ImportTask objects and yields - nothing. + process. """ lib = _reopen_lib(config.lib) + task = None while True: - task = yield - # Don't do anything if we're skipping the album or we're done. - if task.sentinel or task.choice_flag == action.SKIP: - if config.resume is not False: - task.save_progress() + task = yield task + if task.should_skip(): continue # Change metadata, move, and copy. @@ -441,7 +442,8 @@ def apply_choices(config): autotag.apply_item_metadata(task.item, task.info) items = task.items if task.is_album else [task.item] if config.copy and config.delete: - old_paths = [os.path.realpath(syspath(item.path)) for item in items] + task.old_paths = [os.path.realpath(syspath(item.path)) + for item in items] for item in items: if config.copy: item.move(lib, True, task.is_album) @@ -453,8 +455,9 @@ def apply_choices(config): try: if task.is_album: # Add an album. - albuminfo = lib.add_album(task.items, - infer_aa = task.should_infer_aa()) + album = lib.add_album(task.items, + infer_aa = task.should_infer_aa()) + task.album_id = album.id else: # Add tracks. for item in items: @@ -462,18 +465,47 @@ def apply_choices(config): finally: lib.save() - # Get album art if requested. - if config.art and task.should_fetch_art(): +def fetch_art(config): + """A coroutine that fetches and applies album art for albums where + appropriate. + """ + lib = _reopen_lib(config.lib) + task = None + while True: + task = yield task + if task.should_skip(): + continue + + if task.should_fetch_art(): artpath = beets.autotag.art.art_for_album(task.info) + + # Save the art if any was found. if artpath: try: - albuminfo.set_art(artpath) + album = lib.get_album(task.album_id) + album.set_art(artpath) finally: lib.save() +def finalize(config): + """A coroutine that finishes up importer tasks. In particular, the + coroutine sends plugin events, deletes old files, and saves + progress. This is a "terminal" coroutine (it yields None). + """ + lib = _reopen_lib(config.lib) + while True: + task = yield + if task.should_skip(): + if config.resume is not False: + task.save_progress() + continue + + items = task.items if task.is_album else [task.item] + # Announce that we've added an album. if task.is_album: - plugins.send('album_imported', lib=lib, album=albuminfo) + album = lib.get_album(task.album_id) + plugins.send('album_imported', lib=lib, album=album) else: for item in items: plugins.send('item_imported', lib=lib, item=item) @@ -481,7 +513,7 @@ def apply_choices(config): # Finally, delete old files. if config.copy and config.delete: new_paths = [os.path.realpath(item.path) for item in items] - for old_path in old_paths: + for old_path in task.old_paths: # Only delete files that were actually moved. if old_path not in new_paths: os.remove(syspath(old_path)) @@ -571,6 +603,9 @@ def run_import(**kwargs): # When not autotagging, just display progress. stages += [show_progress(config)] stages += [apply_choices(config)] + if config.art: + stages += [fetch_art(config)] + stages += [finalize(config)] pl = pipeline.Pipeline(stages) # Run the pipeline. diff --git a/test/test_importer.py b/test/test_importer.py index 2af2c9376..05c6eb34a 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -164,11 +164,14 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): def tearDown(self): shutil.rmtree(self.libdir) - def _call_apply(self, coro, items, info): + def _call_apply(self, coros, items, info): task = importer.ImportTask(None, None, None) task.is_album = True task.set_choice((info, items)) - coro.send(task) + if not isinstance(coros, list): + coros = [coros] + for coro in coros: + task = coro.send(task) def _call_apply_choice(self, coro, items, choice): task = importer.ImportTask(None, None, items) @@ -176,16 +179,22 @@ class ImportApplyTest(unittest.TestCase, _common.ExtraAsserts): task.set_choice(choice) coro.send(task) - def test_apply_no_delete(self): - coro = importer.apply_choices(_common.iconfig(self.lib, delete=False)) - coro.next() # Prime coroutine. - self._call_apply(coro, [self.i], self.info) + def test_finalize_no_delete(self): + config = _common.iconfig(self.lib, delete=False) + applyc = importer.apply_choices(config) + applyc.next() + finalize = importer.finalize(config) + finalize.next() + self._call_apply([applyc, finalize], [self.i], self.info) self.assertExists(self.srcpath) - def test_apply_with_delete(self): - coro = importer.apply_choices(_common.iconfig(self.lib, delete=True)) - coro.next() # Prime coroutine. - self._call_apply(coro, [self.i], self.info) + def test_finalize_with_delete(self): + config = _common.iconfig(self.lib, delete=True) + applyc = importer.apply_choices(config) + applyc.next() + finalize = importer.finalize(config) + finalize.next() + self._call_apply([applyc, finalize], [self.i], self.info) self.assertNotExists(self.srcpath) def test_apply_asis_uses_album_path(self):