[Squeezebox] Cover download not working

[kungfoolfighting]

I am running 2.3.0 Snapshot and the cover is not being displayed for the cover items that I have. Only very rarely does a cover show, and I don't know what conditions make it work. My item: Image sq_dining_cover "Cover" {channel="squeezebox:squeezeboxplayer:2278B73C-21BC-419C-B96D-5CF10CE3DBD1:8cae4cf81772:coverartdata"}

The sitemap: Image item=sq_dining_cover

I guess the update of the squeezebox binding to use the ESH function for image download might have broken something. I activated debug logging for squeezebox and I am getting a ton of this:

2017-12-28 18:45:13.621 [DEBUG] [ebox.handler.SqueezeBoxPlayerHandler] - Trying to download the content of URL http://192.168.2.115:9000/https://i.scdn.co/image/a5f082179a69f18d056b124f01f9158ccbe57255 
2017-12-28 18:45:53.559 [DEBUG] [ebox.handler.SqueezeBoxPlayerHandler] - Failed to download the content of URL http://192.168.2.115:9000/https://i.scdn.co/image/a5f082179a69f18d056b124f01f9158ccbe57255

The url is nonsense, but the base url is correct (its the server IP of the logitech media server)

[mhilbush]

@phil2fer Based on the broken URL above, when the artwork_url contains https, you might also need to include a check for https here.

https://github.com/openhab/openhab2-addons/pull/2906/files#diff-6abbe31c933b849f493ec1a48d6a5e0eR555

[phil2fer]

@kungfoolfighting could you test this jar file. I check now if the artwork_url contains https org.openhab.binding.squeezebox-b4.jar.pdf

[kungfoolfighting]

@phil2fer I have tested the jar file and it does seem like the error messages are gone. However, sadly, I think I am dealing with two separate issues here. The cover images are still not correctly updating. I think previously the binding was trying to download an image from spotify that was being used on another player and it failed at that due to the https address. But it also has problems just getting the covers from my server.

Only every once in a while does the cover update, normally it doesn't. I can't find a pattern for this. The debug log is very sparse. When just letting it play it looks like this: 2017-12-29 13:43:00.430 [DEBUG] [ebox.handler.SqueezeBoxServerHandler] - Sending command: players 0 over and over. And that is it.

[phil2fer]

Could you check if you can see the cover with this url this is a shortcut URL to return the artwork of the currently playing song for a player

http://<server>:<port>/music/current/cover.jpg?player=<playerid>
where:
    <server> is the ip address or name of the server.
    <port> is the HTTP port of the server (not the same as the CLI port).
    <playerid> is the unique identifier for the player, as above. If omitted, the server will use a random player.

For the playerid you can use the mac adress like this : xx:xx:xx:xx:xx:xx

[kungfoolfighting]

I tried that and it works.

[kungfoolfighting]

So to clarify: The logs show that the binding is trying to download artwort. The links are now correct. But they are the artworks for the other players, not the one that is currently playing. All the other players are paused, and yet, for some reason the binding is trying to download them. I also do not see any such debug message for trying to download the artwork whenever the song on the active player changes.

I have this rule:

rule LivingCover
when
    Item sq_living_cover received update or
    Item sq_living_cover received command or
    Item sq_living_cover changed
then
    logInfo("Livingarea", "Updating unified Cover from Livingroom.")
    if(CurrentlyActiveLivingAreaPlayer == "LivingRoom")
    {
        sq_living_area_unified_cover.postUpdate(sq_living_cover.state)
    }
end

And it very rarely triggers. Only like once in every 10 songs, and even then I am not sure if it does it at the beginning of the song.

[mhilbush]

I'm seeing something similar. I was going to try to debug it a bit this morning, but I got sidetracked on a few other things. When streaming Pandora, I can see in the debug logs (player status message) that the coverart URL changes whenever the track changes.

2018-01-02 16:34:54.614 [TRACE] [ebox.handler.SqueezeBoxServerHandler] - Message received: b8%3A27%3Aeb%3A16%3A3f%3A1e status - 1 subscribe%3A10 tags%3AyagJlNK player_name%3AOffice player_connected%3A1 player_ip%3A192.168.1.132%3A36196 power%3A1 signalstrength%3A0 mode%3Aplay remote%3A1 current_title%3A time%3A222.99387125206 rate%3A1 duration%3A230.79175 mixer%20volume%3A55 playlist%20repeat%3A0 playlist%20shuffle%3A0 playlist%20mode%3Aoff seq_no%3A0 playlist_cur_index%3A0 playlist_timestamp%3A1514910048.28216 playlist_tracks%3A1 digital_volume_control%3A1 remoteMeta%3AHASH(0x997e498) playlist%20index%3A0 id%3A-148397720 title%3ASay%20(All%20I%20Need) year%3A0 artist%3AOneRepublic album%3ADreaming%20Out%20Loud artwork_url%3Ahttp%3A%2F%2Fmediaserver-cont-ch1-2-v4v6.pandora.com%2Fimages%2Fpublic%2Frovi%2Falbumart%2F7%2F9%2F1%2F7%2F602517507197_500W_500H.jpg buttons%3AHASH(0x9e2e5a0)

2018-01-02 16:35:03.316 [TRACE] [ebox.handler.SqueezeBoxServerHandler] - Message received: b8%3A27%3Aeb%3A16%3A3f%3A1e status - 1 subscribe%3A10 tags%3AyagJlNK player_name%3AOffice player_connected%3A1 player_ip%3A192.168.1.132%3A36196 power%3A1 signalstrength%3A0 mode%3Aplay remote%3A1 current_title%3A time%3A0.962505151748657 rate%3A1 duration%3A285.0741875 mixer%20volume%3A55 playlist%20repeat%3A0 playlist%20shuffle%3A0 playlist%20mode%3Aoff seq_no%3A0 playlist_cur_index%3A0 playlist_timestamp%3A1514910048.28216 playlist_tracks%3A1 digital_volume_control%3A1 remoteMeta%3AHASH(0x9dfb040) playlist%20index%3A0 id%3A-148397720 title%3AHear%20You%20Me year%3A0 artist%3AJimmy%20Eat%20World album%3ABleed%20American artwork_url%3Ahttp%3A%2F%2Fcont-1.p-cdn.com%2Fimages%2Fpublic%2Frovi%2Falbumart%2F9%2F2%2F4%2F3%2F600445033429_500W_500H.jpg buttons%3AHASH(0x9df1eb8)

[phil2fer]

@kungfoolfighting sorry i don't understand the goal of your rule.

Why you don't use a item like : Image Lms_cover {channel="squeezebox:squeezeboxplayer:myServer:myplayer:coverartdata"}

and a sitemap like : Image item=Lms_cover

(You dont have to make rule to update cover)

If you have multiple player, and you want to display only the cover of the active player try to use the visibility parameter. something like that : item :

Image  Lms_cover1   {channel="squeezebox:squeezeboxplayer:myServer:myplayer1:coverartdata"}
Image  Lms_cover2   {channel="squeezebox:squeezeboxplayer:myServer:myplayer2:coverartdata"}

sitemap :

Image item=Lms_cover1 visibility=[CurrentlyActiveLivingAreaPlayer == "LivingRoom" ]
Image item=Lms_cover2 visibility=[CurrentlyActiveLivingAreaPlayer == "qqch"]

[mhilbush]

Here's one problem I see. There may be others.

Since the default cover art URL (http://lmshost:lmsport/music/current/cover.jpg?player=mac) is used for most cover art retrievals, the caching feature implemented in this PR always results in the image being pulled from cache. Of course, the image will never change, even when changing to another track because the key used to look up the image from cache is always the same. https://github.com/openhab/openhab2-addons/pull/3035

[mhilbush]

@cweitkamp Do you have any thoughts on this? When creating the caching feature, you might not have realized that the LMS supplies a convenience cover art URL (http://lmshost:lmsport/music/current/cover.jpg?player=mac) , which is used in almost all cases to retrieve the image except in a few edge cases. Since this URL is the key to the image cache, the cover art image will rarely be correct.

[kungfoolfighting]

@phil2fer You misunderstood why I posted the code snippets. The snippet is used for some complicated thing where I want to use only one interface in HabPanel to control two different players, depending on which one is currently playing. In order to do that, I have "unified" variants of all items that are necessary and I update the unified items whenever the actual items of the currently active player are updated. The rule I posted is the rule to do this very thing with the cover art.

The other reason why the rule looks the way it looks is for debugging purposes. I wanted to see when this rule triggers, because this means that the cover art has updated.

Even without using this complicated code, just using the items exactly like you wrote, the item does not update on the sitemap for every new cover.

The caching issue that @mhilbush describes, sounds like it could have something to do with it.

[phil2fer]

@kungfoolfighting i understand better now. Thank you for your explanation.

@mhilbush thank you for reporting this PR. I did not see him. There seems to be 2 distinct problems. One for https url, and one for caching. Can i make my PR (for https) refered to this issue ? or should i open a new one ?

[cweitkamp]

When creating the caching feature, you might not have realized that the LMS supplies a convenience cover art URL (http://lmshost:lmsport/music/current/cover.jpg?player=mac) , which is used in almost all cases to retrieve the image except in a few edge cases. Since this URL is the key to the image cache, the cover art image will rarely be correct.

@mhilbush I have to admit I didn't have that in my mind. Sry. If that's the case I suggest to remove the caching feature again.

How often is the image downloaded from the LMS server? Do you can think of any other / better key for the caching mechanism? I see something like a hash in the responses you posted in https://github.com/openhab/openhab2-addons/issues/3043#issuecomment-355060174.

[mhilbush]

No worries, @cweitkamp. Don't remove caching just yet. Let me look around to see if there's something else that can be used for the key. Keeping the cache is a good idea, as it will avoid requesting the cover art image every several seconds.

[phil2fer]

@kungfoolfighting : Only for test purpose you can install this jar file. I temporaly remove the cache feature org.openhab.binding.squeezebox-b5.jar.pdf Check if your issues are gone

[kungfoolfighting]

@phil2fer I wouldn't mind having the PRs for both issues point to this thread.

Yes, I will try the jar.

I also have another question: the cover art download seems to be triggered by a timer, why? Should it not be enough to only download cover art if a new track starts playing? (or even less frequently when multiple tracks if the same album have the same cover art)

@cweitkamp I don't think you can use that hash, which you mentioned. This seems to be specific to Pandora. I have not seen this sort of syntax in the Spotify urls for cover art.

[mhilbush]

Looking at the LMS docs, I'm wondering if the c tag should be included in the status request in order to use the coverid as the cache key.

Changes starting from Squeezebox Server 7.6
Added tag 'M' for songinfo to return track musicmagic_mixable value.
Added tag 'c' for songinfo to return the track's coverid value, used for artwork URLs. If you were using the track's ID directly in artwork URLs, you should switch to using the coverid value.

I also have another question: the cover art download seems to be triggered by a timer, why?

Not sure what you mean here. The timer is used to periodically remove items from the cache.

Unless you're referring to the periodic player status request...

Can you clarify what you're referring to?

[mhilbush]

The coverid looks encouraging for use as the cache key. I tested with local music and and a few streaming radio stations. Unfortunately, I can't test with Pandora and Slacker, as they are both broken at the moment... http://forums.slimdevices.com/showthread.php?108502-Did-Pandora-stop-working-for-anyone-else

[phil2fer]

@mhilbush : Just an idea, we can add a hash string to the coverart url like this http://lmshost:lmsport/music/current/cover.jpg?player=mac&hash=hashstring with hashstring = md5(original URL)

with this, we can keep the cache feature

[mhilbush]

I'm actually thinking we should use the coverid in order to form the cover art URL. Like this:

http://lmshost:lmsport/music/<coverid>/cover.jpg

This would work nicely with the current cache implementation. But we need to test with multiple different music sources to see if there are any edge cases where this won't work.

[kungfoolfighting]

@mhilbush I can test spotify.

[kungfoolfighting]

@phil2fer I have tried with the new jar and it works very nicely. So I guess it is definitely the caching that is the issue.

[mhilbush]

it is definitely the caching that is the issue

That's good news. Now if we can figure out the best approach to solve the caching issue.

From what I'm seeing...

When coverid is non-negative (i.e. it doesn't start with a -), the cover art can be retrieved using this URL, which is suitable for the cache key. http://lmshost:lmsport/music//cover.jpg

When coverid is negative (i.e. it starts with a -), the cover art can be retrieved using the URL returned in artwork_url, which I think also is suitable for the cache key.

Unfortunately, I can't test with Pandora and Slacker due to the above mentioned issue.

@phil2fer WDYT? Does this handle your edge cases?

[phil2fer]

i make some test with the "c" tag too

When coverid is non-negative (i.e. it doesn't start with a -), the cover art can be retrieved using this URL, which is suitable for the cache key. http://lmshost:lmsport/music/coverid/cover.jpg

ok for that

When coverid is negative (i.e. it starts with a -), the cover art can be retrieved using the URL returned in artwork_url, which I think also is suitable for the cache key. ok but in this case we have to hit the artwork_url (and not http://lmshost:lmsport/music/current/cover.jpg?player=mac)

This work only if Openhab have acces to internet (I think it's 99% the case but)

I see some case when there are no artwork_url and no coverid (when there are no cover) and in this case only the current cover url works (for displaying the "default" cover)

so my opinion : if we use the coverid information, i think it complicating the code to fetch the correct url, maybe the hash parameter could be an alternative solution It's just my opinion (i am newbies in openhab addon developpement..)

and sorry for my very bad english... i try to do the best

[mhilbush]

What about something as simple as clearing the cache when receiving a duration event with duration of 0? This might be ok since each player handler has its own cache.

    @Override
    public void durationEvent(String mac, int duration) {
        if (getThing().getChannel(CHANNEL_DURATION) == null) {
            logger.debug("Channel 'duration' does not exist.  Delete and readd player thing to pick up channel.");
            return;
        }
        if (isMe(mac) && duration == 0) {
            IMAGE_CACHE.clear();
        }
        updateChannel(mac, CHANNEL_DURATION, new DecimalType(duration));
    }

[kungfoolfighting]

@phil2fer @mhilbush From what I understood, @phil2fer wrote that only in the case where there is no cover the coverid would not be there but also no specific url. But in that case we always want to display the same cover (default cover) anyways and as such the default url could be used as a cache key, right?

[mhilbush]

My bad. There's just one cache shared by all players.

[phil2fer]

Could you test this jar file. (Cache + coverid + https) org.openhab.binding.squeezebox-b6.jar.pdf

I add a debug log (info) to display the covertart url

[kungfoolfighting]

@phil2fer I have tested the newest jar and it works fine. The covers are displayed the way they should be for both local music and Spotify. However: I see a constant spam of "coverart url" info traces that are referencing covers for other players that are not even currently playing. I assume that this trace is being written before the cache lookup, so it doesn't lead to downloads? What triggers this message? Should you not only look for new covers whenever a new track starts playing?

[phil2fer]

I assume that this trace is being written before the cache lookup, so it doesn't lead to downloads?

you are right

However: I see a constant spam of "coverart url" info traces that are reverencing covers for other players that are not even currently playing What triggers this message?

I try to explain : Thirst the binding connect to the squeezebox server (telnet) second: it announces (all) the player after that the server send continuously information of the current song, playlist etc.. the binding catch this information for every player b8%3A27%3Aeb%3A39%3A89%3Abe status - 1 subscribe%3A10 tags%3AyagJlNKc player_name%3ApiCorePlayer player_connected%3A1 player_ip%3A192.168.1.34%3A57894 power%3A1 signalstrength%3A0 mode%3Aplay remote%3A1 current_title%3A time%3A1273.66968982697 rate%3A1 mixer%20volume%3A20 playlist%20repeat%3A0 playlist%20shuffle%3A0 playlist%20mode%3Aoff seq_no%3A0 playlist_cur_index%3A0 playlist_timestamp%3A1515090030.85743 playlist_tracks%3A1 digital_volume_control%3A1 remoteMeta%3AHASH(0x4b03ce0) playlist%20index%3A0 id%3A-85307920 title%3ATSF%20Jazz year%3A0 artwork_url%3Ahtml%2Fimages%2Fradio.png coverid%3A-85307920

and the binding split this information into small message like "coverid" or "artwork_url" and analyse it That why you received constant spam

Should you not only look for new covers whenever a new track starts playing?

if you want to do that we have to store and compare the url cover beetween 2 messages, but this is that the new cache feature do..

[mhilbush]

Should you not only look for new covers whenever a new track starts playing?

if you want to do that we have to store and compare the url cover beetween 2 messages, but this is that the new cache feature do..

Not necessarily. As I said above, you can use duration==0 to clear the cache. In addition, you would create a cache for each player (move cache creation to initialize method). It's not really a performance hit because the cache is there primarily to avoid image downloads on each player status message. You also would wrap the body of the downloadImage method with an isMe(mac) check.

[phil2fer]

Could you test this jar file. (Cache + coverid + https + hash) All the url request stay in the local network

org.openhab.binding.squeezebox-b7.jar.pdf

@mhilbush, i don't know if duration work in case of streaming radio

[mhilbush]

@phil2fer I would never say never. But I've not seen a case where duration has not been reset when switching tracks or stations.

While you're in the code, the parseInt call here needs to handle a NumberFormatException. https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.squeezebox/src/main/java/org/openhab/binding/squeezebox/handler/SqueezeBoxServerHandler.java#L593

@pgfeller FYI

[phil2fer]

@mhilbush

While you're in the code, the parseInt call here needs to handle a NumberFormatException. https://github.com/openhab/openhab2-addons/blob/master/addons/binding/org.openhab.binding.squeezebox/src/main/java/org/openhab/binding/squeezebox/handler/SqueezeBoxServerHandler.java#L593

Ok for me

I can push a first PR with b7 jar file (it seems to work correctly). Are you Ok ? I do not have enough skill to code what you want (cache for each player, duration etc..)