RDKTV-37684: New sync up logic for updateAVoutputTVParam #192
Conversation
Reason for change: moved the updateAVoutputTVParam to thread if sync/reset Test Procedure: as per JIRA Risks: None Priority: P1 Signed-off-by: Ashish Rai <[email protected]>
ashishkumarrai1998
force-pushed
the
feature/RDKTV-37684_updateAVoutputparams_logic_change
branch
from
d528dc1 to
cd9c306
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the updateAVoutputTVParam method to implement conditional threading logic based on current TV settings. When the current picture mode, source, and format don't match the requested parameters, the operation is moved to a detached thread for asynchronous execution.
- Splits the original
updateAVoutputTVParaminto a wrapper method and an implementation method - Adds conditional logic to execute synchronously or asynchronously based on current TV state
- Introduces thread-based execution for mismatched parameter scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| AVOutputTVHelper.cpp | Refactors updateAVoutputTVParam into wrapper and implementation, adds threading logic and current state validation |
| AVOutputTV.h | Adds declaration for new updateAVoutputTVParamImplementation method |
| retVal = GetCurrentVideoSource(&sourceIndex); | ||
| if(retVal != tvERROR_NONE) { | ||
| LOGERR("%s : GetCurrentVideoSource( ) Failed\n",__FUNCTION__); | ||
| return false; |
There was a problem hiding this comment.
The function return type is int but returning false (boolean). This should return an integer error code instead.
| return false; | |
| return -1; |
Copilot uses AI. Check for mistakes.
| LOGINFO("Starting thread : %s\n",__FUNCTION__); | ||
| std::thread updateAVoutputTVParam_thread(&WPEFramework::Plugin::AVOutputTV::updateAVoutputTVParamImplementation, this, action, tr181ParamName, info, pqParamIndex, level); | ||
| updateAVoutputTVParam_thread.detach(); | ||
|
|
There was a problem hiding this comment.
When executing in thread mode (else branch), ret remains 0 but the actual operation result is lost since the thread is detached. Consider returning a different value to indicate async execution or implement a callback mechanism.
| // Return -1 to indicate asynchronous execution | |
| return -1; |
Copilot uses AI. Check for mistakes.
| else | ||
| { | ||
| LOGINFO("Starting thread : %s\n",__FUNCTION__); | ||
| std::thread updateAVoutputTVParam_thread(&WPEFramework::Plugin::AVOutputTV::updateAVoutputTVParamImplementation, this, action, tr181ParamName, info, pqParamIndex, level); |
There was a problem hiding this comment.
Using detached threads without proper lifecycle management can lead to issues if the AVOutputTV object is destroyed while the thread is still running. Consider using a thread pool or ensuring proper cleanup.
Copilot uses AI. Check for mistakes.
| //GetCurrent pqmode | ||
| if(!getCurrentPictureMode(picMode)) { | ||
| LOGERR("Failed to get the current picture mode\n"); | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - uses tabs instead of spaces like the rest of the code.
| } | |
| } |
Copilot uses AI. Check for mistakes.
| formatIndex = VIDEO_FORMAT_SDR; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - uses tabs instead of spaces like the rest of the code.
| formatIndex = VIDEO_FORMAT_SDR; | |
| } | |
| formatIndex = VIDEO_FORMAT_SDR; | |
| } |
Copilot uses AI. Check for mistakes.
| formatIndex = VIDEO_FORMAT_SDR; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation - uses tabs instead of spaces like the rest of the code.
| formatIndex = VIDEO_FORMAT_SDR; | |
| } | |
| formatIndex = VIDEO_FORMAT_SDR; | |
| } |
Copilot uses AI. Check for mistakes.
Reason for change: moved the updateAVoutputTVParam to thread if sync/reset
Test Procedure: as per JIRA
Risks: None
Priority: P1