Skip to content

VPAAMP-205: fix reverse trickplay oscillation - guard against download-loop-exit re-entry #1443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pstroffolino wants to merge 2 commits into
base: dev_sprint_25_2
Choose a base branch
from
+393 −8
Open

VPAAMP-205: fix reverse trickplay oscillation - guard against download-loop-exit re-entry #1443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f37f861
VPAAMP-249: fix reverse trickplay oscillation - guard against downloa…
pstroffolino May 13, 2026
b131368
VPAAMP-249: add FOG live-edge unit test (adbreak at manifest index 0)
pstroffolino May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions admanager_mpd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ class PrivateCDAIObjectMPD
std::unordered_map<std::string, AdBreakObject> mAdBreaks; /**< Periodid to adbreakobject map*/
std::unordered_map<std::string, Period2AdData> mPeriodMap; /**< periodId to Ad map */
std::string mCurPlayingBreakId; /**< Currently playing Ad */
std::string mLastCompletedBreakId; /**< ID of the adbreak most recently cleared from mCurPlayingBreakId (set just before the clear); used by SelectSourceOrAdPeriod to guard against end-of-period re-entry in reverse trickplay when "All Ads Finished" fires in the download loop before SelectSourceOrAdPeriod is entered. */
std::thread mAdObjThreadID; /**< ThreadId of Ad fulfillment */
AdNodeVectorPtr mCurAds; /**< Vector of ads from the current Adbreak */
int mCurAdIdx; /**< Currently playing Ad index */
Expand Down
61 changes: 53 additions & 8 deletions fragmentcollector_mpd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9388,19 +9388,61 @@ bool StreamAbstractionAAMP_MPD::SelectSourceOrAdPeriod(bool &periodChanged, bool
// If the playing period changes, it will be detected below [if(currentPeriodId != mCurrentPeriod->GetId())]
periodChanged = false;
}
// Snapshot the currently playing break ID before the state transition clears it;
// used later in the OUTSIDE_ADBREAK block to guard against re-entering the same adbreak.
std::string snapPlayingBreakId;
// Guard (download-loop exit path): "All Ads Finished" may have fired in the
// segment download loop before this SelectSourceOrAdPeriod call. In that case
// mAdState is already OUTSIDE_ADBREAK and mCurPlayingBreakId has been cleared to
// "". The end-of-period mBasePeriodOffset set above would cause the first
// onAdEvent(DEFAULT) call below to immediately re-enter the just-completed adbreak
// via CheckForAdStart's (rate<0)&&(key==end) condition, producing the oscillation
// that feeds GStreamer mismatched init segments (GstPipeline error 80:1).
// mLastCompletedBreakId was set just before mCurPlayingBreakId was cleared; it
// persists across the download-loop/SelectSourceOrAdPeriod boundary. Consume it
// here (one-shot): if mPeriodMap[mBasePeriodId] maps to the same adbreak, skip the
// probe and let the FetcherLoop's BASE_OFFSET_CHANGE path detect any genuinely
// preceding adbreak naturally as mBasePeriodOffset decreases during rewind.
bool skipProbeForDownloadLoopExit = false;
{
std::lock_guard<std::mutex> lock(mCdaiObject->mDaiMtx);
snapPlayingBreakId = mCdaiObject->mCurPlayingBreakId;
if (mPlayRate < AAMP_RATE_PAUSE &&
mCdaiObject->mAdState == AdState::OUTSIDE_ADBREAK &&
!mCdaiObject->mLastCompletedBreakId.empty())
{
auto pit = mCdaiObject->mPeriodMap.find(mBasePeriodId);
if (pit != mCdaiObject->mPeriodMap.end() &&
!pit->second.adBreakId.empty() &&
pit->second.adBreakId == mCdaiObject->mLastCompletedBreakId)
{
AAMPLOG_INFO("[CDAI] Reverse trickplay: skipping end-of-period probe for "
"period[%s] - mPeriodMap adBreakId[%s] matches lastCompletedBreakId; "
"would oscillate back into same adbreak. FetcherLoop will detect "
"preceding adbreak via BASE_OFFSET_CHANGE.",
mBasePeriodId.c_str(), pit->second.adBreakId.c_str());
skipProbeForDownloadLoopExit = true;
}
mCdaiObject->mLastCompletedBreakId.clear(); // one-shot: consume regardless
}
}
// Calling the function to play ads from first ad break(existing logic).
adStateChanged = onAdEvent(AdEvent::DEFAULT);
if(adStateChanged && AdState::OUTSIDE_ADBREAK_WAIT4ADS == mCdaiObject->mAdState)
// Snapshot the currently playing break ID before the state transition clears it;
// used later in the OUTSIDE_ADBREAK block to guard against re-entering the same
// adbreak when "All Ads Finished" fires inside the onAdEvent call below (Scenario B).
std::string snapPlayingBreakId;
if (!skipProbeForDownloadLoopExit)
{
// Adbreak was available, but ads were not available and waited for fulfillment. Now, check if ads are available.
{
std::lock_guard<std::mutex> lock(mCdaiObject->mDaiMtx);
snapPlayingBreakId = mCdaiObject->mCurPlayingBreakId;
}
// Calling the function to play ads from first ad break(existing logic).
adStateChanged = onAdEvent(AdEvent::DEFAULT);
if(adStateChanged && AdState::OUTSIDE_ADBREAK_WAIT4ADS == mCdaiObject->mAdState)
{
// Adbreak was available, but ads were not available and waited for fulfillment. Now, check if ads are available.
adStateChanged = onAdEvent(AdEvent::DEFAULT);
}
}
else
{
adStateChanged = false;
}
// endPeriod for the ad break is not available, so wait for the ad break to complete
if (AdState::IN_ADBREAK_WAIT2CATCHUP == mCdaiObject->mAdState)
Expand Down Expand Up @@ -12248,6 +12290,7 @@ bool StreamAbstractionAAMP_MPD::onAdEvent(AdEvent evt, double &adOffset)
{
AAMPLOG_WARN("[CDAI]: ADBREAK[%s] ENDED. Playing the basePeriod[%s].", mCdaiObject->mCurPlayingBreakId.c_str(), mBasePeriodId.c_str());
mCdaiObject->mAdBreaks[mCdaiObject->mCurPlayingBreakId].mAdFailed = false;
mCdaiObject->mLastCompletedBreakId = mCdaiObject->mCurPlayingBreakId;
mCdaiObject->mCurPlayingBreakId = "";
mCdaiObject->mCurAds = nullptr;
mCdaiObject->mCurAdIdx = -1;
Expand Down Expand Up @@ -12302,6 +12345,7 @@ bool StreamAbstractionAAMP_MPD::onAdEvent(AdEvent evt, double &adOffset)
{
AAMPLOG_WARN("[CDAI]: BUG! BUG!! BUG!!! We should not come here.AdIdx[-1].");
mCdaiObject->mAdBreaks[mCdaiObject->mCurPlayingBreakId].mAdFailed = false;
mCdaiObject->mLastCompletedBreakId = mCdaiObject->mCurPlayingBreakId;
mCdaiObject->mCurPlayingBreakId = "";
mCdaiObject->mCurAds = nullptr;
mCdaiObject->mCurAdIdx = -1;
Expand Down Expand Up @@ -12401,6 +12445,7 @@ bool StreamAbstractionAAMP_MPD::onAdEvent(AdEvent evt, double &adOffset)
reservationEvt2Send = AAMP_EVENT_AD_RESERVATION_END;
reservationEndReason = GetAdReservationEndReason(curAdFailed, curAdCancelled);
sendImmediate = curAdFailed; //Current Ad failed. Hence may not get discontinuity from gstreamer.
mCdaiObject->mLastCompletedBreakId = mCdaiObject->mCurPlayingBreakId; // persist across FetcherLoop boundary before clearing
mCdaiObject->mCurPlayingBreakId = "";
mCdaiObject->mCurAds = nullptr;
mCdaiObject->mCurAdIdx = -1;
Expand Down
Loading
Loading