From 15c23e84e6d49a410354f85fdd0371c1185eecdc Mon Sep 17 00:00:00 2001 From: Nandu Date: Thu, 9 Jul 2026 15:45:00 +0100 Subject: [PATCH 1/2] VPAAMP-684: Fix Tech Fault error on rewind from live edge into CDAI ads 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 --- fragmentcollector_mpd.cpp | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index e7254594b..288558132 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -10744,18 +10744,34 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() if (AdState::OUTSIDE_ADBREAK != mCdaiObject->mAdState) { Period2AdData &curPeriod = mCdaiObject->mPeriodMap[mBasePeriodId]; + bool breakDownloaderLoop = false; if ((mPlayRate < AAMP_RATE_PAUSE && mBasePeriodOffset <= 0) || (mPlayRate > AAMP_RATE_PAUSE && curPeriod.filled && curPeriod.duration <= (uint64_t)(mBasePeriodOffset * 1000))) { AAMPLOG_INFO("[CDAI]: BasePeriod[%s] completed @%lf. Changing to next ", mBasePeriodId.c_str(), mBasePeriodOffset); - break; + breakDownloaderLoop = true; } else if (lastPrdOffset != mBasePeriodOffset && AdState::IN_ADBREAK_AD_NOT_PLAYING == mCdaiObject->mAdState) { // In adbreak, but somehow Ad is not playing. Need to check whether the position reached the next Ad start. adStateChanged = onAdEvent(AdEvent::BASE_OFFSET_CHANGE); if (adStateChanged) - break; + breakDownloaderLoop = true; + } + // 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. + if (breakDownloaderLoop) + { + aamp->GetAampTrackWorkerManager()->WaitForCompletionWithTimeout(MAX_WAIT_TIMEOUT_MS, [this]() { + if (mIsLiveManifest) + { + if (eAAMPSTATUS_OK != UpdateMPD()) + { + AAMPLOG_DEBUG("Failed to refresh MPD"); + } + } + }); + break; } lastPrdOffset = mBasePeriodOffset; } From 627aa0ba7f6d45b9681ea4d44786f8a5eeb72934 Mon Sep 17 00:00:00 2001 From: varshnie Date: Tue, 14 Jul 2026 12:06:31 +0530 Subject: [PATCH 2/2] VPAAMP-684:Tech Fault error leads to Blue Screen when rewinding from live edge into CDAI ads Reason for change:Added L1 test Risks: p1 Signed-off-by: varshnie --- fragmentcollector_mpd.cpp | 15 +- .../fakes/FakeAampTrackWorkerManager.cpp | 8 + .../utests/mocks/MockAampTrackWorkerManager.h | 38 +++++ .../FetcherLoopTests.cpp | 156 ++++++++++++++++++ 4 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 test/utests/mocks/MockAampTrackWorkerManager.h diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index 288558132..c948cdf54 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -10744,23 +10744,25 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() if (AdState::OUTSIDE_ADBREAK != mCdaiObject->mAdState) { Period2AdData &curPeriod = mCdaiObject->mPeriodMap[mBasePeriodId]; - bool breakDownloaderLoop = false; + bool breakInnerFetcherLoop = false; if ((mPlayRate < AAMP_RATE_PAUSE && mBasePeriodOffset <= 0) || (mPlayRate > AAMP_RATE_PAUSE && curPeriod.filled && curPeriod.duration <= (uint64_t)(mBasePeriodOffset * 1000))) { AAMPLOG_INFO("[CDAI]: BasePeriod[%s] completed @%lf. Changing to next ", mBasePeriodId.c_str(), mBasePeriodOffset); - breakDownloaderLoop = true; + breakInnerFetcherLoop = true; } else if (lastPrdOffset != mBasePeriodOffset && AdState::IN_ADBREAK_AD_NOT_PLAYING == mCdaiObject->mAdState) { // In adbreak, but somehow Ad is not playing. Need to check whether the position reached the next Ad start. adStateChanged = onAdEvent(AdEvent::BASE_OFFSET_CHANGE); if (adStateChanged) - breakDownloaderLoop = true; + breakInnerFetcherLoop = true; } - // 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. - if (breakDownloaderLoop) + // We are in any ad-related state (IN_ADBREAK_AD_PLAYING, IN_ADBREAK_AD_NOT_PLAYING, + // IN_ADBREAK_AD_READY2PLAY, IN_ADBREAK_WAIT2CATCHUP, or OUTSIDE_ADBREAK_WAIT4ADS). + // Before transitioning to the next ad or base period, wait for any in-flight + // fragment downloads to complete. + if (breakInnerFetcherLoop) { aamp->GetAampTrackWorkerManager()->WaitForCompletionWithTimeout(MAX_WAIT_TIMEOUT_MS, [this]() { if (mIsLiveManifest) @@ -10771,6 +10773,7 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() } } }); + AAMPLOG_INFO("[CDAI]: In-flight downloads for period[%s] drained. Transitioning to next period.", mBasePeriodId.c_str()); break; } lastPrdOffset = mBasePeriodOffset; diff --git a/test/utests/fakes/FakeAampTrackWorkerManager.cpp b/test/utests/fakes/FakeAampTrackWorkerManager.cpp index 988f1d8a9..2197cf8aa 100644 --- a/test/utests/fakes/FakeAampTrackWorkerManager.cpp +++ b/test/utests/fakes/FakeAampTrackWorkerManager.cpp @@ -18,6 +18,7 @@ */ #include "AampTrackWorkerManager.hpp" +#include "MockAampTrackWorkerManager.h" // Global counter incremented each time AampTrackWorkerManager::SubmitJob is // called. Tests that need to distinguish the synchronous Execute() path from @@ -25,6 +26,8 @@ // inspect it afterwards. int g_submitJobCallCount = 0; +std::shared_ptr g_mockAampTrackWorkerManager{}; + namespace aamp { /** @@ -98,6 +101,11 @@ namespace aamp */ void AampTrackWorkerManager::WaitForCompletionWithTimeout(int timeout, std::function onTimeout) { + if (g_mockAampTrackWorkerManager) + { + g_mockAampTrackWorkerManager->WaitForCompletionWithTimeout(timeout, onTimeout); + return; + } } /** diff --git a/test/utests/mocks/MockAampTrackWorkerManager.h b/test/utests/mocks/MockAampTrackWorkerManager.h new file mode 100644 index 000000000..0f5bea4ea --- /dev/null +++ b/test/utests/mocks/MockAampTrackWorkerManager.h @@ -0,0 +1,38 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef AAMP_MOCK_TRACK_WORKER_MANAGER_H +#define AAMP_MOCK_TRACK_WORKER_MANAGER_H + +#include +#include +#include + +namespace aamp +{ + class MockAampTrackWorkerManager + { + public: + MOCK_METHOD(void, WaitForCompletionWithTimeout, (int timeout, std::function onTimeout)); + }; +} + +extern std::shared_ptr g_mockAampTrackWorkerManager; + +#endif /* AAMP_MOCK_TRACK_WORKER_MANAGER_H */ diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp index a23889faf..b360a007c 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp @@ -36,6 +36,8 @@ #include "MockAampStreamSinkManager.h" #include "MockTSBSessionManager.h" #include "MockAdManager.h" +#include "MockAampTimeBasedBufferManager.h" +#include "MockAampTrackWorkerManager.h" #include "AampTrackWorker.hpp" using ::testing::_; @@ -166,6 +168,21 @@ class FetcherLoopTests : public ::testing::Test mIsFogTSB = value; } + void SetPlayRate(float rate) + { + mPlayRate = rate; + } + + void SetBasePeriodOffset(double offset) + { + mBasePeriodOffset = offset; + } + + void SetBasePeriodId(const std::string &id) + { + mBasePeriodId = id; + } + bool InvokeHandleSeekEOSAndPeriodTransition(double remainingSeek, bool skipToEnd) { return HandleSeekEOSAndPeriodTransition(remainingSeek, skipToEnd); @@ -3430,4 +3447,143 @@ TEST_F(FetcherLoopTests, SegmentBase_MediaFragment_ExecutesSynchronously) << "SegmentBase media fragment must execute via Execute(), not SubmitJob. " "Async submission allows the FetcherLoop to race ahead of IDX loading " "during ABR switches, producing a stale byte-range and a bogus box-size error."; + +} +// Verify that FetcherLoop calls WaitForCompletionWithTimeout() before breaking +// out of the inner loop on a CDAI ad-period completion (live manifest). + +/** + * @brief Verifies that FetcherLoop calls WaitForCompletionWithTimeout() before + * breaking out of the inner loop on a live rewind (rate < 0, + * mBasePeriodOffset ≤ 0) with CDAI state IN_ADBREAK_AD_PLAYING. + */ +TEST_F(FetcherLoopTests, CDAI_LiveRewindPeriodComplete_WaitsForWorkers) +{ + // mLiveManifest is type="dynamic" — sets mIsLiveManifest=true inside Init(). + // Seek to 10 s so Init places the playhead inside p0 (0–30 s), ensuring + // mBasePeriodId="p0" and mCurrentPeriodIdx=0. Without this, NEW_NORMAL on a + // live manifest seeks to the live edge (p1), which would cause + // SelectSourceOrAdPeriod to detect a period change and reset mBasePeriodOffset. + std::string fragmentUrl = std::string(TEST_BASE_URL) + std::string("video_p0_init.mp4"); + EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(fragmentUrl, _, _, _, _, true, _, _, _)) + .WillRepeatedly(Return(true)); + + AAMPStatusType status = InitializeMPD(mLiveManifest, eTUNETYPE_SEEK, 10.0, AAMP_NORMAL_PLAY_RATE); + ASSERT_EQ(status, eAAMPSTATUS_OK); + + // Prevent AdvanceTrack from being called: reporting IsFull()=true makes the + // inner loop skip the per-track download branch and jump straight to the CDAI + // period check, avoiding an infinite spin calling PushNextFragment. + g_mockAampTimeBasedBufferManager = + std::make_shared>(); + ON_CALL(*g_mockAampTimeBasedBufferManager, IsFull()).WillByDefault(Return(true)); + + // Set CDAI state: inside an ad break, the ad is actively playing. + auto cdaiObj = mTestableStreamAbstractionAAMP_MPD->GetCDAIObject(); + cdaiObj->mAdState = AdState::IN_ADBREAK_AD_PLAYING; + // Provide a Period2AdData entry so the mPeriodMap lookup inside FetcherLoop + // does not insert a default entry with undefined content. + cdaiObj->mPeriodMap["p0"] = Period2AdData(); + + // Switch to rewind; place the downloader position at ≤ 0 so the condition + // (mPlayRate < AAMP_RATE_PAUSE && mBasePeriodOffset <= 0) + // is satisfied on the very first inner-loop iteration. + mTestableStreamAbstractionAAMP_MPD->SetPlayRate(-2.0f); + mTestableStreamAbstractionAAMP_MPD->SetBasePeriodOffset(-0.1); + mTestableStreamAbstractionAAMP_MPD->SetBasePeriodId("p0"); + + // DownloadsAreEnabled() call sequence: + // Call 1 – outer loop guard (iteration 1) → true (run inner loop) + // Call 2 – inner loop guard → true (reach CDAI check, + // WaitForCompletion fires, + // break inner loop) + // After break: mIterPeriodIndex-- → -1 (rewind) + // Call 3 – outer loop guard (iteration 2) → true (SelectSourceOrAdPeriod + // returns false immediately + // because mIterPeriodIndex<0, + // then CheckEndOfStream exits) + int downloadsCounter = 0; + EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()) + .Times(AnyNumber()) + .WillRepeatedly([&downloadsCounter]() { + return ++downloadsCounter <= 3; + }); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsLocalAAMPTsbInjection()).WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetTSBSessionManager()).WillRepeatedly(Return(nullptr)); + + g_mockAampTrackWorkerManager = std::make_shared>(); + EXPECT_CALL(*g_mockAampTrackWorkerManager, WaitForCompletionWithTimeout(_, _)) + .Times(testing::AtLeast(1)); + + mTestableStreamAbstractionAAMP_MPD->InvokeInitializeWorkers(); + mTestableStreamAbstractionAAMP_MPD->InvokeFetcherLoop(); + + g_mockAampTrackWorkerManager.reset(); + g_mockAampTimeBasedBufferManager.reset(); +} + +/** + * @brief Verifies that FetcherLoop calls WaitForCompletionWithTimeout() before + * breaking out of the inner loop when the ad period duration is exhausted + * (filled=true, mBasePeriodOffset > period duration) with CDAI state + * IN_ADBREAK_AD_PLAYING during live forward play. + */ +TEST_F(FetcherLoopTests, CDAI_LiveForwardPlayPeriodComplete_WaitsForWorkers) +{ + // mLiveManifest is type="dynamic" — two periods p0 (0–30 s) and p1 (30 s+). + // Seek to 10 s to land inside p0, so mBasePeriodId="p0" and + // mCurrentPeriodIdx=0 after Init. A eTUNETYPE_NEW_NORMAL on a live manifest + // would jump to the live edge (p1) instead, conflicting with the "p0" state + // we configure below and causing SelectSourceOrAdPeriod to reset the offset. + std::string fragmentUrl = std::string(TEST_BASE_URL) + std::string("video_p0_init.mp4"); + EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(fragmentUrl, _, _, _, _, true, _, _, _)) + .WillRepeatedly(Return(true)); + + AAMPStatusType status = InitializeMPD(mLiveManifest, eTUNETYPE_SEEK, 10.0, AAMP_NORMAL_PLAY_RATE); + ASSERT_EQ(status, eAAMPSTATUS_OK); + + // Prevent AdvanceTrack from being called by making the time-based buffer + // report full, so the inner loop reaches the CDAI check without downloading. + g_mockAampTimeBasedBufferManager = + std::make_shared>(); + ON_CALL(*g_mockAampTimeBasedBufferManager, IsFull()).WillByDefault(Return(true)); + + // Set CDAI state: inside an ad break, the ad is actively playing. + auto cdaiObj = mTestableStreamAbstractionAAMP_MPD->GetCDAIObject(); + cdaiObj->mAdState = AdState::IN_ADBREAK_AD_PLAYING; + + // Mark period p0 as fully filled with a 30-second ad (30 000 ms). + // Condition fired: + // curPeriod.filled (true) && curPeriod.duration (30000) <= 30.1*1000 (30100) + cdaiObj->mPeriodMap["p0"] = Period2AdData(true /*filled*/, "p0", 30000 /*ms*/, {}); + + // Downloader position is 100 ms past the end of the ad period. + mTestableStreamAbstractionAAMP_MPD->SetBasePeriodOffset(30.1); + mTestableStreamAbstractionAAMP_MPD->SetBasePeriodId("p0"); + + // DownloadsAreEnabled() call sequence: + // Call 1 – outer loop guard (iteration 1) → true (run inner loop) + // Call 2 – inner loop guard → true (reach CDAI check) + // Call 3 – outer loop guard (iteration 2, after → false (exit cleanly before + // mIterPeriodIndex++ makes it 1 and SelectSourceOrAdPeriod + // SelectSourceOrAdPeriod would try a attempts a period change) + // live period change with IN_ADBREAK state) + int downloadCallCount = 0; + EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()) + .Times(AnyNumber()) + .WillRepeatedly([&downloadCallCount]() { + return ++downloadCallCount <= 2; + }); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsLocalAAMPTsbInjection()).WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetTSBSessionManager()).WillRepeatedly(Return(nullptr)); + + g_mockAampTrackWorkerManager = std::make_shared>(); + EXPECT_CALL(*g_mockAampTrackWorkerManager, WaitForCompletionWithTimeout(_, _)) + .Times(testing::AtLeast(1)); + + mTestableStreamAbstractionAAMP_MPD->InvokeInitializeWorkers(); + mTestableStreamAbstractionAAMP_MPD->InvokeFetcherLoop(); + + g_mockAampTrackWorkerManager.reset(); + g_mockAampTimeBasedBufferManager.reset(); } \ No newline at end of file