Skip to content

Feature/delia 70176 #1228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
varatharajan568 wants to merge 7 commits into
base: dev_sprint_25_2
Choose a base branch
from
Open

Feature/delia 70176 #1228

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
19db08d
Update gstcdmidecryptor.cpp
varatharajan568 Mar 23, 2026
c9af2ff
Update gstcdmidecryptor.cpp
varatharajan568 Mar 23, 2026
41135c4
Update gstcdmidecryptor.cpp
varatharajan568 Mar 23, 2026
9af90a0
Merge branch 'dev_sprint_25_2' into feature/DELIA-70176
varatharajan568 Mar 24, 2026
32bb1bf
Update opencdmsessionadapter.cpp
varatharajan568 Mar 25, 2026
bc1ad7d
Update OcdmGstSessionAdapter.cpp
varatharajan568 Mar 25, 2026
77be84b
Merge branch 'dev_sprint_25_2' into feature/DELIA-70176
varatharajan568 Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions middleware/drm/ocdm/OcdmGstSessionAdapter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ int OCDMGSTSessionAdapter::decrypt(GstBuffer *keyIDBuffer, GstBuffer *ivBuffer,
gst_buffer_add_protection_meta (buffer, crypto_info);
}
retValue = OCDMGSTSessionDecrypt(m_pOpenCDMSession, buffer, caps);
printf("VRN DECRYPT Return[%d] \n",retValue);
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
else
/* CID:328751 - Waiting while holding a lock, got detected due to usage of external API. It may be replaced if approach is redesigned in future */
Expand Down
16 changes: 10 additions & 6 deletions middleware/drm/ocdm/opencdmsessionadapter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void OCDMSessionAdapter::initDRMSystem()

OCDMSessionAdapter::~OCDMSessionAdapter()
{
MW_LOG_WARN("[HHH]OCDMSessionAdapter destructor called! keySystem %s", m_keySystem.c_str());
MW_LOG_WARN("VRN [HHH]OCDMSessionAdapter destructor called! keySystem %s", m_keySystem.c_str());
clearDecryptContext();

if (m_pOpenCDMSystem) {
Expand All @@ -120,7 +120,7 @@ OCDMSessionAdapter::~OCDMSessionAdapter()
void OCDMSessionAdapter::generateDRMSession(const uint8_t *f_pbInitData,
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);
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

std::lock_guard<std::mutex> guard(decryptMutex);
if (m_pOpenCDMSystem == nullptr)
Expand All @@ -141,6 +141,7 @@ void OCDMSessionAdapter::generateDRMSession(const uint8_t *f_pbInitData,

m_OCDMSessionCallbacks.key_update_callback = [](OpenCDMSession* session, void* userData, const uint8_t key[], const uint8_t keySize) {
OCDMSessionAdapter* userSession = reinterpret_cast<OCDMSessionAdapter*>(userData);
MW_LOG_WARN( "VRN TRIGGERED KEY UPDATE OCDM");
userSession->keyUpdateOCDM(key, keySize);
};

Expand All @@ -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();
Comment on lines 142 to 154
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
};
const unsigned char *customDataMessage = customData.empty() ? nullptr:reinterpret_cast<const unsigned char *>(customData.c_str()) ;
Expand Down Expand Up @@ -214,7 +216,7 @@ void OCDMSessionAdapter::processOCDMChallenge(const char destUrl[], const uint8_
}

void OCDMSessionAdapter::keyUpdateOCDM(const uint8_t key[], const uint8_t keySize) {
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);
// Validate input parameters
if (key != nullptr && keySize > 0)
{
Expand All @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Comment on lines 219 to +290
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
int retValue = -1;
const uint8_t* keyMessage = NULL;
uint16_t keyMessageLength = 0;
Expand Down
12 changes: 12 additions & 0 deletions middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,10 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip(
// call decrypt even for clear samples in order to copy it to a secure buffer. If secure buffers are not supported
// decrypt() call will return without doing anything
if (cdmidecryptor->drmSession != NULL)
{
errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps);
MW_LOG_WARN("VRN drmSession->decrypt returns[%d]",errorCode);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Comment on lines 523 to +524
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
else
{ /* If drmSession creation failed, then the call will be aborted here */
result = GST_FLOW_NOT_SUPPORTED;
Expand Down Expand Up @@ -637,16 +640,20 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip(
}

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

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

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++;
Comment on lines 646 to 652
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
else if(cdmidecryptor->hdcpOpProtectionFailCount)
{
MW_LOG_WARN("VRN Enters HDCP PROTEC FAIL COUNT[%d] T[%d]",cdmidecryptor->hdcpOpProtectionFailCount,DECRYPT_FAILURE_THRESHOLD);
Comment on lines +643 to +656
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
if(cdmidecryptor->hdcpOpProtectionFailCount >= DECRYPT_FAILURE_THRESHOLD) {
Comment on lines +648 to 657
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
GstStructure *newmsg = gst_structure_new("HDCPProtectionFailure", "message", G_TYPE_STRING,"HDCP Output Protection Error", NULL);
gst_element_post_message(reinterpret_cast<GstElement*>(cdmidecryptor),gst_message_new_application (GST_OBJECT (cdmidecryptor), newmsg));
Expand All @@ -656,6 +663,7 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip(
else
{
GST_ERROR_OBJECT(cdmidecryptor, "decryption failed; error code %d\n",errorCode);
MW_LOG_WARN("VRN Enters decryption failed; error code %d FC[%d]",errorCode,cdmidecryptor->decryptFailCount);
cdmidecryptor->decryptFailCount++;
if(cdmidecryptor->decryptFailCount >= DECRYPT_FAILURE_THRESHOLD && cdmidecryptor->notifyDecryptError )
{
Expand All @@ -665,11 +673,14 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip(
{
// Failure - 2.2 vs 1.4 HDCP
error = g_error_new(GST_STREAM_ERROR , GST_STREAM_ERROR_FAILED, "HDCP Compliance Check Failure");
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);
Comment on lines +676 to +681
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
MW_LOG_WARN("VRN Decrypt Failed gst_element_post_message:[%d]",errorCode);
gst_element_post_message(reinterpret_cast<GstElement*>(cdmidecryptor), gst_message_new_error (GST_OBJECT (cdmidecryptor), error, "Decrypt Failed"));
g_error_free(error);
result = GST_FLOW_ERROR;
Expand All @@ -679,6 +690,7 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip(
}
else
{
MW_LOG_WARN("VRN Else Scenario [%d]",errorCode);
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
cdmidecryptor->decryptFailCount = 0;
cdmidecryptor->hdcpOpProtectionFailCount = 0;
Comment on lines +693 to 695
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
if (cdmidecryptor->mediaType == eGST_MEDIATYPE_AUDIO)
Expand Down
Loading