From 1d67e19678dcf21fd8c369cc5a3dc33124869e60 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Fri, 15 May 2026 09:54:04 -0400 Subject: [PATCH 1/2] VPAAMP-346 restore period state when UpdateTrackInfo fails in HandleSeekEOSAndPeriodTransition 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. --- fragmentcollector_mpd.cpp | 21 +++- .../FetcherLoopTests.cpp | 110 ++++++++++++++++++ 2 files changed, 130 insertions(+), 1 deletion(-) diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index 546ce142aa..aadf4fa82f 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -1964,6 +1964,19 @@ bool StreamAbstractionAAMP_MPD::HandleSeekEOSAndPeriodTransition(double remainin return false; } + // Snapshot period state before applying the switch. UpdateTrackInfo may fail (e.g. + // malformed or codec-incompatible period discovered during a live manifest refresh). + // Without a rollback, the object is left in a partially-switched state: period index + // and id advanced to nextPeriodIdx while the track contexts still reflect the old + // period. Subsequent fetcher-loop iterations would attempt to download fragments from + // a period that was never successfully initialised. + int savedPeriodIdx = mCurrentPeriodIdx; + IPeriod *savedCurrentPeriod = mCurrentPeriod; + std::string savedBasePeriodId = mBasePeriodId; + double savedPeriodStartTime = mPeriodStartTime; + double savedPeriodDuration = mPeriodDuration; + double savedPeriodEndTime = mPeriodEndTime; + mCurrentPeriodIdx = nextPeriodIdx; mCurrentPeriod = mpd->GetPeriods().at(mCurrentPeriodIdx); mBasePeriodId = mCurrentPeriod->GetId(); @@ -1976,7 +1989,13 @@ bool StreamAbstractionAAMP_MPD::HandleSeekEOSAndPeriodTransition(double remainin AAMPStatusType ret = UpdateTrackInfo(true, true); if (ret != eAAMPSTATUS_OK) { - AAMPLOG_WARN("SeekInPeriod: UpdateTrackInfo failed while switching to period %d", mCurrentPeriodIdx); + AAMPLOG_WARN("SeekInPeriod: UpdateTrackInfo failed switching to period %d; restoring previous period state", mCurrentPeriodIdx); + mCurrentPeriodIdx = savedPeriodIdx; + mCurrentPeriod = savedCurrentPeriod; + mBasePeriodId = savedBasePeriodId; + mPeriodStartTime = savedPeriodStartTime; + mPeriodDuration = savedPeriodDuration; + mPeriodEndTime = savedPeriodEndTime; return false; } diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp index a242cfdf24..2dc67cbebb 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp @@ -165,6 +165,26 @@ class FetcherLoopTests : public ::testing::Test { mIsFogTSB = value; } + + bool InvokeHandleSeekEOSAndPeriodTransition(double remainingSeek, bool skipToEnd) + { + return HandleSeekEOSAndPeriodTransition(remainingSeek, skipToEnd); + } + + int GetNumberOfTracks() const + { + return mNumberOfTracks; + } + + MediaStreamContext* GetMediaStreamContextAt(int idx) + { + return mMediaStreamContext[idx]; + } + + void SetMediaStreamContextAt(int idx, MediaStreamContext *ctx) + { + mMediaStreamContext[idx] = ctx; + } }; PrivateInstanceAAMP *mPrivateInstanceAAMP; @@ -2642,3 +2662,93 @@ INSTANTIATE_TEST_SUITE_P( BasicFetcherLoopMPDTests, AdvancedFetcherLoopTests, ::testing::ValuesIn(testCases)); + +/** + * @brief VPAAMP-346: HandleSeekEOSAndPeriodTransition must restore period state when + * UpdateTrackInfo fails after a period switch attempt. + * + * Scenario: + * - Init a 2-period video+audio manifest at period 0. + * - Mark the video track enabled and eos=true to trigger a forward period switch. + * - Null out the audio track context. UpdateTrackInfo checks all track contexts and + * returns eAAMPSTATUS_MANIFEST_CONTENT_ERROR when it encounters a null context. + * - Without the rollback, mCurrentPeriodIdx (and the other period members) remain + * set to period 1 even though the switch was not completed, leaving the object in + * a partially-switched state that will cause fragment-download failures on the + * next fetcher-loop iteration. + * - With the fix, all period members are restored to their pre-switch values. + */ +TEST_F(FetcherLoopTests, HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored) +{ + AAMPStatusType status; + + static const char *kTwoPeriodVideoAudioManifest = R"( + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +)"; + + EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(_, _, _, _, _, true, _, _, _)) + .WillRepeatedly(Return(true)); + + status = InitializeMPD(kTwoPeriodVideoAudioManifest); + EXPECT_EQ(status, eAAMPSTATUS_OK); + + int periodBefore = mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(); + + // Set the video track eos=true (enabled should already be true after init) so the + // EOS check in HandleSeekEOSAndPeriodTransition fires for period 1. + MediaStreamContext *videoCtx = mTestableStreamAbstractionAAMP_MPD->GetMediaStreamContextAt(eMEDIATYPE_VIDEO); + ASSERT_NE(videoCtx, nullptr); + videoCtx->eos = true; + videoCtx->enabled = true; + + // Null out the audio context. UpdateTrackInfo will encounter a null context and + // return eAAMPSTATUS_MANIFEST_CONTENT_ERROR, triggering the rollback path. + MediaStreamContext *audioCtx = mTestableStreamAbstractionAAMP_MPD->GetMediaStreamContextAt(eMEDIATYPE_AUDIO); + mTestableStreamAbstractionAAMP_MPD->SetMediaStreamContextAt(eMEDIATYPE_AUDIO, nullptr); + + bool transitioned = mTestableStreamAbstractionAAMP_MPD->InvokeHandleSeekEOSAndPeriodTransition(0.0, false); + + // Restore audio context before any teardown path reads it. + mTestableStreamAbstractionAAMP_MPD->SetMediaStreamContextAt(eMEDIATYPE_AUDIO, audioCtx); + + // UpdateTrackInfo failed: the period switch must have been rolled back. + EXPECT_FALSE(transitioned); + EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(), periodBefore); +} From 057bdfc2791c08a507c8157781509c73f4b3eeac Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Sat, 16 May 2026 18:54:45 -0400 Subject: [PATCH 2/2] fix(VPAAMP-346): complete period-transition rollback in HandleSeekEOSAndPeriodTransition StreamSelection() and UpdateTrackInfo() mutate per-track state (adaptationSet, representation, fragmentDescriptor.Number/Time/Bandwidth, eos, timeLineIndex, fragmentRepeatCount, scaledPTO, enabled, adaptationSetIdx, adaptationSetId, representationIndex, profileChanged) one track at a time. The previous rollback only restored the six period-identity scalars (mCurrentPeriodIdx, mCurrentPeriod, mBasePeriodId, mPeriodStartTime, mPeriodDuration, mPeriodEndTime). If UpdateTrackInfo() failed after processing video (track 0) but before processing audio (track 1), the video context was left pointing at the new period's adaptation set and representation while the period index was restored to the old period, putting the object in a state that would cause wrong-period fragment downloads on the next fetcher-loop iteration. Fix: snapshot all mutable per-track fields (via a local TrackSnapshot struct across all mMaxTracks slots) plus mNumberOfTracks, mUpdateStreamInfo, mABRMode, mStreamInfo, and mBitrateIndexVector before calling StreamSelection()/ UpdateTrackInfo(). Restore the full snapshot on UpdateTrackInfo() failure so the object is in exactly the same state as before the attempted transition. test: add HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored assertions The existing test only checked mCurrentPeriodIdx and the boolean return value. Added four accessors to TestableStreamAbstractionAAMP_MPD (GetBasePeriodId, GetPeriodStartTime, GetPeriodDuration, GetPeriodEndTime) and extended the test to snapshot and re-assert every period-identity field plus the video track's adaptationSet pointer, representation pointer, fragmentDescriptor.Number, and eos flag after the failed transition, so an incomplete rollback that restores only the index would cause the test to fail. --- fragmentcollector_mpd.cpp | 94 +++++++++++++++++-- .../FetcherLoopTests.cpp | 55 +++++++++++ 2 files changed, 143 insertions(+), 6 deletions(-) diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index aadf4fa82f..4cea673c96 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -1970,6 +1970,61 @@ bool StreamAbstractionAAMP_MPD::HandleSeekEOSAndPeriodTransition(double remainin // and id advanced to nextPeriodIdx while the track contexts still reflect the old // period. Subsequent fetcher-loop iterations would attempt to download fragments from // a period that was never successfully initialised. + // + // StreamSelection() and UpdateTrackInfo() are not atomic: they iterate over tracks in + // order and mutate each context's adaptation/representation pointers, fragment number, + // eos flag, and timeline index one track at a time. If UpdateTrackInfo() returns an + // error mid-way (e.g. a null context for a later track), earlier tracks already point + // at the new period's data while the period identity fields are about to be restored to + // the old period. Snapshot and restore the full set of mutable per-track and ABR state + // so that a failed transition leaves the object in its pre-attempt state. + struct TrackSnapshot + { + bool enabled; + int adaptationSetIdx; + uint32_t adaptationSetId; + int representationIndex; + bool profileChanged; + const IAdaptationSet *adaptationSet; + const IRepresentation *representation; + BitsPerSecond bandwidth; + uint64_t number; + double time; + bool eos; + int timeLineIndex; + int fragmentRepeatCount; + double scaledPTO; + }; + std::vector trackSnapshots(mMaxTracks); + for (int i = 0; i < mMaxTracks; i++) + { + if (mMediaStreamContext[i]) + { + const auto *ctx = mMediaStreamContext[i]; + trackSnapshots[i] = { + ctx->enabled, + ctx->adaptationSetIdx, + ctx->adaptationSetId, + ctx->representationIndex, + ctx->profileChanged, + ctx->adaptationSet, + ctx->representation, + ctx->fragmentDescriptor.Bandwidth, + ctx->fragmentDescriptor.Number, + ctx->fragmentDescriptor.Time, + ctx->eos, + ctx->timeLineIndex, + ctx->fragmentRepeatCount, + ctx->scaledPTO + }; + } + } + int savedNumberOfTracks = mNumberOfTracks; + bool savedUpdateStreamInfo = mUpdateStreamInfo; + ABRMode savedABRMode = mABRMode; + std::vector savedStreamInfo = mStreamInfo; + std::vector savedBitrateIndexVector = mBitrateIndexVector; + int savedPeriodIdx = mCurrentPeriodIdx; IPeriod *savedCurrentPeriod = mCurrentPeriod; std::string savedBasePeriodId = mBasePeriodId; @@ -1990,12 +2045,39 @@ bool StreamAbstractionAAMP_MPD::HandleSeekEOSAndPeriodTransition(double remainin if (ret != eAAMPSTATUS_OK) { AAMPLOG_WARN("SeekInPeriod: UpdateTrackInfo failed switching to period %d; restoring previous period state", mCurrentPeriodIdx); - mCurrentPeriodIdx = savedPeriodIdx; - mCurrentPeriod = savedCurrentPeriod; - mBasePeriodId = savedBasePeriodId; - mPeriodStartTime = savedPeriodStartTime; - mPeriodDuration = savedPeriodDuration; - mPeriodEndTime = savedPeriodEndTime; + mCurrentPeriodIdx = savedPeriodIdx; + mCurrentPeriod = savedCurrentPeriod; + mBasePeriodId = savedBasePeriodId; + mPeriodStartTime = savedPeriodStartTime; + mPeriodDuration = savedPeriodDuration; + mPeriodEndTime = savedPeriodEndTime; + mNumberOfTracks = savedNumberOfTracks; + mUpdateStreamInfo = savedUpdateStreamInfo; + mABRMode = savedABRMode; + mStreamInfo = std::move(savedStreamInfo); + mBitrateIndexVector = std::move(savedBitrateIndexVector); + for (int i = 0; i < mMaxTracks; i++) + { + if (mMediaStreamContext[i]) + { + auto *ctx = mMediaStreamContext[i]; + const auto &snap = trackSnapshots[i]; + ctx->enabled = snap.enabled; + ctx->adaptationSetIdx = snap.adaptationSetIdx; + ctx->adaptationSetId = snap.adaptationSetId; + ctx->representationIndex = snap.representationIndex; + ctx->profileChanged = snap.profileChanged; + ctx->adaptationSet = snap.adaptationSet; + ctx->representation = snap.representation; + ctx->fragmentDescriptor.Bandwidth = snap.bandwidth; + ctx->fragmentDescriptor.Number = snap.number; + ctx->fragmentDescriptor.Time = snap.time; + ctx->eos = snap.eos; + ctx->timeLineIndex = snap.timeLineIndex; + ctx->fragmentRepeatCount = snap.fragmentRepeatCount; + ctx->scaledPTO = snap.scaledPTO; + } + } return false; } diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp index 2dc67cbebb..ce107bd876 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp @@ -185,6 +185,26 @@ class FetcherLoopTests : public ::testing::Test { mMediaStreamContext[idx] = ctx; } + + std::string GetBasePeriodId() const + { + return mBasePeriodId; + } + + double GetPeriodStartTime() const + { + return mPeriodStartTime; + } + + double GetPeriodDuration() const + { + return mPeriodDuration; + } + + double GetPeriodEndTime() const + { + return mPeriodEndTime; + } }; PrivateInstanceAAMP *mPrivateInstanceAAMP; @@ -2731,6 +2751,15 @@ TEST_F(FetcherLoopTests, HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored) int periodBefore = mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(); + // Snapshot all period-identity and video-track state that the rollback is + // expected to restore. These values characterise "in period 0 before the + // attempted switch" and must be identical after the failed transition. + dash::mpd::IPeriod *periodPtrBefore = mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriod(); + std::string basePeriodIdBefore = mTestableStreamAbstractionAAMP_MPD->GetBasePeriodId(); + double periodStartBefore = mTestableStreamAbstractionAAMP_MPD->GetPeriodStartTime(); + double periodDurBefore = mTestableStreamAbstractionAAMP_MPD->GetPeriodDuration(); + double periodEndBefore = mTestableStreamAbstractionAAMP_MPD->GetPeriodEndTime(); + // Set the video track eos=true (enabled should already be true after init) so the // EOS check in HandleSeekEOSAndPeriodTransition fires for period 1. MediaStreamContext *videoCtx = mTestableStreamAbstractionAAMP_MPD->GetMediaStreamContextAt(eMEDIATYPE_VIDEO); @@ -2738,6 +2767,12 @@ TEST_F(FetcherLoopTests, HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored) videoCtx->eos = true; videoCtx->enabled = true; + // Snapshot video-track fields that UpdateTrackInfo would mutate for period 1 + // before they can be rolled back. + const IAdaptationSet *videoAdaptSetBefore = videoCtx->adaptationSet; + const IRepresentation *videoRepBefore = videoCtx->representation; + uint64_t videoNumberBefore = videoCtx->fragmentDescriptor.Number; + // Null out the audio context. UpdateTrackInfo will encounter a null context and // return eAAMPSTATUS_MANIFEST_CONTENT_ERROR, triggering the rollback path. MediaStreamContext *audioCtx = mTestableStreamAbstractionAAMP_MPD->GetMediaStreamContextAt(eMEDIATYPE_AUDIO); @@ -2750,5 +2785,25 @@ TEST_F(FetcherLoopTests, HandleSeekEOS_UpdateTrackInfoFails_PeriodStateRestored) // UpdateTrackInfo failed: the period switch must have been rolled back. EXPECT_FALSE(transitioned); + + // Period-identity fields — any one of these left pointing at period 1 would cause + // the fetcher loop to download from the wrong period on the next iteration. EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(), periodBefore); + EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriod(), periodPtrBefore); + EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetBasePeriodId(), basePeriodIdBefore); + EXPECT_DOUBLE_EQ(mTestableStreamAbstractionAAMP_MPD->GetPeriodStartTime(), periodStartBefore); + EXPECT_DOUBLE_EQ(mTestableStreamAbstractionAAMP_MPD->GetPeriodDuration(), periodDurBefore); + EXPECT_DOUBLE_EQ(mTestableStreamAbstractionAAMP_MPD->GetPeriodEndTime(), periodEndBefore); + + // Video track context — StreamSelection()/UpdateTrackInfo() switch adaptationSet and + // representation to period 1's objects before the null audio context causes a + // failure. After rollback these must be restored to period 0's objects. + EXPECT_EQ(videoCtx->adaptationSet, videoAdaptSetBefore); + EXPECT_EQ(videoCtx->representation, videoRepBefore); + EXPECT_EQ(videoCtx->fragmentDescriptor.Number, videoNumberBefore); + + // eos was set to true by the test (to trigger the period transition check) and + // must be preserved by the rollback rather than left at the false that + // UpdateTrackInfo writes for the new period. + EXPECT_TRUE(videoCtx->eos); }