Skip to content

Commit

Permalink
Fix mb_artistid, mb_albumartistid, albumtype diff issue
Browse files Browse the repository at this point in the history
  • Loading branch information
snejus committed Dec 14, 2024
1 parent 22163d7 commit 550a9a8
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 1 deletion.
39 changes: 39 additions & 0 deletions beets/autotag/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,43 @@ def apply_album_metadata(album_info: AlbumInfo, album: Album):
_apply_metadata(album_info, album)


def ensure_consistent_list_fields(i: Item) -> None:
"""Synchronise single and list values for the list fields that we use.
That is, ensure the same value in the single field and the first element
in the list.
For context, the value we set as, say, ``mb_artistid`` is simply ignored:
Under the current :class:`MediaFile` implementation, fields ``albumtype``,
``mb_artistid`` and ``mb_albumartistid`` are mapped to the first element of
``albumtypes``, ``mb_artistids`` and ``mb_albumartistids`` respectively.
This means setting ``mb_artistid`` has no effect. However, beets
functionality still assumes that ``mb_artistid`` is independent and stores
its value in the database. If ``mb_artistid`` != ``mb_artistids[0]``,
``beet write`` command thinks that ``mb_artistid`` is modified and tries to
update the field in the file. Of course nothing happens, so the same diff
is shown every time the command is run.
We can avoid this issue by ensuring that ``mb_artistid`` has the same value
as ``mb_artistids[0]``, and that's what this function does.
"""
if aid := i.mb_artistid:
i.mb_artistids = list(dict.fromkeys([aid, *i.mb_artistids]))
elif aids := i.mb_artistids:
i.mb_artistid = aids[0]

if aaid := i.mb_albumartistid:
i.mb_albumartistids = list(dict.fromkeys([aaid, *i.mb_albumartistids]))
elif aaids := i.mb_albumartistids:
i.mb_albumartistid = aaids[0]

if atype := i.albumtype:
i.albumtypes = list(dict.fromkeys([atype, *i.albumtypes]))
elif atypes := i.albumtypes:
i.albumtype = atypes[0]


def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
"""Set the items' metadata to match an AlbumInfo object using a
mapping from Items to TrackInfo objects.
Expand Down Expand Up @@ -250,6 +287,8 @@ def apply_metadata(album_info: AlbumInfo, mapping: Mapping[Item, TrackInfo]):
item.mb_albumartistids = album_info.artists_ids
item.mb_releasegroupid = album_info.releasegroup_id

ensure_consistent_list_fields(item)

# Compilation flag.
item.comp = album_info.va

Expand Down
8 changes: 8 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ Bug fixes:
have before the introduction of Poetry.
:bug:`5531`
:bug:`5526`
* :ref:`write-cmd`: Fix the issue where for certain files differences in
``mb_artistid``, ``mb_albumartistid`` and ``albumtype`` fields are shown on
every attempt to write tags. Note: your music needs to be reimported with
``beet import -LI`` or synchronised with ``beet mbsync`` in order to fix
this!
:bug:`5265`
:bug:`5371`
:bug:`4715`

For packagers:

Expand Down
39 changes: 38 additions & 1 deletion test/test_autotag.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@
import pytest

from beets import autotag, config
from beets.autotag import AlbumInfo, TrackInfo, match
from beets.autotag import (
AlbumInfo,
TrackInfo,
ensure_consistent_list_fields,
match,
)
from beets.autotag.hooks import Distance, string_dist
from beets.library import Item
from beets.test.helper import BeetsTestCase
Expand Down Expand Up @@ -1040,3 +1045,35 @@ def test_ampersand_expansion(self):
def test_accented_characters(self):
dist = string_dist("\xe9\xe1\xf1", "ean")
assert dist == 0.0


@pytest.mark.parametrize(
"single_field,list_field",
[
("mb_artistid", "mb_artistids"),
("mb_albumartistid", "mb_albumartistids"),
("albumtype", "albumtypes"),
],
)
@pytest.mark.parametrize(
"single_value,list_value",
[
(None, []),
(None, ["1"]),
(None, ["1", "2"]),
("1", []),
("1", ["1"]),
("1", ["1", "2"]),
("1", ["2", "1"]),
],
)
def test_ensure_consistent_list_fields(
single_field, list_field, single_value, list_value
):
data = {single_field: single_value, list_field: list_value}
item = Item(**data)

ensure_consistent_list_fields(item)

single_val, list_val = item[single_field], item[list_field]
assert (not single_val and not list_val) or single_val == list_val[0]

0 comments on commit 550a9a8

Please sign in to comment.