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 746a01355..9937cc7d0 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -179,12 +179,25 @@ class ReplayGainLdnsCliMalformedTest(TestHelper, unittest.TestCase): # 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 febacbc1b..ea0d70aa5 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -21,6 +21,7 @@ import collections 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 @@ Gain = collections.namedtuple("Gain", "gain peak") 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 @@ class Backend(object): """ 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 @@ class Bs1770gainBackend(Backend): 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 @@ class Bs1770gainBackend(Backend): 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 @@ class Bs1770gainBackend(Backend): 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 @@ class Bs1770gainBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): "-", ] - 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 @@ class FfmpegBackend(Backend): # 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 @@ class FfmpegBackend(Backend): )] ) # 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 @@ class FfmpegBackend(Backend): .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 @@ class CommandBackend(Backend): ) 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 @@ class CommandBackend(Backend): 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 @@ class CommandBackend(Backend): 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 @@ class CommandBackend(Backend): 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 @@ class GStreamerBackend(Backend): # 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 @@ class GStreamerBackend(Backend): 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 @@ class GStreamerBackend(Backend): 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 @@ class GStreamerBackend(Backend): 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 @@ class GStreamerBackend(Backend): 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 @@ class AudioToolsBackend(Backend): 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 _title_gain(self, rg, audiofile): + def _with_target_level(self, gain, target_level): + """Return `gain` relative to `target_level`. + + 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 @@ class AudioToolsBackend(Backend): 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 @@ class AudioToolsBackend(Backend): # 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 @@ class AudioToolsBackend(Backend): 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 @@ class AudioToolsBackend(Backend): # 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 @@ class ReplayGainPlugin(BeetsPlugin): '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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 ea0d70aa5..a0108b5a0 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -1164,10 +1164,11 @@ class ReplayGainPlugin(BeetsPlugin): '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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): ) 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 @@ class ReplayGainPlugin(BeetsPlugin): 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 9937cc7d0..8a0e3924b 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -143,6 +143,25 @@ class ReplayGainCliTestBase(TestHelper): 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 8a0e3924b..fe0515bee 100644 --- a/test/test_replaygain.py +++ b/test/test_replaygain.py @@ -87,6 +87,16 @@ class ReplayGainCliTestBase(TestHelper): 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 @@ class ReplayGainCliTestBase(TestHelper): 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 a68f3f7fc..fa120b35e 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 2b6740d48..3d55ab7e5 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 a0108b5a0..a618939b8 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -268,10 +268,11 @@ class Bs1770gainBackend(Backend): 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 @@ class FfmpegBackend(Backend): 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 @@ class FfmpegBackend(Backend): # 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 3d55ab7e5..a13f46794 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 fa120b35e..ea119167d 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 ---------------