Conversation
…y caps setting in mp4demux Reason for change:Use codec info from the manifest rather than lazy caps setting in mp4demux Risks: p1 Signed-off-by: varshnie <varshniblue14@gmail.com>
…ek) regression with useMp4Demux=true Reason For change:Added a frame flag while configuring the caps for ec3 codec Risks: p1 Signed-off-by: varshnie <varshniblue14@gmail.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
InitializeSourceForPlayer() now mixes this and the explicit PlayerInstance pointer (_this) for reading config/DRM fields, which can produce incorrect behavior when PlayerInstance differs (and can be triggered by existing pi-cli usage).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses a regression seen on AmLogic for DASH (useMp4Demux=true) by adjusting how GStreamer caps are configured for EC3 (E-AC3) and by optionally applying “encrypted” caps metadata earlier during appsrc source configuration.
Changes:
- Add
framed=trueto EC3 caps duringInitializeSourceForPlayer()for non-HLS paths. - Add optional early encrypted-caps application during
InitializeSourceForPlayer()behind a newConfigs::enableEncryptedCapsflag. - Add unit tests covering encrypted elementary-stream caps application being enabled/disabled.
File summaries
| File | Description |
|---|---|
test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp |
Adds unit tests for the new enableEncryptedCaps behavior during source initialization. |
InterfacePlayerRDK.h |
Extends Configs with a new enableEncryptedCaps configuration toggle. |
InterfacePlayerRDK.cpp |
Refactors encrypted-caps logic into ApplyEncryptedCaps(), adds early encrypted-caps application, and sets EC3 framed=true during source caps setup. |
Review details
Suppressed comments (2)
InterfacePlayerRDK.cpp:1972
- The new EC3 caps adjustment reads
m_gstConfigParam->mediavia implicitthiseven though the function otherwise uses the explicit PlayerInstance pointer (_this). For consistency (and to avoid using the wrong instance when PlayerInstance differs), read media from_this->m_gstConfigParam.
if (caps != NULL && stream->format == GST_FORMAT_AUDIO_ES_EC3 &&
static_cast<GstMediaFormat>(m_gstConfigParam->media) != eGST_MEDIAFORMAT_HLS)
{
InterfacePlayerRDK.cpp:1983
- ApplyEncryptedCaps is invoked with
mDrmSystemvia implicitthiswhile the rest of InitializeSourceForPlayer is driven by the explicit PlayerInstance pointer (_this). Pass_this->mDrmSystemto ensure the DRM system ID comes from the same instance being configured.
MW_LOG_MIL("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format);
ApplyEncryptedCaps(caps, mDrmSystem);
}
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1926
to
+1929
| const bool hasQueuedProtectionEvent = HasQueuedProtectionEvent(privatePlayer->gstPrivateContext, _this->mProtectionLock); | ||
| const bool isElementaryAudioVideoFormat = IsElementaryAudioVideoFormat(stream->format); | ||
| const bool shouldApplyEncryptedCaps = m_gstConfigParam->enableEncryptedCaps && hasQueuedProtectionEvent && isElementaryAudioVideoFormat; | ||
| MW_LOG_MIL("InitializeSourceForPlayer entry type[%d] format[%d] hasQueuedProtectionEvent[%d] elementary[%d] encryptedCapsEnabled[%d]", mediaType, stream->format, hasQueuedProtectionEvent, isElementaryAudioVideoFormat, m_gstConfigParam->enableEncryptedCaps); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason For change:Added a frame flag while configuring the caps for ec3 codec
Risks: p1
Signed-off-by: varshnie varshniblue14@gmail.com