00dani / mpd-now-playable

Expose your MPD server as a 'now playable' app on MacOS
2 stars 0 forks source link

Crashes on songs with multi-valued tags, such as multiple artists #1

Closed kenziewebm closed 3 months ago

kenziewebm commented 4 months ago
kenzie@tangela ~ % /usr/local/bin/mpd-now-playable
Connecting to MPD server localhost:6600...
Connected to MPD v0.23.5
Traceback (most recent call last):
  File "/usr/local/bin/mpd-now-playable", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/cli.py", line 29, in main
    asyncio.run(listen(), loop_factory=make_loop, debug=True)
  File "/usr/local/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 194, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/usr/local/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12/asyncio/base_events.py", line 687, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/cli.py", line 21, in listen
    await listener.loop(now_playing)
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/mpd/listener.py", line 69, in loop
    await self.update_listener(listener)
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/mpd/listener.py", line 90, in update_listener
    art = await self.art_cache.get_cached_artwork(current)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/mpd/artwork_cache.py", line 45, in get_cached_artwork
    art = await self.album_cache.get(calc_album_key(song))
                                     ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/mpd/artwork_cache.py", line 19, in calc_album_key
    return ":".join(t.replace(":", "-") for t in (artist, album))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/mpd_now_playable/mpd/artwork_cache.py", line 19, in <genexpr>
    return ":".join(t.replace(":", "-") for t in (artist, album))
                    ^^^^^^^^^
AttributeError: 'list' object has no attribute 'replace'
kenzie@tangela ~ %

Python version 3.12.4 plsfix?

00dani commented 4 months ago

Hi! It looks like this isn't an issue with MacOS 12, which I use myself - it's something to do with the way mpd-now-playable is handling your music's tags. My guess is that your current song is tagged with multiple artists, so the artist field ended up being a list of artists instead of a single artist's name.

If possible, would you mind sharing the tags from the current song that caused this? I'll still be able to patch the code to support multiple artists regardless, it'll just be easier to verify that I diagnosed the problem correctly. 🐱

00dani commented 4 months ago

Aha, my guess was correct! I believe the reason ffprobe displays with semicolons and ncmpcpp displays with pipes is that the actual file contains neither - instead, it's got two entirely separate tags ARTIST=Future and ARTIST=Metro Boomin, and the individual programs processing the tags are making different decisions about how to show that information. mpc status most likely only uses the first artist tag, ignoring any extras.

Multivalued tags like this are completely allowed and in fact encouraged in Vorbis comment metadata, which is what FLAC files use. So, yeah, I need to patch mpd-now-playable to expect the possibility of multiple tag values. Shouldn't be too complicated. 👍

kenziewebm commented 4 months ago

(sorry for deleting my comment, github made it look like i posted it 7 times and i got scared, but it in fact didnt)

Yeah you appear to be correct - I ran strings on the file and sure enough:

kenzie@tangela ~ % strings Music/kendrick-and-friends/we\ dont\ trust\ you/01\ -\ We\ Don\'t\ Trust\ You.flac | grep ARTIST
ARTIST=Future
ALBUMARTIST=Future
ARTIST=Metro Boomin
ALBUMARTIST=Metro Boomin
DISPLAY ARTIST=Future; Metro Boomin
kenzie@tangela ~ %
00dani commented 3 months ago

I just published mpd-now-playable v1.3.0, which adds proper handling for potentially multivalued music tags! Artist, album, album artist, composer, and genre are now all treated as multivalued and will work just fine. The MacOS API I'm talking to doesn't support multivalued music tags either, so I currently join up multiple tag values with commas while passing them through to MacOS. Commas looked a bit nicer in the UI than pipes or semicolons to me. 🤷‍♀️

This new version also adds configuration file support, which is why I bumped the minor version number, but don't worry about that. The new config file will only be needed to access upcoming features I'm still planning. 😉

kenziewebm commented 3 months ago

Thank you so much, I always knew trans furries (bronies?) make the best software