diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bfd05c718..0736a0a2f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,7 +35,12 @@ jobs: python-version: ${{ matrix.python-version }} cache: poetry - - name: Install PyGobject and release script dependencies on Ubuntu + - name: Install system dependencies on Windows + if: matrix.platform == 'windows-latest' + run: | + choco install mp3gain -y + + - name: Install system dependencies on Ubuntu if: matrix.platform == 'ubuntu-latest' run: | sudo apt update @@ -43,11 +48,12 @@ jobs: ffmpeg \ gobject-introspection \ gstreamer1.0-plugins-base \ - python3-gst-1.0 \ + imagemagick \ libcairo2-dev \ libgirepository-2.0-dev \ + mp3gain \ pandoc \ - imagemagick + python3-gst-1.0 - name: Get changed lyrics files id: lyrics-update diff --git a/beetsplug/replaygain.py b/beetsplug/replaygain.py index e83345059..e69f6b2ee 100644 --- a/beetsplug/replaygain.py +++ b/beetsplug/replaygain.py @@ -34,7 +34,7 @@ from typing import TYPE_CHECKING, Any, ClassVar, Literal, TypeVar from beets import ui from beets.plugins import BeetsPlugin -from beets.util import command_output, displayable_path, syspath +from beets.util import command_output, syspath if TYPE_CHECKING: import optparse @@ -425,7 +425,7 @@ class FfmpegBackend(Backend): # call ffmpeg self._log.debug("analyzing {}", item) cmd = self._construct_cmd(item, peak_method) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stderr.splitlines() # parse output @@ -643,18 +643,20 @@ class CommandBackend(Backend): # tag-writing; this turns the mp3gain/aacgain tool into a gain # calculator rather than a tag manipulator because we take care # of changing tags ourselves. - cmd: list[str] = [self.command, "-o", "-s", "s"] - if self.noclip: - # Adjust to avoid clipping. - cmd = [*cmd, "-k"] - else: - # Disable clipping warning. - cmd = [*cmd, "-c"] - cmd = [*cmd, "-d", str(int(target_level - 89))] - cmd = cmd + [syspath(i.path) for i in items] + cmd = [ + self.command, + "-o", + "-s", + "s", + # Avoid clipping or disable clipping warning + "-k" if self.noclip else "-c", + "-d", + str(int(target_level - 89)), + *[str(i.filepath) for i in items], + ] self._log.debug("analyzing {} files", len(items)) - self._log.debug("executing {}", " ".join(map(displayable_path, cmd))) + self._log.debug("executing {}", " ".join(cmd)) output = call(cmd, self._log).stdout self._log.debug("analysis finished") return self.parse_tool_output( diff --git a/docs/changelog.rst b/docs/changelog.rst index 3938d26a4..d3dfa104a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,8 @@ Bug fixes - :doc:`plugins/musicbrainz`: Fix fetching very large releases that have more than 500 tracks. :bug:`6355` - :doc:`plugins/badfiles`: Fix number of found errors in log message +- :doc:`plugins/replaygain`: Avoid magic Windows prefix in calls to command + backends, such as ``mp3gain``. :bug:`2946` .. For plugin developers diff --git a/test/plugins/test_replaygain.py b/test/plugins/test_replaygain.py index 094349b25..054b70a76 100644 --- a/test/plugins/test_replaygain.py +++ b/test/plugins/test_replaygain.py @@ -14,12 +14,11 @@ import unittest -from typing import ClassVar +from typing import Any, ClassVar import pytest from mediafile import MediaFile -from beets import config from beets.test.helper import ( AsIsImporterMixin, ImportTestCase, @@ -39,10 +38,15 @@ try: except (ImportError, ValueError): GST_AVAILABLE = False -if any(has_program(cmd, ["-v"]) for cmd in ["mp3gain", "aacgain"]): - GAIN_PROG_AVAILABLE = True -else: - GAIN_PROG_AVAILABLE = False + +GAIN_PROG = next( + ( + cmd + for cmd in ["mp3gain", "mp3rgain", "aacgain"] + if has_program(cmd, ["-v"]) + ), + None, +) FFMPEG_AVAILABLE = has_program("ffmpeg", ["-version"]) @@ -63,14 +67,18 @@ class ReplayGainTestCase(PluginMixin, ImportTestCase): plugin = "replaygain" preload_plugin = False - backend: ClassVar[str] + plugin_config: ClassVar[dict[str, Any]] + + @property + def backend(self): + return self.plugin_config["backend"] def setUp(self): # Implemented by Mixins, see above. This may decide to skip the test. self.test_backend() super().setUp() - self.config["replaygain"]["backend"] = self.backend + self.config["replaygain"].set(self.plugin_config) self.load_plugins() @@ -81,8 +89,16 @@ class ThreadedImportMixin: self.config["threaded"] = True -class GstBackendMixin: - backend = "gstreamer" +class BackendMixin: + plugin_config: ClassVar[dict[str, Any]] + has_r128_support: bool + + def test_backend(self): + """Check whether the backend actually has all required functionality.""" + + +class GstBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "gstreamer"} has_r128_support = True def test_backend(self): @@ -90,30 +106,25 @@ class GstBackendMixin: try: # Check if required plugins can be loaded by instantiating a # GStreamerBackend (via its .__init__). - config["replaygain"]["targetlevel"] = 89 - GStreamerBackend(config["replaygain"], None) + self.config["replaygain"]["targetlevel"] = 89 + GStreamerBackend(self.config["replaygain"], None) except FatalGstreamerPluginReplayGainError as e: # Skip the test if plugins could not be loaded. self.skipTest(str(e)) -class CmdBackendMixin: - backend = "command" +class CmdBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = { + "backend": "command", + "command": GAIN_PROG, + } has_r128_support = False - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - -class FfmpegBackendMixin: - backend = "ffmpeg" +class FfmpegBackendMixin(BackendMixin): + plugin_config: ClassVar[dict[str, Any]] = {"backend": "ffmpeg"} has_r128_support = True - def test_backend(self): - """Check whether the backend actually has all required functionality.""" - pass - class ReplayGainCliTest: FNAME: str @@ -332,7 +343,7 @@ class ReplayGainGstCliTest( FNAME = "full" # file contains only silence -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdCliTest( ReplayGainCliTest, ReplayGainTestCase, CmdBackendMixin ): @@ -369,7 +380,7 @@ class ReplayGainGstImportTest(ImportTest, ReplayGainTestCase, GstBackendMixin): pass -@unittest.skipIf(not GAIN_PROG_AVAILABLE, "no *gain command found") +@unittest.skipIf(not GAIN_PROG, "no *gain command found") class ReplayGainCmdImportTest(ImportTest, ReplayGainTestCase, CmdBackendMixin): pass