From da73c8c530b8c3044ac1eb7a360609c0d619361e Mon Sep 17 00:00:00 2001 From: Splinter Suidman Date: Sun, 13 Jun 2021 19:24:26 +0200 Subject: [PATCH] Add support for multiple tags for multiple values MPD supports multiple tags for some values, such as the artist, the composer, and the performer. (See .) 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. --- mopidy_mpd/protocol/current_playlist.py | 8 ++++-- mopidy_mpd/protocol/status.py | 4 ++- mopidy_mpd/translator.py | 38 ++++++++++++++++++------- tests/protocol/__init__.py | 5 +++- tests/test_dispatcher.py | 7 ++++- tests/test_translator.py | 20 +++++++++---- 6 files changed, 61 insertions(+), 21 deletions(-) diff --git a/mopidy_mpd/protocol/current_playlist.py b/mopidy_mpd/protocol/current_playlist.py index cd3d0f3..9006900 100644 --- a/mopidy_mpd/protocol/current_playlist.py +++ b/mopidy_mpd/protocol/current_playlist.py @@ -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(), ) @@ -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 @@ -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, ) diff --git a/mopidy_mpd/protocol/status.py b/mopidy_mpd/protocol/status.py index ff55728..d981184 100644 --- a/mopidy_mpd/protocol/status.py +++ b/mopidy_mpd/protocol/status.py @@ -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, ) diff --git a/mopidy_mpd/translator.py b/mopidy_mpd/translator.py index ab01c09..786755e 100644 --- a/mopidy_mpd/translator.py +++ b/mopidy_mpd/translator.py @@ -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: @@ -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" ) @@ -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)) @@ -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. diff --git a/tests/protocol/__init__.py b/tests/protocol/__init__.py index 5284a81..cde1232 100644 --- a/tests/protocol/__init__.py +++ b/tests/protocol/__init__.py @@ -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 diff --git a/tests/test_dispatcher.py b/tests/test_dispatcher.py index 13445e8..052e45a 100644 --- a/tests/test_dispatcher.py +++ b/tests/test_dispatcher.py @@ -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) diff --git a/tests/test_translator.py b/tests/test_translator.py index daba6d3..a09184b 100644 --- a/tests/test_translator.py +++ b/tests/test_translator.py @@ -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, @@ -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 @@ -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)