GNOME Bugzilla – Bug 559628
Support async loading of playlists
Last modified: 2010-04-27 11:52:16 UTC
I created a m3u list with my favourite webradio stations. (See http://media.ubuntuusers.de/forum/attachments/1669857/webradio.m3u) totem freeze on opening this file, but after waiting 5minutes everything works fine.
i removed the first three streams... then it works.
It seems the second entry is the offender (http://www.chronixradio.com/chronixaggression/listen/listen.asx) and the following URL is one which causes a freeze: http://scfire-nyk-aa04.stream.aol.com:80/stream/1039 The problem is that the server is dead (wget doesn't load the URL either). But Totem shouldn't freeze even with an unresponsive server. The root problem is that totem_pl_parser_parse is blocking and that this freezes the GUI.
I've created bug #561444 to track adding async functionality to totem-pl-parser. This bug can be for using it in Totem.
*** Bug 569175 has been marked as a duplicate of this bug. ***
*** Bug 338908 has been marked as a duplicate of this bug. ***
Created attachment 147605 [details] [review] Bug 559628 — Support async loading of playlists Add async playlist support. This converts most calls to the playlist add MRL function to be async, dealing with the cursor appropriately (such that concurrent operations using a busy cursor don't trample on each others' cursors when they finish). Closes: bgo#559628
Review of attachment 147605 [details] [review]: How does this behave when there's short samples in a playlist? If we're blocking while the playlist is parsing, then we're not fixing the original bug. ::: src/totem-playlist.c @@ +97,3 @@ + /* Cursor ref: 0 if the cursor is unbusy; positive numbers indicate the number of nested calls to set_waiting_cursor() */ + guint cursor_ref; Use an int, and g_atomic_* functions instead. @@ +230,3 @@ { + if (--playlist->priv->cursor_ref < 1) + gdk_window_set_cursor (gtk_widget_get_window (GTK_WIDGET (totem_playlist_get_toplevel (playlist))), NULL); g_atomic_int_dec_and_test() @@ +540,3 @@ + * signal once we're done adding it */ + if (p->next == NULL) + totem_playlist_add_mrl (playlist, filename, title, TRUE, NULL, (GAsyncReadyCallback) _drop_cb, NULL); Don't like _drop_cb(). Could you come up with a more descriptive function name (drop_finished_cb, or something) ::: src/totem-playlist.h @@ +91,3 @@ + GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); +gboolean totem_playlist_add_mrl_finish (TotemPlaylist *playlist, GAsyncResult *result); +gboolean totem_playlist_add_mrl_sync (TotemPlaylist *playlist, const char *mrl, const char *display_name); Split arguments into lines, like in the original code.
(In reply to comment #7) > Review of attachment 147605 [details] [review]: > > How does this behave when there's short samples in a playlist? Fine. Should it behave any differently for short samples as opposed to longer ones? > If we're blocking while the playlist is parsing, then we're not fixing the > original bug. How are we blocking? We make async calls to the playlist parser… > ::: src/totem-playlist.c > @@ +97,3 @@ > > + /* Cursor ref: 0 if the cursor is unbusy; positive numbers indicate the > number of nested calls to set_waiting_cursor() */ > + guint cursor_ref; > > Use an int, and g_atomic_* functions instead. cursor_ref is only ever touched from the main thread, assuming totem_playlist_add_mrl() is called from the main thread. > @@ +540,3 @@ > + * signal once we're done adding it */ > + if (p->next == NULL) > + totem_playlist_add_mrl (playlist, filename, title, TRUE, NULL, > (GAsyncReadyCallback) _drop_cb, NULL); > > Don't like _drop_cb(). Could you come up with a more descriptive function name > (drop_finished_cb, or something) Righto. > ::: src/totem-playlist.h > @@ +91,3 @@ > + GCancellable *cancellable, GAsyncReadyCallback callback, > gpointer user_data); > +gboolean totem_playlist_add_mrl_finish (TotemPlaylist *playlist, GAsyncResult > *result); > +gboolean totem_playlist_add_mrl_sync (TotemPlaylist *playlist, const char > *mrl, const char *display_name); > > Split arguments into lines, like in the original code. OK.
Created attachment 159511 [details] [review] Support async loading of playlists (updated) This version of the patch has been rebased against current master and has the style fixes incorporated. I haven't really tested it much more than what I did before as the test file originally given with this bug seems to have changed. (The URIs no longer time out.)