diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index 7b18a4b0f2..4adceb290c 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -10630,18 +10630,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; } diff --git a/test/utests/fakes/FakeAampTrackWorkerManager.cpp b/test/utests/fakes/FakeAampTrackWorkerManager.cpp index b7d04020b0..b79e002e88 100644 --- a/test/utests/fakes/FakeAampTrackWorkerManager.cpp +++ b/test/utests/fakes/FakeAampTrackWorkerManager.cpp @@ -19,6 +19,12 @@ #include "AampTrackWorkerManager.hpp" +/** + * @brief Global counter incremented each time WaitForCompletionWithTimeout is called. + * Tests can reset this to 0 before invoking FetcherLoop and then check it afterwards. + */ +int g_waitForCompletionWithTimeoutCallCount = 0; + namespace aamp { /** @@ -92,6 +98,7 @@ namespace aamp */ void AampTrackWorkerManager::WaitForCompletionWithTimeout(int timeout, std::function onTimeout) { + g_waitForCompletionWithTimeoutCallCount++; } /** diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp index dfc20bfba4..c76a44eaa7 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp @@ -36,6 +36,7 @@ #include "MockAampStreamSinkManager.h" #include "MockTSBSessionManager.h" #include "MockAdManager.h" +#include "MockAampTimeBasedBufferManager.h" #include "AampTrackWorker.hpp" using ::testing::_; @@ -166,6 +167,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); @@ -3327,4 +3343,179 @@ TEST_F(FetcherLoopTests, SegmentBase_SkipFragments_FloatingPointEpsilon_CrossesF EXPECT_EQ(ctx->fragmentIndex, 1); EXPECT_NEAR(ctx->fragmentTime, 2.0, 0.001); +} + +// ─── VPAAMP-684 Unit Tests ──────────────────────────────────────────────────── +// +// These tests verify the VPAAMP-684 fix: before breaking out of the segment +// downloader loop when an ad period is complete, FetcherLoop must call +// WaitForCompletionWithTimeout() so that any in-flight fragment downloads finish +// before the period transition. The global counter +// g_waitForCompletionWithTimeoutCallCount (defined in +// FakeAampTrackWorkerManager.cpp) is incremented by the fake stub and checked +// after InvokeFetcherLoop() returns. +// +// Both tests use a LIVE manifest (type="dynamic") which is the primary scenario +// described in VPAAMP-684: rewinding from live edge into CDAI ads. +// With mIsLiveManifest=true the WaitForCompletionWithTimeout callback would also +// call UpdateMPD() to refresh the manifest before the period transition. + +extern int g_waitForCompletionWithTimeoutCallCount; + +/** + * @brief VPAAMP-684 – live rewind: period completion in CDAI adbreak calls + * WaitForCompletionWithTimeout before breaking. + * + * Scenario (live stream): + * - Dynamic (live) two-period manifest, CDAI enabled, ad state IN_ADBREAK_AD_PLAYING. + * - Play rate is negative (rewind, -2×) — user has rewound from live edge into an ad. + * - mBasePeriodOffset is set to –0.1 (≤ 0), satisfying + * (mPlayRate < AAMP_RATE_PAUSE && mBasePeriodOffset <= 0). + * - mIsLiveManifest=true means UpdateMPD() executes inside the inner loop + * AND the WaitForCompletionWithTimeout callback would call UpdateMPD() to + * refresh the manifest before the period transition (not verified here + * because the fake stub does not invoke the callback). + * + * Expected (post-fix): WaitForCompletionWithTimeout() is called at least once + * and FetcherLoop terminates without hanging. + */ +TEST_F(FetcherLoopTests, VPAAMP684_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"); + + // Keep downloads enabled throughout so the loop does not exit via the + // disabled path before reaching the CDAI check. + // For live, UpdateMPD() is executed each inner-loop iteration; the + // GetManifest mock (set up in InitializeMPD with WillRepeatedly) handles + // the repeated manifest-refresh calls transparently. + EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()) + .Times(AnyNumber()) + .WillRepeatedly(Return(true)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsLocalAAMPTsbInjection()).WillRepeatedly(Return(false)); + EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetTSBSessionManager()).WillRepeatedly(Return(nullptr)); + + g_waitForCompletionWithTimeoutCallCount = 0; + + mTestableStreamAbstractionAAMP_MPD->InvokeInitializeWorkers(); + mTestableStreamAbstractionAAMP_MPD->InvokeFetcherLoop(); + + // Core assertion: the VPAAMP-684 fix must call WaitForCompletionWithTimeout + // before breaking out of the inner loop, giving in-flight worker-thread + // downloads a chance to finish before the period transition. + EXPECT_GE(g_waitForCompletionWithTimeoutCallCount, 1) + << "WaitForCompletionWithTimeout was not called; VPAAMP-684 fix may be missing"; + + g_mockAampTimeBasedBufferManager.reset(); +} + +/** + * @brief VPAAMP-684 – live forward play: period duration exhausted in CDAI + * adbreak calls WaitForCompletionWithTimeout before breaking. + * + * Scenario (live stream): + * - Dynamic (live) two-period manifest, CDAI enabled, ad state IN_ADBREAK_AD_PLAYING. + * - Play rate is normal (1×). + * - The period is marked filled=true with a 30-second duration. + * - mBasePeriodOffset is set to 30.1 s (30 100 ms > 30 000 ms), satisfying + * (mPlayRate > AAMP_RATE_PAUSE && curPeriod.filled + * && curPeriod.duration <= mBasePeriodOffset*1000). + * - DownloadsAreEnabled() returns false on the third call so FetcherLoop + * exits cleanly through the outer-loop guard after the CDAI break, instead + * of entering a second SelectSourceOrAdPeriod which would attempt a period + * change that requires extra state setup. + * + * Expected (post-fix): WaitForCompletionWithTimeout() is called at least once + * and FetcherLoop terminates without hanging. + */ +TEST_F(FetcherLoopTests, VPAAMP684_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_waitForCompletionWithTimeoutCallCount = 0; + + mTestableStreamAbstractionAAMP_MPD->InvokeInitializeWorkers(); + mTestableStreamAbstractionAAMP_MPD->InvokeFetcherLoop(); + + // Core assertion: the VPAAMP-684 fix must call WaitForCompletionWithTimeout + // before breaking out of the inner loop. + EXPECT_GE(g_waitForCompletionWithTimeoutCallCount, 1) + << "WaitForCompletionWithTimeout was not called; VPAAMP-684 fix may be missing"; + + g_mockAampTimeBasedBufferManager.reset(); } \ No newline at end of file