GNOME Bugzilla – Bug 345480
Last.fm radio support.
Last modified: 2006-08-31 19:03:51 UTC
I'm working on adding support for last.fm radio in rhythmbox by integrating it into the last.fm song submission plugin that already exists in RB. I have a version that works, but only for neighbor radio so far. I would like suggestions about how people think the interface would be best laid out, since the framework is already there to just add other lastfm:// uris.
Created attachment 67742 [details] The modified files from the plugins/audioscrobbler directory It's not perfect, but it does work, please try it -- also, enabling/disabling the plugin crashes RB, I'll try and fix that :)
Created attachment 67758 [details] [review] lastfm radio patch
I haven't tested it out yet, but a couple of comments: rb_lastfm_source_constructor(): + rb_lastfm_source_new_station( + g_strdup_printf("lastfm://user/%s/neighbours", rb_lastfm_source_get_username()), + _("Neighbour Radio"), + source); This will leak the string, you should store it in a variable, and then g_free() it afterward. rb_lastfm_source_new leaks a reference to the proxy config. Add "g_object_unref (proxy_config);" at the end. rb_lastfm_source_get_password leaks the password string, it should g_free() it after generating the md5 hash. Variables need to be defined at the start of functions; a few places (like rb_lastfm_source_do_handshake) define some in the middle. rb_lastfm_source_get_playback_uri(): + rb_debug (g_strdup_printf(source->priv->stream_url)); This looks just wrong, is the g_strdup_printf supposed to be there? Entry-type related: As the entry type is source-specific, it should be registered in the constructor and not have a global rhythmdb_entry_lastfm_get_type function. This also means moving the rb_shell_register_entry_type_for_source call into the constructor. The source should override the impl_delete_thyself RBSource virtual function and delete all the db entried of the type. i.e. "rhythmdb_entry_delete_by_type (source->priv->entry_type);"
This should all really be discussed on the old bug #313049 to keep the discussion all in one place. Closing as a dupe (also moving patch there). *** This bug has been marked as a duplicate of 313049 ***
I've added skip/love/ban buttons and I'm starting to work on an interface to add stations. Right now the server isn't honoring my requests, so I'm not quite sure what's going on, hopefully that's a problem on their end. My idea for a station tuner would be to put a small interface above the entryview, not on the bar with browse, but as part of the source view. I was just thinking a label with instructions, a combo box to select artist or global tag, a textbox and a button to make it. Any other ideas before I try and make it?
Created attachment 68190 [details] [review] new patch If simply replacing the text of the old file with the new works in .diff files, then this is the latest in last.fm support. Ban/Love/Skip work, but right now there seems to be trouble switching stations. With just a little patience, it works tolerably well. Also, the only way to add stations right now is to drag-n-drop last.fm URIs into the source window. (things like lastfm://user/fisxoj/neighbours) It even has a sort of 'smart URI reader' that will name the stream for you. If you drop a custom station URI (one with lots of numbers listed) it will just list the lastfm:// URI because those *should* return a name from the server. That doesn't work yet, though. Enjoy