Skip to content

VPAAMP-1121: Handle partial-ad fallback in UpdateStartTimeOfFirstPTS - #1940

Open
srikanthreddybijjam-comcast wants to merge 1 commit into
dev_sprint_25_2from
feature/VPAAMP-1121
Open

VPAAMP-1121: Handle partial-ad fallback in UpdateStartTimeOfFirstPTS#1940
srikanthreddybijjam-comcast wants to merge 1 commit into
dev_sprint_25_2from
feature/VPAAMP-1121

Conversation

@srikanthreddybijjam-comcast

Copy link
Copy Markdown
Contributor

Reason for change: Added mBaseperiodOffset to mStartTimeOfFirstPTS for a partial ad
Test Procedure: Refer jira ticket VPAAMP-1121
Priority: P1

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new partial-ad fallback branch lacks corresponding unit-test coverage and also removes the prior non-ad fallback log signal when mBasePeriodOffset is not applied.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates DASH (MPD) stream handling in StreamAbstractionAAMP_MPD::UpdateStartTimeOfFirstPTS() to better handle CDAI partial-ad fallback scenarios by offsetting the computed first-PTS start time.

Changes:

  • Add mBasePeriodOffset (seconds) to mStartTimeOfFirstPTS (ms) when ad start time is unavailable (partial-ad fallback path).
  • Add an INFO log when the base-period offset fallback is applied.
File summaries
File Description
fragmentcollector_mpd.cpp Adjusts mStartTimeOfFirstPTS calculation for partial-ad fallback using mBasePeriodOffset.
Review details

Suppressed comments (1)

fragmentcollector_mpd.cpp:10413

  • The previous warning log for the non-ad path was removed; when adStartTimeSec < 0 and mBasePeriodOffset <= 0 this now becomes silent, which reduces observability when adPeriodOffset/ad timing is not available. Consider keeping a log for the "no ad start time" fallback when the base-period offset is not applied.
			if (mBasePeriodOffset > 0)
			{
				mStartTimeOfFirstPTS += (mBasePeriodOffset * 1000.0);
				AAMPLOG_INFO("UpdateStartTimeOfFirstPTS (partial ad fallback): +mBasePeriodOffset=%lf sec : mStartTimeOfFirstPTS=%.0f ms",
					mBasePeriodOffset, mStartTimeOfFirstPTS);
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread fragmentcollector_mpd.cpp
@srikanthreddybijjam-comcast
srikanthreddybijjam-comcast force-pushed the feature/VPAAMP-1121 branch 2 times, most recently from e491c0d to 25cb570 Compare September 2, 2026 11:26
Reason for change: Added mBaseperiodOffset to mStartTimeOfFirstPTS for a partial ad
Test Procedure: Refer jira ticket VPAAMP-1121
Priority: P1

Signed-off-by: srikanthreddybijjam-comcast <[email protected]>
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.

2 participants