Improve Safari user agent string check

[mattjuggins]

The following code block exists in CatchupController.js (and formally PlaybackController.js). It checks the User Agent (UA) string and limits the minimum calculated playback rate change that will warrant the playback rate changing.

dash.js/src/streaming/controllers/CatchupController.js

Lines 122 to 125 in 3793614

	// Detect safari browser (special behavior for low latency streams)
	const ua = typeof navigator !== 'undefined' ? navigator.userAgent.toLowerCase() : '';
	const isSafari = /safari/.test(ua) && !/chrome/.test(ua);
	minPlaybackRateChange = isSafari ? 0.25 : 0.02;

The UA string checking line originates from this commit from October 2018.

Using a chromium build I came across a UA string where 'chrome' was not present but 'chromium' and 'safari' were, meaning the 'safari only' behaviour incorrectly occurred:

mozilla/5.0 (gbm; px aarch64 1.2.14 neo64) applewebkit/537.36 (khtml, like gecko) chromium/89.0.4389.72 safari/537.3

Understandably, UA string checks are not perfect but this could be made slightly more robust, for example: /safari/.test(ua) && !/chrome/.test(ua) && !/chromium/.test(ua).

I'd suggest just making a small change like this unless there are any better suggestions as this 'safari specific' behaviour is still problematic. Any playback rate changes seem to cause stalls (as detailed here and in duplicate issues) so limiting the rate changes is still sensible.

[dsilhavy]

@mattjuggins Thanks for reporting this. Maybe it would make sense to include a small library such as https://www.npmjs.com/package/detect-browser for this. I am open for discussions here. We should scan for additional browser checks in the code and then decide if it makes sense to add such a library.

In theory, I want to avoid any browser/platform specific logic but facing reality we will probably always need them.

Anyone got experience with libraries that do browser detection?

[mattjuggins]

After a bit of digging, it seems there is no reliable way to detect Safari through feature detection due to some changes after Safari v10. This article however does at least confirm 'chromium' needs to be checked for as well as 'chrome.

From what I can see this appears to be the only place in the core dash.js code where the UserAgent string is checked (there are some instances in the samples).

I have used ua-parser-js in the past which has been reliable. However, the library you suggested @dsilhavy appears to have been updated more recently and is considerably smaller. Bowser is one other option.

If this Safari issue did get resolved in the future then there would be a good argument for accurately checking the browser and version to not apply this different minPlaybackRateChange. This might warrant using a library to do this, even if it is only one check.

[dsilhavy]

@mattjuggins I integrtaed ua-parser-js in #4001 . Before I merge this, can you please check if it solves your issue

[mattjuggins]

Hi @dsilhavy. Apologies, I've been away for a few weeks.
I've tested with the problematic UA string I found and it correctly registers it as not being safari.
Looks good to me 👍

[dsilhavy]

Fixed in #4001