Bug 4167 - Press and hold FAVORITES needs to work wherever you can play something
: Press and hold FAVORITES needs to work wherever you can play something
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Player UI
: 6.5b3
: Macintosh Other
: P2 normal (vote)
: ---
Assigned To: Andy Grundman
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-20 13:03 UTC by Blackketter Dean
Modified: 2008-12-18 11:11 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments
new favorites function in Common.pm (2.58 KB, text/plain)
2006-09-21 16:33 UTC, KDF
Details
patch for above (1.63 KB, patch)
2006-09-21 20:42 UTC, KDF
Details | Diff
updated and tested patch (2.83 KB, patch)
2006-09-22 23:35 UTC, KDF
Details | Diff
with upnp (2.87 KB, patch)
2006-09-23 12:47 UTC, KDF
Details | Diff
and without debug code :) (2.84 KB, patch)
2006-09-23 12:48 UTC, KDF
Details | Diff
live365 support (3.75 KB, patch)
2006-09-23 13:20 UTC, KDF
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Blackketter Dean 2006-09-20 13:03:44 UTC
Nothing happens now, it should add the currently displayed song to favorites.

Merge to SN too please.
Comment 1 Blackketter Dean 2006-09-21 08:02:28 UTC
A few more places to add FAVORITES button support:
- browsing opml 
- Now Playing playlist
- trackinfo/remotetrackinfo
- live365
- moodlogic instant mix
- upnp browser/rhapsody
- browsetree
- browsedb (I think dan covered this)
Comment 2 KDF 2006-09-21 15:47:10 UTC
works for Browsetree for me, 7.0 at least.

should work naturally with anything using INPUT.List.
specifically, it looks for the listRef param, so any other modes could populate that to support the function.

moodlogic mix fails because the item is in url form for that list.  adding an objectForURL call fixes this, and may help for other cases that aren't yet using fully expanded objects in their lists.


conversely, perhaps the routine for adding favorites can be rewritten to expect a url and title only, taking these items from objects and grabbing the title info when sent only a url (falling back to the raw url if none found)

that should then solve most other cases.
Comment 3 KDF 2006-09-21 16:33:10 UTC
Created attachment 1567 [details]
new favorites function in Common.pm

I can't easily make a diff from there, but this reworked favorites function for Common.pm reflects what I was describing above.  This now works for individual items in opml browsing (container levels still need a url/title scheme).  

It works for instant mix as well.  Trackinfo and RemoteTrackInfo can be name to work, but needs some thinking.  Trackinfo has a param('track'), while remoteTrackInfo has param('url') and param('title').  We could cover those cases when using the listRef doesn't reveal a valid object.  

Liv365 is a bit harder.  It's so completely different from most of slimserver code style.  the lists are index numbers with externrefs.  We could add the ability to use lookupRefs and externRefs.  Let me know if you'd like me to continue.
Comment 4 Blackketter Dean 2006-09-21 16:48:16 UTC
Subject: Re:  Press and hold FAVORITES needs to work wherever you can play something

Thanks, KDF.  If you can keep going, great.  I'd really like to get  
as much of this in 6.5.1 as possible, the FAVORITES button seems kind  
of broken right now.

Comment 5 KDF 2006-09-21 20:42:46 UTC
Created attachment 1569 [details]
patch for above

proper diff for the above reworked function.
Comment 6 KDF 2006-09-22 23:35:11 UTC
Created attachment 1575 [details]
updated and tested patch

Current status:

Done(only at item level, not category) - browsing opml 
Done - Now Playing playlist
Done/Done - trackinfo/remotetrackinfo
no - live365
Done - moodlogic instant mix
can't test - upnp browser/rhapsody
Done - browsetree
already there - browsedb (I think dan covered this)


the current playlist feature is a nuisance hack, as I'm sure we are having issue with browseplaylistindex().  It would be nice if the current playlist eventually used INPUT.List in some way.

I don't know what to do with rhapsody. I've not had much luck getting the musicmagic upnp to work, and while I did get Twonky working, the trial has run out.  It might just work, being as it uses remoteTrackinfo and INPUT.Choice, but I can't say for sure.

i also have a question about Favourites in general.  I notice now that when we delete, we're erasing that index.  This means the favorites above the deleted index will change the number that you have to press and hold.  A while back, I was made a change in response to a bug report that asked for the indexes to be kept so that the number presses don't change.  What I hadn't done was add in a cleanup to remove the empty slots at the end of the list.  

What is the proper design here?  Is the current functionality the final decided intent?
Comment 7 Andy Grundman 2006-09-23 09:56:19 UTC
Try GMediaServer for simple UPnP.  Or I can look at it.
Comment 8 KDF 2006-09-23 12:47:12 UTC
Created attachment 1580 [details]
with upnp

ok, this adds upnp support.

not really sure what to do with live365.  I'll try and find a way to make a specific hack, but it's a hard plugin to decypher
Comment 9 KDF 2006-09-23 12:48:13 UTC
Created attachment 1581 [details]
and without debug code :)
Comment 10 KDF 2006-09-23 13:20:38 UTC
Created attachment 1582 [details]
live365 support

This adds Live365 support.  I've checked against the presense of the protocol handler instead of the plugin being enabled/disabled.   I think it is just simpler code that way.  This does seem to work, and plays back from favorites listing.  

What I don't know is whether simply fixing the url to live365:// will get the login and everything working if the favorite is selected many days later.

anyway, this should cover the original list of required support.
Comment 11 KDF 2006-09-25 19:39:02 UTC
in trunk at change 10039.  Please test and let me know how it works out for you before I merge to 6.5
Let me know if I've missed anything
Comment 12 KDF 2006-10-06 20:25:29 UTC
fixed in 6.5.1 at change 10255.

marking as fixed, but please reopen if there are any issues.  I've tried out all of the cases above in at least one incarnation, and added a another case for radio links in playlist mode.