search

kingosticks / mopidy-tunein

Mopidy extension for playing music from tunein
Apache License 2.0
65 stars 14 forks source link

Add radio station logo to track. #14

Closed jcass77 closed 9 years ago

jcass77 commented 9 years ago

Display the radio station's logo in front-ends that support this feature (e.g. Mopidy-Musicbox-Webclient).

kingosticks commented 9 years ago

Thanks for the PR. Slight complication is that there are two possible images to present: the station logo and the 'now playing' show logo. I've not had time to check but I think with this code, which one you get depends on how you accessed the station i.e. if you browsed then I think it's the station logo, if you searched then it's the 'now playing' show logo and if you loaded the URI from a local playlist i.e. tunein:station:s32500 then you don't get anything. In the last case, the TuneIn client uses http://opml.radiotime.com/Describe.ashx?id=s32500&c=composite&detail=listing to get the station information and you'll see the data is available as 'logo' rather than 'image' here. However, if we also used http://opml.radiotime.com/Describe.ashx?c=nowplaying&id=s32500 then we can get the show logo too.

I'm not sure if it makes most sense to get the station or show logo. Any opinions?

adamcik commented 9 years ago

I would suggest the station logo for now, and once pushing metadata changes in core becomes possible I would start doing the show ones.

jcass77 commented 9 years ago

Based on what I have seen so far, the TuneIn station reference will contain either an image tag containing the url to the station logo, or a playing_image tag with the currently playing show or song. I agree that the best option is probably to just display the station logo until such time as stream metadata can be changed dynamically.

I'm still finding my way around the Mopidy API and architecture and didn't realise that lookup is not called every time that a track is selected. I've added the code to the search function as well - thank you.

How would I go about adding a station to a local playlist to test the third scenario referred to above?

kingosticks commented 9 years ago

Oops, yeh my mistake regarding searches returning the show logo, they do of course return the station logo so with the extra change that's all good. (I was looking at the wrong line in my search result - a show!)

For the annoying 3rd case, stick something like the following in an .m3u file in the local/playlists_dir directory. Mine is in /var/lib/mopidy/playlists/Streams.m3u.

#EXTM3U
#EXTINF:123, BBC Radio 2 (London)
tunein:station:s24940
#EXTINF:0,Absolute Radio (London)
tunein:station:s16679

EDIT: or if you are using musicbox-webclient you''ll probably get the same effect when entering the Mopidy-TuneIn URI (tunein:station:s24940) directly into the webclient's "streams" page. Note it has to be a station that has not already been returned whilst browsing or searching as the station data is all cached.

kingosticks commented 9 years ago

And out of interest, doesn't returning an album called ' ' confuse musicbox'webclient's search results or is it smart enough to ignore empty strings like that?

adamcik commented 9 years ago

Not sure, but this is one of the many reasons we need a new give me the coverart / logo for a uri API method in libraries. Our current track + album model breaks down for things like radio stations :(

kingosticks commented 9 years ago

Yes indeed. At one point I was contemplating returning the show as the the Track and the station as it's Album but for some reason I didn't bother in the end. Maybe it was due to the general terrible handling of shows/podcasts that I ended up implementing. Perhaps I'll revisit that idea.

kingosticks commented 9 years ago

I gave this a quick try and unfortunately it does look like an Album called ' ' does slightly confuse musicbox-webclient's handling of search results and the queue. I'll have a quick play tomorrow to see if there is an easy fix. But either way, it's a cool feature so it'll get merged anyway and then sort out the different field name for the annoying weird lookup case. Thanks.