RDKEMW-16714: refactor hard-coded array-based approach to a generic#239
RDKEMW-16714: refactor hard-coded array-based approach to a generic#239yuvaramachandran-gurusamy wants to merge 13 commits intodevelopfrom
Conversation
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Refactors the video frame-rate handling from a fixed, array-indexed approach to a map-based representation, and wires initialization/teardown into the device settings Manager lifecycle.
Changes:
- Add global frame-rate map init/dump/deinit helpers and expose them via
frameRate.hpp. - Rework
FrameRateimplementation to use astd::map<dsVideoFrameRate_t, FrameRateInfo>rather than hard-coded arrays. - Call frame-rate init/dump during
Manager::Initialize()and clear it duringManager::DeInitialize().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| ds/manager.cpp | Initializes and deinitializes the new frame-rate map as part of Manager lifecycle. |
| ds/include/frameRate.hpp | Exposes new init/dump/deinit functions for the frame-rate map. |
| ds/frameRate.cpp | Replaces array-based frame-rate data with a map and adds init/dump/deinit + updated lookup/constructors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…statements Agent-Logs-Url: https://github.com/rdkcentral/devicesettings/sessions/2a1df342-47cc-4d33-85c0-b0963487d4a5 Co-authored-by: yuvaramachandran-gurusamy <123441336+yuvaramachandran-gurusamy@users.noreply.github.com>
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…ings into topic/RDKEMW-16714 Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…opic/RDKEMW-16714
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // initialize the frame rates map with supported frame rates. This is required to be done before loading the video port configs as some of the frame rate related APIs depend on this map to return the supported frame rates and its values. | ||
| initializeFrameRates(); | ||
|
|
There was a problem hiding this comment.
initializeFrameRates() is used here but manager.cpp doesn’t include frameRate.hpp directly, relying on transitive includes via other headers. This is brittle (future include refactors can break compilation). Please include frameRate.hpp explicitly in this file to make the dependency clear.
| FrameRate::FrameRate(float value) : _value(value){ | ||
| for (const auto& frameRate : _frameRates) { | ||
| // check if the value matches with any of the supported frame rates in the map, if found initialize the FrameRate object with corresponding name and id | ||
| if (std::fabs(frameRate.second.value - value) < std::numeric_limits<float>::epsilon()) { | ||
| _id = frameRate.first; | ||
| _name = frameRate.second.name; |
There was a problem hiding this comment.
FrameRate(float) compares float values using std::numeric_limits<float>::epsilon(), which is effectively an exact-equality check and is too strict for values like 23.98/29.97 that may come from calculations or external APIs with small rounding differences. Use a practical tolerance (absolute or relative), or compare against the enum id rather than float equality to avoid spurious IllegalArgumentException throws.
RDKEMW-16714: refactor hard-coded array-based approach to a generic
Signed-off-by: yuvaramachandran_gurusamy yuvaramachandran_gurusamy@comcast.com