From 68d68e0ec3d68001ce7b74956def6c1913c1d739 Mon Sep 17 00:00:00 2001 From: Philippe G Date: Sat, 3 Jul 2021 16:47:42 -0700 Subject: [PATCH] songHandler should be currentTrackHandler This completes the previous work on handlers and trying to clarify the roles of the handlers, between the song/track and the streamUrl. Previously, the songHandler was prioritized over the streamUrl handler, but it was still missing once case where the $track is a playlist and in fact it's not that $song (master track) handler that should be used but the _currentTrack (sub-track) handler. This is the case where a remote/http playlist is an opml list that contains tracks servced by protocol handlers (e.g. Deezer) --- DEVELOPERS.txt | 22 ++++++++++++---------- Slim/Player/Protocols/HTTP.pm | 4 ++-- Slim/Player/Song.pm | 2 +- Slim/Player/SongStreamController.pm | 4 ++-- Slim/Player/Squeezebox.pm | 22 +++++++++++----------- Slim/Player/Squeezebox2.pm | 14 +++++++------- 6 files changed, 35 insertions(+), 33 deletions(-) diff --git a/DEVELOPERS.txt b/DEVELOPERS.txt index 881e56c2890..15c96b28f00 100644 --- a/DEVELOPERS.txt +++ b/DEVELOPERS.txt @@ -205,25 +205,27 @@ have been skipped). On further open(), the next sub-track is set in _currentTrac - currentTrack() is a method that returns the actual current track, i.e. _currentTrack || _track >>> Handlers in Slim::Player::Song and Slim::Player::SongStreamingController -- $song->handler (RO) is set at the creation of the song. it depends on track->url at creation but can - be if the PH wants to pass-on to another PH (like thin PH do) +- $song->handler (RO) is set at the creation of the song. it is based on track->url at creation and is + immutable. - $song->_currentTrackHandler (RW) is set when a single track happens to be a playlist, for each item. - It allows each individual sub-tracks to have their own PR when it relates to some of their management. - It means that it is empty most of the time. + It allows each individual sub-tracks to have their own PH when it relates to some of their management. + It means that it is empty most of the time but PH can update it when $track is changed after scanning + (for example when upgrding from HTTP to HTTPS) - $song->currentTrackHandler() is a method that returns (_currentTrackHandler || handler) so it tells what shall be used for all url related to the current (sub) track - $songStreamingController->urlHandler is RO and set at the creation of the song streaming controller, based on the streamUrl (in S::P::Song::open) - $songStreamingController->streamHandler is RO and set at creation of the song streaming controller. It is the class of the *socket* object created by $song->handler->new. -- $songStreamingController->songHandler is a method that returns the $song->handler, so the song main - handler, not the sub-track +- $songStreamingController->currentTrackHandler is a shortcut to $song->currentTrackhandler >>> Protocol Handlers These files contains a base class but whose methods are called in different contexts (the $self) -1- a $song context means they are called with a $song->currentHandler or $song->handler -2- a $sock context means they are called with a $socket (or the object) that was created by $song->handler->new -3- a simple $url context means they are called by $song->currentHandler, $song->Handler or a $songStreamingController->streamHandler +1- a $song context means they are called with a $song->currentTrackHandler or $song->handler +2- a $sock context means they are called with a $socket (or the object) that was created by + $song->handler->new +3- a simple $url context means they are called by $song->currentTrackHandler, $song->Handler or a + $songStreamingController->streamHandler So it's a bit confusing that methods from the same package can be object-oriented called with totally different type of ancestors/context. In fact, some methods can be called in both context is the @@ -231,7 +233,7 @@ ancestor of the PH is capable of (e.g. HTTP). - functions like sysread are only called with a $sock context - functions like getMetadataFor can be called with a $song or an $url context -- functions like requestString cal be called in a $song or $sock context +- functions like requestString can be called in a $song or $sock context >>> Example of requestString That function is to create the HTTP headers to be sent with the GET to grab the actual audio. It diff --git a/Slim/Player/Protocols/HTTP.pm b/Slim/Player/Protocols/HTTP.pm index fa815a2b7d2..dca3ee2a445 100644 --- a/Slim/Player/Protocols/HTTP.pm +++ b/Slim/Player/Protocols/HTTP.pm @@ -262,9 +262,9 @@ sub getFormatForURL { return Slim::Music::Info::typeFromSuffix($url); } -sub getSongHandler { +sub currentTrackHandler { my ($class, $self, $track) = @_; - + # re-evaluate as we might have been upgraded to HTTPS return $class ne __PACKAGE__ ? $class : Slim::Player::ProtocolHandlers->handlerForURL($track->url); } diff --git a/Slim/Player/Song.pm b/Slim/Player/Song.pm index 4143e2905c8..82a3e488266 100644 --- a/Slim/Player/Song.pm +++ b/Slim/Player/Song.pm @@ -255,7 +255,7 @@ sub getNextSong { if ($self->_track() == $track) { # Update of original track, by playlist or redirection $self->_track($newTrack); - $self->init_accessor(handler => $handler->getSongHandler($self, $newTrack)) if $handler->can('getSongHandler'); + $self->_currentTrackHandler($handler->currentTrackHandler($self, $newTrack)) if $handler->can('currentTrackHandler'); main::INFOLOG && $log->info("Track updated by scan: $url -> " . $newTrack->url); diff --git a/Slim/Player/SongStreamController.pm b/Slim/Player/SongStreamController.pm index 3a8633457fb..b72c22e4bf0 100644 --- a/Slim/Player/SongStreamController.pm +++ b/Slim/Player/SongStreamController.pm @@ -63,8 +63,8 @@ sub close { } } -sub songHandler { - return shift->song->handler(); +sub currentTrackHandler { + return shift->song->currentTrackHandler(); } sub isDirect { diff --git a/Slim/Player/Squeezebox.pm b/Slim/Player/Squeezebox.pm index 21de078ca58..7de3ac897c4 100644 --- a/Slim/Player/Squeezebox.pm +++ b/Slim/Player/Squeezebox.pm @@ -136,7 +136,7 @@ sub play { my $params = shift; my $controller = $params->{'controller'}; - my $handler = $controller->songHandler(); + my $handler = $controller->currentTrackHandler(); # Calculate the correct buffer threshold for remote URLs if ( $handler->isRemote() ) { @@ -537,7 +537,7 @@ sub stream_s { my $url = $controller->streamUrl(); my $track = $controller->track(); my $handler = $controller->streamUrlHandler(); - my $songHandler = $controller->songHandler(); + my $currentTrackHandler = $controller->currentTrackHandler(); my $isDirect = $controller->isDirect(); my $master = $client->master(); @@ -565,7 +565,7 @@ sub stream_s { # use getFormatForURL only if the format is not already given # This method is bad because it only looks at the URL suffix and can cause # (for example) Ogg HTTP streams to be played using the mp3 decoder! - my $methodHandler = $songHandler->can('getFormatForURL') ? $songHandler : $handler; + my $methodHandler = $currentTrackHandler->can('getFormatForURL') ? $currentTrackHandler : $handler; $format = $methodHandler->getFormatForURL($url) if !$format && $methodHandler; } @@ -752,8 +752,8 @@ sub stream_s { $outputThreshold = 1; # Handler may override pcmsamplesize (Rhapsody) - if ( $songHandler && $songHandler->can('pcmsamplesize') ) { - $pcmsamplesize = $songHandler->pcmsamplesize( $client, $params ); + if ( $currentTrackHandler && $currentTrackHandler->can('pcmsamplesize') ) { + $pcmsamplesize = $currentTrackHandler->pcmsamplesize( $client, $params ); } # XXX: The use of mp3 as default has been known to cause the mp3 decoder to be used for @@ -774,7 +774,7 @@ sub stream_s { main::INFOLOG && logger('player.streaming.direct')->info("SqueezePlay direct stream: $url"); - my $methodHandler = $songHandler->can('requestString') ? $songHandler : $handler; + my $methodHandler = $currentTrackHandler->can('requestString') ? $currentTrackHandler : $handler; $request_string = $methodHandler->getRequestString($client, $url, undef, $params->{'seekdata'} || $controller->song->seekdata); $autostart += 2; # will be 2 for direct streaming with no autostart, or 3 for direct with autostart @@ -818,8 +818,8 @@ sub stream_s { } $server_port = $port; - # prioritize song's protocol handler at even in direct mode it might change requestString - my $methodHandler = $songHandler->can('requestString') ? $songHandler : $handler; + # prioritize current track's protocol handler at even in direct mode it might change requestString + my $methodHandler = $currentTrackHandler->can('requestString') ? $currentTrackHandler : $handler; $request_string = $methodHandler->requestString($client, $url, undef, $params->{'seekdata'} || $controller->song->seekdata); $autostart += 2; # will be 2 for direct streaming with no autostart, or 3 for direct with autostart @@ -939,10 +939,10 @@ sub stream_s { } # Bug 10567, allow plugins to override transition setting - if ( $songHandler && $songHandler->can('transitionType') ) { - my $override = $songHandler->transitionType( $master, $controller->song(), $transitionType ); + if ( $currentTrackHandler && $currentTrackHandler->can('transitionType') ) { + my $override = $currentTrackHandler->transitionType( $master, $controller->song(), $transitionType ); if ( defined $override ) { - main::INFOLOG && $log->is_info && $log->info("$songHandler changed transition type to $override"); + main::INFOLOG && $log->is_info && $log->info("$currentTrackHandler changed transition type to $override"); $transitionType = $override; } } diff --git a/Slim/Player/Squeezebox2.pm b/Slim/Player/Squeezebox2.pm index 05ce229f45f..92b236217c6 100644 --- a/Slim/Player/Squeezebox2.pm +++ b/Slim/Player/Squeezebox2.pm @@ -491,7 +491,7 @@ sub directHeaders { return unless $controller && $controller->isDirect(); my $url = $controller->streamUrl(); - my $songHandler = $controller->songHandler(); + my $currentTrackHandler = $controller->currentTrackHandler(); # We involve the protocol handler in the header parsing process. # The current iteration of the firmware only knows about HTTP @@ -529,12 +529,12 @@ sub directHeaders { $directlog->warn("Invalid response code ($response) from remote stream $url"); - if ($songHandler && $songHandler->can("handleDirectError")) { + if ($currentTrackHandler && $currentTrackHandler->can("handleDirectError")) { # bug 10407 - make sure ready to stream again $client->readyToStream(1); - $songHandler->handleDirectError($client, $url, $response, $status_line); + $currentTrackHandler->handleDirectError($client, $url, $response, $status_line); } elsif ($track->can('redir') && $track->redir && $track->redir ne $url) { @@ -569,13 +569,13 @@ sub directHeaders { $directlog->info("Processing " . scalar(@headers) . " headers"); } - # prioritize song's protocol handler over streamUrl handler - my $methodHandler = $songHandler->can('parseDirectHeaders') ? $songHandler : $handler; + # prioritize current track's protocol handler over streamUrl handler + my $methodHandler = $currentTrackHandler->can('parseDirectHeaders') ? $currentTrackHandler : $handler; main::INFOLOG && $directlog->info("Calling $methodHandler :: parseDirectHeader"); - # songHandler relates to the current (sub)track while streamUrl handler just use streamUrl + # currentTrackHandler relates to the current (sub)track while streamUrl handler just use streamUrl ($title, $bitrate, $metaint, $redir, $contentType, $length, $body) = - $methodHandler->parseDirectHeaders($client, $methodHandler == $songHandler ? $controller->song()->currentTrack() : $url, @headers); + $methodHandler->parseDirectHeaders($client, $methodHandler == $currentTrackHandler ? $controller->song()->currentTrack() : $url, @headers); $controller->song()->isLive($length ? 0 : 1) if !$redir; # XXX maybe should (also) check $song->scanData()->{$url}->{metadata}->{info}->{broadcast}