RDKEMW-11877 : Metrics Collection for Player Interface Public APIs

[nejuma1]

RDKEMW-11877 : Metrics Collection for Player Interface Public APIs
Reason for change : Integrating code for metrics collection
Test Procedure : Enable info logs to see metrics related logs
Priority : P1
Risks : None
Signed-off-by : nejumathoppil_nazar@comcast.com

[github-actions]

Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

1 out of 3 committers have signed the CLA.
✅ (narenr94)[https://github.com/narenr94]
❌ @rekhap2kandhavelan
❌ @nejuma1
_{You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.}

[rdkcmf-jenkins]

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 2 files pending identification.

• Protex Server Path: /home/blackduck/github/middleware-player-interface/72/rdkcentral/middleware-player-interface

• Commit: 7161fc8

Report detail: gist'

[Copilot]

Pull request overview

This pull request adds metrics collection infrastructure for Player Interface Public APIs through performance profiling capabilities. The changes include a new PerfProfiler module for timing measurements, updates to the base64_Decode function signature to accept source length explicitly, comprehensive unit tests for PlayerUtils and DRM session management, and various bug fixes including spelling corrections in license headers and padding order fixes in base64 URL decoding.

Changes:

• Added PerfProfiler module with ScopedTimer class for automatic function timing and metrics collection
• Updated base64_Decode API to require explicit source length parameter for improved security and correctness
• Added comprehensive unit tests for RawKeyToKeyId conversion and DRM multi-key validation scenarios
• Fixed base64 URL decoding padding order bug and corrected license header typos

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 17 comments.

Show a summary per file

File	Description
PerfProfiler.h / PerfProfiler.cpp	New performance profiling infrastructure with RAII-based timing
baseConversion/_base64.h / _base64.cpp	Updated base64_Decode to require explicit length, removed unsafe overload
drm/DrmSessionManager.cpp	Enhanced multi-key validation with binary/hex normalization logic
drm/ocdm/opencdmsessionadapter.cpp	Refactored keyUpdateOCDM to consistently update usable keys list
InterfacePlayerRDK.cpp	Added profiling macros to numerous public API methods
PlayerUtils.cpp / playerJsonObject/PlayerJsonObject.cpp	Fixed base64 URL decode padding order bug
test/utests/tests/PlayerUtilsTests/*	New comprehensive tests for RawKeyToKeyId hex conversion
test/utests/tests/DrmSessionManagerTests/*	New tests for multi-key DRM scenarios with real PSSH data
test/utests/tests/OCDMSessionAdapter/KeyUpdateTests.cpp	New tests for key update and validation flows
CMakeLists.txt	Added conditional compilation support for profiling feature
externals/contentsecuritymanager/*	Added profiling macros and fixed license header typos
test/utests/fakes/*	New fake implementations for testing infrastructure

Comments suppressed due to low confidence (1)

baseConversion/_base64.h:43

• The removal of the two-parameter base64_Decode overload is a breaking API change. This function is part of the public API (defined in baseConversion/_base64.h which is installed to the include directory). External code that depends on this library and uses the two-parameter version will fail to compile. Consider keeping both overloads for backward compatibility, with the two-parameter version calling the three-parameter version with strlen(src) as the third argument.

/**
 * @fn base64_Decode
 * @param src pointer to cstring containing base64-encoded data
 * @param len receives byte length of returned pointer, or zero upon failure
 * @param srcLen string length of src
 */
unsigned char *base64_Decode(const char *src, size_t *len, size_t srcLen);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The copyright year in the header is 2024, but other new files in this PR use 2025. For consistency with other new files in this PR, this should be 2025.

Suggested change

	* Copyright 2024 RDK Management
	* Copyright 2025 RDK Management

[Copilot]

The std::stoul call can throw std::invalid_argument or std::out_of_range exceptions if the hex string contains invalid characters or values that are too large. These exceptions are not caught, which could cause the application to crash. Consider wrapping this in a try-catch block and handling errors gracefully by logging an error and returning an empty vector or the original key unchanged.

[Copilot]

The PSSH comment describes 5 key IDs but the test code expects 6 entries (line 697: ASSERT_EQ(keyIDs.size(), 6)). The comment lists 5 key IDs with dashes but notes that 'keyIDs.size()' should be 6, with comment "Should have 5 key IDs". This is confusing. Either the comment should say "6 key IDs" or explain that one of the entries is a content ID, not a key ID. Looking at line 973 comment in a similar test, it appears the 6th entry is indeed a content ID. Clarify the comment to indicate 5 key IDs + 1 content ID = 6 total entries.

[Copilot]

Similar to the previous issue, the key ID "962822bf-0c85-c940-aae5-6943547888874" has 37 characters (including dashes) instead of the standard 36 for a UUID. There's an extra '4' at the end making it "...888874" instead of "...88887". Verify whether this is intentional or a typo in the comment.

[Copilot]

The use of std::move on line 791 is unnecessary and potentially misleading. The keyIdEntry variable is a local variable created in the loop iteration, and it will be moved from automatically when passed to push_back due to copy elision or implicit move. The explicit std::move doesn't provide any benefit here and makes the code harder to read. Consider removing it for clarity.

Suggested change

	data.push_back(std::move(keyIdEntry));
	data.push_back(keyIdEntry);

[Copilot]

There is redundant logic in the SOURCES list. Line 228 adds PerfProfiler.cpp unconditionally, but lines 230-232 check ENABLE_MW_PROFILING and append PerfProfiler.cpp again. The conditional block is unnecessary since the file is already in the list at line 228. Remove either the unconditional addition at line 228 or the conditional block at lines 230-232.

Suggested change

	if(ENABLE_MW_PROFILING)
	list(APPEND SOURCES PerfProfiler.cpp)
	endif()

[Copilot]

The comment on line 1030 says "1 key ID + 1 content ID" but the preceding comment on line 1025 (not shown in diff but inferred) mentions parsing PSSH to extract keys. This is consistent with the test expecting 2 entries. However, for consistency with comments elsewhere in the test file, it would be clearer to state "Should have 2 entries: 1 key ID + 1 content ID" to match the pattern used in line 960 comments.

[Copilot]

The comment on line 663 mentions a key ID "1c9e3c69-eb0e-5cc4-3065-421218173f4aa" which has 37 characters (including dashes), but a standard UUID should have 36 characters. There's an extra 'a' at the end making it "...173f4aa" instead of "...173f4a". Verify whether this is intentional or if it's a typo in the comment. The actual binary key used in the test (lines 719-720) appears correct with 16 bytes.

[Copilot]

The fake implementation of base64_Encode always returns NULL, which could cause null pointer dereferences in test code that expects a valid pointer. Consider either returning an empty string (allocated with malloc) or documenting that this is intentionally stubbed and tests should not rely on this function. If tests do use base64_Encode, they will encounter issues.

Suggested change

	* @return Encoded string (stub returns NULL)
	*/
	char* base64_Encode(const unsigned char* src, size_t len)
	{
	return NULL;
	* @return Encoded string (stub returns empty string allocated with malloc)
	*/
	char* base64_Encode(const unsigned char* src, size_t len)
	{
	(void)src;
	(void)len;
	// Return a heap-allocated empty string to avoid NULL dereferences in tests.
	char* encoded = static_cast<char*>(std::malloc(1));
	if (encoded != NULL)
	{
	encoded[0] = '\0';
	}
	return encoded;

[Copilot]

Similar to an earlier issue, the comment states "Should have 1 key ID" but the test expects 2 (line 1158: ASSERT_EQ(keyIDs.size(), 2)). The comment should clarify whether it means 1 key ID + 1 content ID = 2 total entries, or if the test expectation is incorrect.

[rdkcmf-jenkins]

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

• Upvote reason: OK

• Commit: 7161fc8
  '