Bugzilla – Bug 16006
Ogg & some MMS streams metadata not updated on SqueezePlay UIs
Last modified: 2010-05-12 11:00:32 UTC
Works okay on WebUI and PlayerUI(ip3k).
I'm seeing this issue with eg. Steven's Picks -> Progrock.com
Created attachment 6742 [details] proposed patch Modify Slim::Music::Info::setCurrentTitle() to send out all the various notifications about a new title (if passed a Client parameter), effectively moving this functionality from Slim::Music::Info::setDelayedTitle(). In Slim::Player::Protocols::HTTP::parseMetadata(): Include the code for handling a stream artwork-URL along with that for a new stream title, and set them both in a delayed callback as appropriate. For Ogg metadata, also call setCurrentTitle() in the delayed callback. Don't strip out CRLF from metadata before deciding what type it is, as this can mess with the Ogg length-value formatting. Modify the CRLF stripping to replace any sequence of whitespace including CR or LF with '; '. In Slim::Player::Protocols::MMS::parseMetadata(), also call setCurrentTitle() in the delayed callback.
The problem with Ogg medata is that, because setDelayedTitle() is not being called, the 'playlist newsong' notifications were not being issued. SqueezePlay UIs rely on these notifications. The Ogg metadata could also get messed up if any of its length bytes were CR or LF. The artwork-URL code for ICY-metadata was being called after the stream title update and so would likely be missed for the first track of a stream. WMA metadata also appeared to have the same problem as Ogg metadata but I have been unable to find a WMA with WMA metadata (that we recognize) to test this.
== Auto-comment from SVN commit #30519 to the slim repo by ayoung == == http://svn.slimdevices.com/slim?view=revision&revision=30519 == bug 16006: Ogg streams metadata not updated on SqueezePlay UIs Modify Slim::Music::Info::setCurrentTitle() to send out all the various notifications about a new title (if passed a Client parameter), effectively moving this functionality from Slim::Music::Info::setDelayedTitle(). In Slim::Player::Protocols::HTTP::parseMetadata(): Include the code for handling a stream artwork-URL along with that for a new stream title, and set them both in a delayed callback as appropriate. For Ogg metadata, also call setCurrentTitle() in the delayed callback. Don't strip out CRLF from metadata before deciding what type it is, as this can mess with the Ogg length-value formatting. Modify the CRLF stripping to replace any sequence of whitespace including CR or LF with '; '. In Slim::Player::Protocols::MMS::parseMetadata(), also call setCurrentTitle() in the delayed callback.
Hours
== Auto-comment from SVN commit #30520 to the slim repo by ayoung == == http://svn.slimdevices.com/slim?view=revision&revision=30520 == Fixed bug 16006: Ogg & some MMS streams metadata not updated on SqueezePlay UIs
Alan, This fix causes a cosmetic issue with iplayer when playing streams from the web interface.. The url is shown rather than title in the playlist. Reason is that setCurrentTitle now calls getCurrentTitle which can call displayText caching a null title before track->title is updated. Proposed fix is to ensure track->title is set before calling setCurrentTitle: Index: Slim/Music/Info.pm =================================================================== --- Slim/Music/Info.pm (revision 30689) +++ Slim/Music/Info.pm (working copy) @@ -395,8 +395,6 @@ if ( $meta->{title} ) { $attr->{TITLE} = $meta->{title}; - - setCurrentTitle( $url, $meta->{title} ); } if ( my $type = $meta->{ct} ) { @@ -459,6 +457,11 @@ commit => 1, } ); + if ( $meta->{title} ) { + # set current title, after setting track->title so that calls to displayText do not cache empty title + setCurrentTitle( $url, $meta->{title} ); + } + if ( $meta->{bitrate} ) { # Cache the bitrate string so it will appear in TrackInfo $currentBitrates{$url} = $track->prettyBitRate;
Alan/Andy - are you ok with me applying the patch below - urls in the playlist look bad!
Sure, fine with me, thanks.
Adrian, I'm sure you are right but I cannot work out just what this fixes or how. Is it that the on-title-change callbacks get called too early? Given that we now have a current-title, how does a null title get cached?
Alan - will apply, but here's the justification: When a remote stream is played from xmlbrowser from web or cli/comet interface then the title is set using a call to Slim::Music::Info::setRemoteMetadata before the execute command to play the streams. Previously this would call setCurrentTitle before it set the title information in the database (RemoteTrack object emulating the database). This now sets the title in the database object before calling setCurrentTitle. The reason this matters is that your change for this bug causes setCurrentTitle to call getCurrentTitle so it can see if the current title has changed (it did not do this before). This can call displayText which has a cache in it which caches the result from the first call to it for a url. So if setCurrentTitle is called before the title is set in the database for a remote url, its possible to cache undef as the title for that url. If nothing later comes along and updates the title then this results in blank titles on the display and urls rather than titles in the playlist view. So my view of the most broken bit is not setting the title in the database first so maximing the chance of always getting the real title. We can argue about the caching and whether something designed to cache for the button mode display should also be used for the web interface, but this fixes what I am seeing!
== Auto-comment from SVN commit #30737 to the slim repo by adrian == == http://svn.slimdevices.com/slim?view=revision&revision=30737 == Bug: 16006 Description: original change for this bug caused a side effect of showin urls rather than titles in the web inteface under some conditions. Delay setCurrentTitle until after the title is set in the database object. This avoids calls to displayText from getCurrentTitle causing a blank title to be cached within the diplayText cache.