GNOME Bugzilla – Bug 331508
Initial radio import doesn't work with recent totem-plparser
Last modified: 2008-06-03 17:39:26 UTC
Totem-plparser recently started recursively processing URIs, so that it works with more complex playlist files. RB uses totem-plparser to load it's inital iradio station list, and this change break that usage. We either need a way to tell totem-plparser to not to do recursive processing, or load the initial iradio station in a different way.
We need a way to change the "recurse level" before pushing the URI, and make the .pls and other code use it before spitting out the URL. Should I reassign to Totem?
Whether we want to reassign to totem, or leave this here is up to you. Adding a "recurse" property (default true) to the parser object, or something would probably work okay.
Created attachment 59821 [details] [review] patch for totem This patch makes the necessary changes to totem-plparser so that Rhythmbox's initial station loading works again. I don't have any URLs of streams that require fancy recursive processing, but it should work. As well as adding a "recurse" property (default on), I've fixed an issue with PLS parsing where it would reset the recurse-level to 0 and fallback to false.
Created attachment 59822 [details] [review] rhythmbox changes The necessary changes to RB
Thanks James for that, I'll review the code soon, and go through the release-team for that one. Bug #318749 contains one of those streams that need further parsing.
Comment on attachment 59821 [details] [review] patch for totem >diff -u -u -r1.91 totem-pl-parser.c >--- src/plparse/totem-pl-parser.c 6 Feb 2006 20:42:35 -0000 1.91 >+++ src/plparse/totem-pl-parser.c 21 Feb 2006 06:45:46 -0000 >@@ -54,6 +54,15 @@ <snip> >+ "Whether or not to process URLs furthur", Should be "further". <snip> >- if (totem_pl_parser_parse (parser, file, FALSE) != TOTEM_PL_PARSER_RESULT_SUCCESS) { >+ if (totem_pl_parser_parse_internal (parser, file) != TOTEM_PL_PARSER_RESULT_SUCCESS) { > totem_pl_parser_add_one_url_ext (parser, file, title, genre); The "fallback" is different from the recurse. It means that if the parsing is ignored, or fails, the url itself is added to the playlist, otherwise, it would be ignored. <snip> >- >+ Please mind the gratuitous spacing changes.
Created attachment 60187 [details] [review] fixed patch for totem I've fixed the issues you mentioned. I'm still not clear on why turning fallback off for "child" PLS files makes any sense. If it is needed for something, is there any reason why the same isn't done for other formats, like asx?
asx, and the likes need fallback as they could be playlists or data files. If we can't parse the playlist, we assume it's a media file and add it to the playlist. As for the patch, please also add a "-n/--no-recurse" option to test-parser so we can test without the recurse.
Created attachment 60492 [details] [review] patch with test-parser option I've added --no-recurse and -n as options to the test parser to set recurive processing to false.
*** Bug 333137 has been marked as a duplicate of this bug. ***
Patch committed to cvs. I'll fix up the RB one now.
Created attachment 60525 [details] [review] additional totem patch I've notices a few minor issue that can potentially cause problems, patch attached. We shouldn't reset fallback if we are not recursing, because (depending on the playlist) it may not always emit the entries if it's FALSE. @@ -983,7 +983,8 @@ } fallback = parser->priv->fallback; - parser->priv->fallback = FALSE; + if (parser->priv->recurse) + parser->priv->fallback = FALSE; if (strstr (file, "://") != NULL || file[0] == G_DIR_SEPARATOR) { if (totem_pl_parser_parse_internal (parser, file) != TOTEM_PL_PARSER_RESULT_SUCCESS) { Currently, each entry still has it's mime-type looked up, even though we don't use it. If the mime-type cannot be determines by the file name, it's data is look at, which won't work if it's on the 'net and the user has no 'net connection. This makes it not get the mime-type for things other than the initial playlist when we are not recursing. @@ -1738,6 +1739,10 @@ || g_str_has_prefix (url, "rtsp") != FALSE) { totem_pl_parser_add_one_url (parser, url, NULL); return TOTEM_PL_PARSER_RESULT_SUCCESS; + } + + if (!parser->priv->recurse && parser->priv->recurse_level > 0) { + return TOTEM_PL_PARSER_RESULT_UNHANDLED; } mimetype = gnome_vfs_mime_type_from_name (url);
Looks good. Thanks for working on that.
Committed to cvs.
Mass-move from totem to totem-pl-parser. You can remove all messages by searching for this comment.