Skip to content

Feature/vpaamp 237 unify cache fragment resolve symbols#1426

Merged
pstroffolino merged 7 commits into
dev_sprint_25_2from
feature/VPAAMP-237_Unify_Cache_Fragment_Resolve_Symbols
May 18, 2026
Merged

Feature/vpaamp 237 unify cache fragment resolve symbols#1426
pstroffolino merged 7 commits into
dev_sprint_25_2from
feature/VPAAMP-237_Unify_Cache_Fragment_Resolve_Symbols

Conversation

@DomSyna
Copy link
Copy Markdown
Contributor

@DomSyna DomSyna commented May 8, 2026

Summary
Stage 3 completes the rename work begun in stages 1 and 2. No behavioural change — pure mechanical rename, lock-scope tidy, and comment cleanup.

Changes

  • Rename all ring-buffer symbols: mCachedFragmentChunks* to mCachedFragment*, numberOfFragmentChunksCached to numberOfFragmentsCached, fragmentChunkIdxTo* to fragmentIdxTo*, WaitForCachedFragmentChunk* to WaitForCachedFragment*, UpdateTSAfterChunk* to UpdateTSAfter*, GetFetchChunkBuffer to GetFetchBuffer, SetCachedFragmentChunksSize to SetCachedFragmentSize, eAAMPConfig_MaxFragmentChunkCached to eAAMPConfig_MaxFragmentCached
  • Replace fixed-size C array mCachedFragmentChunks[] with std::array<CachedFragment, DEFAULT_LLD_CACHED_FRAGMENTS_PER_TRACK> (RAII, bounds-safe)
  • Consolidate per-track lifetime counter writes under mTrackParamsMutex; make Get*Duration() accessors lock-taking; document lock order
  • Fix stale doxygen and inline comments in streamabstraction.cpp, StreamAbstractionAAMP.h, AampConfig.h, and AampDefine.h. Legitimate "chunk" references (HTTP chunked transfer, CMAF sub-segments, ABR speed estimation, ISOBMFF boxes) left unchanged.
  • Propagate renames to all unit test fakes, mocks, and test files

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the “chunk-cache” → “fragment-cache” mechanical rename across the player and unit tests, while also tightening some lock scoping and converting the per-track cached fragment ring buffer to a std::array-backed storage.

Changes:

  • Renamed MediaTrack/StreamAbstraction/Config symbols from *Chunk* to *Fragment* (including config enum + defaults) and propagated the renames through fakes/mocks/tests.
  • Replaced the per-track fixed C array cache with std::array<CachedFragment, DEFAULT_LLD_CACHED_FRAGMENTS_PER_TRACK> and introduced an “active window” size (mCachedFragmentSize).
  • Consolidated/clarified locking around track lifetime counters and updated related comments/doxygen.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/utests/tests/TrackInjectTests/TrackInjectTests.cpp Updates unit test references to new fragment-cache symbols and config enum.
test/utests/tests/StreamAbstractionAAMP/FunctionalTests.cpp Updates mocked config lookups to eAAMPConfig_MaxLLDFragmentCached.
test/utests/tests/StreamAbstractionAAMP_MPD/StreamSelectionTest.cpp Renames config enum usage in MPD stream selection tests.
test/utests/tests/StreamAbstractionAAMP_MPD/LinearFOGTests.cpp Renames config enum usage in MPD FOG tests.
test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp Renames config enum usage in MPD functional tests.
test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp Renames config enum usage in MPD fetcher loop tests.
test/utests/tests/StreamAbstractionAAMP_MPD/AudioOnlyTests.cpp Renames config enum usage in MPD audio-only tests.
test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp Updates HLS track state tests to new fragment-cache APIs/fields.
test/utests/tests/MediaTrackTests/MediaTrackTests.cpp Updates MediaTrack unit tests to new fragment-cache APIs/fields.
test/utests/tests/MediaStreamContextTests/MediaStreamContextTests.cpp Updates mocked GetFetch* API call names.
test/utests/tests/MediaStreamContextTests/FragmentDownloadTests.cpp Updates mocked GetFetch*/UpdateTSAfter* calls and cached-count fields.
test/utests/tests/FragmentCollectorAdTests/AdSelectionTests.cpp Renames config enum usage in ad selection tests.
test/utests/tests/CacheFragmentTests/CacheFragmentTests.cpp Updates cached-count field name and config enum usage.
test/utests/mocks/MockMediaTrack.h Renames mocked methods to GetFetchBuffer / UpdateTSAfterFetch.
test/utests/fakes/FakeStreamAbstractionAamp.cpp Updates fake shims to delegate to renamed mock methods/APIs.
StreamAbstractionAAMP.h Introduces std::array ring buffer, renames APIs/fields/CVs, adjusts getters.
streamabstraction.cpp Updates core cache/inject logic to renamed fields + adds lock-order docs and new locked accessor.
priv_aamp.cpp Updates stop-path unblock call to renamed API.
MediaStreamContext.cpp Renames calls to fragment-cache APIs (WaitForCachedFragmentInjected, GetFetchBuffer, UpdateTSAfterFetch).
fragmentcollector_mpd.cpp Propagates fragment-cache renames in MPD TSB/reader paths and cache sizing.
fragmentcollector_hls.cpp Propagates fragment-cache renames in HLS fetch paths and comments.
AampDefine.h Renames default constant to DEFAULT_LLD_CACHED_FRAGMENTS_PER_TRACK.
AampConfig.h Renames config enum to eAAMPConfig_MaxLLDFragmentCached.
AampConfig.cpp Updates config lookup table mapping for renamed enum/default.
Comments suppressed due to low confidence (1)

streamabstraction.cpp:4699

  • SetCachedFragmentSize() validates against maxLLDCachedFragmentsPerTrack but does not enforce the actual backing storage capacity (mCachedFragment is a fixed-size std::array). If the config value exceeds DEFAULT_LLD_CACHED_FRAGMENTS_PER_TRACK, mCachedFragmentSize can be set larger than the array and later loops/indexing (e.g., FlushFragments, modulo wraps) will go out-of-bounds. Clamp size to mCachedFragment.size() (and handle invalid/0 sizes by falling back to a safe default) rather than just logging and leaving the previous value unchanged.
void MediaTrack::SetCachedFragmentSize(size_t size)
{
	if (size > 0 && size <= maxLLDCachedFragmentsPerTrack)
	{
		AAMPLOG_TRACE("Set mCachedFragment size:%zu successfully", size);
		mCachedFragmentSize = size;
	}
	else
	{
		AAMPLOG_ERR("Failed to set size:%zu", size);
	}
}

Comment thread streamabstraction.cpp
Comment thread streamabstraction.cpp
Comment on lines 1870 to +1882
void MediaTrack::FlushFragments()
{
AAMPLOG_WARN("[%s]", name);
for (size_t i = 0; i < mCachedFragmentChunksSize; i++)
for (size_t i = 0; i < mCachedFragmentSize; i++)
{
mCachedFragmentChunks[i].Clear();
mCachedFragment[i].Clear();
}
aamp_utils::ClearAndRelease(unparsedBufferChunk);
aamp_utils::ClearAndRelease(parsedBufferChunk);
fragmentChunkIdxToInject = 0;
fragmentChunkIdxToFetch = 0;
fragmentIdxToInject = 0;
fragmentIdxToFetch = 0;
std::lock_guard<std::mutex> guard(mutex);
numberOfFragmentChunksCached = 0;
// We need to revisit if these variables should be also sync using mTrackParamsMutex
totalInjectedChunksDuration = 0;
numberOfFragmentsCached = 0;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already covered by VPAAMP-180

Comment thread StreamAbstractionAAMP.h Outdated
Comment thread test/utests/tests/CacheFragmentTests/CacheFragmentTests.cpp Outdated
Comment thread StreamAbstractionAAMP.h Outdated
Comment thread StreamAbstractionAAMP.h Outdated
Comment thread StreamAbstractionAAMP.h Outdated
Comment thread StreamAbstractionAAMP.h Outdated
Comment thread streamabstraction.cpp Outdated
Comment thread streamabstraction.cpp
Comment thread streamabstraction.cpp Outdated
@pstroffolino pstroffolino marked this pull request as ready for review May 18, 2026 19:35
@pstroffolino pstroffolino requested a review from a team as a code owner May 18, 2026 19:35
@pstroffolino pstroffolino merged commit 297b384 into dev_sprint_25_2 May 18, 2026
9 of 10 checks passed
@pstroffolino pstroffolino deleted the feature/VPAAMP-237_Unify_Cache_Fragment_Resolve_Symbols branch May 18, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants