Skip to content

Commit

Permalink
Add support for multiple tags for multiple values
Browse files Browse the repository at this point in the history
MPD supports multiple tags for some values, such as the artist, the
composer, and the performer. (See
<https://mpd.readthedocs.io/en/latest/protocol.html#tags>.) There
might be multiple artists for a track, in which case mopidy-mpd would
concatenate their names into one value, separated by a semicolon. For
some users and use cases, this can be suboptimal; when tracking the
tracks you listen to on a platform such as Listenbrainz, tracks of
multiple artists do not contribute to the total number of listens of
one of the artists.

This commit changes this behaviour to use multiple tags for values
when applicable instead of concatenating the values.
  • Loading branch information
splintersuidman committed Jun 29, 2021
1 parent 408c7c0 commit da73c8c
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 21 deletions.
8 changes: 5 additions & 3 deletions mopidy_mpd/protocol/current_playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def playlistid(context, tlid=None):
return translator.track_to_mpd_format(tl_tracks[0], position=position)
else:
return translator.tracks_to_mpd_format(
context.core.tracklist.get_tl_tracks().get()
context.core.tracklist.get_tl_tracks().get(),
)


Expand Down Expand Up @@ -281,7 +281,7 @@ def plchanges(context, version):
tracklist_version = context.core.tracklist.get_version().get()
if version < tracklist_version:
return translator.tracks_to_mpd_format(
context.core.tracklist.get_tl_tracks().get()
context.core.tracklist.get_tl_tracks().get(),
)
elif version == tracklist_version:
# A version match could indicate this is just a metadata update, so
Expand All @@ -293,7 +293,9 @@ def plchanges(context, version):
tl_track = context.core.playback.get_current_tl_track().get()
position = context.core.tracklist.index(tl_track).get()
return translator.track_to_mpd_format(
tl_track, position=position, stream_title=stream_title
tl_track,
position=position,
stream_title=stream_title,
)


Expand Down
4 changes: 3 additions & 1 deletion mopidy_mpd/protocol/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ def currentsong(context):
if tl_track is not None:
position = context.core.tracklist.index(tl_track).get()
return translator.track_to_mpd_format(
tl_track, position=position, stream_title=stream_title
tl_track,
position=position,
stream_title=stream_title,
)


Expand Down
38 changes: 28 additions & 10 deletions mopidy_mpd/translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,11 @@ def track_to_mpd_format(track, position=None, stream_title=None):
result = [
("file", track.uri),
("Time", track.length and (track.length // 1000) or 0),
("Artist", concat_multi_values(track.artists, "name")),
("Album", track.album and track.album.name or ""),
]

result += multi_tag_list(track.artists, "name", "Artist")

if stream_title is not None:
result.append(("Title", stream_title))
if track.name:
Expand All @@ -69,9 +70,8 @@ def track_to_mpd_format(track, position=None, stream_title=None):
result.append(("MUSICBRAINZ_ALBUMID", track.album.musicbrainz_id))

if track.album is not None and track.album.artists:
result.append(
("AlbumArtist", concat_multi_values(track.album.artists, "name"))
)
result += multi_tag_list(track.album.artists, "name", "AlbumArtist")

musicbrainz_ids = concat_multi_values(
track.album.artists, "musicbrainz_id"
)
Expand All @@ -84,14 +84,10 @@ def track_to_mpd_format(track, position=None, stream_title=None):
result.append(("MUSICBRAINZ_ARTISTID", musicbrainz_ids))

if track.composers:
result.append(
("Composer", concat_multi_values(track.composers, "name"))
)
result += multi_tag_list(track.composers, "name", "Composer")

if track.performers:
result.append(
("Performer", concat_multi_values(track.performers, "name"))
)
result += multi_tag_list(track.performers, "name", "Performer")

if track.genre:
result.append(("Genre", track.genre))
Expand Down Expand Up @@ -151,6 +147,28 @@ def concat_multi_values(models, attribute):
)


def multi_tag_list(models, attribute, tag):
"""
Format Mopidy model values for output to MPD client in a list with one tag
per value.
:param models: the models
:type models: array of :class:`mopidy.models.Artist`,
:class:`mopidy.models.Album` or :class:`mopidy.models.Track`
:param attribute: the attribute to use
:type attribute: string
:param tag: the name of the tag
:type tag: string
:rtype: list of tuples of string and attribute value
"""

return [
(tag, getattr(m, attribute))
for m in models
if getattr(m, attribute, None) is not None
]


def tracks_to_mpd_format(tracks, start=0, end=None):
"""
Format list of tracks for output to MPD client.
Expand Down
5 changes: 4 additions & 1 deletion tests/protocol/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ class BaseTestCase(unittest.TestCase):
def get_config(self):
return {
"core": {"max_tracklist_length": 10000},
"mpd": {"password": None, "default_playlist_scheme": "dummy"},
"mpd": {
"password": None,
"default_playlist_scheme": "dummy",
},
}

def setUp(self): # noqa: N802
Expand Down
7 changes: 6 additions & 1 deletion tests/test_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@

class MpdDispatcherTest(unittest.TestCase):
def setUp(self): # noqa: N802
config = {"mpd": {"password": None, "command_blacklist": ["disabled"]}}
config = {
"mpd": {
"password": None,
"command_blacklist": ["disabled"],
}
}
self.backend = dummy_backend.create_proxy()
self.dispatcher = MpdDispatcher(config=config)

Expand Down
20 changes: 15 additions & 5 deletions tests/test_translator.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,23 @@
class TrackMpdFormatTest(unittest.TestCase):
track = Track(
uri="à uri",
artists=[Artist(name="an artist")],
artists=[Artist(name="an artist"), Artist(name="yet another artist")],
name="a nàme",
album=Album(
name="an album",
num_tracks=13,
artists=[Artist(name="an other artist")],
artists=[
Artist(name="an other artist"),
Artist(name="still another artist"),
],
uri="urischeme:àlbum:12345",
),
track_no=7,
composers=[Artist(name="a composer")],
performers=[Artist(name="a performer")],
composers=[Artist(name="a composer"), Artist(name="another composer")],
performers=[
Artist(name="a performer"),
Artist(name="another performer"),
],
genre="a genre",
date="1977-01-01",
disc_no=1,
Expand Down Expand Up @@ -69,11 +75,15 @@ def test_track_to_mpd_format_for_nonempty_track(self):
assert ("file", "à uri") in result
assert ("Time", 137) in result
assert ("Artist", "an artist") in result
assert ("Artist", "yet another artist") in result
assert ("Title", "a nàme") in result
assert ("Album", "an album") in result
assert ("AlbumArtist", "an other artist") in result
assert ("AlbumArtist", "still another artist") in result
assert ("Composer", "a composer") in result
assert ("Composer", "another composer") in result
assert ("Performer", "a performer") in result
assert ("Performer", "another performer") in result
assert ("Genre", "a genre") in result
assert ("Track", "7/13") in result
assert ("Date", "1977-01-01") in result
Expand All @@ -82,7 +92,7 @@ def test_track_to_mpd_format_for_nonempty_track(self):
assert ("Id", 122) in result
assert ("X-AlbumUri", "urischeme:àlbum:12345") in result
assert ("Comment", "a comment") not in result
assert len(result) == 15
assert len(result) == 19

def test_track_to_mpd_format_with_last_modified(self):
track = self.track.replace(last_modified=995303899000)
Expand Down

0 comments on commit da73c8c

Please sign in to comment.