Internet radio artwork.

[sfeakes]

Looking over the code, it seems artwork URL's that as embedded in the stream work fine for the web interface, but not for "remotes" like iTunes Remote & Retune. The latter default to .image file with the same name as playlist containing the URL. I was wondering if they was a bug or by design due to some limitations of DACP clients like iTunes Remote.

On a slight tangent to the above, I've gone through the code and ffmpeg will pull the "StreamUrl" tag from the stream, and forked-daapd expects that to be a URL pointing to the artwork of the currently streamed track. What I've seen from the streams I listed to is that isn't always the artwork url, some times it's a url pointing to JSON or XML with further information and the artwork url is contained within that. Since there doesn't seem to be a standard for this, I've modified forked-daapd to accept an optional configuration parameter "stream_image_proxy", that you can set to create your own "plugin" image passer for the particular streams you listen to.
example ::-

configuration file.
stream_image_proxy = http://mywebserver/img_proxy.php

When forked-daapd receives ICY tag StreamUrl of https://listenapi.planetradio.co.uk/api9/eventdata/49620117

it will call 
http://mywebserver/img_proxy.php?url=https%3A%2F%2Flistenapi.planetradio.co.uk%2Fapi9%2Feventdata%2F49620117

Which allows you to create a script that can pass https://listenapi.planetradio.co.uk/api9/eventdata/49620117, pull the appropriate image and return just that image.

Kind-a convoluted, I know but it was the best option I could think of that supports some form of plug-in style image passer/proxy so people can create their own since their is a whole host of different formats out their.

Please let me know If this is something that's inline with intended direction of forked-daapd, and I'll create a pull request.

[sfeakes]

OK, Figured out the problem on the streams not displaying in DACP. Seems all the few streaming servers I was using for testing are returning "image/jpg" for content type and forked-daapd is checking for "image/jpeg" (Notice the e), which is the correct content type. ie image/jpg is invalid. Easy fix since now I can re-write the header with the proxy solution above.

[ejurgensen]

You did a great job looking into this, that is much appreciated! With regard to jpeg/jpg I suppose the easiest fix is to just look for both?

As you say the script/proxy-server approach makes it rather complicated for the user. Can we think of something more simple?

I'm also wondering why on earth these radio stations are putting non-URL's in a field called StreamUrl. How many radio stations have you surveyed?

[sfeakes]

There are a few things going on here that I'm looking into and trying to find the best solution for that's inline with how the code is written and not having to customize it for the myriad of different formats.

First, the icy tag StreamUrl when it does return a valid image it can be something like

https://assets.planetradio.co.uk/artist/1-1/320x320/202.jpg?ver=1465083269
http://img.radioparadise.com/covers/l/B00X8Z0W48.jpg
https://assets.planetradio.co.uk/artist/1-1/?ver=1465083269

And the web server can return content-type image/jpeg (correct), or image/jpg or no content type at all. Since forked-daapd checks the URL ends in .jpg or .png AND also checks content type, seems many fails those two checks. So seems a bit redundant to check both content type and the extension. So I was going to modify the code as such (just check content type).

Second, Seems if you have you stream within a playlist ie station.m3u | pls and station.jpg | png then the first request any client makes for artwork will do the correct thing and use StreamUrl, but all further requests (except web ui) will pickup the "cached" station.jpg and no further requests to StreamUrl will be done. Easy fix is to delete station.jpg there for forked-daapd has no cache image, but then you have no fallback if StreamUrl tags go astray for some reason (which does happen). Currently looking into best way to fix this. Think it may be best to simply reverse the order of image checks for streams, ie (StreamUrl, cache) rather than using the mp3 order (cache, directory, StreamUrl) that seems to be using now.

Third, yup the image proxy is a pain for users, I'll give it some more thought see if I can come up with a better solution. As an example.

This Stream
http://ais.absoluteradio.co.uk/absoluteradiohigh.aac

Has StreamURL tag of

https://listenapi.planetradio.co.uk/api9/eventdata/49648827

If you plug the last URL into your browser you'll see the JSON that's returned, (have more information about the current track being played) and that JSON has another tag EventImageUrl that the actual image.

[sfeakes]

OK, think I have got it all sorted and in a neat manor.

After reading the ICY spec for StreamUrl is does indeed state you can put a like to extra data or image artwork in that tag. So what some stations are doing is to spec. There is however no spec (that I could find) defining the format of what could be returned in the "extra data" behind the StreamUrl. So I have coded to the following -

make request to StreamUrl and :--
  a) If a http content type of jpeg or png is returned or url has .jpg or .png, then load image and continue as normal.
  b) if http content type of json is returned, then go to forked-daap configuration and pick out 
      stream_urlimage_tags = { "eventImageUrl", "StreamURL", "ImageUrl" }
      search json for the tags and if match found, make a new request for an image from that URL

That way when new tags are found in stream "extra data" you can simply add them to the config file and not have to hard code all the different implementations.

I also made a small change so streams can also use playlist.png images if the above fails, but doesn't cache the result. I've noticed some streams show no images on when song isn't playing (like DJ talking / advert etc).

Need to clean it up, but if you want it let me know and I'll create a pull request.

[ejurgensen]

Sounds like a very good solution to me, I would appreciate a PR.

I'm not sure what I think about the other change, so please separate the commits. If you are putting the stream artwork handler above the cache it sounds like we will be making a number of extra http requests. But let me take a look at it.

[sfeakes]

The cache change was a simple one, easy to back out. I changed the "playlist own" from "cache = ON_SUCCESS | ON_FAILURE" to "cache = STASH" (in artwork.c)

One other change I forgot to mention, I also modified the json sent to the web UI to add a few more checks. Currently It just passed queue_item->artwork_url if artwork_url was http(s). Obviously that doesn't work when you need to make an extra HTTP call to get the actual image URL, so now it also check for the queue_item->artwork_url containing .jpg or .png, and passes that direct if it does, and if not passed the local artwork URL to force webIU to request image through forked-daapd. Again, simple to back-out.

I'll make a PR tomorrow and you can see, if you want modifications / multiple PR's to fit within the overall design of forked-daapd, just let me know. I tried to follow how it's currently been written / architected as best I could.

[ejurgensen]

I've now merged #1006 which should include the improvements you suggest, except I haven't gotten around to the issue about clearing metadata yet (I'll make a separate issue for that). Hope it works.

Will close this issue along with PR #1007. Thanks for your contribution with this.

BTW the default for the time will be to not process StreamUrl resources unless the user has enabled this (currently only possible via the API). The reason for that is that I haven't seen many of these type of radio stations (most seem to just have nothing in the StreamUrl). Perhaps I am just not looking properly, so if I hear about more of these streams I will change the default.