Persistent & Cache streaming options alltogether

[philippe44]

I think I have found a pretty neat way to alleviate some concerns due to buffered HTTP and have the option of "Persistent" (aka Reliable) connections. I thought it was not possible when I did the "Reliable" plugin but with what I've learnt during the development of "Buffered" I think this works. It requires much more testing, though.

It's different from buffer as it does not subclass HTTPS, instead it is installed at the core of HTTP. What is really nice is that as long as the original session does not fail, it is exactly that session which is used for streaming (as opposed to "Reliable" which was always creating its own session). When the connection fails (0 bytes returned while not all expected bytes received, then it will kick a new, hidden, session that is a clone of the original one and resume streaming from there. The original socket is not active anymore but I've not seen in LMS a case where opened() would be called once body is streamed. If there is, we can always add an opened() method as well. But worse case, it works as LMS does today.

I've like to keep the "Buffered" option because it can player nice with some services but you can't have "buffered" and "persistent" at the same time as "buffered" relies on adding an onRead() to original socket and if it fails, then it will not be called anymore. Now (and I still need to make that), if a plugin decides to create a "Buffered" ProtocolHandler, the "persistent" will simply disabled.

[philippe44]

I've been able to fold the "Buffered" version under ¨S::P::P::HTTP as well. that's pretty cool because now all subclass of S::P::P::HTTP can use either Persistent or Buffered, depending on system's settings. There is no need for plugins to know about what's happening, so I've removed the changes in Tidal. It is also dynamic, i.e. no need to restart LMS.

Still, this PR is not ready for merge because there is one pending issue with HTTPS: I need to have a call to HTTP's close() so that I can cleanup Persistent/Buffered but as HTTPS is multiple inheritance, then its close() calls IO::Socket::SSL, and not S::P::P::HTTP::close(), so objects are not properly destroyed.

[philippe44]

Last PR takes properly care of HTTPS' multiple inheritance case. Now we have the option of "Persistent" or "Buffered" mode for any HTTP(S) connection and PH that depend on it and they don't need to make anything special. When playing tracks with unknown content-length (webradio) "Bufferred" will automatically downgrade to "Persistent".

I've tried to handle the case of the user changing the mode while a track is playing, so any track started in a mode will continue to be handled as it was created.

NB: with the way it works now, we can easily have a blacklist of URLs for which this is disabled

[philippe44]

I think this is now complete. I've added a --tmpdir option to slimserver.pl command line so that there is a choice where to put tmp files but that's not bloating the UI (only 'motivated' users will find the option)

[michaelherger]

This PR looks amazing! (says the man who didn't understand half of it)

Please see the few comments.

[michaelherger]

Please define a const for the various enhanced modes to make such conditions self explanatory.

[philippe44]

Done, except that I don't know how I could use these in the setting's HTML file

[michaelherger]

I think this check would deserve a little helper function, as it's being done in a hand full of places... I know, more changes in unrelated places. But as you're doing a great job changing some of the core already, let's take the opportunity to get rid of some of the cruft at the same time.

[philippe44]

It will be super-thin over crackURL, really (one line). Unless you agree to add a one return parameter to crackUrl to include the proxied url, something like

sub crackURL {
	my ($string) = @_;

	my $urlstring = join('|', Slim::Player::ProtocolHandlers->registeredHandlers);

	$string =~ m|(?:$urlstring)://(?:([^\@\/:]+):?([^\@\/]*)\@)?([^:/]+):*(\d*)(\S*)|i;

	my ($user, $pass, $host, $port, $path) = ($1, $2, $3, $4, $5);

	$path ||= '/';
	$port ||= ((Slim::Networking::Async::HTTP->hasSSL() && $string =~ /^https/) ? 443 : 80);
	
	my $proxied = "http://$host:$port$path" if $prefs->get('webproxy') && $host !~ /(?:localhost|127.0.0.1)/;

	if ( main::DEBUGLOG && $ospathslog->is_debug ) {
		$ospathslog->debug("Cracked: $string with [$host],[$port],[$path]");
		$ospathslog->debug("   user: [$user]") if $user;
		$ospathslog->debug("   pass: [$pass]") if $pass;
	}

	return ($host, $port, $path, $user, $pass, $proxied);
}

[philippe44]

Done that

[mherger]

Let's give this a try! Unless you want to fix that minor typo in the latest comment?

[mherger]

Many, many thanks!

[philippe44]

Fingers crossed 😄