Skip to content

VPAAMP-346 restore period state when UpdateTrackInfo fails in HandleS… #1454

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 1 commit into
base: dev_sprint_25_2
Choose a base branch
from
Open

VPAAMP-346 restore period state when UpdateTrackInfo fails in HandleS… #1454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 20 additions & 1 deletion fragmentcollector_mpd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
Comment on lines +1993 to +1998
return false;
}

Expand Down
110 changes: 110 additions & 0 deletions test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"(<?xml version="1.0" encoding="utf-8"?>
<MPD xmlns="urn:mpeg:dash:schema:mpd:2011" maxSegmentDuration="PT2S" minBufferTime="PT4S"
profiles="urn:dvb:dash:profile:dvb-dash:2014" type="static">
<Period id="p0" start="PT0S">
<AdaptationSet id="0" contentType="video">
<Representation id="0" mimeType="video/mp4" codecs="avc1.640028" bandwidth="800000">
<SegmentTemplate timescale="2500" initialization="video_p0_init.mp4"
media="video_p0_$Number$.m4s" startNumber="1">
<SegmentTimeline><S t="0" d="5000" r="14"/></SegmentTimeline>
</SegmentTemplate>
</Representation>
</AdaptationSet>
<AdaptationSet id="1" contentType="audio" lang="eng">
<Representation id="0" mimeType="audio/mp4" codecs="mp4a.40.2" bandwidth="128000">
<SegmentTemplate timescale="2500" initialization="audio_p0_init.mp4"
media="audio_p0_$Number$.m4s" startNumber="1">
<SegmentTimeline><S t="0" d="5000" r="14"/></SegmentTimeline>
</SegmentTemplate>
</Representation>
</AdaptationSet>
</Period>
<Period id="p1" start="PT30S">
<AdaptationSet id="0" contentType="video">
<Representation id="0" mimeType="video/mp4" codecs="avc1.640028" bandwidth="800000">
<SegmentTemplate timescale="2500" initialization="video_p1_init.mp4"
media="video_p1_$Number$.m4s" startNumber="16">
<SegmentTimeline><S t="75000" d="5000" r="14"/></SegmentTimeline>
</SegmentTemplate>
</Representation>
</AdaptationSet>
<AdaptationSet id="1" contentType="audio" lang="eng">
<Representation id="0" mimeType="audio/mp4" codecs="mp4a.40.2" bandwidth="128000">
<SegmentTemplate timescale="2500" initialization="audio_p1_init.mp4"
media="audio_p1_$Number$.m4s" startNumber="16">
<SegmentTimeline><S t="75000" d="5000" r="14"/></SegmentTimeline>
</SegmentTemplate>
</Representation>
</AdaptationSet>
</Period>
</MPD>)";

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);
}
Loading