Create "Buffered" player protocol

[philippe44]

I might have found a "neat" (please note the use of conditional here 😄) solution for protocol handlers that want to inherit from S::P::P::HTTP(S) but have a problem with servers that do not like long streaming session and would rather prefer the whole file to be downloaded as quickly as possible.

This solution makes use of the recent modifications in Temp directory usage and in the HTTP(S) inheritance process (use of sysread and SUPER::_sysread), so it feels like all adding up nicely.

Basically, the idea is to let the header's acquisition work as usual (blocking read and direct use of HTTP(S)' sysread) in the new() and once this is done, it installs a reader on the socket so that it can read as fast as possible and store body in a file located in "temp". Then the _sysread continues reading from that file instead of direct access to HTTP(S)' sysread. The file is then removed at the end of the streaming session.

I've quiclky tested it with MixCloud and Tidal but it requires more scrutiny at this point but I thought I'd submit it for feedback & opinions.

[michaelherger]

I understand that this protocol handler would need to be used by any plugin/code which would want to take advantage of that feature. As it's based on HTTPS, plain text http connections could not be supported this way. Is there any other reason why this wouldn't be the default way of dealing with https connections if proxied streaming was enabled?

Ok, disk usage. Some people might not like the idea of writing to their SD cards. Could it optionally be used for those cases? With podcasts?

I guess that users (plugins) would need an update to take advantage of this.

[philippe44]

I think we have made now changes so that HTTP url are supported S::P::P::HTTPS is used, but I'll check.

If it works, it could be used as the default handler in the LMS core but I'm not sure that all services are always happy by a quick download, I don't know.

[philippe44]

Also, this should really NOT being used with webradio and continuous streams 😅

[michaelherger]

Also, this should really NOT being used with webradio and continuous streams 😅

That much even I understood 😉!

[philippe44]

Now, I probably can look for content-length and disable the buffering if none is found

[michaelherger]

How are you testing this change? Did you modify any other code to make use of it? If so, anything I could inherit for the testing?

[philippe44]

Yes, you can modify Slim::Player::ProtocolHandler.pm the following way

# the protocolHandlers hash contains the modules that handle specific URLs,
# indexed by the URL protocol.  built-in protocols are exist in the hash, but
# have a zero value
my %protocolHandlers = ( 
	file     => main::LOCALFILE ? qw(Slim::Player::Protocols::LocalFile) : qw(Slim::Player::Protocols::File),
	tmp      => qw(Slim::Player::Protocols::Volatile),
	#http     => qw(Slim::Player::Protocols::HTTP),
	#https    => Slim::Networking::Async::HTTP->hasSSL() ? qw(Slim::Player::Protocols::HTTPS) : qw(Slim::Player::Protocols::HTTP),
	http     => qw(Slim::Player::Protocols::Buffered),
	https    => qw(Slim::Player::Protocols::Buffered),
	icy      => qw(Slim::Player::Protocols::HTTP),
	mms      => qw(Slim::Player::Protocols::MMS),
	spdr     => qw(Slim::Player::Protocols::SqueezePlayDirect),
	playlist => 0,
	db       => 1,
);

Which forces usage of "Buffered" very broadly across LMS

[michaelherger]

But this wouldn't work with PHs which inherit from S::P::P::HTTP, would it? Eg. for TIDAL I'd have to modify TIDAL's PH?

[philippe44]

But this wouldn't work with PHs which inherit from S::P::P::HTTP, would it? Eg. for TIDAL I'd have to modify TIDAL's PH?

You're correct, it would only work with "native" services. For Tidal, you'd have to do what I did for mixcloud, which is simply inheriting from Buffered. This is how I tested your plugin.

[michaelherger]

Ok, here's something I'm not sure whether it's related to this change, or SqueezeAMP: I was playing that TIDAL mix on SqueezeAMP. Had been for a few hours. Suddenly it stopped at the end of a track. LMS logged this:

[21-04-14 14:02:30.8829] Slim::Control::Request::execute (1888) Error: While trying to run function coderef [Slim::Control::Commands::playcontrolCommand]: [Can't use an undefined value as a symbol reference at /opt/git/slimserver/Slim/Player/Protocols/Buffered.pm line 25.]

And the player disappeared. I tend to think that the above message was the consequence of the player crashing. It didn't come back automatically. I had to pull the plug on it.

Good news: no buffering file was left behind.

[philippe44]

I added a clean fail when the $socket cannot be created by S::P::P::HTTPS

[philippe44]

With your changes, I think we are ready to give it a try

[mherger]

Great thanks!

[mw9]

Ok, disk usage. Some people might not like the idea of writing to their SD cards. Could it optionally be used for those cases? With podcasts?

I haven't been following this PR or its predecessors, so I can't comment sensibly. But a thought occurs:

Many "SD Card using people" have learned over the years to mount their system /tmp directory onto RAM, on the premise that these files do not need to persist after reboot, and that their SD Cards will be happier as a result.

The temporary directory returned by Slim::Utils::Misc::getTempDir seems to lie within LMS's cache hierarchy, which must persist after reboot.

Does this "buffered" temporary directory or its contents need to to persist after reboot ? I guess I'm just wondering if the system's /tmp directory might be a better place for it.

[philippe44]

Not this buffered is totally temp and in fact is wiped-out at every LMS start, just in case. The choice of not using /tmp is precisely for the reason you describe which is that /tmp can be rather small

[michaelherger]

Buffering to the file system currently is off by default. I might change this, depending on the experience we'll have. But I do see your point and actually planned to look into not enabling it on systems running on SD card or similar. If you have a good idea how to discover whether a system is running on a "weak" storage medium, please let me know.

[mw9]

The choice of not using /tmp is precisely for the reason you describe which is that /tmp can be rather small

I thought that might be an issue. I guess an educated user might "learn" to suitably size and mount a "RAM" based directory prior to starting LMS. It will be interesting to find out what the size will need to be in real use.

If you have a good idea how to discover whether a system is running on a "weak" storage medium, please let me know.

I wish.

On a Linux based system one can look for the obvious candidates /dev/mmc*, /dev/mtd*, and perhaps /dev/ubi*. That would identify some systems that have "weak" storage systems on board. But by no means all, because that would not identify systems running with CompactFlash, USB flash drives, or even SSD drives. So distinctly < 100%, and perhaps not good enough to use as a fail-safe check.

Nor does it tell us that the relevant "weak" storage system is actually in point. But perhaps that's secondary.

I'll communicate if I find or come up with anything further.

[michaelherger]

TBH: I believe the whole "don't write to [put your media here]" is overrated - in particular when it comes to SSD. I've been running LMS on pCP for about 4 years now and haven't had a single SD card failure.

[mw9]

You may well be right.

I was well bitten about ten/twelve years ago by SDHC/MMC, and USB flash, and my outlook has been informed by that experience. So I learned to take “precautions”. And I haven’t had a problem since then.

But, I believe that SDHC/MMC technology has improved significantly during the intervening period, and default file system settings are now probably more suited to the media. I suspect the same carries over to USB flash drives. I have no experience of SSD drives, other than one inside my iMac, which seems fine and, I suppose, is as reliable as the accompanying magnetic drive.

So perhaps I overestimate the need for caution.