mirror of
https://github.com/beetbox/beets.git
synced 2025-12-06 00:24:25 +01:00
Fix convert --format with never_convert_lossy_files (#6171)
## Description Fixes #5625 When `convert.never_convert_lossy_files` is enabled, `beet convert` was ignoring the explicit `--format` option and just copying the lossy files without transcoding them. For example: - `beet convert format:mp3 --format opus` would still produce MP3 files instead of OPUS. Change: - Allows to override options `never_convert_lossy_files`, `max_bitrate` or `no_convert` for `beet convert` as well as trying to convert to the same format as existing already with a new option `--force`. That way, for example lossy files selected by the query are transcoded to the requested format anyway. - Keeps existing behavior for automatic conversion on import (no CLI override there). - Adds tests to cover checking whether `--force` correctly overrides settings or CLI options. - Documents the behavior in the convert plugin docs Co-authored-by: J0J0 Todos <jojo@peek-a-boo.at>
This commit is contained in:
parent
53931279a3
commit
2bd77b9895
4 changed files with 90 additions and 9 deletions
|
|
@ -95,12 +95,18 @@ def in_no_convert(item: Item) -> bool:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
def should_transcode(item, fmt):
|
def should_transcode(item, fmt, force: bool = False):
|
||||||
"""Determine whether the item should be transcoded as part of
|
"""Determine whether the item should be transcoded as part of
|
||||||
conversion (i.e., its bitrate is high or it has the wrong format).
|
conversion (i.e., its bitrate is high or it has the wrong format).
|
||||||
|
|
||||||
|
If ``force`` is True, safety checks like ``no_convert`` and
|
||||||
|
``never_convert_lossy_files`` are ignored and the item is always
|
||||||
|
transcoded.
|
||||||
"""
|
"""
|
||||||
|
if force:
|
||||||
|
return True
|
||||||
if in_no_convert(item) or (
|
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 item.format.lower() not in LOSSLESS_FORMATS
|
||||||
):
|
):
|
||||||
return False
|
return False
|
||||||
|
|
@ -236,6 +242,16 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
drive, relative paths pointing to media files
|
drive, relative paths pointing to media files
|
||||||
will be used.""",
|
will be used.""",
|
||||||
)
|
)
|
||||||
|
cmd.parser.add_option(
|
||||||
|
"-F",
|
||||||
|
"--force",
|
||||||
|
action="store_true",
|
||||||
|
dest="force",
|
||||||
|
help=(
|
||||||
|
"force transcoding. Ignores no_convert, "
|
||||||
|
"never_convert_lossy_files, and max_bitrate"
|
||||||
|
),
|
||||||
|
)
|
||||||
cmd.parser.add_album_option()
|
cmd.parser.add_album_option()
|
||||||
cmd.func = self.convert_func
|
cmd.func = self.convert_func
|
||||||
return [cmd]
|
return [cmd]
|
||||||
|
|
@ -259,6 +275,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
link,
|
link,
|
||||||
playlist,
|
playlist,
|
||||||
|
force,
|
||||||
) = self._get_opts_and_config(empty_opts)
|
) = self._get_opts_and_config(empty_opts)
|
||||||
|
|
||||||
items = task.imported_items()
|
items = task.imported_items()
|
||||||
|
|
@ -272,6 +289,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
threads,
|
threads,
|
||||||
items,
|
items,
|
||||||
|
force,
|
||||||
)
|
)
|
||||||
|
|
||||||
# Utilities converted from functions to methods on logging overhaul
|
# Utilities converted from functions to methods on logging overhaul
|
||||||
|
|
@ -347,6 +365,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
pretend=False,
|
pretend=False,
|
||||||
link=False,
|
link=False,
|
||||||
hardlink=False,
|
hardlink=False,
|
||||||
|
force=False,
|
||||||
):
|
):
|
||||||
"""A pipeline thread that converts `Item` objects from a
|
"""A pipeline thread that converts `Item` objects from a
|
||||||
library.
|
library.
|
||||||
|
|
@ -372,11 +391,11 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
if keep_new:
|
if keep_new:
|
||||||
original = dest
|
original = dest
|
||||||
converted = item.path
|
converted = item.path
|
||||||
if should_transcode(item, fmt):
|
if should_transcode(item, fmt, force):
|
||||||
converted = replace_ext(converted, ext)
|
converted = replace_ext(converted, ext)
|
||||||
else:
|
else:
|
||||||
original = item.path
|
original = item.path
|
||||||
if should_transcode(item, fmt):
|
if should_transcode(item, fmt, force):
|
||||||
dest = replace_ext(dest, ext)
|
dest = replace_ext(dest, ext)
|
||||||
converted = dest
|
converted = dest
|
||||||
|
|
||||||
|
|
@ -406,7 +425,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
)
|
)
|
||||||
util.move(item.path, original)
|
util.move(item.path, original)
|
||||||
|
|
||||||
if should_transcode(item, fmt):
|
if should_transcode(item, fmt, force):
|
||||||
linked = False
|
linked = False
|
||||||
try:
|
try:
|
||||||
self.encode(command, original, converted, pretend)
|
self.encode(command, original, converted, pretend)
|
||||||
|
|
@ -577,6 +596,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
link,
|
link,
|
||||||
playlist,
|
playlist,
|
||||||
|
force,
|
||||||
) = self._get_opts_and_config(opts)
|
) = self._get_opts_and_config(opts)
|
||||||
|
|
||||||
if opts.album:
|
if opts.album:
|
||||||
|
|
@ -613,6 +633,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
threads,
|
threads,
|
||||||
items,
|
items,
|
||||||
|
force,
|
||||||
)
|
)
|
||||||
|
|
||||||
if playlist:
|
if playlist:
|
||||||
|
|
@ -735,7 +756,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
else:
|
else:
|
||||||
hardlink = self.config["hardlink"].get(bool)
|
hardlink = self.config["hardlink"].get(bool)
|
||||||
link = self.config["link"].get(bool)
|
link = self.config["link"].get(bool)
|
||||||
|
force = getattr(opts, "force", False)
|
||||||
return (
|
return (
|
||||||
dest,
|
dest,
|
||||||
threads,
|
threads,
|
||||||
|
|
@ -745,6 +766,7 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
link,
|
link,
|
||||||
playlist,
|
playlist,
|
||||||
|
force,
|
||||||
)
|
)
|
||||||
|
|
||||||
def _parallel_convert(
|
def _parallel_convert(
|
||||||
|
|
@ -758,13 +780,21 @@ class ConvertPlugin(BeetsPlugin):
|
||||||
hardlink,
|
hardlink,
|
||||||
threads,
|
threads,
|
||||||
items,
|
items,
|
||||||
|
force,
|
||||||
):
|
):
|
||||||
"""Run the convert_item function for every items on as many thread as
|
"""Run the convert_item function for every items on as many thread as
|
||||||
defined in threads
|
defined in threads
|
||||||
"""
|
"""
|
||||||
convert = [
|
convert = [
|
||||||
self.convert_item(
|
self.convert_item(
|
||||||
dest, keep_new, path_formats, fmt, pretend, link, hardlink
|
dest,
|
||||||
|
keep_new,
|
||||||
|
path_formats,
|
||||||
|
fmt,
|
||||||
|
pretend,
|
||||||
|
link,
|
||||||
|
hardlink,
|
||||||
|
force,
|
||||||
)
|
)
|
||||||
for _ in range(threads)
|
for _ in range(threads)
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,8 @@ New features:
|
||||||
- :doc:`plugins/mbpseudo`: Add a new `mbpseudo` plugin to proactively receive
|
- :doc:`plugins/mbpseudo`: Add a new `mbpseudo` plugin to proactively receive
|
||||||
MusicBrainz pseudo-releases as recommendations during import.
|
MusicBrainz pseudo-releases as recommendations during import.
|
||||||
- Added support for Python 3.13.
|
- Added support for Python 3.13.
|
||||||
|
- :doc:`/plugins/convert`: ``force`` can be passed to override checks like
|
||||||
|
no_convert, never_convert_lossy_files, same format, and max_bitrate
|
||||||
- :doc:`plugins/titlecase`: Add the `titlecase` plugin to allow users to
|
- :doc:`plugins/titlecase`: Add the `titlecase` plugin to allow users to
|
||||||
resolve differences in metadata source styles.
|
resolve differences in metadata source styles.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -51,6 +51,11 @@ instead, passing ``-H`` (``--hardlink``) creates hard links. Note that album art
|
||||||
embedding is disabled for files that are linked. Refer to the ``link`` and
|
embedding is disabled for files that are linked. Refer to the ``link`` and
|
||||||
``hardlink`` options below.
|
``hardlink`` options below.
|
||||||
|
|
||||||
|
The ``-F`` (or ``--force``) option forces transcoding even when safety options
|
||||||
|
such as ``no_convert``, ``never_convert_lossy_files``, or ``max_bitrate`` would
|
||||||
|
normally cause a file to be copied or skipped instead. This can be combined with
|
||||||
|
``--format`` to explicitly transcode lossy inputs to a chosen target format.
|
||||||
|
|
||||||
The ``-m`` (or ``--playlist``) option enables the plugin to create an m3u8
|
The ``-m`` (or ``--playlist``) option enables the plugin to create an m3u8
|
||||||
playlist file in the destination folder given by the ``-d`` (``--dest``) option
|
playlist file in the destination folder given by the ``-d`` (``--dest``) option
|
||||||
or the ``dest`` configuration. The path to the playlist file can either be
|
or the ``dest`` configuration. The path to the playlist file can either be
|
||||||
|
|
@ -104,15 +109,21 @@ The available options are:
|
||||||
with high bitrates, even if they are already in the same format as the output.
|
with high bitrates, even if they are already in the same format as the output.
|
||||||
Note that this does not guarantee that all converted files will have a lower
|
Note that this does not guarantee that all converted files will have a lower
|
||||||
bitrate---that depends on the encoder and its configuration. Default: none.
|
bitrate---that depends on the encoder and its configuration. Default: none.
|
||||||
|
This option will be overridden by the ``--force`` flag
|
||||||
- **no_convert**: Does not transcode items matching the query string provided
|
- **no_convert**: Does not transcode items matching the query string provided
|
||||||
(see :doc:`/reference/query`). For example, to not convert AAC or WMA formats,
|
(see :doc:`/reference/query`). For example, to not convert AAC or WMA formats,
|
||||||
you can use ``format:AAC, format:WMA`` or ``path::\.(m4a|wma)$``. If you only
|
you can use ``format:AAC, format:WMA`` or ``path::\.(m4a|wma)$``. If you only
|
||||||
want to transcode WMA format, you can use a negative query, e.g.,
|
want to transcode WMA format, you can use a negative query, e.g.,
|
||||||
``^path::\.(wma)$``, to not convert any other format except WMA.
|
``^path::\.(wma)$``, to not convert any other format except WMA. This option
|
||||||
|
will be overridden by the ``--force`` flag
|
||||||
- **never_convert_lossy_files**: Cross-conversions between lossy codecs---such
|
- **never_convert_lossy_files**: Cross-conversions between lossy codecs---such
|
||||||
as mp3, ogg vorbis, etc.---makes little sense as they will decrease quality
|
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:
|
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. To explicitly transcode lossy files in spite of this, use the
|
||||||
|
``--force`` option with the ``convert`` command (optionally together with
|
||||||
|
``--format`` to choose a target format)
|
||||||
- **paths**: The directory structure and naming scheme for the converted files.
|
- **paths**: The directory structure and naming scheme for the converted files.
|
||||||
Uses the same format as the top-level ``paths`` section (see
|
Uses the same format as the top-level ``paths`` section (see
|
||||||
:ref:`path-format-config`). Default: Reuse your top-level path format
|
:ref:`path-format-config`). Default: Reuse your top-level path format
|
||||||
|
|
|
||||||
|
|
@ -236,6 +236,16 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand):
|
||||||
self.convert_dest / "converted.ogg", "ogg"
|
self.convert_dest / "converted.ogg", "ogg"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_force_overrides_max_bitrate_and_same_formats(self):
|
||||||
|
self.config["convert"]["max_bitrate"] = 5000
|
||||||
|
self.config["convert"]["format"] = "ogg"
|
||||||
|
|
||||||
|
with control_stdin("y"):
|
||||||
|
self.run_convert("--force")
|
||||||
|
|
||||||
|
converted = self.convert_dest / "converted.ogg"
|
||||||
|
assert self.file_endswith(converted, "ogg")
|
||||||
|
|
||||||
def test_transcode_when_maxbr_set_low_and_same_formats(self):
|
def test_transcode_when_maxbr_set_low_and_same_formats(self):
|
||||||
self.config["convert"]["max_bitrate"] = 5
|
self.config["convert"]["max_bitrate"] = 5
|
||||||
self.config["convert"]["format"] = "ogg"
|
self.config["convert"]["format"] = "ogg"
|
||||||
|
|
@ -260,6 +270,21 @@ class ConvertCliTest(ConvertTestCase, ConvertCommand):
|
||||||
self.run_convert("--playlist", "playlist.m3u8", "--pretend")
|
self.run_convert("--playlist", "playlist.m3u8", "--pretend")
|
||||||
assert not (self.convert_dest / "playlist.m3u8").exists()
|
assert not (self.convert_dest / "playlist.m3u8").exists()
|
||||||
|
|
||||||
|
def test_force_overrides_no_convert(self):
|
||||||
|
self.config["convert"]["formats"]["opus"] = {
|
||||||
|
"command": self.tagged_copy_cmd("opus"),
|
||||||
|
"extension": "ops",
|
||||||
|
}
|
||||||
|
self.config["convert"]["no_convert"] = "format:ogg"
|
||||||
|
|
||||||
|
[item] = self.add_item_fixtures(ext="ogg")
|
||||||
|
|
||||||
|
with control_stdin("y"):
|
||||||
|
self.run_convert_path(item, "--format", "opus", "--force")
|
||||||
|
|
||||||
|
converted = self.convert_dest / "converted.ops"
|
||||||
|
assert self.file_endswith(converted, "opus")
|
||||||
|
|
||||||
|
|
||||||
@_common.slow_test()
|
@_common.slow_test()
|
||||||
class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand):
|
class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand):
|
||||||
|
|
@ -301,6 +326,19 @@ class NeverConvertLossyFilesTest(ConvertTestCase, ConvertCommand):
|
||||||
converted = self.convert_dest / "converted.ogg"
|
converted = self.convert_dest / "converted.ogg"
|
||||||
assert not self.file_endswith(converted, "mp3")
|
assert not self.file_endswith(converted, "mp3")
|
||||||
|
|
||||||
|
def test_force_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", "--force")
|
||||||
|
|
||||||
|
converted = self.convert_dest / "converted.ops"
|
||||||
|
assert self.file_endswith(converted, "opus")
|
||||||
|
|
||||||
|
|
||||||
class TestNoConvert:
|
class TestNoConvert:
|
||||||
"""Test the effect of the `no_convert` option."""
|
"""Test the effect of the `no_convert` option."""
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue