Skip to content

Commit

Permalink
Fix the no_convert option of the convert plugin stopping conversion w…
Browse files Browse the repository at this point in the history
…hen there is only a partial match. (#5376)

I was running the `convert` plugin with the following config:

```
no_convert: samplerate:..48000 bitdepth:..16
```

but anything that was 24/48 was also not being converted, this was due
to the code returning `False` for `should_transcode` if any part of the
query matches, rather than considering the whole query. This meant that
`bitdepth:...16` was being ignored.

I have changed this so the `no_convert` value is considered as one
query.
  • Loading branch information
snejus authored Oct 27, 2024
2 parents f8b1071 + 4b78abd commit f0f87cc
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 7 deletions.
16 changes: 9 additions & 7 deletions beetsplug/convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,20 @@ def get_format(fmt=None):

return (command.encode("utf-8"), extension.encode("utf-8"))

def in_no_convert(item: Item) -> bool:
no_convert_query = config["convert"]["no_convert"].as_str()

if no_convert_query:
query, _ = parse_query_string(no_convert_query, Item)
return query.match(item)
else:
return False

Check failure on line 95 in beetsplug/convert.py

View workflow job for this annotation

GitHub Actions / Check linting

Ruff (W293)

beetsplug/convert.py:95:1: W293 Blank line contains whitespace
def should_transcode(item, fmt):
"""Determine whether the item should be transcoded as part of
conversion (i.e., its bitrate is high or it has the wrong format).
"""
no_convert_queries = config["convert"]["no_convert"].as_str_seq()
if no_convert_queries:
for query_string in no_convert_queries:
query, _ = parse_query_string(query_string, Item)
if query.match(item):
return False
if (
if in_no_convert(item) or (
config["convert"]["never_convert_lossy_files"]
and item.format.lower() not in LOSSLESS_FORMATS
):
Expand Down
4 changes: 4 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ Bug fixes:
* :doc:`plugins/lyrics`: Update ``tekstowo`` backend to fetch lyrics directly
since recent updates to their website made it unsearchable.
:bug:`5456`
* :doc:`plugins/convert`: Fixed the convert plugin ``no_convert`` option so
that it no longer treats "and" and "or" queries the same. To maintain
previous behaviour add commas between your query keywords. For help see
:ref:`combiningqueries`.

For packagers:

Expand Down
22 changes: 22 additions & 0 deletions test/plugins/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@
import re
import sys
import unittest
import pytest

from mediafile import MediaFile

from beets import util
from beetsplug import convert
from beets.library import Item
from beets.test import _common
from beets.test.helper import (
AsIsImporterMixin,
Expand Down Expand Up @@ -335,3 +338,22 @@ def test_transcode_from_lossy_prevented(self):
self.run_convert_path(item.path)
converted = os.path.join(self.convert_dest, b"converted.ogg")
self.assertNoFileTag(converted, "mp3")


class TestNoConvert:
"""Test the effect of the `no_convert` option."""

@pytest.mark.parametrize(
"config_value, should_skip",
[
("", False),
("bitrate:320", False),
("bitrate:320 format:ogg", False),
("bitrate:320 , format:ogg", True),
],
)

def test_no_convert_skip(self, config_value, should_skip):
item = Item(format="ogg", bitrate=256)
convert.config["convert"]["no_convert"] = config_value
assert convert.in_no_convert(item) == should_skip

0 comments on commit f0f87cc

Please sign in to comment.