From 51d524cb66bdd16446d1bba077232294893f3afe Mon Sep 17 00:00:00 2001 From: Splinter Suidman Date: Sun, 13 Jun 2021 19:24:26 +0200 Subject: [PATCH 1/2] 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/translator.py | 38 ++++++++++++++++++++++++++++---------- tests/test_translator.py | 20 +++++++++++++++----- 2 files changed, 43 insertions(+), 15 deletions(-) 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/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) From 380e52ba908fa5009c16720498335bc8716b827d Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Sat, 11 Sep 2021 11:18:03 +0200 Subject: [PATCH 2/2] Apply my own review comments ...to have something to help force a build. --- mopidy_mpd/translator.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/mopidy_mpd/translator.py b/mopidy_mpd/translator.py index 786755e..b3b396c 100644 --- a/mopidy_mpd/translator.py +++ b/mopidy_mpd/translator.py @@ -42,11 +42,10 @@ def track_to_mpd_format(track, position=None, stream_title=None): result = [ ("file", track.uri), ("Time", track.length and (track.length // 1000) or 0), + *multi_tag_list(track.artists, "name", "Artist"), ("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: @@ -147,14 +146,14 @@ def concat_multi_values(models, attribute): ) -def multi_tag_list(models, attribute, tag): +def multi_tag_list(objects, attribute, tag): """ - Format Mopidy model values for output to MPD client in a list with one tag - per value. + Format multiple objects 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 objects: the model objects + :type objects: 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 @@ -163,9 +162,9 @@ def multi_tag_list(models, attribute, tag): """ return [ - (tag, getattr(m, attribute)) - for m in models - if getattr(m, attribute, None) is not None + (tag, getattr(obj, attribute)) + for obj in objects + if getattr(obj, attribute, None) is not None ]