GNOME Bugzilla – Bug 313049
last.fm radio stream support
Last modified: 2009-07-28 00:20:37 UTC
The quite popular site audioscrobbler.com was relaunched under last.fm, with streaming music support. This requires their own QT-based streaming music client; however, the (portable) source code is published under the BSD licence: http://www.last.fm/help/player/ Direct support in Rythmbox would be cool.
Yes, please.
I don't have QT4 on my machine at the moment, so I can't say anything about their client (yet), but the protocol is a fairly simple one using HTTP. I think the biggest thing that needs to get done is to have a discussion of how the functionality fits into Rhythmbox, and the UI for it. I can't make any comments until I've played with the existing client, but if anyone wants to offer suggestions of how it should work and then later make some mockups that'd be great.
last.fm has also an authentication protocol in order to let the listener to manage his prefered music (by clicking on 'i love it' directly from the player). The player also connects lastfm's audioscrobbler server in order to set an history of "who listen to what" for future webservice exploitation.
Suggestions for the integration into the UI: Add an item "Last.fm Recommendations" to the source list on the left. If the protocol provides a song list (what's up next...) then show those in the listview on the right. otherwise just disable it. Concerning the "I love it" button: add to main toolbar, but show it only when the last.fm support has been enabled in the preferences.
The way I would probably add it would be to make the "submit songs to last.fm" separate from entering your user/password. If you have entered the info, a "last.fm radio" source shows up. The source would be sort of a cross between the last.fm player and the normal Internet radio source. Having a "last.fm suggestions" or a play order that choosing things based on your profile would be separate (but nice).
*** Bug 324460 has been marked as a duplicate of this bug. ***
I've had a quick look at what we'd need to do to support last.fm radio streams. Core changes: - per-source toolbar items (for the 'love this song'/'ban this song' buttons) - per-source behaviour for the previous/next buttons (disable previous, make next do the same as 'skip this track' in the last.fm player) - ability to override the default entry-activated signal handler for the entry view. (this may already exist) We'd have a few different 'create station' dialogs, for user stations (personal, neighbour, loved tracks), groups, tags (user and global) and similar artist stations. Each created station would appear as an entry in the last.fm radio source. When the user first activates last.fm radio streams, we'd create their neighbour station, and if they're a subscriber, their personal and loved tracks stations. The entry-activated handler for the entry view would create a last.fm radio stream connection if we didn't already have one, and then invoke the web service used to control the stream. Changing radio station doesn't require a new stream connection.
*** Bug 330482 has been marked as a duplicate of this bug. ***
Rhythmbox already has a ratings UI, shouldn't this be used to handle love/hate submissions?
FYI, there is a last.fm player for gtk here: http://www.o-hand.com/~iain/last-exit/ May be able to steal some code to get this working in RB if wanted.
http://bugzilla.gnome.org/show_bug.cgi?id=345480 I'm working on support, and the GUI suggestions here sound good. I'll look into the ban/love buttons,and I suppose a skip button too.
Created attachment 67840 [details] [review] Moved patch from duplicate bug #345480 Moving patch and comments here from duplicate bug. >>>> Initial comment from Matt N: 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. >>>> Response comment from Doc Livingston: 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);"
*** Bug 345480 has been marked as a duplicate of this bug. ***
Created attachment 68281 [details] [review] New stuff Ban/Love/Skip work, but right now there seems to be trouble switching stations. This seems to be related to other bugs I caught involving using g_strconcat(). If anyone knows of some property of strings made with that function that prevents them from being sent as URLs, that'd be nice to know. 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 -- and give feedback! What should the station creation interface be like? Should there be a "connect" button instead of it handshaking when you first doubleclick on an entry? (That's certainly a terrible way to trigger the handshake)
Created attachment 68282 [details] [review] latest, greatest I fixed the station switching problems. I was handling encoding the url badly.
Seems to work OK. In rb-lastfm-source.c, you've got a call to gtk_paned_pack2 before the paned widget is created. It'd be good to have some feedback from the skip/ban/love buttons indicating it'll take a while to take effect, otherwise people will press the buttons multiple times. I very nearly did. Pulsing the progress bar would be a reasonable way to do this. To do this, call rb_source_notify_status_changed() and have impl_get_status set the progress value to -1.0 (based on a flag in the source object, I guess).
Works very good, but there are some small issues that could improve usability: - Displaying the name of the current track - artist. - Make the ban/love unsensitive after pressing them. - Make the button skip unsensitive while the next song is being cached. Great work so far! I am using it and really like it.
Last.fm sends the metadata *in* the stream, which means I can't get at it yet. I've been told there's actually some work in progress to make gstreamer backend relay the info, and when that happens, it'll be very easy for me to stik the track and artist wherever. The ban/love/skip I can only make insensitive until the server responds with an OK. This doesn't mean the new song has actually cached yet, but I have no other indicator until the backend starts sending metedata. I'm working on the interface, so tell me what sort of stations you want to be able to make. So far, similar artist and global tag are implemented. I'm not sure if there's a way to check if your station is valid, and you can't remove them yet. I did implement the pulsing statusbar, and improved the status text a bit. Also, changing stations seems to require restarting the entry a few times. I don't know why though.
Created attachment 68585 [details] [review] I think this is the best I can do without being able to get at the stream metadata Station changing seems to be working, the status messages are informative, though it can only tell you when the server says it's changing the song. I think I'd count this as a first stable-ish release.
Nice work! Could a right-click context menu to radio stations be added with a "Delete" entry to remove radio stations? e.g. sometimes a tag doesn't exist or I otherwise want to remove a station, curently there appears to be no way to do this.
(In reply to comment #18) > Last.fm sends the metadata *in* the stream, which means I can't get at it yet. > I've been told there's actually some work in progress to make gstreamer backend > relay the info, and when that happens, it'll be very easy for me to stik the > track and artist wherever. Is there a bugzilla bug on that?
Also it doesn't seem to save the radio stations you have created from Artist or Tags. They are lost on a restart of rhythmbox.
As suggested in comment #10, the last-exit player: http://www.o-hand.com/~iain/last-exit/ has some code (in C#) that parses the metadata from interogating the session URL or somesuch, perhaps it could be adapted? It doesn't seem to require reading it via gstreamer.
Alex: You are absolutely right. I just found the code in last-exit. I'll start implementing it tomorrow!
Created attachment 68771 [details] [review] updated to CVS *Updated to CVS: rhythmdb_entry_register_type() now expects the db as a parameter. *Corrected a small typo: "handsake" -> "handshake".
I've been testing attachment 68771 [details] [review] Built fine, and basic playback seems to work - which is great. Problem One ----------- Neighbour radio seems to play BUT there is a conflict with the album art plugin now that bug 345688 has been committed. This is probably due to the lastfm:// protocol being unknown to gnomevfs: ...
+ Trace 69287
self.uri = gnomevfs.URI (db.entry_get (entry,rhythmdb.PROP_LOCATION))
This would be trivial to fix as special case hack in the artdisplay plugin. Or as a more general solution, maybe we should just catch this exception and give up on the "local cover art search"? Problem Two ----------- When switching stations (e.g. to an "artist" or "tag" channel, or back to "neighbour radio") I usually get popup message: Couldn't start playback Could not determine type of stream. There is then a "red no entry" icon on the station. Sometimes it works on retrying. Problem Three ------------- No meta data but I see in comment 24 that might be comming soon :) I haven't tried the love/ban icons yet as I would rather know which Artist I am doing this to. Problem Four ------------ The tool bar icons for the Last.fm source are more spread out than the other sources - just single clicking on each source in turn makes this difference very obvious in my computer. If you don't see this maybe its related to large font settings?
Disclaimer: I have not read the HIG, the following are purely my own opinions. And my suggestions for shorter icon captions is based on the fact that all the other (English) toolbar icons in Rhythmbox tend to be one word (Previous, Play, Next, Repeat, Shuffle, Browse). Next Song Icon -------------- Why have you disabled the existing "Next (Start playing the next song)" icon (third along) and added a new "Next Song (Skip the current track)" icon? Couldn't you just use the existing button? And why use "Next Song" rather than just "Next" instead? Love Song Icon -------------- You have used a plus icon, with the text "< 3 Song" and tooltip "Mark this song as loved" I'm guessing the "< 3 Song" is something about rating over three? I found it odd/confusing. How about just "Love Song" or "Love!" instead? Ban Song Icon ------------- You have used a red cross icon, with the text "Ban Song" and tooltip "Ban the current track from being played again". Why not just "Ban"? Also, to be clear that this ban action only affects Last.fm (and not Rhythmbox as a whole) maybe change the tooltip to use something like "Ask Last.fm not to play the current track again" instead?
"I'm guessing the "< 3 Song" is something about rating over three? I found it odd/confusing." Haha! I'm sorry, I meant to change that one. It's supposed to look like a sideways heart, and I knew it wasn't a good label for the button, I was just joking around. I will change the others to one word. Also, the extra next button is because I don't know how to / think it would be inconvenient, to handle the next track like that. If someone knows how to get the regular 'next' button to emit a callback I can use, using that button will be wonderfully simple, otherwise I have no idea. With regard to the 'could not determine the type of stream' error, I'm pretty clueless, though a month ago RB wouldn't play the streams at all, and things have only been getting better. When I get this error, -debug spits this out: rb-shell-player.c:2505: playback error while playing: Could not determine type of stream. So, I'm not sure if I can fix that particular thing.
Now you've explained the <3 it makes sense, but it should definitely be changed ;) Trivial point - you might want to spell check the patch, e.g. rb_debug ("Last.fm commuinicating with %s", url); I don't know how rhythmbox does its strings for internationalisation or localisation, but I guess that could wait until after the basic code is checked in and tested. Also, the three functions rb_lastfm_source_love_track, rb_lastfm_source_skip_track and rb_lastfm_source_ban_track share a lot of code - how about a generic function which takes an extra argument of the string "love", "skip" or "ban" instead? Finally, your object RBLastfmSourcePrivate has a boolean "banned" which seems be for when the server says the user is banned from last.fm - nothing to do with banned tracks as I first assumed. Maybe use "banned_user" instead, or do you think its clear enough as it is?
(In reply to comment #26) > Problem One > ----------- > Neighbour radio seems to play BUT there is a conflict with the album art plugin > now that bug 345688 has been committed. This is probably due to the lastfm:// > protocol being unknown to gnomevfs: > > ... > File "/usr/local/lib/rhythmbox/plugins/artdisplay/LocalCoverArtSearch.py", > line 43, in search > self.uri = gnomevfs.URI (db.entry_get (entry,rhythmdb.PROP_LOCATION)) > TypeError: could not parse URI > > This would be trivial to fix as special case hack in the artdisplay plugin. Or > as a more general solution, maybe we should just catch this exception and give > up on the "local cover art search"? I've just fixed that in cvs, we now use entry.get_playback_uri() instead of getting the LOCATION, and don't try to get the directory listing for http(s) URI schemes. However it doesn't handle all of the cases, we still don't handle URIs that gnomevfs can't parse. Catching the TypeError exception and giving up sound like a good plan. (In reply to comment #26) >rb_debug ("Last.fm commuinicating with %s", url); > >I don't know how rhythmbox does its strings for internationalisation or >localisation, but I guess that could wait until after the basic code is checked >in and tested. In most cases, you just replace "message" with _("message"). However debugging messages shouldn't be marked for translation.
Last.fm players get notifications of metadata updates by means of a four byte keyword (SYNC) in the stream. Out in the open by itself. Last-exit acquires this notification by means of a hack that probes the data from a GstPad for that combination of letters.... The stream information is then retrieved by querying a PHP page that returns plaintext. My question is, of course, if they were going to do it like that, why not use a multipart/x-mixed-replace page? But, there's this weird keyword instead. Essentially, it involves diving heavily into the guts of the gstreamer pipeline to fire off a signal or something.
Is this gstreamer bug of any interest?, it explicitly mentions Last.fm streams starting with the SYNC keyword: gstreamer Bug 331690 – playbin won't play my last.fm stream In other news, Amarok recently got Last.fm support (Amarok 'Fast Forward' 1.4.1) http://amarok.kde.org/blog/archives/147-Hooray,-last.fm-stream-support-in-Amarok!.html There might be some interesting code there too.
I feel that the most elegant solution to this particular issue would be to implement a GStreamer source specifically for last.fm streams. It could parse lastfm:// URIs and handle retrieving the stream, including injecting id3 tags and updating them when the SYNC keyword arrives (possibly also replacing it with zeroes), providing a properly tagged mp3 iradio stream. Opinions?
I thought about that too. But I don't think writing gstreamer elements is easy. It could get into rhythmbox as a plugin for a while. If everything goes fine, perhaps someone will make a gstreamer element from it. BTW, the first thing you could do now is writing a gstreamer element for radio stations. I would be able to listen radio stations from totem :D
In this particular instance the element should be fairly simple to write, because you can make a gstreamer element out of a pipeline (and upon that you mess with the Pads and add the various variable capabilities required). The difficultly comes in the user interaction from Rhythmbox (skip song/love song/ban song) if we can't assume that the backend being used is gstreamer.
This sounds like a reasonably good idea. It may also help in the possible process split, since we'd just need a gstreamer plugin directory for the player process, rather than some nightmarish out of process pad probe concept. The source element would contain a real http source (somehow), and would issue the request to get the updated metadata on song changes, then send the information down the pipeline as tags (not injecting id3 tags into the stream; that wouldn't work anyway). It wouldn't need to be involved with any of the user interaction, as far as I can tell.
Needing to just sends tags down the pipeline instead of injecting id3 into the stream makes life much easier :) One idea I had for user interaction would be to include the session id of the stream as a tag....
It'd be easier to log in and get the session ID outside the element, then pass it in as part of the URL.
The session ID is a necessary part of the stream URL. How would a Gstreamer element integrate the next/ban/love functions? I'm not at all familiar with how plugins for Gstreamer work, but I'm not sure how generic last.fm is, when it comes to reducing the whole thing to a new type of stream to be supported. If someone wants to try and make a Gstreamer element, I'd be happy to share what I've learned or try and collaborate, but for now I think I'm just going to try and improve the plugin.
I have good news and bad news. The good news is that I've managed to successfully create a gstreamer element that parses the SYNC keyword from a last.fm stream and retrieves tag data. The bad news is, I little idea how to integrate it into rhythmbox. Once possibility is to have an audio/lastfm mimetype and handle that by the lastfm element, but that involves chagnes to the gstreamer core I think. At the moment it's not a source element, but a filter than you put between a source (eg. gnomevfssrc) and a "audio/mpeg, mpegversion=1, layer=3" decoder. It actually works quite well, but the stream catches up with the changed tag very slowly. I don't think that implementing next/ban/love functions in a Gstreamer element is the best approach, however it would be possible via GObject signals.
Making it a source element rather than a filter would be better, as it's hard to make the filter work without doing lots of dodgy hacks (like are done for icecast metadata). One option would be to make a GstLastFmSource which is derived from GstBin, and have it contain an element which does the actual http access (created with gst_element_make_from_uri), with it's source pad ghosted to the bin's. It would add a pad data probe to watch for SYNC and generate tag events after grabbing the metadata. I don't think skip/love/ban should be done by the element, as it would be horribly messy. The element could in theory handle login, which would allow it to be used by any program, but it would also make things more complicated as the application would need to get details from it for s/l/b. WRT the earlier comment about using the existing Next button, we could do that if we changed all the normal GtkAction handler to use CONNECT_AFTER, which would allow things to override the behaviour. The last.fm source could then connect a signal handler (and stop further signal emission) when it's the playing source.
Source element > filter -- definitely agree there. I've just now been messing around with making the filter work with typefind, and have discovered that my hacking into the source element to retrieve session information from the location uri no longer works. The option you described for implementing a source sounds roughly like what I was planning to do in the long term. As for the skip/love/ban -> Messiness will happen however hard we try to avoid it. I'm beginning to lean towards have the element able to do it, and activate it with gobject signals (Badness: the application will have to know what signals to emit) over the application interacting directly with the last.fm servers (Badness: the application will have to somehow find the session ID -- this is possible by monitoring gstreamer for tags, but do we really want to do this?)
My filter code as of a few minutes ago is up at http://fj.whittle.googlepages.com/lastfmforgstreamer if anyone wants to have a play with it.
The tagging in the source element works fine now, however the internals of rhythmbox need quite a bit of hacking up in order to show up the tag information properly. Filter element updated at the same location -> now works with autoplugging :)
Created attachment 69444 [details] [review] updated patch fixes several leaks, various reformatting, makes metadata work
Works for me. Metadata is working too.
Write down a note here so it won't get lost. Rhythmbox should handle lastfm:// from web browsers if last.fm radio plugin is enabled. The gconf schema can be stolen from last-exit.
Created attachment 69860 [details] [review] updated patch Saves stations into the on-disk db, fixes a few memory related issues and changes some C++-style comments to C-style.
(In reply to comment #48) > Created an attachment (id=69860) [edit] > updated patch > > Saves stations into the on-disk db, fixes a few memory related issues and > changes some C++-style comments to C-style. Works for me. Still no way of deleting stations, however.
Functionality-wise good enough to check into CVS IMHO.
I second Alex on this one, we should put this in CVS to get a wider test audience.
I haven't seen the list saving yet, but CVS certainly doesn't require the shiniest of implementations. With the metadata and the station saving, it's already as far as I'd hoped when I decided to start. Hopefully people can come up with cool things we haven't already thought of if it goes into CVS.
(In reply to comment #51) > I second Alex on this one, we should put this in CVS to get a wider test > audience. Ryan, perhaps if you check with Doc on IRC and check it in to CVS (like you did with the artwork plugin) if you don't hear otherwise from him, so it can get better testing?
Created attachment 70436 [details] [review] new station type recognized and a bugfix This patch fixes the bug where a station is loaded a few times when clcked on, making one song start and then another in quick succession. I also added code to recognize the recommended tracks stations.
Don't use that patch! I used some logical operators wrong, and now it removes all of the track metadata for anything OTHER than last.fm streams... fixing it now...
Created attachment 70451 [details] [review] the less stupid patch I mixed up == with !=, so it was removing metadata from every entry that *wasn't* a last.fm stream. Everything's ok now. I promise
Too late :( I will check when return home...
The new patch seems OK. A few things I've noticed: 1) Sometimes after playing a few track and then skipping forward using "Next" a few tracks the audio stops, although the metadata for each track is being updated as I click "Next". 2) I have noticed particular choppy audio on some radio stations that weren't choppy before on several different occasions at different times of day, could be the last.fm server. 3) The next track signal may not be emitted at the right time in order for the artdisplay plugin to pick up the new metadata it got from the last.fm server and lookup cover art on Amazon. A few times it gets it, but most times it is attempting to lookup "Unknown" artist and "Unknown" album. Seems to happen most often when the "Next" track is selected (i.e. the current track is skipped). This results in an "Unknown - Unknown.rb-blist" file written in ~/.gnome2/rhythmbox/covers/
Talked to doc tonight. He is going to do a release in the near (hopefully very near) future. Once that release gets out the door, this patch will go in. Please keep hammering on it.
I guess with Doc's new job, so we're probably not going to get a release soon, perhaps we could commit this now? At least we'd have the benefit of more testing while we're waiting.
Does someone have a quick way of just asking doc about his opinion for this issue?
(In reply to comment #61) > Does someone have a quick way of just asking doc about his opinion for this > issue? I think I remember him saying on IRC that he would be off the Internet for a while, while he was moving to his new job. Perhaps he might respond to a direct e-mail? I haven't seen him on #rhythmbox for a while.
I was planning to get a release out before I left, but it didn't happen. I've notified the gnome-18n list, so I can get one out next weekend. Marking as commit-after-freeze so it gets committed after that.
If we commit the iradio plugin patch first, we can rework this to use the extra metadata fields that adds, rather than storing transient data in the persistant title/artist/album fields. This would make it slightly less weird.
Which iradio patch? I looked through the bugs list for them earlier so I could see what changes I'd have to make. Could you list a bug #? I hope we can get this into CVS soon, now that 0.9.6 is out.
The iradio plugin patch is on bug 347097 and requires a patch from bug 345592. I'll update this patch after I check it all in.
Created attachment 73979 [details] [review] use extra-metadata signals Also fixes some leaks, code style things, marks more (all?) user-visible strings for translation.
Created attachment 73994 [details] [review] allow deletion of stations Works fine for me, and could probably be committed to get more widespread testing. This patch now allows the deletion of last.fm stations. Some things that probably fixing: * add station label should be left-aligned, not in the centre * add context menu to entry-view and source * lastfm icon for source * last played should probably update when you start to play a station It might also be nice to have a history of the recently played last.fm tracks (or a general RB one I guess), with an icon indicating if you skipped/banned/loved them. We should also be getting people to add their name to the plugin copyright bit in the .rb-plugin file for big changes like this. Code-wise, a couple of suggestions: * rather than having a "event" signal with a string parameter, it might be better to have it use the signal detail so things can connect to "event::lastfm". It might also be worth giving it a gpointer parameter, so events can have data as well as a name. * rather than doing the handshake when the user first tries to play something, and then making then do it again, perhaps we could do the handshake when the source is first viewed (i.e. from impl_activate) ? * it might be worth using "x-rb-lastfm://" instead of "lastfm://" for the URIs we pass to gstreamer (and the src element handles), this way they won't accidently get mixed up with 'normal' lastfm:// URIs that we could potentially get passed from browsers et al.
(In reply to comment #68) This applies and works fine for me. > allow deletion of stations How can you delete stations? I installed the updated plugin, but I can't see any UI to delete a station. There was no right-click context menu, or new toolbar button to delete a station. Is there a missing glade file or something in that patch? > Works fine for me, and could probably be committed to get more widespread > testing. This patch now allows the deletion of last.fm stations. Yes, I think it would be good to commit now to get more testing.
I've committed the patch to cvs, with the changes I mentioned above under "code changes". (In reply to comment #69) > > allow deletion of stations > > How can you delete stations? I installed the updated plugin, but I can't see > any UI to delete a station. There was no right-click context menu, or new > toolbar button to delete a station. Is there a missing glade file or something > in that patch? Edit->Remove. As above, a context menu would be good.
(In reply to comment #70) > I've committed the patch to cvs, with the changes I mentioned above under "code > changes". Hmm with this patch now in CVS now none of my lastfm stations will play. I get a: "Couldn't start playback Unknown playback error" dialog box, and the following messages appear on the console: (rhythmbox:32531): GStreamer-CRITICAL **: gst_element_make_from_uri: assertion `gst_uri_is_valid (uri)' failed (rhythmbox:32531): GStreamer-CRITICAL **: gst_uri_get_protocol: assertion `gst_uri_is_valid (uri)' failed I removed some stations and re-added them, as I thought that the rhythmdb.xml might have changed the URIs, but I get the same problem.
Here's what I get in the debug log (running with "-d"): (15:45:34) [0x815c1f0] [error_cb] /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell-player.c:2768: ignoring error (no source): Invalid URI "x-rb-lastfm://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2".
It seems related somehow to the lastfm:// to x-rb-lastfm:// change between the last patch and what was committed to CVS.
GStreamer currently doesn't support hyphens in URI schemes, which I'd fixed in my local copy of it. I've made us use "xrblastfm" in cvs.
I (19:14:50) [0x815c1f0] [rb_lastfm_source_new_station] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:977: adding lastfm: lastfm://user/*/neighbours, Neighbour Radio sys:1: Warning: g_date_set_julian: assertion `g_date_valid_julian (j)' failed sys:1: Warning: g_date_get_year: assertion `g_date_valid (d)' failed sys:1: Warning: g_date_set_dmy: assertion `g_date_valid_dmy (day, m, y)' failed sys:1: Warning: g_date_get_julian: assertion `g_date_valid (d)' failed (19:14:56) [0x815c1f0] [rb_lastfm_source_do_handshake] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:742: Last.fm sending handshake (19:14:56) [0x815c1f0] [rb_lastfm_perform] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:793: Last.fm communicating with http://ws.audioscrobbler.com/radio/handshake.php?version=1.1.1&platform=linux&debug=0&partner= (19:14:58) [0x815c1f0] [rb_lastfm_message_cb] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:840: response body: session=8a221d2ac7d8bf3acf1433303ae043f2 stream_url=http://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2 subscriber=0 framehack=0 base_url=ws.audioscrobbler.com base_path=/radio (19:14:58) [0x815c1f0] [rb_lastfm_message_cb] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:858: session ID: 8a221d2ac7d8bf3acf1433303ae043f2 (19:14:58) [0x815c1f0] [rb_lastfm_message_cb] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:862: stream url: http://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2 (19:15:04) [0x815c1f0] [rb_lastfm_source_get_playback_uri] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:775: playback uri: xrblastfm://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2 (19:15:04) [0x815c1f0] [playing_source_changed_cb] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:1430: connecting buffering signal handler (19:15:04) [0x815c1f0] [rb_lastfm_source_get_playback_uri] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-source.c:775: playback uri: xrblastfm://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2 (19:15:04) [0x815c1f0] [rb_lastfm_src_init] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-gst-src.c:100: creating rb last.fm src element (19:15:04) [0x815c1f0] [rb_lastfm_src_set_uri] /home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/audioscrobbler/rb-lastfm-gst-src.c:129: stream uri: http://streamer2.last.fm:80/last.mp3?Session=8a221d2ac7d8bf3acf1433303ae043f2
Here is a gdb backtrace when the frozen UI is interrupted using Ctrl-C. It appears to be hanging on network access. This requires the process to be killed and restarted and can lose data, e.g. lastfm stations that you've added in that session will disappear because it appears that they are written to rhythmdb.xml on shutdown. (gdb) bt
+ Trace 75022
Backtrace indicates a GStreamer problem. gst_element_set_state () is supposed to never block, but it leads up to waiting for gnomevfssrc to establish the network connection. This seems to be a known issue and is the subject of bug #338827.
I've just committed a (single line) change to make playback of last.fm streams start in a separate thread, which should at least stop the UI freezing when there are connection problems.
With this change, I can't get a stream to play at all, and I still get a UI hang. What happens is that as soon as I double click a last.fm station (or select it and then press play) it goes into "Pause" mode. When I click "Play" again, I get a UI freeze and the following trace: (gdb) thread apply all bt
+ Trace 75356
Thread 1 (Thread -1211783472 (LWP 11214))
Backing out the last CVS change, means that it hangs the UI immediately, so I guess it's an improvement. Perhaps if it fails, it should prevent the user from reselecting that station at least until a restart of rhythmbox.
Why can't it pop up a little no-entry flag like the iradio stations do?
Sorry for more bug spam, but to summarise: the UI doesn't hang straight away when I attempt to play a station and there's network problem (which is good), but if I've attempted to play at least one station, it locks up again even if I attempt to play anything else outside the lastfm source (e.g. a Library track), so the net effect is still the same as before. I suspect gstreamer is still waiting on connecting to that station, and hence blocks playback of anything else.
Well, as I understand it the quick fix by Jonathan is just supposed to keep the GUI from freezing, to allow you to shut down rather gracefully (saving the db without losing data). Of course you can't play anything else... the thread calling gst_element_set_state on the pipeline (normally the main thread) is hanging there until the blocking vfs calls error out. This is not supposed to happen, gnomevfssrc is seriously broken there I'd say.
(In reply to comment #83) > Well, as I understand it the quick fix by Jonathan is just supposed to keep the > GUI from freezing, to allow you to shut down rather gracefully (saving the db > without losing data). Fair enough, but there's nothing to indicate that to the user (because it simply shows that the station is in a Paused state), so naturally the user will attempt to unpause, or play something else. > Of course you can't play anything else... the thread calling > gst_element_set_state on the pipeline (normally the main thread) is > hanging there until the blocking vfs calls error out. This is not supposed to > happen, gnomevfssrc is seriously broken there I'd say. Indeed, but it doesn't look easy to fix judging by the comments on bug #338827. :-(
*wish* the offical last.fm client has a bar showing how much of the song is done. it would be nice if this could be replicated using rhythmbox's seek bar. *wish2* the keyboard hotkey forward still takes effect (possibly back as well?). could this be either disabled or used to skip tracks? maybe even use the back button for banning songs? (tho that's getting alittle complex)
(In reply to comment #85) > *wish* > the offical last.fm client has a bar showing how much of the song is done. it > would be nice if this could be replicated using rhythmbox's seek bar. > > *wish2* > the keyboard hotkey forward still takes effect (possibly back as well?). could > this be either disabled or used to skip tracks? maybe even use the back button > for banning songs? (tho that's getting alittle complex) Now that this plugin in CVS and works (modulo the occasional issues with gstreamer discussed above, which are outside rhythmbox), I'm closing this bug as it is getting very long. New bugs or enhancements should be opened in separate bugs.