From 07ac01fdbf2b3206b92a029a0407e0f93206315a 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 when the `multiple_tags' configuration variable is set to true, to use multiple tags for values when applicable instead of concatenating the values. --- mopidy_mpd/__init__.py | 1 + mopidy_mpd/dispatcher.py | 5 ++ mopidy_mpd/ext.conf | 1 + mopidy_mpd/protocol/current_playlist.py | 23 ++++++-- mopidy_mpd/protocol/music_db.py | 20 +++++-- mopidy_mpd/protocol/status.py | 5 +- mopidy_mpd/protocol/stored_playlists.py | 4 +- mopidy_mpd/translator.py | 73 ++++++++++++++++++++----- tests/test_translator.py | 33 +++++++++-- 9 files changed, 136 insertions(+), 29 deletions(-) diff --git a/mopidy_mpd/__init__.py b/mopidy_mpd/__init__.py index 49f47e1..6de76a8 100644 --- a/mopidy_mpd/__init__.py +++ b/mopidy_mpd/__init__.py @@ -26,6 +26,7 @@ def get_config_schema(self): schema["zeroconf"] = config.String(optional=True) schema["command_blacklist"] = config.List(optional=True) schema["default_playlist_scheme"] = config.String() + schema["multiple_tags"] = config.Boolean(optional=True) return schema def setup(self, registry): diff --git a/mopidy_mpd/dispatcher.py b/mopidy_mpd/dispatcher.py index 25e3220..b80b440 100644 --- a/mopidy_mpd/dispatcher.py +++ b/mopidy_mpd/dispatcher.py @@ -239,6 +239,9 @@ class MpdContext: #: The subsytems that we want to be notified about in idle mode. subscriptions = None + #: Whether to use multiple tags for e.g. artists. + multiple_tags = None + _uri_map = None def __init__( @@ -248,6 +251,8 @@ def __init__( self.session = session if config is not None: self.password = config["mpd"]["password"] + # TODO: don't default here. + self.multiple_tags = config["mpd"].get("multiple_tags", False) self.core = core self.events = set() self.subscriptions = set() diff --git a/mopidy_mpd/ext.conf b/mopidy_mpd/ext.conf index ee518a8..7691904 100644 --- a/mopidy_mpd/ext.conf +++ b/mopidy_mpd/ext.conf @@ -8,3 +8,4 @@ connection_timeout = 60 zeroconf = Mopidy MPD server on $hostname command_blacklist = listall,listallinfo default_playlist_scheme = m3u +multiple_tags = false diff --git a/mopidy_mpd/protocol/current_playlist.py b/mopidy_mpd/protocol/current_playlist.py index cd3d0f3..892c84c 100644 --- a/mopidy_mpd/protocol/current_playlist.py +++ b/mopidy_mpd/protocol/current_playlist.py @@ -188,7 +188,9 @@ def playlistfind(context, tag, needle): if not tl_tracks: return None position = context.core.tracklist.index(tl_tracks[0]).get() - return translator.track_to_mpd_format(tl_tracks[0], position=position) + return translator.track_to_mpd_format( + tl_tracks[0], position=position, multiple_tags=context.multiple_tags + ) raise exceptions.MpdNotImplemented # TODO @@ -207,10 +209,13 @@ def playlistid(context, tlid=None): if not tl_tracks: raise exceptions.MpdNoExistError("No such song") position = context.core.tracklist.index(tl_tracks[0]).get() - return translator.track_to_mpd_format(tl_tracks[0], position=position) + return translator.track_to_mpd_format( + tl_tracks[0], position=position, multiple_tags=context.multiple_tags + ) else: return translator.tracks_to_mpd_format( - context.core.tracklist.get_tl_tracks().get() + context.core.tracklist.get_tl_tracks().get(), + multiple_tags=context.multiple_tags, ) @@ -241,7 +246,9 @@ def playlistinfo(context, parameter=None): raise exceptions.MpdArgError("Bad song index") if end and end > len(tl_tracks): end = None - return translator.tracks_to_mpd_format(tl_tracks, start, end) + return translator.tracks_to_mpd_format( + tl_tracks, start, end, multiple_tags=context.multiple_tags + ) @protocol.commands.add("playlistsearch") @@ -281,7 +288,8 @@ 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(), + multiple_tags=context.multiple_tags, ) elif version == tracklist_version: # A version match could indicate this is just a metadata update, so @@ -293,7 +301,10 @@ 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, + multiple_tags=context.multiple_tags, ) diff --git a/mopidy_mpd/protocol/music_db.py b/mopidy_mpd/protocol/music_db.py index da4f0a3..12a1f24 100644 --- a/mopidy_mpd/protocol/music_db.py +++ b/mopidy_mpd/protocol/music_db.py @@ -154,7 +154,9 @@ def find(context, *args): if "album" not in query: result_tracks += [_album_as_track(a) for a in _get_albums(results)] result_tracks += _get_tracks(results) - return translator.tracks_to_mpd_format(result_tracks) + return translator.tracks_to_mpd_format( + result_tracks, multiple_tags=context.multiple_tags + ) @protocol.commands.add("findadd") @@ -338,7 +340,11 @@ def listallinfo(context, uri=None): else: for tracks in lookup_future.get().values(): for track in tracks: - result.extend(translator.track_to_mpd_format(track)) + result.extend( + translator.track_to_mpd_format( + track, multiple_tags=context.multiple_tags + ) + ) return result @@ -390,7 +396,11 @@ def lsinfo(context, uri=None): else: for tracks in lookup_future.get().values(): if tracks: - result.extend(translator.track_to_mpd_format(tracks[0])) + result.extend( + translator.track_to_mpd_format( + tracks[0], multiple_tags=context.multiple_tags + ) + ) if uri in (None, "", "/"): result.extend(protocol.stored_playlists.listplaylists(context)) @@ -444,7 +454,9 @@ def search(context, *args): artists = [_artist_as_track(a) for a in _get_artists(results)] albums = [_album_as_track(a) for a in _get_albums(results)] tracks = _get_tracks(results) - return translator.tracks_to_mpd_format(artists + albums + tracks) + return translator.tracks_to_mpd_format( + artists + albums + tracks, multiple_tags=context.multiple_tags + ) @protocol.commands.add("searchadd") diff --git a/mopidy_mpd/protocol/status.py b/mopidy_mpd/protocol/status.py index ff55728..237da53 100644 --- a/mopidy_mpd/protocol/status.py +++ b/mopidy_mpd/protocol/status.py @@ -44,7 +44,10 @@ 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, + multiple_tags=context.multiple_tags, ) diff --git a/mopidy_mpd/protocol/stored_playlists.py b/mopidy_mpd/protocol/stored_playlists.py index 072e7e8..d6e09d9 100644 --- a/mopidy_mpd/protocol/stored_playlists.py +++ b/mopidy_mpd/protocol/stored_playlists.py @@ -63,7 +63,9 @@ def listplaylistinfo(context, name): for uri in track_uris: tracks.extend(tracks_map[uri]) playlist = playlist.replace(tracks=tracks) - return translator.playlist_to_mpd_format(playlist) + return translator.playlist_to_mpd_format( + playlist, multiple_tags=context.multiple_tags + ) @protocol.commands.add("listplaylists") diff --git a/mopidy_mpd/translator.py b/mopidy_mpd/translator.py index ab01c09..723ed7d 100644 --- a/mopidy_mpd/translator.py +++ b/mopidy_mpd/translator.py @@ -18,7 +18,9 @@ def normalize_path(path, relative=False): return "/".join(parts) -def track_to_mpd_format(track, position=None, stream_title=None): +def track_to_mpd_format( + track, position=None, stream_title=None, multiple_tags=False +): """ Format track for output to MPD client. @@ -28,6 +30,8 @@ def track_to_mpd_format(track, position=None, stream_title=None): :type position: integer :param stream_title: the current streams title :type position: string + :param multiple_tags: whether to use multiple tags for e.g. artists + :type multiple_tags: boolean :rtype: list of two-tuples """ if isinstance(track, TlTrack): @@ -42,10 +46,14 @@ 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 ""), ] + if multiple_tags: + result += multi_tag_list(track.artists, "name", "Artist") + else: + result.append(("Artist", concat_multi_values(track.artists, "name"))) + if stream_title is not None: result.append(("Title", stream_title)) if track.name: @@ -69,9 +77,16 @@ 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")) - ) + if multiple_tags: + result += multi_tag_list(track.album.artists, "name", "AlbumArtist") + else: + result.append( + ( + "AlbumArtist", + concat_multi_values(track.album.artists, "name"), + ) + ) + musicbrainz_ids = concat_multi_values( track.album.artists, "musicbrainz_id" ) @@ -84,14 +99,20 @@ 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")) - ) + if multiple_tags: + result += multi_tag_list(track.composers, "name", "Composer") + else: + result.append( + ("Composer", concat_multi_values(track.composers, "name")) + ) if track.performers: - result.append( - ("Performer", concat_multi_values(track.performers, "name")) - ) + if multiple_tags: + result += multi_tag_list(track.performers, "name", "Performer") + else: + result.append( + ("Performer", concat_multi_values(track.performers, "name")) + ) if track.genre: result.append(("Genre", track.genre)) @@ -151,7 +172,29 @@ def concat_multi_values(models, attribute): ) -def tracks_to_mpd_format(tracks, start=0, end=None): +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, multiple_tags=False): """ Format list of tracks for output to MPD client. @@ -164,6 +207,8 @@ def tracks_to_mpd_format(tracks, start=0, end=None): :type start: int (positive or negative) :param end: position after last track to include in output :type end: int (positive or negative) or :class:`None` for end of list + :param multiple_tags: whether to use multiple tags for e.g. artists + :type multiple_tags: boolean :rtype: list of lists of two-tuples """ if end is None: @@ -173,7 +218,9 @@ def tracks_to_mpd_format(tracks, start=0, end=None): assert len(tracks) == len(positions) result = [] for track, position in zip(tracks, positions): - formatted_track = track_to_mpd_format(track, position) + formatted_track = track_to_mpd_format( + track, position, multiple_tags=multiple_tags + ) if formatted_track: result.append(formatted_track) return result diff --git a/tests/test_translator.py b/tests/test_translator.py index daba6d3..8d145f8 100644 --- a/tests/test_translator.py +++ b/tests/test_translator.py @@ -9,7 +9,7 @@ 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", @@ -18,8 +18,8 @@ class TrackMpdFormatTest(unittest.TestCase): 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, @@ -68,12 +68,37 @@ def test_track_to_mpd_format_for_nonempty_track(self): ) assert ("file", "à uri") in result assert ("Time", 137) in result + assert ("Artist", "an 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 ("Composer", "another composer;a composer") in result + assert ("Performer", "another performer;a performer") in result + assert ("Genre", "a genre") in result + assert ("Track", "7/13") in result + assert ("Date", "1977-01-01") in result + assert ("Disc", 1) in result + assert ("Pos", 9) in result + assert ("Id", 122) in result + assert ("X-AlbumUri", "urischeme:àlbum:12345") in result + assert ("Comment", "a comment") not in result + assert len(result) == 15 + + def test_track_to_mpd_format_for_nonempty_track_with_multiple_tags(self): + result = translator.track_to_mpd_format( + TlTrack(122, self.track), position=9, multiple_tags=True + ) + 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 ("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 +107,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) == 18 def test_track_to_mpd_format_with_last_modified(self): track = self.track.replace(last_modified=995303899000)