From b9063a0240e1d9d53f5910b389767fd351b3a9be Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Mon, 22 Jul 2019 12:49:50 +0200 Subject: [PATCH 1/8] fix bs1770gain test This test caused other tests to fail due to missing cleanup. --- test/test_replaygain.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 746a01355a..9937cc7d0d 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -179,12 +179,25 @@ def setUp(self, call_patch): # Patch call to return nothing, bypassing the bs1770gain installation # check. - call_patch.return_value = None - self.load_plugins('replaygain') + call_patch.return_value = CommandOutput(stdout=b"", stderr=b"") + try: + self.load_plugins('replaygain') + except Exception: + import sys + exc_info = sys.exc_info() + try: + self.tearDown() + except Exception: + pass + six.reraise(exc_info[1], None, exc_info[2]) for item in self.add_album_fixture(2).items(): reset_replaygain(item) + def tearDown(self): + self.teardown_beets() + self.unload_plugins() + @patch('beetsplug.replaygain.call') def test_malformed_output(self, call_patch): # Return malformed XML (the ampersand should be &) From 0c8eead45978addf6337b81ddd9efe65c0786f33 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 27 Oct 2018 21:04:51 +0200 Subject: [PATCH 2/8] replaygain: pass target_level and peak to backends Configure the replaygain analysis by passing arguments to the Backends. This avoids the difference between ReplayGain and EBU r128 backends; every Backend can now fulfil both tasks. Additionally it eases Backend development as the difference between the two tag formats is now completely handled in the main Plugin, not in the Backends. --- beetsplug/replaygain.py | 278 +++++++++++++++++++++++----------------- 1 file changed, 161 insertions(+), 117 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index febacbc1be..ea0d70aa55 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -21,6 +21,7 @@ import math import sys import warnings +import enum import xml.parsers.expat from six.moves import zip @@ -74,6 +75,15 @@ def db_to_lufs(db): return db - 107 +def lufs_to_db(db): + """Convert LUFS to db. + + According to https://wiki.hydrogenaud.io/index.php?title= + ReplayGain_2.0_specification#Reference_level + """ + return db + 107 + + # Backend base and plumbing classes. # gain: in LU to reference level @@ -84,6 +94,12 @@ def db_to_lufs(db): AlbumGain = collections.namedtuple("AlbumGain", "album_gain track_gains") +class Peak(enum.Enum): + none = 0 + true = 1 + sample = 2 + + class Backend(object): """An abstract class representing engine for calculating RG values. """ @@ -94,22 +110,18 @@ def __init__(self, config, log): """ self._log = log - def compute_track_gain(self, items): + def compute_track_gain(self, items, target_level, peak): """Computes the track gain of the given tracks, returns a list of Gain objects. """ raise NotImplementedError() - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): """Computes the album gain of the given album, returns an AlbumGain object. """ raise NotImplementedError() - def use_ebu_r128(self): - """Set this Backend up to use EBU R128.""" - pass - # bsg1770gain backend class Bs1770gainBackend(Backend): @@ -117,44 +129,51 @@ class Bs1770gainBackend(Backend): its flavors EBU R128, ATSC A/85 and Replaygain 2.0. """ + methods = { + -24: "atsc", + -23: "ebu", + -18: "replaygain", + } + def __init__(self, config, log): super(Bs1770gainBackend, self).__init__(config, log) config.add({ 'chunk_at': 5000, - 'method': 'replaygain', + 'method': '', }) self.chunk_at = config['chunk_at'].as_number() - self.method = '--' + config['method'].as_str() + # backward compatibility to `method` config option + self.__method = config['method'].as_str() cmd = 'bs1770gain' try: - call([cmd, self.method]) + call([cmd, "--help"]) self.command = cmd except OSError: raise FatalReplayGainError( - u'Is bs1770gain installed? Is your method in config correct?' + u'Is bs1770gain installed?' ) if not self.command: raise FatalReplayGainError( u'no replaygain command found: install bs1770gain' ) - def compute_track_gain(self, items): + def compute_track_gain(self, items, target_level, peak): """Computes the track gain of the given tracks, returns a list of TrackGain objects. """ - output = self.compute_gain(items, False) + output = self.compute_gain(items, target_level, False) return output - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): """Computes the album gain of the given album, returns an AlbumGain object. """ # TODO: What should be done when not all tracks in the album are # supported? - output = self.compute_gain(items, True) + output = self.compute_gain(items, target_level, True) if not output: raise ReplayGainError(u'no output from bs1770gain') @@ -179,7 +198,7 @@ def isplitter(self, items, chunk_at): else: break - def compute_gain(self, items, is_album): + def compute_gain(self, items, target_level, is_album): """Computes the track or album gain of a list of items, returns a list of TrackGain objects. When computing album gain, the last TrackGain object returned is @@ -200,22 +219,38 @@ def compute_gain(self, items, is_album): i = 0 for chunk in self.isplitter(items, self.chunk_at): i += 1 - returnchunk = self.compute_chunk_gain(chunk, is_album) + returnchunk = self.compute_chunk_gain( + chunk, + is_album, + target_level + ) albumgaintot += returnchunk[-1].gain albumpeaktot = max(albumpeaktot, returnchunk[-1].peak) returnchunks = returnchunks + returnchunk[0:-1] returnchunks.append(Gain(albumgaintot / i, albumpeaktot)) return returnchunks else: - return self.compute_chunk_gain(items, is_album) + return self.compute_chunk_gain(items, is_album, target_level) - def compute_chunk_gain(self, items, is_album): + def compute_chunk_gain(self, items, is_album, target_level): """Compute ReplayGain values and return a list of results dictionaries as given by `parse_tool_output`. """ + # choose method + target_level = db_to_lufs(target_level) + if self.__method != "": + # backward compatibility to `method` option + method = self.__method + elif target_level in self.methods: + method = self.methods[target_level] + gain_adjustment = 0 + else: + method = self.methods[-23] + gain_adjustment = target_level - lufs_to_db(-23) + # Construct shell command. cmd = [self.command] - cmd += [self.method] + cmd += ["--" + method] cmd += ['--xml', '-p'] # Workaround for Windows: the underlying tool fails on paths @@ -232,6 +267,12 @@ def compute_chunk_gain(self, items, is_album): self._log.debug(u'analysis finished: {0}', output) results = self.parse_tool_output(output, path_list, is_album) + + if gain_adjustment != 0: + for i in range(len(results)): + orig = results[i] + results[i] = Gain(orig.gain + gain_adjustment, orig.peak) + self._log.debug(u'{0} items, {1} results', len(items), len(results)) return results @@ -299,10 +340,6 @@ def end_element_handler(name): out.append(album_gain["album"]) return out - def use_ebu_r128(self): - """Set this Backend up to use EBU R128.""" - self.method = '--ebu' - # ffmpeg backend class FfmpegBackend(Backend): @@ -310,11 +347,6 @@ class FfmpegBackend(Backend): """ def __init__(self, config, log): super(FfmpegBackend, self).__init__(config, log) - config.add({ - "peak": "true" - }) - self._peak_method = config["peak"].as_str() - self._target_level = db_to_lufs(config['targetlevel'].as_number()) self._ffmpeg_path = "ffmpeg" # check that ffmpeg is installed @@ -341,18 +373,7 @@ def __init__(self, config, log): u"the --enable-libebur128 configuration option is required." ) - # check that peak_method is valid - valid_peak_method = "true", "sample" - if self._peak_method not in valid_peak_method: - raise ui.UserError( - u"Selected ReplayGain peak method {0} is not supported. " - u"Please select one of: {1}".format( - self._peak_method, - u', '.join(valid_peak_method) - ) - ) - - def compute_track_gain(self, items): + def compute_track_gain(self, items, target_level, peak): """Computes the track gain of the given tracks, returns a list of Gain objects (the track gains). """ @@ -361,15 +382,19 @@ def compute_track_gain(self, items): gains.append( self._analyse_item( item, + target_level, + peak, count_blocks=False, )[0] # take only the gain, discarding number of gating blocks ) return gains - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): """Computes the album gain of the given album, returns an AlbumGain object. """ + target_level_lufs = db_to_lufs(target_level) + # analyse tracks # list of track Gain objects track_gains = [] @@ -381,8 +406,9 @@ def compute_album_gain(self, items): n_blocks = 0 for item in items: - track_gain, track_n_blocks = self._analyse_item(item) - + track_gain, track_n_blocks = self._analyse_item( + item, target_level, peak + ) track_gains.append(track_gain) # album peak is maximum track peak @@ -393,7 +419,7 @@ def compute_album_gain(self, items): n_blocks += track_n_blocks # convert `LU to target_level` -> LUFS - track_loudness = self._target_level - track_gain.gain + track_loudness = target_level_lufs - track_gain.gain # This reverses ITU-R BS.1770-4 p. 6 equation (5) to convert # from loudness to power. The result is the average gating # block power. @@ -412,7 +438,7 @@ def compute_album_gain(self, items): else: album_gain = -70 # convert LUFS -> `LU to target_level` - album_gain = self._target_level - album_gain + album_gain = target_level_lufs - album_gain self._log.debug( u"{0}: gain {1} LU, peak {2}" @@ -436,16 +462,23 @@ def _construct_cmd(self, item, peak_method): "-", ] - def _analyse_item(self, item, count_blocks=True): + def _analyse_item(self, item, target_level, peak, count_blocks=True): """Analyse item. Return a pair of a Gain object and the number of gating blocks above the threshold. If `count_blocks` is False, the number of gating blocks returned will be 0. """ + target_level_lufs = db_to_lufs(target_level) + peak_method = { + Peak.none: "none", + Peak.true: "true", + Peak.sample: "sample", + }[peak] + # call ffmpeg self._log.debug(u"analyzing {0}".format(item)) - cmd = self._construct_cmd(item, self._peak_method) + cmd = self._construct_cmd(item, peak_method) self._log.debug( u'executing {0}', u' '.join(map(displayable_path, cmd)) ) @@ -453,12 +486,12 @@ def _analyse_item(self, item, count_blocks=True): # parse output - if self._peak_method == "none": + if peak is Peak.none: peak = 0 else: line_peak = self._find_line( output, - " {0} peak:".format(self._peak_method.capitalize()).encode(), + " {0} peak:".format(peak_method.capitalize()).encode(), start_line=len(output) - 1, step_size=-1, ) peak = self._parse_float( @@ -481,7 +514,7 @@ def _analyse_item(self, item, count_blocks=True): )] ) # convert LUFS -> LU from target level - gain = self._target_level - gain + gain = target_level_lufs - gain # count BS.1770 gating blocks n_blocks = 0 @@ -553,11 +586,6 @@ def _parse_float(self, line): .format(value) ) - def use_ebu_r128(self): - """Set this Backend up to use EBU R128.""" - self._target_level = -23 - self._peak_method = "none" # R128 tags do not need peak - # mpgain/aacgain CLI tool backend. class CommandBackend(Backend): @@ -592,18 +620,16 @@ def __init__(self, config, log): ) self.noclip = config['noclip'].get(bool) - target_level = config['targetlevel'].as_number() - self.gain_offset = int(target_level - 89) - def compute_track_gain(self, items): + def compute_track_gain(self, items, target_level, peak): """Computes the track gain of the given tracks, returns a list of TrackGain objects. """ supported_items = list(filter(self.format_supported, items)) - output = self.compute_gain(supported_items, False) + output = self.compute_gain(supported_items, target_level, False) return output - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): """Computes the album gain of the given album, returns an AlbumGain object. """ @@ -615,7 +641,7 @@ def compute_album_gain(self, items): self._log.debug(u'tracks are of unsupported format') return AlbumGain(None, []) - output = self.compute_gain(supported_items, True) + output = self.compute_gain(supported_items, target_level, True) return AlbumGain(output[-1], output[:-1]) def format_supported(self, item): @@ -627,7 +653,7 @@ def format_supported(self, item): return False return True - def compute_gain(self, items, is_album): + def compute_gain(self, items, target_level, is_album): """Computes the track or album gain of a list of items, returns a list of TrackGain objects. @@ -654,7 +680,7 @@ def compute_gain(self, items, is_album): else: # Disable clipping warning. cmd = cmd + ['-c'] - cmd = cmd + ['-d', str(self.gain_offset)] + cmd = cmd + ['-d', str(int(target_level - 89))] cmd = cmd + [syspath(i.path) for i in items] self._log.debug(u'analyzing {0} files', len(items)) @@ -717,8 +743,6 @@ def __init__(self, config, log): # to rganalsys should have their gain computed, even if it # already exists. self._rg.set_property("forced", True) - self._rg.set_property("reference-level", - config["targetlevel"].as_number()) self._sink = self.Gst.ElementFactory.make("fakesink", "sink") self._pipe = self.Gst.Pipeline() @@ -779,7 +803,7 @@ def _import_gst(self): self.GLib = GLib self.Gst = Gst - def compute(self, files, album): + def compute(self, files, target_level, album): self._error = None self._files = list(files) @@ -788,6 +812,8 @@ def compute(self, files, album): self._file_tags = collections.defaultdict(dict) + self._rg.set_property("reference-level", target_level) + if album: self._rg.set_property("num-tracks", len(self._files)) @@ -796,8 +822,8 @@ def compute(self, files, album): if self._error is not None: raise self._error - def compute_track_gain(self, items): - self.compute(items, False) + def compute_track_gain(self, items, target_level, peak): + self.compute(items, target_level, False) if len(self._file_tags) != len(items): raise ReplayGainError(u"Some tracks did not receive tags") @@ -808,9 +834,9 @@ def compute_track_gain(self, items): return ret - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): items = list(items) - self.compute(items, True) + self.compute(items, target_level, True) if len(self._file_tags) != len(items): raise ReplayGainError(u"Some items in album did not receive tags") @@ -1024,14 +1050,21 @@ def init_replaygain(self, audiofile, item): return return rg - def compute_track_gain(self, items): + def compute_track_gain(self, items, target_level, peak): """Compute ReplayGain values for the requested items. :return list: list of :class:`Gain` objects """ - return [self._compute_track_gain(item) for item in items] + return [self._compute_track_gain(item, target_level) for item in items] + + def _with_target_level(self, gain, target_level): + """Return `gain` relative to `target_level`. - def _title_gain(self, rg, audiofile): + Assumes `gain` is relative to 89 db. + """ + return gain + (target_level - 89) + + def _title_gain(self, rg, audiofile, target_level): """Get the gain result pair from PyAudioTools using the `ReplayGain` instance `rg` for the given `audiofile`. @@ -1041,14 +1074,15 @@ def _title_gain(self, rg, audiofile): try: # The method needs an audiotools.PCMReader instance that can # be obtained from an audiofile instance. - return rg.title_gain(audiofile.to_pcm()) + gain, peak = rg.title_gain(audiofile.to_pcm()) except ValueError as exc: # `audiotools.replaygain` can raise a `ValueError` if the sample # rate is incorrect. self._log.debug(u'error in rg.title_gain() call: {}', exc) raise ReplayGainError(u'audiotools audio data error') + return self._with_target_level(gain, target_level), peak - def _compute_track_gain(self, item): + def _compute_track_gain(self, item, target_level): """Compute ReplayGain value for the requested item. :rtype: :class:`Gain` @@ -1058,13 +1092,15 @@ def _compute_track_gain(self, item): # Each call to title_gain on a ReplayGain object returns peak and gain # of the track. - rg_track_gain, rg_track_peak = self._title_gain(rg, audiofile) + rg_track_gain, rg_track_peak = self._title_gain( + rg, audiofile, target_level + ) self._log.debug(u'ReplayGain for track {0} - {1}: {2:.2f}, {3:.2f}', item.artist, item.title, rg_track_gain, rg_track_peak) return Gain(gain=rg_track_gain, peak=rg_track_peak) - def compute_album_gain(self, items): + def compute_album_gain(self, items, target_level, peak): """Compute ReplayGain values for the requested album and its items. :rtype: :class:`AlbumGain` @@ -1079,7 +1115,9 @@ def compute_album_gain(self, items): track_gains = [] for item in items: audiofile = self.open_audio_file(item) - rg_track_gain, rg_track_peak = self._title_gain(rg, audiofile) + rg_track_gain, rg_track_peak = self._title_gain( + rg, audiofile, target_level + ) track_gains.append( Gain(gain=rg_track_gain, peak=rg_track_peak) ) @@ -1089,6 +1127,7 @@ def compute_album_gain(self, items): # After getting the values for all tracks, it's possible to get the # album values. rg_album_gain, rg_album_peak = rg.album_gain() + rg_album_gain = self._with_target_level(rg_album_gain, target_level) self._log.debug(u'ReplayGain for album {0}: {1:.2f}, {2:.2f}', items[0].album, rg_album_gain, rg_album_peak) @@ -1112,7 +1151,10 @@ class ReplayGainPlugin(BeetsPlugin): "ffmpeg": FfmpegBackend, } - r128_backend_names = ["bs1770gain", "ffmpeg"] + peak_methods = { + "true": Peak.true, + "sample": Peak.sample, + } def __init__(self): super(ReplayGainPlugin, self).__init__() @@ -1124,7 +1166,8 @@ def __init__(self): 'backend': u'command', 'targetlevel': 89, 'r128': ['Opus'], - 'per_disc': False + 'per_disc': False, + "peak": "true", }) self.overwrite = self.config['overwrite'].get(bool) @@ -1138,6 +1181,16 @@ def __init__(self): u', '.join(self.backends.keys()) ) ) + peak_method = self.config["peak"].as_str() + if peak_method not in self.peak_methods: + raise ui.UserError( + u"Selected ReplayGain peak method {0} is not supported. " + u"Please select one of: {1}".format( + peak_method, + u', '.join(self.peak_methods.keys()) + ) + ) + self._peak_method = self.peak_methods[peak_method] # On-import analysis. if self.config['auto']: @@ -1154,8 +1207,6 @@ def __init__(self): raise ui.UserError( u'replaygain initialization failed: {0}'.format(e)) - self.r128_backend_instance = '' - def should_use_r128(self, item): """Checks the plugin setting to decide whether the calculation should be done using the EBU R128 standard and use R128_ tags instead. @@ -1208,6 +1259,20 @@ def store_album_r128_gain(self, item, album_gain): self._log.debug(u'applied r128 album gain {0} LU', item.r128_album_gain) + def tag_specific_values(self, items): + """Return some tag specific values. + + Returns a tuple (store_track_gain, store_album_gain). + """ + if any([self.should_use_r128(item) for item in items]): + store_track_gain = self.store_track_r128_gain + store_album_gain = self.store_album_r128_gain + else: + store_track_gain = self.store_track_gain + store_album_gain = self.store_album_gain + + return store_track_gain, store_album_gain + def handle_album(self, album, write, force=False): """Compute album and track replay gain store it in all of the album's items. @@ -1229,16 +1294,8 @@ def handle_album(self, album, write, force=False): u" for some tracks in album {0}".format(album) ) - if any([self.should_use_r128(item) for item in album.items()]): - if self.r128_backend_instance == '': - self.init_r128_backend() - backend_instance = self.r128_backend_instance - store_track_gain = self.store_track_r128_gain - store_album_gain = self.store_album_r128_gain - else: - backend_instance = self.backend_instance - store_track_gain = self.store_track_gain - store_album_gain = self.store_album_gain + tag_vals = self.tag_specific_values(album.items()) + store_track_gain, store_album_gain = tag_vals discs = dict() if self.per_disc: @@ -1251,7 +1308,11 @@ def handle_album(self, album, write, force=False): for discnumber, items in discs.items(): try: - album_gain = backend_instance.compute_album_gain(items) + album_gain = self.backend_instance.compute_album_gain( + items, + self.config['targetlevel'].as_number(), + self._peak_method, + ) if len(album_gain.track_gains) != len(items): raise ReplayGainError( u"ReplayGain backend failed " @@ -1282,17 +1343,15 @@ def handle_track(self, item, write, force=False): self._log.info(u'analyzing {0}', item) - if self.should_use_r128(item): - if self.r128_backend_instance == '': - self.init_r128_backend() - backend_instance = self.r128_backend_instance - store_track_gain = self.store_track_r128_gain - else: - backend_instance = self.backend_instance - store_track_gain = self.store_track_gain + tag_vals = self.tag_specific_values([item]) + store_track_gain, store_album_gain = tag_vals try: - track_gains = backend_instance.compute_track_gain([item]) + track_gains = self.backend_instance.compute_track_gain( + [item], + self.config['targetlevel'].as_number(), + self._peak_method, + ) if len(track_gains) != 1: raise ReplayGainError( u"ReplayGain backend failed for track {0}".format(item) @@ -1307,21 +1366,6 @@ def handle_track(self, item, write, force=False): raise ui.UserError( u"Fatal replay gain error: {0}".format(e)) - def init_r128_backend(self): - backend_name = self.config["backend"].as_str() - if backend_name not in self.r128_backend_names: - backend_name = "bs1770gain" - - try: - self.r128_backend_instance = self.backends[backend_name]( - self.config, self._log - ) - except (ReplayGainError, FatalReplayGainError) as e: - raise ui.UserError( - u'replaygain initialization failed: {0}'.format(e)) - - self.r128_backend_instance.use_ebu_r128() - def imported(self, session, task): """Add replay gain info to items or albums of ``task``. """ From e7e2c424e7dd0efb202c9550ce35438e7915d979 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Mon, 22 Jul 2019 13:20:28 +0200 Subject: [PATCH 3/8] replaygain: targetlevel and peak_method depends on tag format Allow to configure the target level for R128_* tags separately from REPLAYGAIN_* tags and skip peak calculation for R128_* tags if possible. --- beetsplug/replaygain.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index ea0d70aa55..a0108b5a0c 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1164,10 +1164,11 @@ def __init__(self): 'overwrite': False, 'auto': True, 'backend': u'command', + 'per_disc': False, + 'peak': 'true', 'targetlevel': 89, 'r128': ['Opus'], - 'per_disc': False, - "peak": "true", + 'r128_targetlevel': lufs_to_db(-23), }) self.overwrite = self.config['overwrite'].get(bool) @@ -1262,16 +1263,21 @@ def store_album_r128_gain(self, item, album_gain): def tag_specific_values(self, items): """Return some tag specific values. - Returns a tuple (store_track_gain, store_album_gain). + Returns a tuple (store_track_gain, store_album_gain, target_level, + peak_method). """ if any([self.should_use_r128(item) for item in items]): store_track_gain = self.store_track_r128_gain store_album_gain = self.store_album_r128_gain + target_level = self.config['r128_targetlevel'].as_number() + peak = Peak.none # R128_* tags do not store the track/album peak else: store_track_gain = self.store_track_gain store_album_gain = self.store_album_gain + target_level = self.config['targetlevel'].as_number() + peak = self._peak_method - return store_track_gain, store_album_gain + return store_track_gain, store_album_gain, target_level, peak def handle_album(self, album, write, force=False): """Compute album and track replay gain store it in all of the @@ -1295,7 +1301,7 @@ def handle_album(self, album, write, force=False): ) tag_vals = self.tag_specific_values(album.items()) - store_track_gain, store_album_gain = tag_vals + store_track_gain, store_album_gain, target_level, peak = tag_vals discs = dict() if self.per_disc: @@ -1309,9 +1315,7 @@ def handle_album(self, album, write, force=False): for discnumber, items in discs.items(): try: album_gain = self.backend_instance.compute_album_gain( - items, - self.config['targetlevel'].as_number(), - self._peak_method, + items, target_level, peak ) if len(album_gain.track_gains) != len(items): raise ReplayGainError( @@ -1344,13 +1348,11 @@ def handle_track(self, item, write, force=False): self._log.info(u'analyzing {0}', item) tag_vals = self.tag_specific_values([item]) - store_track_gain, store_album_gain = tag_vals + store_track_gain, store_album_gain, target_level, peak = tag_vals try: track_gains = self.backend_instance.compute_track_gain( - [item], - self.config['targetlevel'].as_number(), - self._peak_method, + [item], target_level, peak ) if len(track_gains) != 1: raise ReplayGainError( From 5a8bdb67f7f2d24ac5b1af3fb69b3436a09e6d79 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 27 Oct 2018 22:39:45 +0200 Subject: [PATCH 4/8] replaygain: add target_level test Assert that analysing the same track with different target levels yields different gain adjustments. --- test/test_replaygain.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 9937cc7d0d..8a0e3924b2 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -143,6 +143,25 @@ def test_cli_saves_album_gain_to_file(self): self.assertNotEqual(max(gains), 0.0) self.assertNotEqual(max(peaks), 0.0) + def test_target_level_has_effect(self): + item = self.lib.items()[0] + + def analyse(target_level): + self.config['replaygain']['targetlevel'] = target_level + self._reset_replaygain(item) + self.run_command(u'replaygain', '-f') + mediafile = MediaFile(item.path) + return mediafile.rg_track_gain + + gain_relative_to_84 = analyse(84) + gain_relative_to_89 = analyse(89) + + # check that second calculation did work + if gain_relative_to_84 is not None: + self.assertIsNotNone(gain_relative_to_89) + + self.assertNotEqual(gain_relative_to_84, gain_relative_to_89) + @unittest.skipIf(not GST_AVAILABLE, u'gstreamer cannot be found') class ReplayGainGstCliTest(ReplayGainCliTestBase, unittest.TestCase): From 88ab5474c597d441e883d80dc47f6fe1fbe7c4e0 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 27 Oct 2018 22:49:21 +0200 Subject: [PATCH 5/8] replaygain: add R128_* tag test Assert that the replaygain plugin does not write REPLAYGAIN_* tags but R128_* tags, when instructed to do so. This test is skipped for the `command` backend as it does not support OPUS. --- test/test_replaygain.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/test_replaygain.py b/test/test_replaygain.py index 8a0e3924b2..fe0515bee5 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -87,6 +87,16 @@ def tearDown(self): self.teardown_beets() self.unload_plugins() + def _reset_replaygain(self, item): + item['rg_track_peak'] = None + item['rg_track_gain'] = None + item['rg_album_peak'] = None + item['rg_album_gain'] = None + item['r128_track_gain'] = None + item['r128_album_gain'] = None + item.write() + item.store() + def test_cli_saves_track_gain(self): for item in self.lib.items(): self.assertIsNone(item.rg_track_peak) @@ -143,6 +153,26 @@ def test_cli_saves_album_gain_to_file(self): self.assertNotEqual(max(gains), 0.0) self.assertNotEqual(max(peaks), 0.0) + def test_cli_writes_only_r128_tags(self): + if self.backend == "command": + # opus not supported by command backend + return + + album = self.add_album_fixture(2, ext="opus") + for item in album.items(): + self._reset_replaygain(item) + + self.run_command(u'replaygain', u'-a') + + for item in album.items(): + mediafile = MediaFile(item.path) + # does not write REPLAYGAIN_* tags + self.assertIsNone(mediafile.rg_track_gain) + self.assertIsNone(mediafile.rg_album_gain) + # writes R128_* tags + self.assertIsNotNone(mediafile.r128_track_gain) + self.assertIsNotNone(mediafile.r128_album_gain) + def test_target_level_has_effect(self): item = self.lib.items()[0] From fbc8cc484de67c497036a4f68302d8c0eb56b221 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Sat, 27 Oct 2018 21:30:18 +0200 Subject: [PATCH 6/8] update replaygain target level documentation - document `r128_targetlevel` - explain difference between `targetlevel` and `r128_targetlevel` - deprecate `method` option: use `targetlevel` instead. --- docs/plugins/replaygain.rst | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index a68f3f7fc0..fa120b35e5 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -96,8 +96,13 @@ configuration file. The available options are: Default: ``command``. - **overwrite**: Re-analyze files that already have ReplayGain tags. Default: ``no``. -- **targetlevel**: A number of decibels for the target loudness level. - Default: 89. +- **targetlevel**: A number of decibels for the target loudness level for files + using ``REPLAYGAIN_`` tags. + Default: ``89``. +- **r128_targetlevel**: The target loudness level in decibels (i.e. + `` + 107``) for files using ``R128_`` tags. + Default: 84 (Use ``83`` for ATSC A/85, ``84`` for EBU R128 or ``89`` for + ReplayGain 2.0.) - **r128**: A space separated list of formats that will use ``R128_`` tags with integer values instead of the common ``REPLAYGAIN_`` tags with floating point values. Requires the "ffmpeg" backend. @@ -120,6 +125,13 @@ This option only works with the "ffmpeg" backend: - **peak**: Either ``true`` (the default) or ``sample``. ``true`` is more accurate but slower. +This option is deprecated: + +- **method**: The loudness scanning standard: either `replaygain` for + ReplayGain 2.0, `ebu` for EBU R128, or `atsc` for ATSC A/85. This dictates + the reference level: -18, -23, or -24 LUFS respectively. Only supported by + "bs1770gain" backend. + Manual Analysis --------------- From da602d779ce2b67101bfd72e2d9d266a6069e020 Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Wed, 7 Nov 2018 19:48:22 +0100 Subject: [PATCH 7/8] changelog: new `r128_targetlevel` option Add changelog entries for the introduction of the `r128_targetlevel` configuration option, superseding the `method` option. --- docs/changelog.rst | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2b6740d483..3d55ab7e57 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -42,6 +42,13 @@ New features: new ``discogs_albumid`` field from the Discogs API. Thanks to :user:`thedevilisinthedetails`. :bug:`465` :bug:`3322` +* :doc:`plugins/replaygain`: ``r128_targetlevel`` is a new configuration option + for the ReplayGain plugin: It defines the reference volume for files using + ``R128_`` tags. ``targtelevel`` only configures the reference volume for + ``REPLAYGAIN_`` files. + This also deprecates the ``bs1770gain`` ReplayGain backend's ``method`` + option. Use ``targetlevel`` and ``r128_targetlevel`` instead. + :bug:`3065` Fixes: @@ -86,6 +93,10 @@ For plugin developers: * There were sporadic failures in ``test.test_player``. Hopefully these are fixed. If they resurface, please reopen the relevant issue. :bug:`3309` :bug:`3330` +* The internal structure of the replaygain plugin had some changes: There are no + longer separate R128 backend instances. Instead the targetlevel is passed to + ``compute_album_gain`` and ``compute_track_gain``. + :bug:`3065` For packagers: From a9f70f81518903b26968056cc81b03589a90e10c Mon Sep 17 00:00:00 2001 From: Zsin Skri Date: Thu, 25 Jul 2019 23:19:10 +0200 Subject: [PATCH 8/8] apply suggested improvements Apply improvements suggested in GitHub PullRequest #3065: - be idiomatic - 0 is falsy - check enum equality, not identity - mutate list by constructing a new one - improve documentation - fix a typo - do not mention deprecation of a config option --- beetsplug/replaygain.py | 17 +++++++---------- docs/changelog.rst | 2 +- docs/plugins/replaygain.rst | 7 ------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index a0108b5a0c..a618939b83 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -268,10 +268,11 @@ def compute_chunk_gain(self, items, is_album, target_level): self._log.debug(u'analysis finished: {0}', output) results = self.parse_tool_output(output, path_list, is_album) - if gain_adjustment != 0: - for i in range(len(results)): - orig = results[i] - results[i] = Gain(orig.gain + gain_adjustment, orig.peak) + if gain_adjustment: + results = [ + Gain(res.gain + gain_adjustment, res.peak) + for res in results + ] self._log.debug(u'{0} items, {1} results', len(items), len(results)) return results @@ -470,11 +471,7 @@ def _analyse_item(self, item, target_level, peak, count_blocks=True): will be 0. """ target_level_lufs = db_to_lufs(target_level) - peak_method = { - Peak.none: "none", - Peak.true: "true", - Peak.sample: "sample", - }[peak] + peak_method = peak.name # call ffmpeg self._log.debug(u"analyzing {0}".format(item)) @@ -486,7 +483,7 @@ def _analyse_item(self, item, target_level, peak, count_blocks=True): # parse output - if peak is Peak.none: + if peak == Peak.none: peak = 0 else: line_peak = self._find_line( diff --git a/docs/changelog.rst b/docs/changelog.rst index 3d55ab7e57..a13f467949 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -44,7 +44,7 @@ New features: :bug:`465` :bug:`3322` * :doc:`plugins/replaygain`: ``r128_targetlevel`` is a new configuration option for the ReplayGain plugin: It defines the reference volume for files using - ``R128_`` tags. ``targtelevel`` only configures the reference volume for + ``R128_`` tags. ``targetlevel`` only configures the reference volume for ``REPLAYGAIN_`` files. This also deprecates the ``bs1770gain`` ReplayGain backend's ``method`` option. Use ``targetlevel`` and ``r128_targetlevel`` instead. diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index fa120b35e5..ea119167dc 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -125,13 +125,6 @@ This option only works with the "ffmpeg" backend: - **peak**: Either ``true`` (the default) or ``sample``. ``true`` is more accurate but slower. -This option is deprecated: - -- **method**: The loudness scanning standard: either `replaygain` for - ReplayGain 2.0, `ebu` for EBU R128, or `atsc` for ATSC A/85. This dictates - the reference level: -18, -23, or -24 LUFS respectively. Only supported by - "bs1770gain" backend. - Manual Analysis ---------------