VPAAMP-346 restore period state when UpdateTrackInfo fails in HandleS…#1454
Open
pstroffolino wants to merge 1 commit into
Open
VPAAMP-346 restore period state when UpdateTrackInfo fails in HandleS…#1454pstroffolino wants to merge 1 commit into
pstroffolino wants to merge 1 commit into
Conversation
…eekEOSAndPeriodTransition HandleSeekEOSAndPeriodTransition updates six period-identity members (mCurrentPeriodIdx, mCurrentPeriod, mBasePeriodId, mPeriodStartTime, mPeriodDuration, mPeriodEndTime) before calling UpdateTrackInfo. If UpdateTrackInfo returns failure (malformed period, no codec-compatible representation, null track context after live manifest refresh), the function returned false but left all six members pointing at the new, uninitialised period. The next fetcher-loop iteration would read mCurrentPeriodIdx as the new value and attempt to download fragments from a period whose tracks were never set up, producing download errors or incorrect init-segment selection. Fix: snapshot all six members before the switch. On UpdateTrackInfo failure, restore them and return false so the caller sees a clean no-op. Unit test: HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored forces UpdateTrackInfo to fail by nulling the audio track context (UpdateTrackInfo returns MANIFEST_CONTENT_ERROR on a null context) and verifies that mCurrentPeriodIdx is restored to its pre-switch value.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make MPD period transitions safer by rolling back period state when HandleSeekEOSAndPeriodTransition() cannot complete UpdateTrackInfo(), preventing the fetch loop from continuing with a partially switched period.
Changes:
- Adds period-state snapshot/restore logic around the period switch path.
- Adds an L1 regression test that forces
UpdateTrackInfo()failure during a forward period transition.
L1 test verdict: Mostly valid, but needs revision.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
fragmentcollector_mpd.cpp |
Restores selected period identity fields when UpdateTrackInfo() fails during EOS-driven period transition. |
test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp |
Adds test accessors and a regression test for failed period-switch rollback. |
Comment on lines
+1993
to
+1998
| mCurrentPeriodIdx = savedPeriodIdx; | ||
| mCurrentPeriod = savedCurrentPeriod; | ||
| mBasePeriodId = savedBasePeriodId; | ||
| mPeriodStartTime = savedPeriodStartTime; | ||
| mPeriodDuration = savedPeriodDuration; | ||
| mPeriodEndTime = savedPeriodEndTime; |
|
|
||
| // UpdateTrackInfo failed: the period switch must have been rolled back. | ||
| EXPECT_FALSE(transitioned); | ||
| EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(), periodBefore); |
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.
…eekEOSAndPeriodTransition
HandleSeekEOSAndPeriodTransition updates six period-identity members (mCurrentPeriodIdx, mCurrentPeriod, mBasePeriodId, mPeriodStartTime, mPeriodDuration, mPeriodEndTime) before calling UpdateTrackInfo. If UpdateTrackInfo returns failure (malformed period, no codec-compatible representation, null track context after live manifest refresh), the function returned false but left all six members pointing at the new, uninitialised period. The next fetcher-loop iteration would read mCurrentPeriodIdx as the new value and attempt to download fragments from a period whose tracks were never set up, producing download errors or incorrect init-segment selection.
Fix: snapshot all six members before the switch. On UpdateTrackInfo failure, restore them and return false so the caller sees a clean no-op.
Unit test: HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored forces UpdateTrackInfo to fail by nulling the audio track context (UpdateTrackInfo returns MANIFEST_CONTENT_ERROR on a null context) and verifies that mCurrentPeriodIdx is restored to its pre-switch value.