-
Notifications
You must be signed in to change notification settings - Fork 5
Feature/rdk 56050 #224
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.
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
base: dev_sprint_25_2
Are you sure you want to change the base?
Feature/rdk 56050 #224
Conversation
Reason for change: Migration to dev_sprint_25_2 from 117308 (patchset 12) Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: Migration to dev_sprint_25_2 from 117308 (patchset 12) Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
removed .rej
…into feature/RDK-56050
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Reason for change: for dev_Sprint_25_2 Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Coverity Issue - Dereference before null checkNull-checking "respData" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. Medium Impact, CWE-476 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"drmHelper" is passed-by-value as parameter to "std::shared_ptr::shared_ptr(std::shared_ptr const &) /explicit =default/", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""drmHelper"")" instead of "drmHelper". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Use after freeCalling "g_object_get" dereferences freed pointer "this->m_gstElement". High Impact, CWE-416 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"data" is copied in call to copy assignment for class "std::vector<std::vector<unsigned char, std::allocator >, std::allocator<std::vector<unsigned char, std::allocator > > >", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""data"")" instead of "data". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Use after freePassing freed pointer "this->m_gstElement" as an argument to "logprintf". High Impact, CWE-416 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"licenseResponse" is passed-by-value as parameter to "std::shared_ptr::shared_ptr(std::shared_ptr const &) /explicit =default/", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""licenseResponse"")" instead of "licenseResponse". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"watermarkSessionUpdateCallback" is passed-by-value as parameter to "std::function<void (unsigned int, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const &)>::function(std::function<void (unsigned int, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const &)> const &)", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""watermarkSessionUpdateCallback"")" instead of "watermarkSessionUpdateCallback". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Double unlock"~PlayerScheduler" unlocks "this->mExLock" while it is unlocked. Medium Impact, CWE-765 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Variable copied when it could be moved"waterMarkSessionUpdateCB" is passed-by-value as parameter to "std::function<void (unsigned int, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const &)>::function(std::function<void (unsigned int, unsigned int, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const &)> const &)", when it could be moved instead. Low Impact, CWE-none How to fixUse "std::move(""waterMarkSessionUpdateCB"")" instead of "waterMarkSessionUpdateCB". Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Change-Id: I77d0d29b44a01ae8d9fb3ddd455815700a3dbc76
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
1 similar comment
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Reason for change: comp issues Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
middleware/subtec/libsubtec/SubtecAttribute.hpp | ||
middleware/PlayerMetadata.hpp | ||
) | ||
set(AAMP_SUBTEC_CLASS_SOURCES subtec/subtecparser/WebvttSubtecDevParser.cpp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a follow-up ticket to move these out of AAMP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes Vinish, As part of RDK-58626, we will take separation activity too.
@@ -411,7 +411,8 @@ AAMPGstPlayer::AAMPGstPlayer(PrivateInstanceAAMP *aamp, id3_callback_t id3Handle | |||
} | |||
InitializePlayerConfigs(this,playerInstance); | |||
playerInstance->SetPlayerName(PLAYER_NAME); | |||
playerInstance->setEncryption((void*)aamp); | |||
playerInstance->setEncryption((void*)aamp, (void*)aamp->mDRMLicenseManager->mDrmSessionManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revisit why aamp is required here.
return _this->HandleContentProtectionData(drmHelper, streamType, keyId, contentProtectionUpd); | ||
}); | ||
/** Register the profiler update callback for TriggerProfileBeginCb */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up, need a single callback with all the profiling data.
aampInstance->mConfig->IsConfigSet(eAAMPConfig_PropagateURIParam), | ||
aampInstance->mIsFakeTune | ||
); | ||
aampInstance->mConfig->IsConfigSet(eAAMPConfig_UseSecManager), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should look at the history of why we need all these configs, especially
eAAMPConfig_WideVineKIDWorkaround, eAAMPConfig_PropagateURIParam
drm/AampDRMLicManager.cpp
Outdated
@@ -166,8 +206,8 @@ void AampDRMLicenseManager::renewLicense(std::shared_ptr<DrmHelper> drmHelper, v | |||
try | |||
{ | |||
mLicenseRenewalThreads[sessionSlot] = std::thread([this, drmHelper, sessionSlot, aampInstance] { | |||
this->licenseRenewalThread(drmHelper, sessionSlot, aampInstance); | |||
}); | |||
this->licenseRenewalThread(drmHelper, sessionSlot, aampInstance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -45,6 +45,13 @@ using namespace WPEFramework; | |||
|
|||
#define THUNDER_RPC_TIMEOUT 5000 | |||
|
|||
#define ARRAY_SIZE(A) (sizeof(A)/sizeof(A[0])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefix with MW, as similar macros are present in AAMP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
@@ -642,10 +639,10 @@ plugin_init (GstPlugin * plugin) | |||
#define VERSION "0.1.0" | |||
#endif | |||
#ifndef PACKAGE | |||
#define PACKAGE "AAMPGstreamerPlugins" | |||
#define PACKAGE "GstreamerPlugins" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RDKGstreamerPlugins or MWGstreamerPlugins
aamp, | ||
"Advanced Adaptive Media Player", | ||
plugin, | ||
"Interface Player", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MW Interface Player?
@@ -21,7 +21,9 @@ | |||
#define AMLOGIC_SOC_INTERFACE_H | |||
|
|||
#include "SocInterface.h" | |||
|
|||
#if defined(AMLOGIC) | |||
#include "gst_svp_meta.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed , can bring svp while during vendor migration
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
Reason for change: adressing review comments Test Procedure: Refer Ticket Risks: Low Priority: P1 Change-Id: If0a31d066478806bae29f09ab829b1108d7856ba Signed-off-by: Deepikasri N <[email protected]
Coverity Issue - Unchecked return valueCalling "wait_for" without checking return value (as is done elsewhere 13 out of 14 times). Medium Impact, CWE-252 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
pull request upon dev_sprint_25_2