diff --git a/beetsplug/convert.py b/beetsplug/convert.py index e72f8c75a..4c874425f 100644 --- a/beetsplug/convert.py +++ b/beetsplug/convert.py @@ -95,13 +95,18 @@ def in_no_convert(item: Item) -> bool: return False -def should_transcode(item, fmt): +def should_transcode(item, fmt, override_never_lossy: bool = False): """Determine whether the item should be transcoded as part of conversion (i.e., its bitrate is high or it has the wrong format). + + If ``override_never_lossy`` is True, the ``never_convert_lossy_files`` + check is ignored (used when the user explicitly requests a format + on the CLI). """ if in_no_convert(item) or ( - config["convert"]["never_convert_lossy_files"] + config["convert"]["never_convert_lossy_files"].get(bool) and item.format.lower() not in LOSSLESS_FORMATS + and not override_never_lossy ): return False maxbr = config["convert"]["max_bitrate"].get(Optional(int)) @@ -259,6 +264,7 @@ class ConvertPlugin(BeetsPlugin): hardlink, link, playlist, + fmt_from_cli, ) = self._get_opts_and_config(empty_opts) items = task.imported_items() @@ -272,6 +278,7 @@ class ConvertPlugin(BeetsPlugin): hardlink, threads, items, + fmt_from_cli, ) # Utilities converted from functions to methods on logging overhaul @@ -347,6 +354,7 @@ class ConvertPlugin(BeetsPlugin): pretend=False, link=False, hardlink=False, + fmt_from_cli=False, ): """A pipeline thread that converts `Item` objects from a library. @@ -372,11 +380,15 @@ class ConvertPlugin(BeetsPlugin): if keep_new: original = dest converted = item.path - if should_transcode(item, fmt): + if should_transcode( + item, fmt, override_never_lossy=fmt_from_cli + ): converted = replace_ext(converted, ext) else: original = item.path - if should_transcode(item, fmt): + if should_transcode( + item, fmt, override_never_lossy=fmt_from_cli + ): dest = replace_ext(dest, ext) converted = dest @@ -406,7 +418,7 @@ class ConvertPlugin(BeetsPlugin): ) util.move(item.path, original) - if should_transcode(item, fmt): + if should_transcode(item, fmt, override_never_lossy=fmt_from_cli): linked = False try: self.encode(command, original, converted, pretend) @@ -577,6 +589,7 @@ class ConvertPlugin(BeetsPlugin): hardlink, link, playlist, + fmt_from_cli, ) = self._get_opts_and_config(opts) if opts.album: @@ -613,6 +626,7 @@ class ConvertPlugin(BeetsPlugin): hardlink, threads, items, + fmt_from_cli, ) if playlist: @@ -715,6 +729,7 @@ class ConvertPlugin(BeetsPlugin): path_formats = ui.get_path_formats(self.config["paths"] or None) + fmt_from_cli = opts.format is not None fmt = opts.format or self.config["format"].as_str().lower() playlist = opts.playlist or self.config["playlist"].get() @@ -745,6 +760,7 @@ class ConvertPlugin(BeetsPlugin): hardlink, link, playlist, + fmt_from_cli, ) def _parallel_convert( @@ -758,13 +774,21 @@ class ConvertPlugin(BeetsPlugin): hardlink, threads, items, + fmt_from_cli, ): """Run the convert_item function for every items on as many thread as defined in threads """ convert = [ self.convert_item( - dest, keep_new, path_formats, fmt, pretend, link, hardlink + dest, + keep_new, + path_formats, + fmt, + pretend, + link, + hardlink, + fmt_from_cli, ) for _ in range(threads) ] diff --git a/docs/changelog.rst b/docs/changelog.rst index c5a0dab53..3a322769a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -46,7 +46,11 @@ Bug fixes: - :doc:`/plugins/web`: repair broken `/item/values/…` and `/albums/values/…` endpoints. Previously, due to single-quotes (ie. string literal) in the SQL query, the query eg. `GET /item/values/albumartist` would return the literal - "albumartist" instead of a list of unique album artists. + "albumartist" instead of a list of unique album artists. - + :doc:`/plugins/convert`: beet convert ignored an explicit --format option when + never_convert_lossy_files was enabled. Lossy files selected by a query and + given a target format on the CLI are now transcoded instead of being copied + unchanged. :bug:`5625` For plugin developers: diff --git a/docs/plugins/convert.rst b/docs/plugins/convert.rst index ecf60a85b..c1ec9e23d 100644 --- a/docs/plugins/convert.rst +++ b/docs/plugins/convert.rst @@ -112,7 +112,12 @@ The available options are: - **never_convert_lossy_files**: Cross-conversions between lossy codecs---such as mp3, ogg vorbis, etc.---makes little sense as they will decrease quality even further. If set to ``yes``, lossy files are always copied. Default: - ``no``. + ``no``. When ``never_convert_lossy_files`` is enabled, lossy source files (for + example MP3 or Ogg Vorbis) are normally not transcoded and are instead copied + or linked as-is. If you explicitly pass a target format on the command line + with ``--format``, this override is respected: lossy files matching the query + will be transcoded to the requested format even when + ``never_convert_lossy_files`` is enabled. - **paths**: The directory structure and naming scheme for the converted files. Uses the same format as the top-level ``paths`` section (see :ref:`path-format-config`). Default: Reuse your top-level path format diff --git a/test/plugins/test_convert.py b/test/plugins/test_convert.py index 1452686a7..d0e162b98 100644 --- a/test/plugins/test_convert.py +++ b/test/plugins/test_convert.py @@ -301,6 +301,20 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand): converted = self.convert_dest / "converted.ogg" assert not self.file_endswith(converted, "mp3") + def test_cli_format_overrides_never_convert_lossy_files(self): + self.config["convert"]["formats"]["opus"] = { + "command": self.tagged_copy_cmd("opus"), + "extension": "ops", + } + + [item] = self.add_item_fixtures(ext="ogg") + + with control_stdin("y"): + self.run_convert_path(item, "--format", "opus") + + converted = self.convert_dest / "converted.ops" + assert self.file_endswith(converted, "opus") + class TestNoConvert: """Test the effect of the `no_convert` option."""