dev sync by narenr94 · Pull Request #1083 · rdkcentral/aamp

narenr94 · 2026-02-26T03:22:32Z

No description provided.

Reason for change:cancelReservation UVE call Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: varshnie <varshniblue14@gmail.com>

…le VODTrickplay_rew_32_4 (#1073) VPLAY-12760: AAMP Trackworker Q causes early EOS and L2 2025 unreliable VODTrickplay_rew_32_4 (#1073) Reason for change: During VOD trickplay, it can cause early EOS due to missing check in ChekForEndOfStream Risks: Low Test Procedure: Test with L2 2025 multiple times Priority: P1 Signed-off-by: Nandakishor U M <nu641001@gmail.com>

VPLAY-12734 Generate CMCD documentation and review gaps Reason for Change: add generated CMCD documentation to repo --------- Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>

… as appropriate (#1012) VPLAY-12671 Replace calls to AppendBytes method with insert or assign as appropriate Reason for Change: tech debt: replacing AppendBytes() method with modern vector like alternatives assign() and insert(). --------- Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>

VPLAY-12795 L2 1006 unreliable stdout mixup (#1075) Reason for change: L2 1006 is unreliable Use AAMPCLI_PRINTF(...) instead of std::cout to give less opportunity of thread switch part way through writing a log line Also adds bounds checking --------- Co-authored-by: Patrick Conduit <pconduit@synamedia.com>

…vice reboot (#1065) VPLAY-12757: Crash occured once during linear channel tuning after device reboot (#1065) Reason for change: Crash occurred once during linear channel tuning after device reboot. Risks: Low Test Procedure: Test with linear Priority: P1 Signed-off-by: Nandakishor U M <nu641001@gmail.com> Co-authored-by: pstroffolino <Philip_Stroffolino@cable.comcast.com>

…ld in PCTune_split to Indicate VIPA vs Non-VIPA Playback (#1015) Reason for Change : To better collect and analyze tune metrics, use the existing player source string field to differentiate VIPA Playback Test Procedure: Tune to linear/VOD content , verify Tune log Priority : p1 Risks: Low Signed-off-by: haripriya_molakalapalli <haripriya_molakalapalli@comcast.com>

Copilot

Pull request overview

This PR represents a "dev sync" that includes multiple categories of changes:

Changes:

Modernization of AampGrowableBuffer API by adding assign() and insert() methods to complement the legacy AppendBytes() method
Refactoring of buffer operations throughout the codebase to use the new modern C++ style methods
Addition of CancelReservation API for ad management across the entire stack
Bug fixes including off-by-one array bounds check in Aampcli.cpp
Improvements to concurrency handling (recursive_mutex, lock_guard usage)
Addition of comprehensive CMCD documentation
Worker job execution pattern improvements for non-parallel download mode
New tests for AampProfiler VIPA tagging functionality

Reviewed changes

Copilot reviewed 55 out of 55 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
AampGrowableBuffer.h	Added new `insert()` and `assign()` methods accepting void* parameters
tsDemuxer.cpp	Refactored to use insert() instead of AppendBytes()
CachedFragment.cpp/h	Updated Copy() method signature to remove length parameter, refactored to use assign()
test/aampcli/Aampcli.cpp	Fixed off-by-one bug in array bounds check, added assert statements
fragmentcollector_mpd.cpp	Added conditional worker job execution, changed to recursive_mutex with lock_guard
MediaStreamContext.h/cpp	Changed mutex to recursive_mutex, refactored lock usage to RAII pattern
admanager_mpd.h/cpp	Added CancelReservation API implementation
AampProfiler.cpp	Added VIPA tagging for Firebolt SDK, increased buffer size
streamabstraction.cpp	Refactored buffer operations to use insert/assign
Multiple test files	Updated to use new buffer API methods
docs/CMCD-*.md	Added comprehensive CMCD documentation (3 new files)
Fake implementations	Added stubs for new CancelReservation API

Copilot · 2026-02-26T03:27:47Z

+			if(ISCONFIGSET(eAAMPConfig_DashParallelFragDownload))
+			{
+				aamp->GetAampTrackWorkerManager()->SubmitJob(eMEDIATYPE_VIDEO , dashWorkerJob);
+			}
+			else
+			{
+				dashWorkerJob->Execute();
+			}


The conditional check for eAAMPConfig_DashParallelFragDownload determines whether to submit the job to the worker thread or execute it immediately. This is a good defensive fix that ensures the code works correctly even when parallel download is disabled. However, ensure this pattern is consistently applied throughout the codebase wherever worker jobs are submitted. Consider documenting this pattern or creating a helper method to encapsulate this logic.

Copilot · 2026-02-26T03:27:47Z

 	enabled = false;
 	unsigned int licenseAcqNWTime = bucketDuration(PROFILE_BUCKET_LA_NETWORK);
-	char tuneTimeStrPrefix[64];
+	char tuneTimeStrPrefix[128];


The buffer size is increased from 64 to 128 bytes to accommodate the additional "_VIPA" suffix that can be added to the app name. However, verify that this is sufficient for the worst case. The snprintf on line 323 uses the full app name plus player mode and other formatting. Consider calculating the maximum size needed: playerActiveMode.length() + appName.length() + strlen("_VIPA") + strlen(" PLAYER[] APP: IP_AAMP_TUNETIME") + digits_for_playerId + 1 to ensure the buffer is appropriately sized.

Suggested change

char tuneTimeStrPrefix[128];

char tuneTimeStrPrefix[512];

Copilot · 2026-02-26T03:27:47Z

-		this->fragment.AppendBytes(other->fragment.GetPtr(), len);
+	// Copy fragment data vector from other to this using assign method
+	if (!other->fragment.GetVector().empty()) 
+  {


Inconsistent indentation: Line 77 uses spaces while the surrounding code uses tabs. This should be corrected to maintain consistency with the codebase style which uses hard tabs for indentation.

Suggested change

{

{

Copilot · 2026-02-26T03:27:48Z

+	void insert(typename std::vector<uint8_t>::const_iterator pos, const void *first, const void *last)
 	{
+		const uint8_t* start = static_cast<const uint8_t*>(first);
+		const uint8_t* end = static_cast<const uint8_t*>(last);
 		const size_t oldCap = buffer.capacity();

-		buffer.insert(pos, first, last);
+		buffer.insert(pos, start, end);
 		if (oldCap == 0 && buffer.capacity() > 0)
 		{
 			NETMEMORY_PLUS();
 		}
 	}
+	void assign(const void *first, const void *last)
+	{
+		const uint8_t* start = static_cast<const uint8_t*>(first);
+		const uint8_t* end = static_cast<const uint8_t*>(last);
+		const size_t oldCap = buffer.capacity();
+
+		// Perform the actual vector assignment
+		buffer.assign(start, end);
+
+		const size_t newCap = buffer.capacity();

+		// Honour the AAMP memory tracking logic
+		if (oldCap == 0 && newCap > 0)
+		{
+			NETMEMORY_PLUS();
+		}
+	}


The insert and assign methods accept const void* parameters and perform casts to const uint8_t*. While this provides flexibility, it bypasses type safety. Consider whether these methods should accept const uint8_t* directly (like AppendBytes does), or provide overloads for common types (char*, uint8_t*) to make the type conversions explicit at call sites. This would improve type safety and make the code intent clearer.

Copilot · 2026-02-26T03:27:48Z

 	const auto playerId = mSingleton->GetId();

-	if (mPlayerSessionID.size() >= playerId)
+	assert(playerId >= 0);


The assert statement is used in production code (test/aampcli/). According to the coding guidelines (CodingGuidelineID: 1000003), assert statements are prohibited in production code as they are disabled in release builds. Replace with proper error handling and logging, such as checking the condition and logging an error or returning early if the condition fails.

Copilot · 2026-02-26T03:27:48Z

 	const auto playerId = mSingleton->GetId();

-	if (mPlayerSessionID.size() >= playerId)
+	assert(playerId >= 0);


The assert statement is used in production code (test/aampcli/). According to the coding guidelines (CodingGuidelineID: 1000003), assert statements are prohibited in production code as they are disabled in release builds. Replace with proper error handling and logging.

Copilot · 2026-02-26T03:27:48Z

+	// Copy fragment data vector from other to this using assign method
+	if (!other->fragment.GetVector().empty()) 
+  {
+		const auto& otherVec = other->fragment.GetVector();
+		this->fragment.assign(otherVec.data(), otherVec.data() + otherVec.size());
 	}


The condition !other->fragment.GetVector().empty() checks if the vector is non-empty before copying. However, this doesn't verify that other is not null. If other is null, this will cause a null pointer dereference. Add a null check: if (other && !other->fragment.GetVector().empty()) to match the pattern used earlier in the function where other member variables are copied unconditionally from other.

varshnie and others added 7 commits February 23, 2026 16:34

VPLAY-12568:Implement cancelReservation UVE-API

f9e733c

Reason for change:cancelReservation UVE call Test Procedure: Refer jira ticket Priority: P1 Signed-off-by: varshnie <varshniblue14@gmail.com>

VPLAY-12734 Generate CMCD documentation and review gaps (#1076)

53664dd

VPLAY-12734 Generate CMCD documentation and review gaps Reason for Change: add generated CMCD documentation to repo --------- Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>

narenr94 requested a review from a team as a code owner February 26, 2026 03:22

Copilot AI review requested due to automatic review settings February 26, 2026 03:22

narenr94 merged commit 1175ac3 into feature/RDKEMW-13801 Feb 26, 2026
7 of 8 checks passed

Copilot started reviewing on behalf of narenr94 February 26, 2026 03:23 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev sync#1083

dev sync#1083
narenr94 merged 7 commits into
feature/RDKEMW-13801from
dev_sprint_25_2

narenr94 commented Feb 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Copilot AI Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

narenr94 commented Feb 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants