-
Notifications
You must be signed in to change notification settings - Fork 3
RDKEMW-5197 : Advance PQParams #174
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
Conversation
#include "UtilsSearchRDKProfile.h" | ||
|
||
#define API_VERSION_NUMBER_MAJOR 1 | ||
#define API_VERSION_NUMBER_MINOR 2 |
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.
Keep version for old platfroms in code
#define API_VERSION_NUMBER_MAJOR 1
#define API_VERSION_NUMBER_MINOR 1
#define API_VERSION_NUMBER_PATCH 0
define new version in yocto recipe for new platfroms
API_VERSION_NUMBER_MAJOR 1
API_VERSION_NUMBER_MINOR 2
API_VERSION_NUMBER_PATCH 0
EXTRA_OEMAKE += "
'CXXFLAGS+=-DAPI_VERSION_NUMBER_MAJOR="${API_VERSION_NUMBER_MAJOR}"'
'CXXFLAGS+=-DAPI_VERSION_NUMBER_MINOR=${API_VERSION_NUMBER_MINOR}'
'CXXFLAGS+=-DAPI_VERSION_NUMBER_PATCH="${API_VERSION_NUMBER_PATCH}"'
"
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.
Pull Request Overview
Adds advanced picture quality (PQ) parameter support by introducing new JSON-RPC methods, associated capability parsing, and bumps the plugin API version to 1.2.0.
- Update CHANGELOG and bump API version to 1.2.0
- Introduce new JSON-RPC endpoints, helpers, and members in AVOutputTV.h for PQParams
- Switch to metadata-based service registration in AVOutput.cpp
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
AVOutput/CHANGELOG.md | Added version 1.2.0 entry for advance PQParams |
AVOutput/AVOutputTV.h | Added JSON-RPC methods, capability parsers, and internal fields |
AVOutput/AVOutput.cpp | Defined API_VERSION constants and updated SERVICE_REGISTRATION |
Comments suppressed due to low confidence (3)
AVOutput/CHANGELOG.md:16
- The version 1.2.0 entry is dated before the existing 1.1.2 entry (2025-07-01). Ensure changelog entries are in chronological order.
## [1.2.0] - 2025-06-25
AVOutput/CHANGELOG.md:18
- [nitpick] Consider rephrasing to "Advanced PQ parameters" or "Add advanced PQ parameters" for clearer intent in the CHANGELOG.
- Advance PQ Params
AVOutput/AVOutput.cpp:44
- The SERVICE_REGISTRATION macro originally took three arguments (major, minor); passing a fourth (patch) may not match its signature. Verify the macro's definition or remove the extra argument.
SERVICE_REGISTRATION(AVOutput, API_VERSION_NUMBER_MAJOR, API_VERSION_NUMBER_MINOR, API_VERSION_NUMBER_PATCH);
size_t* num_control, tvContextCaps_t ** context_caps); | ||
#define HAL_NOT_READY 0 | ||
#if HAL_NOT_READY | ||
#define CAPABLITY_FILE_NAMEV2 "/opt/panel/pq_capabilities.json" |
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.
The macro name "CAPABLITY_FILE_NAMEV2" is misspelled; it should be "CAPABILITY_FILE_NAMEV2".
#define CAPABLITY_FILE_NAMEV2 "/opt/panel/pq_capabilities.json" | |
#define CAPABILITY_FILE_NAMEV2 "/opt/panel/pq_capabilities.json" |
Copilot uses AI. Check for mistakes.
tvDataComponentColor_t getComponentColorEnum(std::string colorName); | ||
tvError_t getParamsCaps(std::string param, capVectors_t &vecInfo); | ||
int GetPanelID(char *panelid); | ||
int ReadCapablitiesFromConf(std::string param, capDetails_t& info); |
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.
The method name "ReadCapablitiesFromConf" is misspelled; consider renaming to "ReadCapabilitiesFromConf" for clarity.
int ReadCapablitiesFromConf(std::string param, capDetails_t& info); | |
int ReadCapabilitiesFromConf(std::string param, capDetails_t& info); |
Copilot uses AI. Check for mistakes.
DECLARE_JSON_RPC_METHOD(getDigitalNoiseReductionCaps) | ||
DECLARE_JSON_RPC_METHOD(getAISuperResolutionCaps) | ||
DECLARE_JSON_RPC_METHOD(getMEMCCaps) | ||
DECLARE_JSON_RPC_METHOD(getMultiPointWBCaps) |
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.
Remove methods that are not implemented or stubbed before merging to dvt1.1 branch
int *max_offset, tvWBColor_t **color, | ||
tvWBControl_t **control, size_t* num_color, | ||
size_t* num_control, tvContextCaps_t ** context_caps); | ||
#define HAL_NOT_READY 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.
Remove it if its not tested against HAL before merging to dvt1.1
tvError_t GetAISuperResolutionCaps(int* maxAISuperResolution, tvContextCaps_t** context_caps); | ||
tvError_t GetMEMCCaps(int* maxMEMC, tvContextCaps_t** context_caps); | ||
#else | ||
#define CAPABLITY_FILE_NAMEV2 "/etc/pq_capabilities.json" |
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.
No need to use pq_capabilities.json from plugin. Use HAL only for dvt1.1
@@ -296,13 +296,13 @@ namespace Plugin { | |||
{ | |||
tvDimmingMode_t index = tvDimmingMode_MAX; | |||
|
|||
if(mode.compare("Local") == 0 ) { | |||
if(mode.compare("local") == 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.
"Local" is correct string i belive. Confirm with AVOP doc.
Same applies to Fixed and Global
@@ -296,13 +296,13 @@ namespace Plugin { | |||
{ | |||
tvDimmingMode_t index = tvDimmingMode_MAX; | |||
|
|||
if(mode.compare("Local") == 0 ) { | |||
if(mode.compare("local") == 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.
"Local" is correct i belive. please confirm
New PR #175 created against support branch. |
No description provided.