Skip to content

VPLAY-12461: better documentation for LLDASH configuration needed#1137

Open
Reshma-JO07 wants to merge 1 commit intodev_sprint_25_2from
feature/VPLAY-12461
Open

VPLAY-12461: better documentation for LLDASH configuration needed#1137
Reshma-JO07 wants to merge 1 commit intodev_sprint_25_2from
feature/VPLAY-12461

Conversation

@Reshma-JO07
Copy link
Contributor

Reason for change: Add better documentation for LLD configs
Test Procedure: Not Applicable.
Risks: Low

Copilot AI review requested due to automatic review settings March 5, 2026 10:22
@Reshma-JO07 Reshma-JO07 requested a review from a team as a code owner March 5, 2026 10:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the documentation for Low Latency DASH (LLDASH) configuration parameters in both the code-level Doxygen comments (AampConfig.h) and the public API documentation (AAMP-UVE-API.md). The goal is to provide clearer, more actionable descriptions of what each latency-related and ABR buffer config parameter does, including the specific playback rate effects.

Changes:

  • Updated Doxygen comments for eAAMPConfig_MinABRNWBufferRampDown and eAAMPConfig_LowLatencyMinBuffer in AampConfig.h with more descriptive explanations.
  • Rewrote descriptions for lowLatencyMinValue, lowLatencyTargetValue, lowLatencyMaxValue, maxABRBufferRampup, and minABRBufferRampdown in the API documentation to clarify their behavioral effects.
  • Removed lowLatencyMinBuffer and lowLatencyTargetBuffer entries from the API documentation table.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
AampConfig.h Updated Doxygen comments for two config enum values with more descriptive documentation.
AAMP-UVE-API.md Rewrote descriptions for several LLDASH and ABR config parameters; removed two still-valid config entries (lowLatencyMinBuffer, lowLatencyTargetBuffer).

@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from eac6067 to 31805fa Compare March 5, 2026 10:43
Copilot AI review requested due to automatic review settings March 5, 2026 10:57
@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from 31805fa to 49d8186 Compare March 5, 2026 10:57
@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from 49d8186 to 5a32ad1 Compare March 5, 2026 10:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

@nu641001
Copy link
Contributor

nu641001 commented Mar 5, 2026

Changes look good

@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch 2 times, most recently from 6b514db to 662997d Compare March 13, 2026 12:22
Copilot AI review requested due to automatic review settings March 13, 2026 12:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from 662997d to cb427bc Compare March 16, 2026 06:33
Copilot AI review requested due to automatic review settings March 19, 2026 05:48
@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from cb427bc to 97945cf Compare March 19, 2026 05:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from 97945cf to 59caa3a Compare March 23, 2026 06:50
Reason for change: Add better documentation for LLD configs
Test Procedure: Not Applicable.
Risks: Low

Signed-off-by: Reshma-JO07 <sreshmaraphaelk@gmail.com>
Copilot AI review requested due to automatic review settings March 24, 2026 10:47
@Reshma-JO07 Reshma-JO07 force-pushed the feature/VPLAY-12461 branch from 59caa3a to 7e39151 Compare March 24, 2026 10:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comment on lines 152 to 156
| contentProtectionDataUpdateTimeout | Number | 5000 | Timeout for Content Protection Data Update on Dynamic Key Rotation (milliseconds). Player waits for [setContentProtectionDataConfig](#setcontentprotectiondataconfig_json-string) API update within the timeout interval. On timeout, uses last configured values. Also refer API [setContentProtectionDataUpdateTimeout](#setcontentprotectiondataupdatetimeout_timeout). |
| disableLowLatencyABR | Boolean | true | Enable Low Latency ABR handling. |
| disablePlaylistIndexEvent | Boolean | true | Enable/disable generation of playlist indexed event by AAMP on tune/trickplay/seek. |
| downloadBufferChunks | Number | 20 | Low Latency fragment chunk cache length. |
| enableLowLatencyCorrection | Boolean | true | Enable latency correction. If disabled, latency may gradually drift from the live edge, especially under poor network conditions. |
| enableLowLatencyDash | Boolean | true | Enable Low Latency DASH playback mode. Allows media chunks to be injected earlier (even before full fragment download completes), allowing player to start and stay closer to live edge. |
| enableSubscribedTags | Boolean | true | Enable/disable subscribed tags. |
| enableVideoEndEvent | Boolean | true | Enable/disable Video End event generation. |
| enableVideoRectangle | Boolean | true | Enable/disable setting of rectangle property for sink element. |
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main initConfig() properties table no longer lists several LL-DASH/LLD-related configuration keys (e.g., enableLowLatencyDash, disableLowLatencyABR, enableLowLatencyCorrection, latencyMonitorDelayMs, latencyMonitorIntervalMs, etc.), but these keys are still registered in AampConfig.cpp and can be passed via initConfig(). Please re-add them to this table or add a clear pointer here to the dedicated LLD section so users can discover all supported config keys.

Copilot uses AI. Check for mistakes.
Comment on lines +2999 to +3007
### Configuration

All LLD configuration properties are grouped below by function.

#### Core Enable / Disable

| Property | Type | Default | Description |
| -------- | ---- | ------- | ----------- |
| enableLowLatencyDash | Boolean | true | Enable Low Latency DASH playback mode. Allows media chunks to be injected earlier (even before full fragment download completes), allowing player to start and stay closer to live edge. |
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section says “All LLD configuration properties are grouped below by function.”, but the list is incomplete: several LLD-related initConfig properties are supported (registered in AampConfig.cpp) but not documented here (e.g., disableLowLatencyABR, enableLowLatencyCorrection, downloadBufferChunks, latencyMonitorDelayMs, latencyMonitorIntervalMs, and the latency playback-rate properties). Please document the full set (or explicitly state what is intentionally unsupported/ignored).

Copilot uses AI. Check for mistakes.
| -------- | ---- | ------- | ----------- |
| lowLatencyMinValue | Number | 3 | If latency drops below this (seconds), playback slows to 0.97x until latency recovers to lowLatencyTargetValue. |
| lowLatencyTargetValue | Number | 6 | The latency value (seconds) at which an active rate correction stops and playback returns to 1x. |
| lowLatencyMaxValue | Number | 9 | If latency exceeds this (seconds) and buffer is at or above lowLatencyTargetBuffer, playback speeds up to 1.03x until latency drops to lowLatencyTargetValue. |
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowLatencyMaxValue’s description references lowLatencyTargetBuffer, but lowLatencyTargetBuffer (and lowLatencyMinBuffer) are not documented anywhere in this LLD section even though they are valid initConfig properties (see AampConfig.cpp). Please either document these buffer-threshold properties here or remove/replace the reference so the table is self-contained.

Suggested change
| lowLatencyMaxValue | Number | 9 | If latency exceeds this (seconds) and buffer is at or above lowLatencyTargetBuffer, playback speeds up to 1.03x until latency drops to lowLatencyTargetValue. |
| lowLatencyMaxValue | Number | 9 | If latency exceeds this (seconds), playback speeds up to 1.03x until latency drops to lowLatencyTargetValue. |

Copilot uses AI. Check for mistakes.
player.initConfig({
enableLowLatencyDash: true,
lowLatencyMinValue: 3,
lowLatencyTargetValue: 6,
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “Example Configuration” comment says “lower latency target (4 sec)”, but the example sets lowLatencyTargetValue: 6. Please update the comment or the values so the example is internally consistent (and matches the intended guidance).

Suggested change
lowLatencyTargetValue: 6,
lowLatencyTargetValue: 4,

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants