Bugzilla – Bug 4316
Adding radio station as Favorite creates 'Empty' favorite
Last modified: 2009-09-08 09:22:26 UTC
Odd behavior reported by customer... If you load WTOP FM (http://www.wtop.com/) into the Tune In URL box using this URL: http://sc1.liquidviewer.com:9062/ ...the station loads and plays fine. But if you go right on the player and add it as a Favorite, it creates a entry labeled "Empty". That empty entry actually works to load the station again. Creating a favorite out of a few other URLs I entered works fine and creates a favorite labeled correctly.
as of chagne 10255 (oct 7 nightly builds), as a workaround you can press-and-hold favorites in the "now playing" display to add this type of url as a favorite and it should save with the right title information. still not sure how to make it work for the "press right...". While it may seem redundant, older remotes can't press and hold 'favorites', but maybe press right on this item can use the same code instead of what it is doing now?
Created attachment 1626 [details] getCurrentTitle for radio urls for radio titles that aren't added to the db, we need to retrieve the current cached title. While investigating, I noticed this problem swas also showing up as a missing line in the current playlist. this patch includes that fix as well.
triode, can you verify that this patch makes sense in the Slim/Web/Pages/Playlist section? I'm fairly sure the use of getCurrentTitle does need to be reversed for this case. Resulting output seems to work.
I think it depends if you want to have the currently playing song info (or whatever the metadata the remote streams sends you) on the playlist or the name of the radio station. As it was, it would ensure it is the name of the radio station, which seems more correct to me?
ah, then I guess that would indicate that the radio station is sending nothing for the station name, and sending what appers to be the station name as the song metadata. perhaps the right solution is (standardTitle || getCurrentTitle) for a remote url? I know getCurrent falls back to standardTitle but what else can we do?
How about standardTitle || url - we don't really want the song name? so I'm not sure what getCurrentTitle would add? Is this probably really due to tune in as it does not set a title in the database? Should it be setting the title to the url if nothing else exists?
in the case of this particular bug, the url above ends up as "WTOP Radio Network" for currentTitle. url makes good glocal sense. I guess some radio stations just have to be different. having url would also be perfectly good for avoiding the empty favorite. cc'ing dean for the official word on what should be the final ui result.
ping Dean. Also changing severity since few stations are affected.
Right, we do _not_ want the song title. If there's a station name (from the ic[ey]-name|x-audiocast-name header) or some other place is there, then use that. Otherwise, the URL will have to do.
2006-10-10 18:15:22.9131 bless({ "content-type" => "audio/mpeg", "icy-br" => 20, "icy-genre" => "News", "icy-metaint" => 8192, "icy-name" => "WTOP Radio Network", "icy-notice1" => "<BR>This stream requires <a href=\"http://www.winamp.com/\">Winamp</a><BR>", "icy-notice2" => "SHOUTcast Distributed Network Audio Server/Linux v1.9.5<BR>", "icy-pub" => 1, "icy-url:http" => "//www.wtopnews.com", }, "HTTP::Headers") the question is, then, why does the icy-name not end up as the $obj->itle. posting a diff shortly that at least covers the fallback to the url if title fails.
Created attachment 1633 [details] proposed patch fall back to using url if title fails. Also affects one case in Utils/Favorites.pm.
that's a very good question, kdf. Where did you print that out? Andy: why doesn't that work?
that is from the log, d_http_async, line 286 of Async::HTTP. Not sure if that was the initial scan result, as I has already played the url earlier.
I think the problem may be that we only call setCurrentTitle for that header, not setTitle. Index: Slim/Formats/HTTP.pm =================================================================== --- Slim/Formats/HTTP.pm (revision 10280) +++ Slim/Formats/HTTP.pm (working copy) @@ -143,7 +143,7 @@ if (!defined ${*$self}{'create'} || ${*$self}{'create'} != 0) { - Slim::Music::Info::setCurrentTitle( $self->url, $self->title ); + Slim::Music::Info::setTitle( $self->url, $self->title ); } } macbook:~/dev/Slim/6.5/server andy$ svn diff Slim/Player/ Index: Slim/Player/Squeezebox2.pm =================================================================== --- Slim/Player/Squeezebox2.pm (revision 10280) +++ Slim/Player/Squeezebox2.pm (working copy) @@ -371,7 +371,7 @@ # update bitrate, content-type title for this URL... Slim::Music::Info::setContentType($url, $contentType) if $contentType; Slim::Music::Info::setBitrate($url, $bitrate) if $bitrate; - Slim::Music::Info::setCurrentTitle($url, $title) if $title; + Slim::Music::Info::setTitle($url, $title) if $title; # Bitrate may have been set in Scanner by reading the mp3 stream if ( !$bitrate ) {
Subject: Re: Adding radio station as Favorite creates 'Empty' favorite Ah, good find. Now, should we only set it there _if_ there's not already a valid title? i.e. If somebody has given a station a name, would we be overriding it?
committed initial patch to trunk and 6.5.1 as of change 10333 and change 10334. This still needs the patch from andy, but modified if we want to ensure that an existing title is not overwritten. might try to work such a patch tonight.
how about this for only setting a title if one doesn't already exist? Index: Slim/Formats/HTTP.pm =================================================================== --- Slim/Formats/HTTP.pm (revision 10331) +++ Slim/Formats/HTTP.pm (working copy) @@ -143,7 +143,8 @@ if (!defined ${*$self}{'create'} || ${*$self}{'create'} != 0) { - Slim::Music::Info::setCurrentTitle( $self->url, $self->title ); + # Bug 4316: set title in DB if not already set + Slim::Music::Info::setTitle( $self->url, $self->title ) if !$self->title; } } Index: Slim/Player/Squeezebox2.pm =================================================================== --- Slim/Player/Squeezebox2.pm (revision 10331) +++ Slim/Player/Squeezebox2.pm (working copy) @@ -371,8 +371,11 @@ # update bitrate, content-type title for this URL... Slim::Music::Info::setContentType($url, $contentType) if $contentType; Slim::Music::Info::setBitrate($url, $bitrate) if $bitrate; - Slim::Music::Info::setCurrentTitle($url, $title) if $title; + if $title && ! Slim::Music::Info::title( $url ) { + Slim::Music::Info::setTitle($url, $title); + } + # Bitrate may have been set in Scanner by reading the mp3 stream if ( !$bitrate ) { $bitrate = Slim::Music::Info::getBitrate( $url );
second part needs a slight correction: Index: Slim/Player/Squeezebox2.pm =================================================================== --- Slim/Player/Squeezebox2.pm (revision 10331) +++ Slim/Player/Squeezebox2.pm (working copy) @@ -371,8 +371,11 @@ # update bitrate, content-type title for this URL... Slim::Music::Info::setContentType($url, $contentType) if $contentType; Slim::Music::Info::setBitrate($url, $bitrate) if $bitrate; - Slim::Music::Info::setCurrentTitle($url, $title) if $title; + if ($title && ! Slim::Music::Info::title( $url )) { + Slim::Music::Info::setTitle($url, $title); + } + # Bitrate may have been set in Scanner by reading the mp3 stream if ( !$bitrate ) { $bitrate = Slim::Music::Info::getBitrate( $url );
Should we commit this and close this bug?
can someone test against the condistion Dean mentioned, just to make sure my suggested patch works? I'm not really sure myself how to set up something like that. Where can users define the station name? Having said that, nothing in this patch should cause a problem, and it does treat the initial bug symptoms. It would just be nice to confirm that it doesn't overwrite an existing title.
I just tried the provided url, in an M3U file and a different name in the #EXTINF field. restarted and did a clear and rescan. Playing the playlist, I get the "current" title of WTOP Radio Network, but press and hld favorites stores the Favorite under the name given in the #EXTINF field. This would seem to indicate that thigns are working normally, given that this particular station is rather awkwardly sending it's station name as the current song data. I think it would be ok to get this into a nightly, resolve this one and give it the couple of days to see if there are any reported issues. If no one else gets to it, I'll deal with it tonight
committed to trunk at change 10376 and 6.5.1 at change 10377 I think this covers all of the issues expressed so far, so marking as fixed. Please reopen if there are any problems.