Feature/delia 70176 by varatharajan568 · Pull Request #1228 · rdkcentral/aamp

varatharajan568 · 2026-03-24T14:22:02Z

No description provided.

Copilot

Pull request overview

This PR adds additional runtime logging around drmSession->decrypt() execution and decrypt/HDCP error-handling paths in the GStreamer CDMI decryptor element.

Changes:

Added braces around a drmSession != NULL block before calling decrypt().
Added multiple MW_LOG_WARN statements to log decrypt return codes and various error branches (including HDCP-related cases).
Added logging around error creation and posting of decrypt failure messages.

Copilot · 2026-03-24T14:25:26Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

 	if (errorCode != 0 || cdmidecryptor->hdcpOpProtectionFailCount)
 	{
+		MW_LOG_WARN("VRN Enters errorcode[%d]",errorCode);
 	if(errorCode == HDCP_OUTPUT_PROTECTION_FAILURE)
 	{
+		MW_LOG_WARN("VRN Enters HDCP O/P PROTEC errorcode[%d]",errorCode);
 		cdmidecryptor->hdcpOpProtectionFailCount++;


The added WARN line at the start of the error-handling block is followed by control flow that is not indented consistently (if(errorCode == ...) is at the same indentation as the parent block). Since this region is already complex, please re-indent (and add missing spaces around parentheses/keywords) so the nesting is unambiguous.

Copilot · 2026-03-24T14:25:27Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

+	MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

 	cdmidecryptor->streamEncrypted = true;
 	if (errorCode != 0 || cdmidecryptor->hdcpOpProtectionFailCount)
 	{
+		MW_LOG_WARN("VRN Enters errorcode[%d]",errorCode);
 	if(errorCode == HDCP_OUTPUT_PROTECTION_FAILURE)
 	{
+		MW_LOG_WARN("VRN Enters HDCP O/P PROTEC errorcode[%d]",errorCode);
 		cdmidecryptor->hdcpOpProtectionFailCount++;
 	}
 	else if(cdmidecryptor->hdcpOpProtectionFailCount)
 	{
+		MW_LOG_WARN("VRN Enters HDCP PROTEC FAIL COUNT[%d] T[%d]",cdmidecryptor->hdcpOpProtectionFailCount,DECRYPT_FAILURE_THRESHOLD);


The "VRN" prefix in these new log messages doesn't match other logging in this element (which uses GST_*_OBJECT messages without per-developer tags). Consider removing the tag or replacing it with a consistent component prefix so logs remain searchable and standardized.

Suggested change

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

cdmidecryptor->streamEncrypted = true;

if (errorCode != 0 || cdmidecryptor->hdcpOpProtectionFailCount)

{

MW_LOG_WARN("VRN Enters errorcode[%d]",errorCode);

if(errorCode == HDCP_OUTPUT_PROTECTION_FAILURE)

{

MW_LOG_WARN("VRN Enters HDCP O/P PROTEC errorcode[%d]",errorCode);

cdmidecryptor->hdcpOpProtectionFailCount++;

}

else if(cdmidecryptor->hdcpOpProtectionFailCount)

{

MW_LOG_WARN("VRN Enters HDCP PROTEC FAIL COUNT[%d] T[%d]",cdmidecryptor->hdcpOpProtectionFailCount,DECRYPT_FAILURE_THRESHOLD);

GST_WARNING_OBJECT(cdmidecryptor, "drmSession->decrypt returns[%d]", errorCode);

cdmidecryptor->streamEncrypted = true;

if (errorCode != 0 || cdmidecryptor->hdcpOpProtectionFailCount)

{

GST_WARNING_OBJECT(cdmidecryptor, "Decrypt error code[%d]", errorCode);

if(errorCode == HDCP_OUTPUT_PROTECTION_FAILURE)

{

GST_WARNING_OBJECT(cdmidecryptor, "HDCP output protection error code[%d]", errorCode);

cdmidecryptor->hdcpOpProtectionFailCount++;

}

else if(cdmidecryptor->hdcpOpProtectionFailCount)

{

GST_WARNING_OBJECT(cdmidecryptor, "HDCP protection failure count[%d] threshold[%d]", cdmidecryptor->hdcpOpProtectionFailCount, DECRYPT_FAILURE_THRESHOLD);

Copilot · 2026-03-24T14:25:27Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

 			if (cdmidecryptor->drmSession != NULL)
+			{
 			   errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);
+				MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);


gst_cdmidecryptor_transform_ip() is on the per-buffer hot path and this MW_LOG_WARN runs for every clear buffer when secure-buffer copying is enabled. Logging at WARN level (and while holding cdmidecryptor->mutex) can significantly impact throughput and flood logs; consider downgrading to DEBUG/TRACE and/or only logging on non-zero errorCode / state transitions.

Suggested change

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

if (errorCode != 0)

{

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]", errorCode);

}

Copilot · 2026-03-24T14:25:27Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

 	}
 	else
 	{
+		MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);


These newly added MW_LOG_WARN calls will execute for every encrypted buffer (including success path via "Else Scenario"). Since this function runs per-sample, WARN-level logging here risks severe log spam and performance regression; consider removing these or gating them to error conditions / threshold events, and using DEBUG/TRACE for routine diagnostics.

Suggested change

MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);

MW_LOG_DEBUG("VRN Else Scenario [%d]",errorCode);

Copilot · 2026-03-24T14:25:27Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

+				MW_LOG_WARN("VRN g_error_new HDCP Compliance Check Failure[%d] Err[%p]",errorCode,error);
 			}
 			else
 			{
 				error = g_error_new(GST_STREAM_ERROR , GST_STREAM_ERROR_FAILED, "Decrypt Error: code %d", errorCode);
+				MW_LOG_WARN("VRN g_error_new Decrypt Error:[%d] Err[%p]",errorCode, error);


Avoid logging raw pointer values (Err[%p]) in production logs, since this can leak memory addresses and reduce the effectiveness of ASLR. Prefer omitting the pointer or logging structured error details (domain/code/message) instead.

Suggested change

MW_LOG_WARN("VRN g_error_new HDCP Compliance Check Failure[%d] Err[%p]",errorCode,error);

}

else

{

error = g_error_new(GST_STREAM_ERROR , GST_STREAM_ERROR_FAILED, "Decrypt Error: code %d", errorCode);

MW_LOG_WARN("VRN g_error_new Decrypt Error:[%d] Err[%p]",errorCode, error);

MW_LOG_WARN("VRN g_error_new HDCP Compliance Check Failure[%d] domain:%u code:%d message:%s",

errorCode, error->domain, error->code, error->message);

}

else

{

error = g_error_new(GST_STREAM_ERROR , GST_STREAM_ERROR_FAILED, "Decrypt Error: code %d", errorCode);

MW_LOG_WARN("VRN g_error_new Decrypt Error:[%d] domain:%u code:%d message:%s",

errorCode, error->domain, error->code, error->message);

Copilot · 2026-03-24T14:25:28Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

 			   errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);
+				MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);


New logging lines introduce inconsistent indentation (mixed spaces/tabs) and missing spaces after commas in function calls (e.g., ...,errorCode). Please align indentation with the surrounding block and apply consistent spacing to keep this hot-path code readable and consistent with the project's formatting rules.

Suggested change

errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]", errorCode);

Copilot

Pull request overview

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

Copilot · 2026-03-25T04:47:50Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

 	}

 	errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);
+	MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);


MW_LOG_WARN is emitted on every decrypt call regardless of success/failure. Since gst_cdmidecryptor_transform_ip() is called per buffer, this is likely to flood logs and affect playback performance; prefer GST_TRACE/DEBUG, or log only on non-zero errorCode (optionally rate-limited).

Suggested change

MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

GST_TRACE_OBJECT(cdmidecryptor, "VRN drmSession->decrypt returns[%d]", errorCode);

Copilot · 2026-03-25T04:47:50Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

+		MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);
 		cdmidecryptor->decryptFailCount = 0;
 	cdmidecryptor->hdcpOpProtectionFailCount = 0;


The success path logs "VRN Else Scenario" at WARN level for every decrypted buffer, which is high-frequency and will overwhelm logs in normal playback. Please remove this log or downgrade it to TRACE/DEBUG behind a feature flag / build-time guard.

Suggested change

MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);

cdmidecryptor->decryptFailCount = 0;

cdmidecryptor->hdcpOpProtectionFailCount = 0;

#ifdef FUNCTION_DEBUG

MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);

#endif

cdmidecryptor->decryptFailCount = 0;

cdmidecryptor->hdcpOpProtectionFailCount = 0;

Copilot · 2026-03-25T04:47:50Z

middleware/drm/ocdm/opencdmsessionadapter.cpp

 		uint32_t f_cbInitData, std::string &customData)
 {
-	MW_LOG_INFO("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
+	MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);


This changes a diagnostic log from INFO to WARN and prints raw pointer values (this, system/session pointers). If this is meant for routine tracing, WARN is too high-severity and pointer addresses are noisy/sensitive; please keep this at INFO/DEBUG (or remove) and avoid logging addresses unless strictly needed.

Suggested change

MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);

MW_LOG_INFO("generateDRMSession called (system=%s, session=%s)",

m_pOpenCDMSystem ? "set" : "null",

m_pOpenCDMSession ? "set" : "null");

Copilot · 2026-03-25T04:47:51Z

middleware/drm/ocdm/opencdmsessionadapter.cpp

@@ -149,6 +150,7 @@ void OCDMSessionAdapter::generateDRMSession(const uint8_t *f_pbInitData,

 		m_OCDMSessionCallbacks.keys_updated_callback = [](const OpenCDMSession* session, void* userData) {
 			OCDMSessionAdapter* userSession = reinterpret_cast<OCDMSessionAdapter*>(userData);
+			MW_LOG_WARN( "VRN TRIGGERED KEY UPDATED OCDM");
 			userSession->keysUpdatedOCDM();


These callbacks now emit WARN logs on key update events. Key update/keys updated can be frequent during playback; WARN-level will spam logs and may impact performance. Please downgrade to DEBUG/TRACE and consider including minimal, actionable context (or rate-limit) instead of a constant string.

Copilot · 2026-03-25T04:47:51Z

middleware/drm/ocdm/OcdmGstSessionAdapter.cpp

 					gst_buffer_add_protection_meta (buffer, crypto_info);
 							}
 						retValue = OCDMGSTSessionDecrypt(m_pOpenCDMSession, buffer, caps);
+				printf("VRN DECRYPT Return[%d] \n",retValue);


printf() in the decrypt path will write to stdout/stderr outside the project logging framework and is very expensive in a per-buffer hot path. Please remove it or replace with the existing logging macros at an appropriate level (e.g., TRACE/DEBUG), ideally gated/rate-limited.

Suggested change

printf("VRN DECRYPT Return[%d] \n",retValue);

MW_LOG_TRACE("VRN DECRYPT Return[%d]", retValue);

Copilot · 2026-03-25T04:47:51Z

middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp

+		MW_LOG_WARN("VRN Enters errorcode[%d]",errorCode);
 	if(errorCode == HDCP_OUTPUT_PROTECTION_FAILURE)
 	{
+		MW_LOG_WARN("VRN Enters HDCP O/P PROTEC errorcode[%d]",errorCode);
 		cdmidecryptor->hdcpOpProtectionFailCount++;
 	}
 	else if(cdmidecryptor->hdcpOpProtectionFailCount)
 	{
+		MW_LOG_WARN("VRN Enters HDCP PROTEC FAIL COUNT[%d] T[%d]",cdmidecryptor->hdcpOpProtectionFailCount,DECRYPT_FAILURE_THRESHOLD);
 		if(cdmidecryptor->hdcpOpProtectionFailCount >= DECRYPT_FAILURE_THRESHOLD) {


These statements introduce a lot of extra WARN-level logging in the hot decryption path (e.g., "Enters errorcode..."). Several messages duplicate the existing GST_ERROR_OBJECT and don’t add actionable context. Consider removing them or downgrading to DEBUG, and keep WARN/ERROR for genuinely actionable, infrequent events.

Copilot · 2026-03-25T04:47:52Z

middleware/drm/ocdm/opencdmsessionadapter.cpp

@@ -224,6 +226,7 @@ void OCDMSessionAdapter::keyUpdateOCDM(const uint8_t key[], const uint8_t keySiz
 		if (m_pOpenCDMSession)
 		{
 			m_keyStatus = opencdm_session_status(m_pOpenCDMSession, key, keySize);
+			MW_LOG_WARN("VRN OCDM SESSION UPDATE[%d]",m_keyStatus);
 			m_keyStateIndeterminate = false;
 		}
 		else
@@ -239,21 +242,22 @@ void OCDMSessionAdapter::keyUpdateOCDM(const uint8_t key[], const uint8_t keySiz
 			// Check if this key already exists to avoid duplicates
 			if (std::find(m_usableKeys.begin(), m_usableKeys.end(), keyData) == m_usableKeys.end())
 			{
+				MW_LOG_WARN("VRN ADDED TO USABLE KEYS");
 				m_usableKeys.push_back(keyData);
 			}
 		}
 	}
 }

 void OCDMSessionAdapter::keysUpdatedOCDM() {
-	MW_LOG_INFO("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
+	MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
 	m_keyStatusReady.signal();
 }


 DrmData * OCDMSessionAdapter::generateKeyRequest(string& destinationURL, uint32_t timeout)
 {
-	MW_LOG_INFO("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
+	MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
 	DrmData * result = NULL;

 	m_eKeyState = KEY_ERROR;
@@ -283,7 +287,7 @@ DrmData * OCDMSessionAdapter::generateKeyRequest(string& destinationURL, uint32_

 int OCDMSessionAdapter::processDRMKey(DrmData* key, uint32_t timeout)
 {
-	MW_LOG_INFO("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
+	MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);


This changes routine session tracing from INFO to WARN. If it’s needed for debugging, prefer DEBUG/TRACE to avoid raising log severity for normal execution (and avoid pointer address logging at WARN).

varatharajan568 added 3 commits March 23, 2026 16:09

Update gstcdmidecryptor.cpp

19db08d

Update gstcdmidecryptor.cpp

c9af2ff

Update gstcdmidecryptor.cpp

41135c4

varatharajan568 requested a review from a team as a code owner March 24, 2026 14:22

Copilot AI review requested due to automatic review settings March 24, 2026 14:22

Merge branch 'dev_sprint_25_2' into feature/DELIA-70176

9af90a0

Copilot started reviewing on behalf of varatharajan568 March 24, 2026 14:22 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

varatharajan568 added 2 commits March 25, 2026 10:12

Update opencdmsessionadapter.cpp

32bb1bf

Update OcdmGstSessionAdapter.cpp

bc1ad7d

Copilot AI review requested due to automatic review settings March 25, 2026 04:44

Copilot started reviewing on behalf of varatharajan568 March 25, 2026 04:45 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Merge branch 'dev_sprint_25_2' into feature/DELIA-70176

77be84b

-				MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);
+			   if (errorCode != 0)
+			   {
+				   MW_LOG_WARN("VRN drmSession->decrypt returns[%d]", errorCode);
+			   }

	MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);
	MW_LOG_DEBUG("VRN Else Scenario [%d]",errorCode);

		errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);
		MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);

	MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);
	GST_TRACE_OBJECT(cdmidecryptor, "VRN drmSession->decrypt returns[%d]", errorCode);

-	MW_LOG_WARN("at %p, with %p, %p", this , m_pOpenCDMSystem, m_pOpenCDMSession);
+	MW_LOG_INFO("generateDRMSession called (system=%s, session=%s)",
+			m_pOpenCDMSystem ? "set" : "null",
+			m_pOpenCDMSession ? "set" : "null");

	printf("VRN DECRYPT Return[%d] \n",retValue);
	MW_LOG_TRACE("VRN DECRYPT Return[%d]", retValue);

Conversation

varatharajan568 commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants