diff --git a/appveyor.yml b/appveyor.yml index 5a0f32135..233fa3178 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -9,8 +9,6 @@ environment: matrix: - PYTHON: C:\Python27 TOX_ENV: py27-test - - PYTHON: C:\Python35 - TOX_ENV: py35-test - PYTHON: C:\Python36 TOX_ENV: py36-test - PYTHON: C:\Python37 diff --git a/beets/config_default.yaml b/beets/config_default.yaml index f3e9acad1..dd140675f 100644 --- a/beets/config_default.yaml +++ b/beets/config_default.yaml @@ -7,6 +7,7 @@ import: move: no link: no hardlink: no + reflink: no delete: no resume: ask incremental: no diff --git a/beets/importer.py b/beets/importer.py index 77cce4bd3..c5701ff30 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -222,19 +222,31 @@ class ImportSession(object): iconfig['resume'] = False iconfig['incremental'] = False - # Copy, move, link, and hardlink are mutually exclusive. + if iconfig['reflink']: + iconfig['reflink'] = iconfig['reflink'] \ + .as_choice(['auto', True, False]) + + # Copy, move, reflink, link, and hardlink are mutually exclusive. if iconfig['move']: iconfig['copy'] = False iconfig['link'] = False iconfig['hardlink'] = False + iconfig['reflink'] = False elif iconfig['link']: iconfig['copy'] = False iconfig['move'] = False iconfig['hardlink'] = False + iconfig['reflink'] = False elif iconfig['hardlink']: iconfig['copy'] = False iconfig['move'] = False iconfig['link'] = False + iconfig['reflink'] = False + elif iconfig['reflink']: + iconfig['copy'] = False + iconfig['move'] = False + iconfig['link'] = False + iconfig['hardlink'] = False # Only delete when copying. if not iconfig['copy']: @@ -707,7 +719,7 @@ class ImportTask(BaseImportTask): item.update(changes) def manipulate_files(self, operation=None, write=False, session=None): - """ Copy, move, link or hardlink (depending on `operation`) the files + """ Copy, move, link, hardlink or reflink (depending on `operation`) the files as well as write metadata. `operation` should be an instance of `util.MoveOperation`. @@ -1536,6 +1548,8 @@ def manipulate_files(session, task): operation = MoveOperation.LINK elif session.config['hardlink']: operation = MoveOperation.HARDLINK + elif session.config['reflink']: + operation = MoveOperation.REFLINK else: operation = None diff --git a/beets/library.py b/beets/library.py index f9b5a2ab4..c5d25e5a9 100644 --- a/beets/library.py +++ b/beets/library.py @@ -807,6 +807,16 @@ class Item(LibModel): util.hardlink(self.path, dest) plugins.send("item_hardlinked", item=self, source=self.path, destination=dest) + elif operation == MoveOperation.REFLINK: + util.reflink(self.path, dest, fallback=False) + plugins.send("item_reflinked", item=self, source=self.path, + destination=dest) + elif operation == MoveOperation.REFLINK_AUTO: + util.reflink(self.path, dest, fallback=True) + plugins.send("item_reflinked", item=self, source=self.path, + destination=dest) + else: + assert False, 'unknown MoveOperation' # Either copying or moving succeeded, so update the stored path. self.path = dest @@ -1147,6 +1157,12 @@ class Album(LibModel): util.link(old_art, new_art) elif operation == MoveOperation.HARDLINK: util.hardlink(old_art, new_art) + elif operation == MoveOperation.REFLINK: + util.reflink(old_art, new_art, fallback=False) + elif operation == MoveOperation.REFLINK_AUTO: + util.reflink(old_art, new_art, fallback=True) + else: + assert False, 'unknown MoveOperation' self.artpath = new_art def move(self, operation=MoveOperation.MOVE, basedir=None, store=True): diff --git a/beets/plugins.py b/beets/plugins.py index 695725cb8..3abd911c9 100644 --- a/beets/plugins.py +++ b/beets/plugins.py @@ -301,6 +301,11 @@ def find_plugins(): currently loaded beets plugins. Loads the default plugin set first. """ + if _instances: + # After the first call, use cached instances for performance reasons. + # See https://github.com/beetbox/beets/pull/3810 + return list(_instances.values()) + load_plugins() plugins = [] for cls in _classes: diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 384609ee6..248096730 100644 --- a/beets/util/__init__.py +++ b/beets/util/__init__.py @@ -134,6 +134,8 @@ class MoveOperation(Enum): COPY = 1 LINK = 2 HARDLINK = 3 + REFLINK = 4 + REFLINK_AUTO = 5 def normpath(path): @@ -549,6 +551,35 @@ def hardlink(path, dest, replace=False): traceback.format_exc()) +def reflink(path, dest, replace=False, fallback=False): + """Create a reflink from `dest` to `path`. + + Raise an `OSError` if `dest` already exists, unless `replace` is + True. If `path` == `dest`, then do nothing. + + If reflinking fails and `fallback` is enabled, try copying the file + instead. Otherwise, raise an error without trying a plain copy. + + May raise an `ImportError` if the `reflink` module is not available. + """ + import reflink as pyreflink + + if samefile(path, dest): + return + + if os.path.exists(syspath(dest)) and not replace: + raise FilesystemError(u'file exists', 'rename', (path, dest)) + + try: + pyreflink.reflink(path, dest) + except (NotImplementedError, pyreflink.ReflinkImpossibleError): + if fallback: + copy(path, dest, replace) + else: + raise FilesystemError(u'OS/filesystem does not support reflinks.', + 'link', (path, dest), traceback.format_exc()) + + def unique_path(path): """Returns a version of ``path`` that does not exist on the filesystem. Specifically, if ``path` itself already exists, then diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 3b2595585..b1b1593cd 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -505,6 +505,12 @@ class DiscogsPlugin(BeetsPlugin): for subtrack in subtracks: if not subtrack.get('artists'): subtrack['artists'] = index_track['artists'] + # Concatenate index with track title when index_tracks + # option is set + if self.config['index_tracks']: + for subtrack in subtracks: + subtrack['title'] = '{}: {}'.format( + index_track['title'], subtrack['title']) tracklist.extend(subtracks) else: # Merge the subtracks, pick a title, and append the new track. @@ -557,7 +563,8 @@ class DiscogsPlugin(BeetsPlugin): title = track['title'] if self.config['index_tracks']: prefix = ', '.join(divisions) - title = ': '.join([prefix, title]) + if prefix: + title = '{}: {}'.format(prefix, title) track_id = None medium, medium_index, _ = self.get_track_index(track['position']) artist, artist_id = MetadataSourcePlugin.get_artist( diff --git a/beetsplug/fish.py b/beetsplug/fish.py index b842ac70f..12f474e88 100644 --- a/beetsplug/fish.py +++ b/beetsplug/fish.py @@ -133,6 +133,13 @@ class FishPlugin(BeetsPlugin): fish_file.write(totstring) +def _escape(name): + # Escape ? in fish + if name == "?": + name = "\\" + name + return name + + def get_cmds_list(cmds_names): # Make a list of all Beets core & plugin commands substr = '' @@ -201,6 +208,8 @@ def get_subcommands(cmd_name_and_help, nobasicfields, extravalues): # Formatting for Fish to complete our fields/values word = "" for cmdname, cmdhelp in cmd_name_and_help: + cmdname = _escape(cmdname) + word += "\n" + "# ------ {} -------".format( "fieldsetups for " + cmdname) + "\n" word += ( @@ -232,6 +241,8 @@ def get_all_commands(beetcmds): names = [alias for alias in cmd.aliases] names.append(cmd.name) for name in names: + name = _escape(name) + word += "\n" word += ("\n" * 2) + "# ====== {} =====".format( "completions for " + name) + "\n" diff --git a/beetsplug/keyfinder.py b/beetsplug/keyfinder.py index a75b8d972..702003f0f 100644 --- a/beetsplug/keyfinder.py +++ b/beetsplug/keyfinder.py @@ -76,7 +76,14 @@ class KeyFinderPlugin(BeetsPlugin): item.path) continue - key_raw = output.rsplit(None, 1)[-1] + try: + key_raw = output.rsplit(None, 1)[-1] + except IndexError: + # Sometimes keyfinder-cli returns 0 but with no key, usually + # when the file is silent or corrupt, so we log and skip. + self._log.error(u'no key returned for path: {0}', item.path) + continue + try: key = util.text_string(key_raw) except UnicodeDecodeError: diff --git a/beetsplug/lastgenre/__init__.py b/beetsplug/lastgenre/__init__.py index cf90facbd..59beddfa9 100644 --- a/beetsplug/lastgenre/__init__.py +++ b/beetsplug/lastgenre/__init__.py @@ -111,6 +111,7 @@ class LastGenrePlugin(plugins.BeetsPlugin): 'auto': True, 'separator': u', ', 'prefer_specific': False, + 'title_case': True, }) self.setup() @@ -224,12 +225,17 @@ class LastGenrePlugin(plugins.BeetsPlugin): # c14n only adds allowed genres but we may have had forbidden genres in # the original tags list - tags = [x.title() for x in tags if self._is_allowed(x)] + tags = [self._format_tag(x) for x in tags if self._is_allowed(x)] return self.config['separator'].as_str().join( tags[:self.config['count'].get(int)] ) + def _format_tag(self, tag): + if self.config["title_case"]: + return tag.title() + return tag + def fetch_genre(self, lastfm_obj): """Return the genre for a pylast entity or None if no suitable genre can be found. Ex. 'Electronic, House, Dance' diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py index 5591598ae..00d65b578 100644 --- a/beetsplug/lyrics.py +++ b/beetsplug/lyrics.py @@ -424,6 +424,7 @@ def _scrape_strip_cruft(html, plain_text_out=False): html = re.sub(r' +', ' ', html) # Whitespaces collapse. html = BREAK_RE.sub('\n', html) #
eats up surrounding '\n'. html = re.sub(r'(?s)<(script).*?', '', html) # Strip script tags. + html = re.sub(u'\u2005', " ", html) # replace unicode with regular space if plain_text_out: # Strip remaining HTML tags html = COMMENT_RE.sub('', html) diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py index 39b045f9b..599aa7631 100644 --- a/beetsplug/mpdstats.py +++ b/beetsplug/mpdstats.py @@ -18,6 +18,7 @@ from __future__ import division, absolute_import, print_function import mpd import socket import select +import sys import time import os @@ -52,7 +53,13 @@ class MPDClientWrapper(object): self.music_directory = ( mpd_config['music_directory'].as_str()) - self.client = mpd.MPDClient(use_unicode=True) + if sys.version_info < (3, 0): + # On Python 2, use_unicode will enable the utf-8 mode for + # python-mpd2 + self.client = mpd.MPDClient(use_unicode=True) + else: + # On Python 3, python-mpd2 always uses Unicode + self.client = mpd.MPDClient() def connect(self): """Connect to the MPD. diff --git a/beetsplug/parentwork.py b/beetsplug/parentwork.py index b3e464e60..a5e042344 100644 --- a/beetsplug/parentwork.py +++ b/beetsplug/parentwork.py @@ -96,7 +96,7 @@ class ParentWorkPlugin(BeetsPlugin): item.try_write() command = ui.Subcommand( 'parentwork', - help=u'fetche parent works, composers and dates') + help=u'fetch parent works, composers and dates') command.parser.add_option( u'-f', u'--force', dest='force', @@ -129,6 +129,7 @@ class ParentWorkPlugin(BeetsPlugin): if 'artist-relation-list' in work_info['work']: for artist in work_info['work']['artist-relation-list']: if artist['type'] == 'composer': + composer_exists = True parent_composer.append(artist['artist']['name']) parent_composer_sort.append(artist['artist']['sort-name']) if 'end' in artist.keys(): diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index 0ba04783d..9d6fa23c4 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -24,12 +24,17 @@ import warnings import enum import re import xml.parsers.expat -from six.moves import zip +from six.moves import zip, queue +import six + +from multiprocessing.pool import ThreadPool, RUN +from threading import Thread, Event +import signal from beets import ui from beets.plugins import BeetsPlugin from beets.util import (syspath, command_output, bytestring_path, - displayable_path, py3_path) + displayable_path, py3_path, cpu_count) # Utilities. @@ -110,6 +115,8 @@ class Backend(object): """An abstract class representing engine for calculating RG values. """ + do_parallel = False + def __init__(self, config, log): """Initialize the backend with the configuration view for the plugin. @@ -141,6 +148,8 @@ class Bs1770gainBackend(Backend): -18: "replaygain", } + do_parallel = True + def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) config.add({ @@ -352,8 +361,7 @@ class Bs1770gainBackend(Backend): except xml.parsers.expat.ExpatError: raise ReplayGainError( u'The bs1770gain tool produced malformed XML. ' - 'Using version >=0.4.10 may solve this problem.' - ) + u'Using version >=0.4.10 may solve this problem.') if len(per_file_gain) != len(path_list): raise ReplayGainError( @@ -378,6 +386,9 @@ class Bs1770gainBackend(Backend): class FfmpegBackend(Backend): """A replaygain backend using ffmpeg's ebur128 filter. """ + + do_parallel = True + def __init__(self, config, log): super(FfmpegBackend, self).__init__(config, log) self._ffmpeg_path = "ffmpeg" @@ -620,6 +631,7 @@ class FfmpegBackend(Backend): # mpgain/aacgain CLI tool backend. class CommandBackend(Backend): + do_parallel = True def __init__(self, config, log): super(CommandBackend, self).__init__(config, log) @@ -748,7 +760,6 @@ class CommandBackend(Backend): # GStreamer-based backend. class GStreamerBackend(Backend): - def __init__(self, config, log): super(GStreamerBackend, self).__init__(config, log) self._import_gst() @@ -1168,6 +1179,33 @@ class AudioToolsBackend(Backend): ) +class ExceptionWatcher(Thread): + """Monitors a queue for exceptions asynchronously. + Once an exception occurs, raise it and execute a callback. + """ + + def __init__(self, queue, callback): + self._queue = queue + self._callback = callback + self._stopevent = Event() + Thread.__init__(self) + + def run(self): + while not self._stopevent.is_set(): + try: + exc = self._queue.get_nowait() + self._callback() + six.reraise(exc[0], exc[1], exc[2]) + except queue.Empty: + # No exceptions yet, loop back to check + # whether `_stopevent` is set + pass + + def join(self, timeout=None): + self._stopevent.set() + Thread.join(self, timeout) + + # Main plugin logic. class ReplayGainPlugin(BeetsPlugin): @@ -1195,6 +1233,8 @@ class ReplayGainPlugin(BeetsPlugin): 'overwrite': False, 'auto': True, 'backend': u'command', + 'threads': cpu_count(), + 'parallel_on_import': False, 'per_disc': False, 'peak': 'true', 'targetlevel': 89, @@ -1204,12 +1244,15 @@ class ReplayGainPlugin(BeetsPlugin): self.overwrite = self.config['overwrite'].get(bool) self.per_disc = self.config['per_disc'].get(bool) - backend_name = self.config['backend'].as_str() - if backend_name not in self.backends: + + # Remember which backend is used for CLI feedback + self.backend_name = self.config['backend'].as_str() + + if self.backend_name not in self.backends: raise ui.UserError( u"Selected ReplayGain backend {0} is not supported. " u"Please select one of: {1}".format( - backend_name, + self.backend_name, u', '.join(self.backends.keys()) ) ) @@ -1226,13 +1269,15 @@ class ReplayGainPlugin(BeetsPlugin): # On-import analysis. if self.config['auto']: + self.register_listener('import_begin', self.import_begin) + self.register_listener('import', self.import_end) self.import_stages = [self.imported] # Formats to use R128. self.r128_whitelist = self.config['r128'].as_str_seq() try: - self.backend_instance = self.backends[backend_name]( + self.backend_instance = self.backends[self.backend_name]( self.config, self._log ) except (ReplayGainError, FatalReplayGainError) as e: @@ -1264,30 +1309,40 @@ class ReplayGainPlugin(BeetsPlugin): (not item.rg_album_gain or not item.rg_album_peak) for item in album.items()]) + def _store(self, item): + """Store an item to the database. + When testing, item.store() sometimes fails non-destructively with + sqlite.OperationalError. + This method is here to be patched to a retry-once helper function + in test_replaygain.py, so that it can still fail appropriately + outside of these tests. + """ + item.store() + def store_track_gain(self, item, track_gain): item.rg_track_gain = track_gain.gain item.rg_track_peak = track_gain.peak - item.store() + self._store(item) self._log.debug(u'applied track gain {0} LU, peak {1} of FS', item.rg_track_gain, item.rg_track_peak) def store_album_gain(self, item, album_gain): item.rg_album_gain = album_gain.gain item.rg_album_peak = album_gain.peak - item.store() + self._store(item) self._log.debug(u'applied album gain {0} LU, peak {1} of FS', item.rg_album_gain, item.rg_album_peak) def store_track_r128_gain(self, item, track_gain): item.r128_track_gain = track_gain.gain - item.store() + self._store(item) self._log.debug(u'applied r128 track gain {0} LU', item.r128_track_gain) def store_album_r128_gain(self, item, album_gain): item.r128_album_gain = album_gain.gain - item.store() + self._store(item) self._log.debug(u'applied r128 album gain {0} LU', item.r128_album_gain) @@ -1322,8 +1377,6 @@ class ReplayGainPlugin(BeetsPlugin): self._log.info(u'Skipping album {0}', album) return - self._log.info(u'analyzing {0}', album) - if (any([self.should_use_r128(item) for item in album.items()]) and not all(([self.should_use_r128(item) for item in album.items()]))): self._log.error( @@ -1331,6 +1384,8 @@ class ReplayGainPlugin(BeetsPlugin): album) return + self._log.info(u'analyzing {0}', album) + tag_vals = self.tag_specific_values(album.items()) store_track_gain, store_album_gain, target_level, peak = tag_vals @@ -1344,21 +1399,35 @@ class ReplayGainPlugin(BeetsPlugin): discs[1] = album.items() for discnumber, items in discs.items(): - try: - album_gain = self.backend_instance.compute_album_gain( - items, target_level, peak - ) - if len(album_gain.track_gains) != len(items): + def _store_album(album_gain): + if not album_gain or not album_gain.album_gain \ + or len(album_gain.track_gains) != len(items): + # In some cases, backends fail to produce a valid + # `album_gain` without throwing FatalReplayGainError + # => raise non-fatal exception & continue raise ReplayGainError( - u"ReplayGain backend failed " - u"for some tracks in album {0}".format(album) + u"ReplayGain backend `{}` failed " + u"for some tracks in album {}" + .format(self.backend_name, album) ) - - for item, track_gain in zip(items, album_gain.track_gains): + for item, track_gain in zip(items, + album_gain.track_gains): store_track_gain(item, track_gain) store_album_gain(item, album_gain.album_gain) if write: item.try_write() + self._log.debug(u'done analyzing {0}', item) + + try: + self._apply( + self.backend_instance.compute_album_gain, args=(), + kwds={ + "items": [i for i in items], + "target_level": target_level, + "peak": peak + }, + callback=_store_album + ) except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: @@ -1376,54 +1445,186 @@ class ReplayGainPlugin(BeetsPlugin): self._log.info(u'Skipping track {0}', item) return - self._log.info(u'analyzing {0}', item) - tag_vals = self.tag_specific_values([item]) store_track_gain, store_album_gain, target_level, peak = tag_vals - try: - track_gains = self.backend_instance.compute_track_gain( - [item], target_level, peak - ) - if len(track_gains) != 1: + def _store_track(track_gains): + if not track_gains or len(track_gains) != 1: + # In some cases, backends fail to produce a valid + # `track_gains` without throwing FatalReplayGainError + # => raise non-fatal exception & continue raise ReplayGainError( - u"ReplayGain backend failed for track {0}".format(item) + u"ReplayGain backend `{}` failed for track {}" + .format(self.backend_name, item) ) store_track_gain(item, track_gains[0]) if write: item.try_write() + self._log.debug(u'done analyzing {0}', item) + + try: + self._apply( + self.backend_instance.compute_track_gain, args=(), + kwds={ + "items": [item], + "target_level": target_level, + "peak": peak, + }, + callback=_store_track + ) except ReplayGainError as e: self._log.info(u"ReplayGain error: {0}", e) except FatalReplayGainError as e: - raise ui.UserError( - u"Fatal replay gain error: {0}".format(e)) + raise ui.UserError(u"Fatal replay gain error: {0}".format(e)) + + def _has_pool(self): + """Check whether a `ThreadPool` is running instance in `self.pool` + """ + if hasattr(self, 'pool'): + if isinstance(self.pool, ThreadPool) and self.pool._state == RUN: + return True + return False + + def open_pool(self, threads): + """Open a `ThreadPool` instance in `self.pool` + """ + if not self._has_pool() and self.backend_instance.do_parallel: + self.pool = ThreadPool(threads) + self.exc_queue = queue.Queue() + + signal.signal(signal.SIGINT, self._interrupt) + + self.exc_watcher = ExceptionWatcher( + self.exc_queue, # threads push exceptions here + self.terminate_pool # abort once an exception occurs + ) + self.exc_watcher.start() + + def _apply(self, func, args, kwds, callback): + if self._has_pool(): + def catch_exc(func, exc_queue, log): + """Wrapper to catch raised exceptions in threads + """ + def wfunc(*args, **kwargs): + try: + return func(*args, **kwargs) + except ReplayGainError as e: + log.info(e.args[0]) # log non-fatal exceptions + except Exception: + exc_queue.put(sys.exc_info()) + return wfunc + + # Wrap function and callback to catch exceptions + func = catch_exc(func, self.exc_queue, self._log) + callback = catch_exc(callback, self.exc_queue, self._log) + + self.pool.apply_async(func, args, kwds, callback) + else: + callback(func(*args, **kwds)) + + def terminate_pool(self): + """Terminate the `ThreadPool` instance in `self.pool` + (e.g. stop execution in case of exception) + """ + # Don't call self._as_pool() here, + # self.pool._state may not be == RUN + if hasattr(self, 'pool') and isinstance(self.pool, ThreadPool): + self.pool.terminate() + self.pool.join() + # self.exc_watcher.join() + + def _interrupt(self, signal, frame): + try: + self._log.info('interrupted') + self.terminate_pool() + exit(0) + except SystemExit: + # Silence raised SystemExit ~ exit(0) + pass + + def close_pool(self): + """Close the `ThreadPool` instance in `self.pool` (if there is one) + """ + if self._has_pool(): + self.pool.close() + self.pool.join() + self.exc_watcher.join() + + def import_begin(self, session): + """Handle `import_begin` event -> open pool + """ + threads = self.config['threads'].get(int) + + if self.config['parallel_on_import'] \ + and self.config['auto'] \ + and threads: + self.open_pool(threads) + + def import_end(self, paths): + """Handle `import` event -> close pool + """ + self.close_pool() def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ - if task.is_album: - self.handle_album(task.album, False) - else: - self.handle_track(task.item, False) + if self.config['auto']: + if task.is_album: + self.handle_album( + task.album, + self.config['auto'].get(bool), + self.config['overwrite'].get(bool) + ) + else: + self.handle_track( + task.item, + self.config['auto'].get(bool), + self.config['overwrite'].get(bool) + ) def commands(self): """Return the "replaygain" ui subcommand. """ def func(lib, opts, args): - write = ui.should_write(opts.write) - force = opts.force + try: + write = ui.should_write(opts.write) + force = opts.force - if opts.album: - for album in lib.albums(ui.decargs(args)): - self.handle_album(album, write, force) + # Bypass self.open_pool() if called with `--threads 0` + if opts.threads != 0: + threads = opts.threads or self.config['threads'].get(int) + self.open_pool(threads) - else: - for item in lib.items(ui.decargs(args)): - self.handle_track(item, write, force) + if opts.album: + albums = lib.albums(ui.decargs(args)) + self._log.info( + "Analyzing {} albums ~ {} backend..." + .format(len(albums), self.backend_name) + ) + for album in albums: + self.handle_album(album, write, force) + else: + items = lib.items(ui.decargs(args)) + self._log.info( + "Analyzing {} tracks ~ {} backend..." + .format(len(items), self.backend_name) + ) + for item in items: + self.handle_track(item, write, force) + + self.close_pool() + except (SystemExit, KeyboardInterrupt): + # Silence interrupt exceptions + pass cmd = ui.Subcommand('replaygain', help=u'analyze for ReplayGain') cmd.parser.add_album_option() + cmd.parser.add_option( + "-t", "--threads", dest="threads", type=int, + help=u'change the number of threads, \ + defaults to maximum available processors' + ) cmd.parser.add_option( "-f", "--force", dest="force", action="store_true", default=False, help=u"analyze all files, including those that " diff --git a/docs/changelog.rst b/docs/changelog.rst index b48818685..e6d33d9f1 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -6,6 +6,7 @@ Changelog New features: +* Add ``title_case`` config option to lastgenre to make TitleCasing optional. * When config is printed with no available configuration a new message is printed. :bug:`3779` * When importing a duplicate album it ask if it should "Keep all" instead of "Keep both". @@ -13,6 +14,9 @@ New features: * :doc:`/plugins/chroma`: Update file metadata after generating fingerprints through the `submit` command. * :doc:`/plugins/lastgenre`: Added more heavy metal genres: https://en.wikipedia.org/wiki/Heavy_metal_genres to genres.txt and genres-tree.yaml * :doc:`/plugins/subsonicplaylist`: import playlist from a subsonic server. +* A new :ref:`reflink` config option instructs the importer to create fast, + copy-on-write file clones on filesystems that support them. Thanks to + :user:`rubdos`. * A new :ref:`extra_tags` configuration option allows more tagged metadata to be included in MusicBrainz queries. * A new :doc:`/plugins/fish` adds `Fish shell`_ tab autocompletion to beets @@ -166,6 +170,9 @@ New features: https://github.com/alastair/python-musicbrainzngs/pull/247 and https://github.com/alastair/python-musicbrainzngs/pull/266 . Thanks to :user:`aereaux`. +* :doc:`/plugins/replaygain` now does its analysis in parallel when using + the ``command``, ``ffmpeg`` or ``bs1770gain`` backends. + :bug:`3478` * Fields in queries now fall back to an item's album and check its fields too. Notably, this allows querying items by an album flex attribute, also in path configuration. @@ -174,6 +181,9 @@ New features: Fixes: +* :bug:`/plugins/discogs`: Fixed a bug with ``index_tracks`` options that + sometimes caused the index to be discarded. Also remove the extra semicolon + that was added when there is no index track. * :doc:`/plugins/subsonicupdate`: REST was using `POST` method rather `GET` method. Also includes better exception handling, response parsing, and tests. * :doc:`/plugins/the`: Fixed incorrect regex for 'the' that matched any @@ -284,6 +294,12 @@ Fixes: :bug:`3773` :bug:`3774` * Fix a bug causing PIL to generate poor quality JPEGs when resizing artwork. :bug:`3743` +* :doc:`plugins/keyfinder`: Catch output from ``keyfinder-cli`` that is missing key. + :bug:`2242` +* :doc:`plugins/replaygain`: Disable parallel analysis on import by default. + :bug:`3819` +* :doc:`/plugins/mpdstats`: Fix Python 2/3 compatibility + :bug:`3798` For plugin developers: diff --git a/docs/dev/plugins.rst b/docs/dev/plugins.rst index 563775fd6..a6aa3d6d7 100644 --- a/docs/dev/plugins.rst +++ b/docs/dev/plugins.rst @@ -164,6 +164,10 @@ The events currently available are: created for a file. Parameters: ``item``, ``source`` path, ``destination`` path +* `item_reflinked`: called with an ``Item`` object whenever a reflink is + created for a file. + Parameters: ``item``, ``source`` path, ``destination`` path + * `item_removed`: called with an ``Item`` object every time an item (singleton or album's part) is removed from the library (even when its file is not deleted from disk). diff --git a/docs/plugins/index.rst b/docs/plugins/index.rst index 1d5d1b815..2b16d96d8 100644 --- a/docs/plugins/index.rst +++ b/docs/plugins/index.rst @@ -275,7 +275,7 @@ Here are a few of the plugins written by the beets community: * `beet-amazon`_ adds Amazon.com as a tagger data source. -* `copyartifacts`_ helps bring non-music files along during import. +* `beets-copyartifacts`_ helps bring non-music files along during import. * `beets-check`_ automatically checksums your files to detect corruption. @@ -326,7 +326,7 @@ Here are a few of the plugins written by the beets community: .. _beets-barcode: https://github.com/8h2a/beets-barcode .. _beets-check: https://github.com/geigerzaehler/beets-check -.. _copyartifacts: https://github.com/sbarakat/beets-copyartifacts +.. _beets-copyartifacts: https://github.com/adammillerio/beets-copyartifacts .. _dsedivec: https://github.com/dsedivec/beets-plugins .. _beets-artistcountry: https://github.com/agrausem/beets-artistcountry .. _beetFs: https://github.com/jbaiter/beetfs diff --git a/docs/plugins/lastgenre.rst b/docs/plugins/lastgenre.rst index dee4260de..cbc54f1cc 100644 --- a/docs/plugins/lastgenre.rst +++ b/docs/plugins/lastgenre.rst @@ -146,6 +146,8 @@ configuration file. The available options are: - **whitelist**: The filename of a custom genre list, ``yes`` to use the internal whitelist, or ``no`` to consider all genres valid. Default: ``yes``. +- **title_case**: Convert the new tags to TitleCase before saving. + Default: ``yes``. Running Manually ---------------- diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index 9602618da..fa0e10b75 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -13,12 +13,16 @@ Installation This plugin can use one of many backends to compute the ReplayGain values: GStreamer, mp3gain (and its cousin, aacgain), Python Audio Tools or ffmpeg. ffmpeg and mp3gain can be easier to install. mp3gain supports less audio formats -then the other backend. +than the other backend. Once installed, this plugin analyzes all files during the import process. This can be a slow process; to instead analyze after the fact, disable automatic analysis and use the ``beet replaygain`` command (see below). +To speed up analysis with some of the avalaible backends, this plugin processes +tracks or albums (when using the ``-a`` option) in parallel. By default, +a single thread is used per logical core of your CPU. + GStreamer ````````` @@ -35,6 +39,8 @@ the GStreamer backend by adding this to your configuration file:: replaygain: backend: gstreamer +The GStreamer backend does not support parallel analysis. + mp3gain and aacgain ``````````````````` @@ -73,6 +79,8 @@ On OS X, most of the dependencies can be installed with `Homebrew`_:: brew install mpg123 mp3gain vorbisgain faad2 libvorbis +The Python Audio Tools backend does not support parallel analysis. + .. _Python Audio Tools: http://audiotools.sourceforge.net ffmpeg @@ -92,6 +100,15 @@ configuration file. The available options are: - **auto**: Enable ReplayGain analysis during import. Default: ``yes``. +- **threads**: The number of parallel threads to run the analysis in. Overridden + by ``--threads`` at the command line. + Default: # of logical CPU cores +- **parallel_on_import**: Whether to enable parallel analysis during import. + As of now this ReplayGain data is not written to files properly, so this option + is disabled by default. + If you wish to enable it, remember to run ``beet write`` after importing to + actually write to the imported files. + Default: ``no`` - **backend**: The analysis backend; either ``gstreamer``, ``command``, ``audiotools`` or ``ffmpeg``. Default: ``command``. @@ -143,8 +160,15 @@ whether ReplayGain tags are written into the music files, or stored in the beets database only (the default is to use :ref:`the importer's configuration `). +To execute with a different number of threads, call ``beet replaygain --threads N``:: + + $ beet replaygain --threads N [-Waf] [QUERY] + +with N any integer. To disable parallelism, use ``--threads 0``. + ReplayGain analysis is not fast, so you may want to disable it during import. Use the ``auto`` config option to control this:: replaygain: auto: no + diff --git a/docs/reference/config.rst b/docs/reference/config.rst index 6aa9f5f53..9dd7447a4 100644 --- a/docs/reference/config.rst +++ b/docs/reference/config.rst @@ -475,13 +475,35 @@ hardlink ~~~~~~~~ Either ``yes`` or ``no``, indicating whether to use hard links instead of -moving or copying or symlinking files. (It conflicts with the ``move``, +moving, copying, or symlinking files. (It conflicts with the ``move``, ``copy``, and ``link`` options.) Defaults to ``no``. As with symbolic links (see :ref:`link`, above), this will not work on Windows and you will want to set ``write`` to ``no``. Otherwise, metadata on the original file will be modified. +.. _reflink: + +reflink +~~~~~~~ + +Either ``yes``, ``no``, or ``auto``, indicating whether to use copy-on-write +`file clones`_ (a.k.a. "reflinks") instead of copying or moving files. +The ``auto`` option uses reflinks when possible and falls back to plain +copying when necessary. +Defaults to ``no``. + +This kind of clone is only available on certain filesystems: for example, +btrfs and APFS. For more details on filesystem support, see the `pyreflink`_ +documentation. Note that you need to install ``pyreflink``, either through +``python -m pip install beets[reflink]`` or ``python -m pip install reflink``. + +The option is ignored if ``move`` is enabled (i.e., beets can move or +copy files but it doesn't make sense to do both). + +.. _file clones: https://blogs.oracle.com/otn/save-disk-space-on-linux-by-cloning-files-on-btrfs-and-ocfs2 +.. _pyreflink: https://reflink.readthedocs.io/en/latest/ + resume ~~~~~~ diff --git a/setup.py b/setup.py index 2c3cb2b55..41050307a 100755 --- a/setup.py +++ b/setup.py @@ -122,6 +122,7 @@ setup( 'pyxdg', 'responses>=0.3.0', 'requests_oauthlib', + 'reflink', ] + ( # Tests for the thumbnails plugin need pathlib on Python 2 too. ['pathlib'] if (sys.version_info < (3, 4, 0)) else [] @@ -163,6 +164,7 @@ setup( 'scrub': ['mutagen>=1.33'], 'bpd': ['PyGObject'], 'replaygain': ['PyGObject'], + 'reflink': ['reflink'], }, # Non-Python/non-PyPI plugin dependencies: # chroma: chromaprint or fpcalc diff --git a/test/_common.py b/test/_common.py index 8e3b1dd18..e44fac48b 100644 --- a/test/_common.py +++ b/test/_common.py @@ -25,6 +25,8 @@ import six import unittest from contextlib import contextmanager +import reflink + # Mangle the search path to include the beets sources. sys.path.insert(0, '..') @@ -55,6 +57,7 @@ _item_ident = 0 # OS feature test. HAVE_SYMLINK = sys.platform != 'win32' HAVE_HARDLINK = sys.platform != 'win32' +HAVE_REFLINK = reflink.supported_at(tempfile.gettempdir()) def item(lib=None): diff --git a/test/test_files.py b/test/test_files.py index 13a8b4407..ab82c192e 100644 --- a/test/test_files.py +++ b/test/test_files.py @@ -86,6 +86,24 @@ class MoveTest(_common.TestCase): self.i.move(operation=MoveOperation.COPY) self.assertExists(self.path) + def test_reflink_arrives(self): + self.i.move(operation=MoveOperation.REFLINK_AUTO) + self.assertExists(self.dest) + + def test_reflink_does_not_depart(self): + self.i.move(operation=MoveOperation.REFLINK_AUTO) + self.assertExists(self.path) + + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_force_reflink_arrives(self): + self.i.move(operation=MoveOperation.REFLINK) + self.assertExists(self.dest) + + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_force_reflink_does_not_depart(self): + self.i.move(operation=MoveOperation.REFLINK) + self.assertExists(self.path) + def test_move_changes_path(self): self.i.move() self.assertEqual(self.i.path, util.normpath(self.dest)) @@ -268,6 +286,17 @@ class AlbumFileTest(_common.TestCase): self.assertTrue(os.path.exists(oldpath)) self.assertTrue(os.path.exists(self.i.path)) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_albuminfo_move_reflinks_file(self): + oldpath = self.i.path + self.ai.album = u'newAlbumName' + self.ai.move(operation=MoveOperation.REFLINK) + self.ai.store() + self.i.load() + + self.assertTrue(os.path.exists(oldpath)) + self.assertTrue(os.path.exists(self.i.path)) + def test_albuminfo_move_to_custom_dir(self): self.ai.move(basedir=self.otherdir) self.i.load() @@ -549,6 +578,12 @@ class SafeMoveCopyTest(_common.TestCase): self.assertExists(self.dest) self.assertExists(self.path) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_successful_reflink(self): + util.reflink(self.path, self.dest) + self.assertExists(self.dest) + self.assertExists(self.path) + def test_unsuccessful_move(self): with self.assertRaises(util.FilesystemError): util.move(self.path, self.otherpath) @@ -557,6 +592,11 @@ class SafeMoveCopyTest(_common.TestCase): with self.assertRaises(util.FilesystemError): util.copy(self.path, self.otherpath) + @unittest.skipUnless(_common.HAVE_REFLINK, "need reflink") + def test_unsuccessful_reflink(self): + with self.assertRaises(util.FilesystemError): + util.reflink(self.path, self.otherpath) + def test_self_move(self): util.move(self.path, self.path) self.assertExists(self.path) diff --git a/test/test_keyfinder.py b/test/test_keyfinder.py index a9ac43a27..c8735e47f 100644 --- a/test/test_keyfinder.py +++ b/test/test_keyfinder.py @@ -76,6 +76,16 @@ class KeyFinderTest(unittest.TestCase, TestHelper): item.load() self.assertEqual(item['initial_key'], 'F') + def test_no_key(self, command_output): + item = Item(path='/file') + item.add(self.lib) + + command_output.return_value = util.CommandOutput(b"", b"") + self.run_command('keyfinder') + + item.load() + self.assertEqual(item['initial_key'], None) + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_parentwork.py b/test/test_parentwork.py index df6a98d79..b124785bb 100644 --- a/test/test_parentwork.py +++ b/test/test_parentwork.py @@ -20,12 +20,52 @@ from __future__ import division, absolute_import, print_function import os import unittest from test.helper import TestHelper +from mock import patch from beets.library import Item from beetsplug import parentwork -class ParentWorkTest(unittest.TestCase, TestHelper): +work = {'work': {'id': '1', + 'title': 'work', + 'work-relation-list': [{'type': 'parts', + 'direction': 'backward', + 'work': {'id': '2'}}], + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random'}}]}} +dp_work = {'work': {'id': '2', + 'title': 'directparentwork', + 'work-relation-list': [{'type': 'parts', + 'direction': 'backward', + 'work': {'id': '3'}}], + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random' + }}]}} +p_work = {'work': {'id': '3', + 'title': 'parentwork', + 'artist-relation-list': [{'type': 'composer', + 'artist': {'name': + 'random composer', + 'sort-name': + 'composer, random'}}]}} + + +def mock_workid_response(mbid, includes): + if mbid == '1': + return work + elif mbid == '2': + return dp_work + elif mbid == '3': + return p_work + + +class ParentWorkIntegrationTest(unittest.TestCase, TestHelper): def setUp(self): """Set up configuration""" self.setup_beets() @@ -35,12 +75,15 @@ class ParentWorkTest(unittest.TestCase, TestHelper): self.unload_plugins() self.teardown_beets() + # test how it works with real musicbrainz data @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') - def test_normal_case(self): + def test_normal_case_real(self): item = Item(path='/file', - mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53') + mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53') item.add(self.lib) self.run_command('parentwork') @@ -52,11 +95,13 @@ class ParentWorkTest(unittest.TestCase, TestHelper): @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') - def test_force(self): + def test_force_real(self): self.config['parentwork']['force'] = True item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-b8ebc18e8c53', - mb_parentworkid=u'XXX') + mb_parentworkid=u'XXX', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53', parentwork='whatever') item.add(self.lib) self.run_command('parentwork') @@ -68,10 +113,12 @@ class ParentWorkTest(unittest.TestCase, TestHelper): @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') - def test_no_force(self): - self.config['parentwork']['force'] = True + def test_no_force_real(self): + self.config['parentwork']['force'] = False item = Item(path='/file', mb_workid=u'e27bda6e-531e-36d3-9cd7-\ - b8ebc18e8c53', mb_parentworkid=u'XXX') + b8ebc18e8c53', mb_parentworkid=u'XXX', + parentwork_workid_current=u'e27bda6e-531e-36d3-9cd7-\ + b8ebc18e8c53', parentwork='whatever') item.add(self.lib) self.run_command('parentwork') @@ -85,7 +132,7 @@ class ParentWorkTest(unittest.TestCase, TestHelper): @unittest.skipUnless( os.environ.get('INTEGRATION_TEST', '0') == '1', 'integration testing not enabled') - def test_direct_parent_work(self): + def test_direct_parent_work_real(self): mb_workid = u'2e4a3668-458d-3b2a-8be2-0b08e0d8243a' self.assertEqual(u'f04b42df-7251-4d86-a5ee-67cfa49580d1', parentwork.direct_parent_id(mb_workid)[0]) @@ -93,6 +140,56 @@ class ParentWorkTest(unittest.TestCase, TestHelper): parentwork.work_parent_id(mb_workid)[0]) +class ParentWorkTest(unittest.TestCase, TestHelper): + def setUp(self): + """Set up configuration""" + self.setup_beets() + self.load_plugins('parentwork') + self.patcher = patch('musicbrainzngs.get_work_by_id', + side_effect=mock_workid_response) + self.patcher.start() + + def tearDown(self): + self.unload_plugins() + self.teardown_beets() + self.patcher.stop() + + def test_normal_case(self): + item = Item(path='/file', mb_workid='1', parentwork_workid_current='1') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], '3') + + def test_force(self): + self.config['parentwork']['force'] = True + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', + parentwork_workid_current='1', parentwork='parentwork') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], '3') + + def test_no_force(self): + self.config['parentwork']['force'] = False + item = Item(path='/file', mb_workid='1', mb_parentworkid=u'XXX', + parentwork_workid_current='1', parentwork='parentwork') + item.add(self.lib) + + self.run_command('parentwork') + + item.load() + self.assertEqual(item['mb_parentworkid'], u'XXX') + + def test_direct_parent_work(self): + self.assertEqual('2', parentwork.direct_parent_id('1')[0]) + self.assertEqual('3', parentwork.work_parent_id('1')[0]) + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 969f5c230..0100b520e 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -28,6 +28,7 @@ from mediafile import MediaFile from beetsplug.replaygain import (FatalGstreamerPluginReplayGainError, GStreamerBackend) + try: import gi gi.require_version('Gst', '1.0') @@ -55,12 +56,13 @@ def reset_replaygain(item): item['rg_album_gain'] = None item.write() item.store() + item.store() + item.store() class ReplayGainCliTestBase(TestHelper): - def setUp(self): - self.setup_beets() + self.setup_beets(disk=True) self.config['replaygain']['backend'] = self.backend try: