Feature/vplay 13092 migrate fragmentcollector hls header to std vector#1220
Conversation
…entcollector_hls_header_to_std_vector
There was a problem hiding this comment.
Pull request overview
This PR migrates key in-memory HLS manifest/playlist buffers in the HLS fragment collector from the legacy AampGrowableBuffer wrapper to std::vector<uint8_t>, with corresponding updates to production code call sites and unit tests.
Changes:
- Replace
AampGrowableBuffermembers (mainManifest,thumbnailManifest, and per-trackplaylist) withstd::vector<uint8_t>infragmentcollector_hls.h. - Update
fragmentcollector_hls.cppto usestd::vectorAPIs and adjust cache/GetFile interactions accordingly. - Refactor HLS-related unit tests to build manifests/playlists as
std::vector<uint8_t>and add parameterized coverage forIs4KStreamand tag parsing.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
fragmentcollector_hls.h |
Switch HLS manifest/playlist buffer members to std::vector<uint8_t> and update helper function signature. |
fragmentcollector_hls.cpp |
Update buffer handling, cache interactions, and GetFormatFromFragmentExtension() to operate on vectors. |
test/utests/fakes/FakeFragmentCollector_HLS.cpp |
Align fake StreamAbstractionAAMP_HLS constructor initialization with the new member types. |
test/utests/tests/fragmentcollector_hls/byteRangeTests.cpp |
Update format-detection test to pass a std::vector<uint8_t> buffer and modernize the test table iteration. |
test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp |
Refactor tests to use vector-backed manifests; add parameterized tests for Is4KStream and tag smoke parsing. |
test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp
Outdated
Show resolved
Hide resolved
| TrackState::~TrackState() | ||
| { | ||
| playlist.Free(); | ||
| // We could remove this. This is already done in MediaTrack destructor | ||
| int maxCachedFragmentsPerTrack = GETCONFIGVALUE(eAAMPConfig_MaxFragmentCached); | ||
| for (int j=0; j< maxCachedFragmentsPerTrack; j++) | ||
| { | ||
| aamp_utils::ClearAndRelease(mCachedFragment[j].fragment); | ||
| } | ||
| FlushIndex(); | ||
| memset( mDrmInfo.iv, 0, sizeof(mDrmInfo.iv) ); | ||
| } |
There was a problem hiding this comment.
TrackState::~TrackState() became empty. Previously it explicitly zeroed mDrmInfo.iv; with the destructor now empty, the IV bytes can remain in freed memory until overwritten. Please restore secure clearing of sensitive DRM fields (e.g., explicitly zero mDrmInfo.iv, ideally using a clearing routine that won’t be optimized out).
There was a problem hiding this comment.
If a bad actor can access the released memory to read the transient init vector data they can also read the data structure, which will be much easier to find. For it to be properly secure it would have to live in the TEE.
The Init vector is often in the clear, so not really that secret.
…entcollector_hls_header_to_std_vector
Summary
Pure mechanical refactor of fragmentcollector_hls.h — no behaviour changes. Replaces AampGrowableBuffer member fields with std::vector<uint8_t>, eliminating a legacy manual-memory type from the HLS abstraction layer.
Changes