Show proper metadata including album and artist whenever possible #739
Conversation
Currently But of course, this solution with lastfm is better, and sharing the metadata through mpris is great too. |
mps_youtube/mpris.py
Outdated
| @@ -301,7 +301,9 @@ def setproperty(self, name, val): | |||
| '/CurrentPlaylist/ytid/' + trackid, variant_level=1), | |||
| 'mpris:length' : dbus.Int64(length * 10**6, variant_level=1), | |||
| 'mpris:artUrl' : dbus.String(arturl, variant_level=1), | |||
| 'xesam:title' : dbus.String(title, variant_level=1) } | |||
| 'xesam:title' : dbus.String(title, variant_level=1), | |||
| 'xesam:artist' : dbus.Array(artist), | |||
There was a problem hiding this comment.
please specify also here signature and variant level explicitly. See how it is done on line 162.
mps_youtube/player.py
Outdated
| @@ -374,14 +375,24 @@ def _get_input_file(): | |||
| def _launch_player(song, songdata, cmd): | |||
| """ Launch player application. """ | |||
|
|
|||
| #song.title | |||
mps_youtube/player.py
Outdated
| else : | ||
| arturl = metadata['album_art_url'] | ||
| temp = (song.ytid, metadata['track_title'], song.length, arturl, [metadata['artist']], metadata['album']) | ||
| metadata = temp |
There was a problem hiding this comment.
we can assign to metadata directly, right?
There was a problem hiding this comment.
I don't know why I did that. XD
|
Thanks again for this. Seems to work reasonably well for most of the songs. Is usage of last.fm API anyhow limited? I'm just worried if all of our users start to use this, we might get blocked. For this reason, there should be a way to change the API key. It can be similar to what we have for youtube API key. |
|
@hrnr I don't think the last.fm API would be problem. Their terms of service says the limit is 5 requests per ip per sec (averaged over 5 minutes). That is not happening. But we will have to give them proper credits for their data. I was thinking about implementing this for downloads too. Is dependency on mutagen a problem? |
|
Thanks. I have tested the MPRIS side and all looks good now (all datatypes correct etc.). |
I'm hesitant to add non-optional dependencies, but mutagen seems to be commonly packaged by distributions (Arch and Debian, at least), so it should be fine. |
|
We could also implement the musicbrainz project as a source for music information instead of last.fm. It also does not require an API key so the functionality won't break if the hardcoded key is revoked in the future. We're also not attributing them when using their data(we also need to show a link to https://last.fm). |
|
@Razesdark Do they provide the album art with the json response? Their web service don't expose many things. Is there a way to search using both title and artist? I tried a few songs and the top result don't match (Which means more work to find the correct result). If there is way to search both title and artist this would improve. But still I couldn't see the ablum art in the json response. I have seen many open source projects using last.fm (Jockey and Phonograph, to name some). |
Now fixes metadata when playing songs. This could be extended to fixing metadata (track title, album, artist, album art) when downloading songs. But that would add mutagen as a dependency; so I was not sure of implementing that (That could be a future PR). Switches to old method of displaying thumbnails if no proper metadata was found. Please see attached image. Fixes #209 and #495 .