From bc56686fc46c837daa0cde7eaea5db0b6e6489a6 Mon Sep 17 00:00:00 2001 From: Danielle McLean Date: Sun, 23 Jun 2024 17:24:37 +1000 Subject: [PATCH] Support multivalued song tags (fixes #1) python-mpd2 unreliably returns either a single value or a list of values for commands like currentsong, which is super fun if you're trying to write type stubs for it that statically describe its behaviour. Whee. Anyway, I ended up changing my internal song model to always use lists for tags like artist and genre which are likely to have multiple values. There's some finagling involved in massaging python-mpd2's output into lists every time. However it's much nicer to work with an object that always has a list of artists, even if it's a list of one or zero artists, rather than an object that can have a single artist, a list of multiple artists, or a null. So it's worth it. The MPNowPlayingInfoCenter in MacOS only works with single string values for these tags, not lists, so we have to join the artists and such into a single string for its consumption. I'm using commas for the separator at the moment, but I may make this a config option later on if there's interest. --- src/mpd_now_playable/cocoa/now_playing.py | 14 +++++-- src/mpd_now_playable/mpd/artwork_cache.py | 9 +++-- src/mpd_now_playable/mpd/listener.py | 12 +++--- src/mpd_now_playable/mpd/types.py | 16 ++++---- src/mpd_now_playable/song.py | 13 +++--- src/mpd_now_playable/tools/types.py | 48 +++++++++++++++++++++-- 6 files changed, 81 insertions(+), 31 deletions(-) diff --git a/src/mpd_now_playable/cocoa/now_playing.py b/src/mpd_now_playable/cocoa/now_playing.py index 15225de..13681d7 100644 --- a/src/mpd_now_playable/cocoa/now_playing.py +++ b/src/mpd_now_playable/cocoa/now_playing.py @@ -78,6 +78,12 @@ def playback_state_to_cocoa(state: PlaybackState) -> MPMusicPlaybackState: return mapping[state] +def join_plural_field(field: list[str]) -> str | None: + if field: + return ", ".join(field) + return None + + def song_to_media_item(song: Song) -> NSMutableDictionary: nowplaying_info = nothing_to_media_item() nowplaying_info[MPNowPlayingInfoPropertyMediaType] = MPNowPlayingInfoMediaTypeAudio @@ -88,12 +94,12 @@ def song_to_media_item(song: Song) -> NSMutableDictionary: nowplaying_info[MPMediaItemPropertyPersistentID] = song_to_persistent_id(song) nowplaying_info[MPMediaItemPropertyTitle] = song.title - nowplaying_info[MPMediaItemPropertyArtist] = song.artist - nowplaying_info[MPMediaItemPropertyAlbumTitle] = song.album + nowplaying_info[MPMediaItemPropertyArtist] = join_plural_field(song.artist) + nowplaying_info[MPMediaItemPropertyAlbumTitle] = join_plural_field(song.album) nowplaying_info[MPMediaItemPropertyAlbumTrackNumber] = song.track nowplaying_info[MPMediaItemPropertyDiscNumber] = song.disc - nowplaying_info[MPMediaItemPropertyGenre] = song.genre - nowplaying_info[MPMediaItemPropertyComposer] = song.composer + nowplaying_info[MPMediaItemPropertyGenre] = join_plural_field(song.genre) + nowplaying_info[MPMediaItemPropertyComposer] = join_plural_field(song.composer) nowplaying_info[MPMediaItemPropertyPlaybackDuration] = song.duration # MPD can't play back music at different rates, so we just want to set it diff --git a/src/mpd_now_playable/mpd/artwork_cache.py b/src/mpd_now_playable/mpd/artwork_cache.py index 6ec5a9a..3d9ed1b 100644 --- a/src/mpd_now_playable/mpd/artwork_cache.py +++ b/src/mpd_now_playable/mpd/artwork_cache.py @@ -6,6 +6,7 @@ from yarl import URL from ..cache import Cache, make_cache from ..tools.asyncio import run_background_task +from ..tools.types import un_maybe_plural from .types import CurrentSongResponse, MpdStateHandler CACHE_TTL = 60 * 60 # seconds = 1 hour @@ -16,9 +17,11 @@ class ArtCacheEntry(TypedDict): def calc_album_key(song: CurrentSongResponse) -> str: - artist = song.get("albumartist", song.get("artist", "Unknown Artist")) - album = song.get("album", "Unknown Album") - return ":".join(t.replace(":", "-") for t in (artist, album)) + artist = sorted( + un_maybe_plural(song.get("albumartist", song.get("artist", "Unknown Artist"))) + ) + album = sorted(un_maybe_plural(song.get("album", "Unknown Album"))) + return ":".join(";".join(t).replace(":", "-") for t in (artist, album)) def calc_track_key(song: CurrentSongResponse) -> str: diff --git a/src/mpd_now_playable/mpd/listener.py b/src/mpd_now_playable/mpd/listener.py index 1a8c107..62229b0 100644 --- a/src/mpd_now_playable/mpd/listener.py +++ b/src/mpd_now_playable/mpd/listener.py @@ -9,7 +9,7 @@ from yarl import URL from ..config.model import MpdConfig from ..player import Player from ..song import PlaybackState, Song, SongListener -from ..tools.types import convert_if_exists +from ..tools.types import convert_if_exists, un_maybe_plural from .artwork_cache import MpdArtworkCache from .types import CurrentSongResponse, StatusResponse @@ -27,11 +27,11 @@ def mpd_current_to_song( current.get("musicbrainz_releasetrackid"), UUID ), title=current.get("title"), - artist=current.get("artist"), - album=current.get("album"), - album_artist=current.get("albumartist"), - composer=current.get("composer"), - genre=current.get("genre"), + artist=un_maybe_plural(current.get("artist")), + album=un_maybe_plural(current.get("album")), + album_artist=un_maybe_plural(current.get("albumartist")), + composer=un_maybe_plural(current.get("composer")), + genre=un_maybe_plural(current.get("genre")), track=convert_if_exists(current.get("track"), int), disc=convert_if_exists(current.get("disc"), int), duration=float(status["duration"]), diff --git a/src/mpd_now_playable/mpd/types.py b/src/mpd_now_playable/mpd/types.py index a5aa8a3..8782ddc 100644 --- a/src/mpd_now_playable/mpd/types.py +++ b/src/mpd_now_playable/mpd/types.py @@ -1,5 +1,7 @@ from typing import Literal, NotRequired, Protocol, TypedDict +from ..tools.types import MaybePlural + class MpdStateHandler(Protocol): async def get_art(self, file: str) -> bytes | None: ... @@ -47,19 +49,19 @@ class StatusResponse(TypedDict): # tagged, since then it can pass more information on to Now Playing, but it # should work fine with completely untagged music too. class CurrentSongTags(TypedDict, total=False): - artist: str - albumartist: str - artistsort: str - albumartistsort: str + artist: MaybePlural[str] + albumartist: MaybePlural[str] + artistsort: MaybePlural[str] + albumartistsort: MaybePlural[str] title: str - album: str + album: MaybePlural[str] track: str date: str originaldate: str - composer: str + composer: MaybePlural[str] disc: str label: str - genre: str + genre: MaybePlural[str] musicbrainz_albumid: str musicbrainz_albumartistid: str musicbrainz_releasetrackid: str diff --git a/src/mpd_now_playable/song.py b/src/mpd_now_playable/song.py index cf72cfa..76f17a8 100644 --- a/src/mpd_now_playable/song.py +++ b/src/mpd_now_playable/song.py @@ -21,18 +21,17 @@ class Song: musicbrainz_trackid: UUID | None musicbrainz_releasetrackid: UUID | None title: str | None - artist: str | None - composer: str | None - album: str | None - album_artist: str | None + artist: list[str] + composer: list[str] + album: list[str] + album_artist: list[str] track: int | None disc: int | None - genre: str | None + genre: list[str] duration: float elapsed: float art: bytes | None = field(repr=lambda a: "" if a else "") class SongListener(Protocol): - def update(self, song: Song | None) -> None: - ... + def update(self, song: Song | None) -> None: ... diff --git a/src/mpd_now_playable/tools/types.py b/src/mpd_now_playable/tools/types.py index 7ee4ffe..1d0f5cc 100644 --- a/src/mpd_now_playable/tools/types.py +++ b/src/mpd_now_playable/tools/types.py @@ -1,11 +1,51 @@ -from typing import Callable, TypeVar +from collections.abc import Callable +from typing import Any, TypeAlias, TypeVar -__all__ = ("convert_if_exists",) +__all__ = ( + "AnyExceptList", + "MaybePlural", + "convert_if_exists", + "un_maybe_plural", +) -T = TypeVar("T") +# Accept as many types as possible that are not lists. Yes, having to identify +# them manually like this is kind of a pain! TypeScript can express this +# restriction using a conditional type, and with another conditional type it +# can correctly type a version of un_maybe_plural that *does* accept lists, but +# Python's type system isn't quite that bonkers powerful. Yet? +AnyExceptList = ( + int + | float + | complex + | bool + | str + | bytes + | set[Any] + | dict[Any, Any] + | tuple[Any, ...] + | Callable[[Any], Any] + | type +) -def convert_if_exists(value: str | None, converter: Callable[[str], T]) -> T | None: +U = TypeVar("U") + + +def convert_if_exists(value: str | None, converter: Callable[[str], U]) -> U | None: if value is None: return None return converter(value) + + +T = TypeVar("T", bound=AnyExceptList) +MaybePlural: TypeAlias = list[T] | T + + +def un_maybe_plural(value: MaybePlural[T] | None) -> list[T]: + match value: + case None: + return [] + case list(values): + return values[:] + case item: + return [item]