VPAAMP-684: Fix Tech Fault error on rewind from live edge into CDAI ads#1711
Open
nu641001 wants to merge 2 commits into
Open
VPAAMP-684: Fix Tech Fault error on rewind from live edge into CDAI ads#1711nu641001 wants to merge 2 commits into
nu641001 wants to merge 2 commits into
Conversation
Reason for change: Wait for scheduled fragments to be downloaded and cached before moving to next period when Ad is either ended or not playing. This will avoid Tech Fault error due to wrong init fragment being downloaded or wrong bitrate during period transition. Risks: Low Test Procedure: Test with Fog CDAI Priority: P1 Change-Id: I9d9fbb37797bcbc1455fff680a37c106f44f9be9 Signed-off-by: Nandakishor U M <nu641001@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the DASH (MPD) fetcher loop CDAI period-transition handling to reduce Tech Fault errors when rewinding from the live edge into CDAI ads by waiting for scheduled fragment downloads to complete before advancing periods.
Changes:
- Adds a period-transition gate (
breakDownloaderLoop) and waits for track-worker completion before breaking out to the next period when base-period completion/offset change triggers a transition. - Refreshes the MPD during the wait loop for live manifests.
Comment on lines
+10647
to
+10648
| // State is not OUTSIDE_ADBREAK, Ad is either ended or not playing | ||
| // Wait for scheduled fragments to be downloaded and cached before moving to next period. |
Comment on lines
+10651
to
+10660
| aamp->GetAampTrackWorkerManager()->WaitForCompletionWithTimeout(MAX_WAIT_TIMEOUT_MS, [this]() { | ||
| if (mIsLiveManifest) | ||
| { | ||
| if (eAAMPSTATUS_OK != UpdateMPD()) | ||
| { | ||
| AAMPLOG_DEBUG("Failed to refresh MPD"); | ||
| } | ||
| } | ||
| }); | ||
| break; |
9a44a9d to
309bc10
Compare
…live edge into CDAI ads Reason for change:Added L1 test Risks: p1 Signed-off-by: varshnie <varshniblue14@gmail.com>
Comment on lines
+3422
to
+3424
| EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()) | ||
| .Times(AnyNumber()) | ||
| .WillRepeatedly(Return(true)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Wait for scheduled fragments to be downloaded and cached before moving to next period when Ad is either ended or not playing. This will avoid Tech Fault error due to wrong init fragment being downloaded or wrong bitrate during period transition. Risks: Low
Test Procedure: Test with Fog CDAI
Priority: P1
Change-Id: I9d9fbb37797bcbc1455fff680a37c106f44f9be9
Signed-off-by: Nandakishor U M nu641001@gmail.com