-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add autoLowLatencyMode config #2861
Conversation
@michellezhuogg This is related to your work in low latency, can you review? |
Hello @avelad , Thank you! What are the scenarios to use this config? How would it interact with lowLatencyMode config? |
@michellezhuogg
In a Live TV app with a mix of providers and mix of low latency
In stream A and D the player automatically chooses the low latency mode, in stream B and C the player takes the conservative values of not low latency Note: the application does not know which of the previous streams is low latency, but in the streams in which it is available it activates it by default. |
@avelad, low latency mode would inhibit response filters for media segments (but not for manifests or license responses), would require the accurate manifests ( Which of these things is a problem for your scenario? If some of your content is low-latency, could you not just turn on low latency mode? In fact, for DASH, and even for VOD, |
@joeyparrish The scenario is not knowing if the stream is low latency or not. I always use For VOD I have no problem. |
Any news? |
Hello @avelad , Sorry for the late reply.
Thank you! |
HI @michellezhuogg,
I have been carefully studying the need for this, and the alternative is to use filters to determine if the manifest is low latency and change the rebufferingGoal setting. Thanks, |
Makes sense to me, and we need better document or comments to explain how lowLatencyMode / autoLowLatencyMode work for other developers. |
@@ -383,6 +383,13 @@ shaka.dash.DashParser = class { | |||
} | |||
|
|||
// The segments are available earlier than the availability start time. | |||
if (this.minTotalAvailabilityTimeOffset_ && !this.lowLatencyMode_) { | |||
const autoLowLatencyMode = this.playerInterface_.isAutoLowLatencyMode(); | |||
if (autoLowLatencyMode) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments here to explain how lowLatencyMode & autoLowLatencyMode works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
* lowLatencyMode is set to true, inaccurateManifestTolerance is set to 0 | ||
* unless specified, and rebufferingGoal to 0.01 unless specified at the same | ||
* time. | ||
* @property {boolean} autoLowLatencyMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add more comment here to clarify how autoLowLatencyMode interacts with lowLatencyMode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
All tests passed! |
@michellezhuogg Is there anything else I can do so this PR can be merged? |
It is not always known if a stream is low latency or not (for example, when adding streams from multiple providers). To avoid having to use a responseFilter, a configuration is added to enable low latency automatically.
Related to #1525