Why are retryAttempts and retryIntervals modified in low latency mode?

[jmar777]

While testing error recovery, I noticed that the retryAttempts and retryIntervals seemed to be getting ignored. Upon further inspection of the source, I learned that, while these settings aren't outright disregarded, they do become modified when running in low latency mode.

Source for getRetryAttemptsForType(type):

const LOW_LATENCY_MULTIPLY_FACTOR = 5; // line 44

function getRetryAttemptsForType(type) {
    return settings.get().streaming.lowLatencyEnabled ? settings.get().streaming.retryAttempts[type] * LOW_LATENCY_MULTIPLY_FACTOR : settings.get().streaming.retryAttempts[type];
}

Source for getRetryIntervalsForType(type):

const LOW_LATENCY_REDUCTION_FACTOR = 10; // line 43

function getRetryIntervalsForType(type) {
    return settings.get().streaming.lowLatencyEnabled ? settings.get().streaming.retryIntervals[type] / LOW_LATENCY_REDUCTION_FACTOR : settings.get().streaming.retryIntervals[type];
}

I appreciate why low latency mode benefits from different defaults here, but it strikes me as unexpected to modify explicitly provided settings. This also seems to have the potential of unintentionally flooding a backend media server with a far higher frequency and volume of requests than expected. A developer who wants the player to make 5 attempts over a 5 second window would presumably enter 5 for retryAttempts and 1000 for retryIntervals, whereas the code cited above will turn this into 25 attempts over 2.5 second window.

I tried searching, and I couldn't find this behavior documented anywhere, much less the rationale for it. IMO, it seems best to remove this behavior, but at a bare minimum, this looks like it deserves some documentation. I haven't yet contributed back, but I'm happy to submit a pull request for either; I just need some feedback on which solution would be considered better. :)

[dsilhavy]

Hi @jmar777 , I agree that values set by the user should take precedence over default values. Downside of the Settings.js is that we do not maintain information if a value was changed by the user.
In my opinion the cleanest way would be to have a single constants file with default values and a settings object with values coming from the user. Settings object would override default values.

For now I suggest you add the two low latency factors to the Settings object as well. That way you can change the values if required.

[dsilhavy]

@jmar777 Can you please check if this PR solves your issue: #3279
You can configure the low latency factors using the settings object.

[dsilhavy]

Fixed in #3279