Bug 6839 - showBriefly popup doesn't show cover art for remote tracks
: showBriefly popup doesn't show cover art for remote tracks
Status: CLOSED FIXED
Product: SB Controller
Classification: Unclassified
Component: UI
: unspecified
: Macintosh Other
: P3 normal (vote)
: 7.0
Assigned To: Ben Klaas
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-28 14:39 UTC by Andy Grundman
Modified: 2009-09-08 09:28 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
Patch to move showBriefly for jump into command handler (2.50 KB, patch)
2008-02-02 07:16 UTC, Adrian Smith
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Grundman 2008-01-28 14:39:23 UTC
The showBriefly popup for playing/pausing a track doesn't show the artwork if the art is a remote URL (Pandora, Rhapsody, Slacker, etc).
Comment 1 Blackketter Dean 2008-01-28 20:11:11 UTC
Ben: Is this a hard thing to fix?
Comment 2 Ben Klaas 2008-01-31 20:59:24 UTC
After spending several hours with this tonight, I'd say the answer is a resounding YES.

I can't figure out where the showBriefly for these events are being generated. I've tried backtracing it to no avail.

Data:Dump shows this as the message getting sent out--

  jive    => {
               "icon-id" => 0,
               text => ["Paused", "Thievery Corporation Radio"],
               type => "song",
             },
  line    => [
               "Paused",
               "Even After All by Finley Quaye from Maverick A Strike",
             ],
  overlay => [undef, "\1"],
}


what needs to change is 'icon-id' => 0 needs to change to 'icon' => the imageURL or the path to the service Icon
problem is, I cannot figure out where the data for this is being constructed. The very first pop-up (initial play command) comes from Commands.pm, but not subsequent track advances.

right now I'm stumped.
Comment 3 Ben Klaas 2008-02-01 08:50:24 UTC
SC change 17096 and Jive r1735 has a partial fix for this. There is a remaining problem that some remote tracks that buffer (e.g., Rhapsody) are not sending the correct showBriefly to Jive

jive-side, in jive.slim.Player.lua:

Remote URLs for artwork are sent via an 'icon' not 'icon-id' flag
Resize thumbs to 56 pixels for showBrieflys
Allow for rendering of static image paths in uri generator (e.g., radio icon). This section comes largely pasted from SlimBrowser


SC-side, in Player::Player::Songlines:

this should work for remote streams without artwork, both regular internet streams (which will show the static radio.png image) and those carried by service providers, e.g., Live365 (this will show the service provider icon)

what is still broken is the wrong showBriefly is getting sent to jive on track change-

for example, if you hit next to go from Rhapsody Track A to Rhapsody Track B,
you will get a showBriefly on Jive that says 'Now Playing - Track A' with the artwork for Track A.

In other words, there is a showBriefly sent before the "buffering" showBrieflys that go to the playerUI, but nothing afterwards.



Comment 4 Adrian Smith 2008-02-01 17:01:36 UTC
Ben - so the reason for the out of date popups is as follows - looking for the best way to fix...

When you press fwd, a jump command is executed followed immediately by showBriefly showing the currentSongLines.
currentSongLines currently displays the "playingSongIndex"  However when we are buffering a new remote stream it looks like the "streamingSongIndex" is set to the song we want to display and the playingSong is the previous one.

The buffing display hides this for traditional players as it uses the streamingSongIndex, however for jive it is too noisy so we don't send that - this means you see the original showbriefly which occurs at the time of pressing fwd.

I think the right thing to do is to find some logic for currentSongLines to display the buffering song when the player is buffering a new remote song and the player is not yet playing.  Do people agree - if so whats the simplest state variables to look at for this - really want something to tell the player is currently buffering a remote stream but not playing an existing remote stream [I assume we need to check the latter for music services].
Comment 5 Adrian Smith 2008-02-02 06:33:06 UTC
Hum - Ignore the above - I was obviously not recreating your problem last night.  For normal remote streams I think it works fine.  Whenever Slim::Player::Source::jumpto is called it will clear the streaming queue and hence after this point playingSongIndex and streamingSongIndex will be the same.

However I have found that Slim::Control::Commands::playlistJumpCommand allows jumpto to be called in a callback for protocol handlers that have and onJump command.  I think this is possibly the cause of your out of order problem as Pandora does an async function before the callback in called.  However the showBriefly for the fwd action is synchronous and happens as soon as Slim::Control::Commands::playlistJumpCommand returns.

I will try and see if I can move the showBriefly into the callback, but need to understand what else this impacts..
Comment 6 Adrian Smith 2008-02-02 07:16:25 UTC
Created attachment 2803 [details]
Patch to move showBriefly for jump into command handler

Patch which moves where the showBriefly occurs - looking for feedback whether this  works for the problem cases identified by Ben.  Specifically it should move the showBriefly into the callback and hence it should display the correct track info when you jump to a new track when the new track uses an asyc callback for the jump command.

Patch also removes showBriefly for pause as this is already handled in the command handler (we duplicate at present)
Comment 7 Adrian Smith 2008-02-03 13:38:57 UTC
I've been using this patch for the last day and the player display messages look to work correctly for me for local and remote streams, so if it fixes your case Ben I think the patch is good to use.  Please apply after testing as I may not be around for the next two days..
Comment 8 Ben Klaas 2008-02-04 14:33:02 UTC
change 17149 fixes this.

I had thought last night it only mostly covered it, but I can't find anywhere now where they aren't working correctly.

moving to RESOLVED
Comment 9 James Richardson 2008-05-15 13:02:37 UTC
This bug has recently been fixed in the latest release of SqueezeCenter 7.0.1

Please try that version, if you still see the error, then reopen this bug.

To download this version, please navigate to: http://www.slimdevices.com/su_downloads.html