diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e2b74cd87b..748de174bc 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -682,70 +682,110 @@ def format_supported(self, item: Item) -> bool: def compute_track_gain(self, task: AnyRgTask) -> AnyRgTask: """Compute the track gain for each FLAC item in the task.""" - track_gains = [] - for item in filter(self.format_supported, task.items): - self._add_replay_gain([item]) - track_gains.append( - self._read_gain(item, "TRACK", task.target_level) - ) + items = list(filter(self.format_supported, task.items)) + + if not items: + task.track_gains = None + return task + + results = self._read_gain(items, task.target_level) + + track_gains: list[Gain] = [] + for item in items: + track_gain_value = results[item][1] + track_gains.append(track_gain_value) + task.track_gains = track_gains return task def compute_album_gain(self, task: AnyRgTask) -> AnyRgTask: """Compute the album gain and per-track gains for the FLAC items.""" items = list(task.items) - if not items or not all(self.format_supported(i) for i in items): + supported_items = list(filter(self.format_supported, items)) + + if not items or len(supported_items) != len(items): task.album_gain = None task.track_gains = None return task - self._add_replay_gain(items) - task.track_gains = [ - self._read_gain(item, "TRACK", task.target_level) for item in items - ] - task.album_gain = self._read_gain(items[0], "ALBUM", task.target_level) + results = self._read_gain(items, task.target_level) + + track_gains: list[Gain] = [] + for item in items: + track_gain_value = results[item][1] + track_gains.append(track_gain_value) + + album_gain = results[items[0]][0] + + task.album_gain = album_gain + task.track_gains = track_gains return task - def _add_replay_gain(self, items: Sequence[Item]) -> None: - """Run ``metaflac --add-replay-gain`` on the given files.""" + def _read_gain( + self, items: Sequence[Item], target_level: float + ) -> dict[Item, tuple[Gain, Gain]]: + """Run ``metaflac --scan-replay-gain`` on the given files""" paths = [str(item.filepath) for item in items] - call([self.command, "--add-replay-gain", *paths], self._log) - def _read_gain(self, item: Item, kind: str, target_level: float) -> Gain: - """Read the REPLAYGAIN gain and peak tags back from a file.""" - gain_tag = f"REPLAYGAIN_{kind}_GAIN" - peak_tag = f"REPLAYGAIN_{kind}_PEAK" - command = [ - self.command, - f"--show-tag={gain_tag}", - f"--show-tag={peak_tag}", - str(item.filepath), - ] - tags = self._parse_tags(call(command, self._log).stdout) try: - gain = self._parse_gain(tags[gain_tag]) - peak = float(tags[peak_tag]) - except (KeyError, IndexError, ValueError) as exc: - raise ReplayGainError( - f"could not read metaflac replaygain tags for {item}: {exc!r}" + output = call( + [self.command, "--scan-replay-gain", *paths], self._log + ).stdout.decode("utf-8", "ignore") + except ReplayGainError as exc: + raise FatalReplayGainError( + f"metaflac --scan-replay-gain failed" + f" (you might need to update metaflac): {exc!r}" ) - # metaflac uses an 89 dB reference, like the other backends - return Gain(gain=gain + (target_level - 89.0), peak=peak) - @staticmethod - def _parse_tags(output: bytes) -> dict[str, str]: - """Turn metaflac's NAME=VALUE output into a dict.""" - tags: dict[str, str] = {} - for line in output.decode("utf-8", "ignore").splitlines(): - name, sep, value = line.partition("=") - if sep: - tags[name.strip().upper()] = value.strip() - return tags + gain_by_path = self._parse_output(output) + + results: dict[Item, tuple[Gain, Gain]] = {} + for item in items: + path = str(item.filepath) + + try: + album_gain, album_peak, track_gain, track_peak = gain_by_path[ + path + ] + except KeyError as exc: + raise ReplayGainError( + f"metaflac output missing replaygain values for {path!r}: {exc!r}" + ) + + # metaflac uses an 89 dB reference, like the other backends + offset = target_level - 89.0 + + results[item] = ( + Gain(gain=album_gain + offset, peak=album_peak), + Gain(gain=track_gain + offset, peak=track_peak), + ) + return results @staticmethod - def _parse_gain(value: str) -> float: - """Turn a '-7.89 dB' tag value into a float.""" - return float(value.split()[0]) + def _parse_output( + text: str, + ) -> dict[str, tuple[float, float, float, float]]: + """Parse the output of ``metaflac --scan-replay-gain``.""" + out: dict[str, tuple[float, float, float, float]] = {} + + for line in text.splitlines(): + path, sep, values = line.partition(": ") + + if not sep: + continue + + try: + album_gain, album_peak, track_gain, track_peak = ( + float(v) for v in values.split() + ) + except ValueError as exc: + raise ReplayGainError( + f"could not parse metaflac output for file {path!r}: {exc!r}" + ) + + out[path] = (album_gain, album_peak, track_gain, track_peak) + + return out # GStreamer-based backend. diff --git a/docs/changelog.rst b/docs/changelog.rst index 4d98aa214f..d32f70c8fd 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -50,6 +50,8 @@ Bug fixes relevance order and truncated to ``search_limit``. For artists with more releases than that window, the track being imported was never among the candidates offered. +- :doc:`plugins/replaygain`: Fix ReplayGain metaflac backend altering input + files. :bug:`6915` .. For plugin developers diff --git a/docs/plugins/replaygain.rst b/docs/plugins/replaygain.rst index 92b40924fd..66d4d27baf 100644 --- a/docs/plugins/replaygain.rst +++ b/docs/plugins/replaygain.rst @@ -145,8 +145,9 @@ metaflac This backend uses the metaflac_ command-line tool (part of the FLAC tools) to compute ReplayGain values for FLAC files. It only supports FLAC; files in other -formats are skipped. To use it, install the ``flac`` package, which provides -``metaflac``, and select the ``metaflac`` backend in your configuration file: +formats are skipped. To use it, install the ``flac`` package (version >= 1.3.2 +is required), which provides ``metaflac``, and select the ``metaflac`` backend +in your configuration file: .. code-block:: yaml diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 33464860a8..111ba32a74 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -4,6 +4,7 @@ import pytest from mediafile import MediaFile +from beets.library import Item from beets.test.helper import ( AsIsImporterMixin, ImportHelper, @@ -14,6 +15,7 @@ FatalGstreamerPluginReplayGainError, GStreamerBackend, MetaflacBackend, + RgTask, ) try: @@ -406,16 +408,41 @@ def _add_album(self, *args, **kwargs): return super()._add_album(*args, **kwargs) -def test_metaflac_backend_parses_replaygain_tags(): +def test_metaflac_backend_parses_output(): output = ( - b"REPLAYGAIN_TRACK_GAIN=-11.55 dB\nREPLAYGAIN_TRACK_PEAK=0.99998772\n" + "01.flac: -1.234567 0.123456 1.987654 0.456789\n" + "02.flac: -1.234567 0.123456 -1.987654 0.987654\n" ) - tags = MetaflacBackend._parse_tags(output) - assert MetaflacBackend._parse_gain(tags["REPLAYGAIN_TRACK_GAIN"]) == ( - pytest.approx(-11.55) - ) - assert float(tags["REPLAYGAIN_TRACK_PEAK"]) == pytest.approx(0.99998772) - assert MetaflacBackend._parse_gain("+4.56 dB") == pytest.approx(4.56) + + results = MetaflacBackend._parse_output(output) + + assert set(results) == {"01.flac", "02.flac"} + + album_gain_1, album_peak_1, track_gain_1, track_peak_1 = results["01.flac"] + album_gain_2, album_peak_2, track_gain_2, track_peak_2 = results["02.flac"] + + # Album gain/peak is shared across every file in the batch... + assert album_gain_1 == pytest.approx(album_gain_2) + assert album_peak_1 == pytest.approx(album_peak_2) + + # ...but each file keeps its own distinct track gain/peak. + assert track_gain_1 == pytest.approx(1.987654) + assert track_peak_1 == pytest.approx(0.456789) + + assert track_gain_2 == pytest.approx(-1.987654) + assert track_peak_2 == pytest.approx(0.987654) + + +def test_metaflac_backend_cannot_compute_album_gain_with_mixed_formats(): + backend = MetaflacBackend.__new__(MetaflacBackend) + + items = [Item(format="FLAC"), Item(format="MP3"), Item(format="FLAC")] + task = RgTask(items, None, 89.0, None, "metaflac", None) + + result = backend.compute_album_gain(task) + + assert result.track_gains is None + assert result.album_gain is None class ImportTest(AsIsImporterMixin):