RDKEMW-9781: Video format issue on MTK #199

kiruba115 · 2026-01-12T17:39:05Z

RDKEMW-9781: Video format issue on MTK
Enhance the retry mechanism for AudioPort and VideoDeviceInit as well

Enhance the retry mechanism for AudioPort and VideoDeviceInit as well

Copilot

Pull request overview

This PR addresses RDKEMW-9781, a video format issue on MTK platforms by enhancing the initialization retry mechanism. The changes extract the existing retry logic into a reusable helper function and extend retry capabilities to AudioPort and VideoDevice initialization, which previously had no retry mechanism.

Changes:

Introduced a new retryInitialization helper function with configurable retry logic
Extended retry mechanism to dsAudioPortInit, dsVideoPortInit, and dsVideoDeviceInit (previously only dsDisplayInit had retries)
Refactored Manager::Initialize to use the new helper function for cleaner code

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-12T18:29:10Z

ds/manager.cpp

+ * @param[in] maxRetries Maximum number of retry attempts. Defaults to 25. The function will
+ *                       retry up to this many times with a 100ms delay between attempts.


The documentation states that the function "retries up to this many times with a 100ms delay between attempts" for the maxRetries parameter. However, due to the loop logic issue, the actual behavior may differ. The delay happens after a failed attempt but before checking the retry count, and the post-increment in the condition means the number of executions may not match the documented behavior precisely. This documentation should be clarified once the loop logic is corrected.

ds/manager.cpp

Copilot · 2026-01-12T18:29:11Z

ds/manager.cpp

+            err = retryInitialization("dsAudioPortInit", dsAudioPortInit);
            CHECK_RET_VAL(err);
-	    	err = dsVideoPortInit();
+
+            err = retryInitialization("dsVideoPortInit", dsVideoPortInit);


The retry behavior has changed for dsAudioPortInit and dsVideoPortInit. Previously, these functions were called directly without retries. Now they use the retry mechanism with default parameters (maxRetries=25, checkInvalidState=false), meaning they will retry on ANY error up to 25 times. This is a significant behavioral change that could mask real errors and delay failure detection. Consider whether retrying on all errors is appropriate for these functions, or if they should only retry on specific errors like dsERR_INVALID_STATE similar to dsDisplayInit.

Copilot · 2026-01-12T18:29:11Z

ds/manager.cpp

+dsError_t retryInitialization(const char* functionName, 
+                                   std::function<dsError_t()> initFunc, 
+                                   unsigned int maxRetries = 25,
+                                   bool checkInvalidState = false) 
+{
+	dsError_t err = dsERR_GENERAL;
+	unsigned int retryCount = 0;
+
+	do {
+		err = initFunc();
+		printf("Manager::Initialize:%s result :%d retryCount :%d\n", 
+				functionName, err, retryCount);
+		if (dsERR_NONE == err) break;
+		usleep(100000);
+	} while ((checkInvalidState ? (dsERR_INVALID_STATE == err) : true) && 
+				(retryCount++ < maxRetries));
+
+	return err;
+}


The new retryInitialization function and the modified retry behavior in Manager::Initialize lack test coverage. Given that the test directory contains tests for other device settings components, consider adding tests to verify the retry logic works correctly, including edge cases like maximum retries reached, successful retry after failures, and the checkInvalidState parameter behavior.

ds/manager.cpp

Copilot · 2026-01-12T18:29:12Z

ds/manager.cpp

+dsError_t retryInitialization(const char* functionName, 
+                                   std::function<dsError_t()> initFunc, 
+                                   unsigned int maxRetries = 25,
+                                   bool checkInvalidState = false) 


The default parameter value for maxRetries (25) is specified in the function definition, but this is a static function with a forward declaration at line 66. Default parameter values should only be specified in the function declaration, not in the definition. The compiler may accept this, but it's not standard practice and could cause issues. Move the default parameter values to the declaration at line 66.

Copilot · 2026-01-12T18:29:12Z

ds/manager.cpp

-	    	VideoDeviceConfig::getInstance().load();
-	    }
+
+            err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit);


The retry behavior has changed for dsVideoDeviceInit. Previously, this function was called directly without retries. Now it uses the retry mechanism with default parameters (maxRetries=25, checkInvalidState=false), meaning it will retry on ANY error up to 25 times. This is a significant behavioral change that could mask real errors and delay failure detection. Consider whether retrying on all errors is appropriate for this function, or if it should only retry on specific errors like dsERR_INVALID_STATE similar to dsDisplayInit.

Suggested change

err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit);

err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit, 25, true);

ds/manager.cpp

apatel859

fix

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ds/manager.cpp

Copilot · 2026-01-13T17:22:15Z

ds/manager.cpp

+        if (IsInitialized == 0) {
+            needInit = true;
+        }
+        IsInitialized++;


Potential race condition: If an exception is thrown after IsInitialized is incremented (line 152) but before the initialization completes, the decrement in the catch block (line 179) may not fully restore the correct state. Consider moving the IsInitialized increment to after successful initialization to ensure accurate state tracking.

Copilot · 2026-01-13T17:22:15Z

ds/manager.cpp

+    {std::lock_guard<std::mutex> lock(gManagerInitMutex);
+        printf("Entering %s count %d with thread id %lu\n",__FUNCTION__,IsInitialized,pthread_self());
+        if (IsInitialized == 0) {
+            needInit = true;
+        }
+        IsInitialized++;


Inconsistent indentation: The opening brace on line 147 uses spaces instead of tabs, which is inconsistent with the rest of the file. This should use consistent indentation with the rest of the codebase.

Suggested change

{std::lock_guard<std::mutex> lock(gManagerInitMutex);

printf("Entering %s count %d with thread id %lu\n",__FUNCTION__,IsInitialized,pthread_self());

if (IsInitialized == 0) {

needInit = true;

}

IsInitialized++;

{std::lock_guard<std::mutex> lock(gManagerInitMutex);

printf("Entering %s count %d with thread id %lu\n",__FUNCTION__,IsInitialized,pthread_self());

if (IsInitialized == 0) {

needInit = true;

}

IsInitialized++;

ds/manager.cpp

…MTK-Video-format-cb-issue

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ds/manager.cpp

mplement retry logic for dsVideoPortInit

9ea0c78

Enhance the retry mechanism for AudioPort and VideoDeviceInit as well

kiruba115 requested a review from a team as a code owner January 12, 2026 17:39

Add CHECK_RET_VAL

c1e2f88

kiruba115 requested a review from apatel859 January 12, 2026 18:21

apatel859 requested a review from Copilot January 12, 2026 18:26

Copilot started reviewing on behalf of apatel859 January 12, 2026 18:26 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

apatel859 reviewed Jan 12, 2026

View reviewed changes

ds/manager.cpp Outdated Show resolved Hide resolved

apatel859 reviewed Jan 12, 2026

View reviewed changes

ds/manager.cpp Outdated Show resolved Hide resolved

apatel859 requested changes Jan 12, 2026

View reviewed changes

kiruba115 added 3 commits January 12, 2026 14:22

Address review comments

096cc9f

Address review comments

9ba57f7

Error check for retry logic updated

21a9b99

apatel859 previously approved these changes Jan 13, 2026

View reviewed changes

Rebase to latest develop

c952aa6

kiruba115 dismissed apatel859’s stale review via c952aa6 January 13, 2026 17:18

Copilot AI review requested due to automatic review settings January 13, 2026 17:18

Copilot started reviewing on behalf of kiruba115 January 13, 2026 17:19 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

kiruba115 added 2 commits January 13, 2026 12:38

Merge remote-tracking branch 'origin/develop' into topic/RDKEMW-9781-…

8a023ed

…MTK-Video-format-cb-issue

Removed unwanted merge backup file

ca0378c

Copilot AI review requested due to automatic review settings January 13, 2026 17:42

Copilot started reviewing on behalf of kiruba115 January 13, 2026 17:42 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

ds/manager.cpp Show resolved Hide resolved

ds/manager.cpp Show resolved Hide resolved

ds/manager.cpp Show resolved Hide resolved

apatel859 approved these changes Jan 13, 2026

View reviewed changes

apatel859 merged commit f90a986 into develop Jan 13, 2026
14 of 15 checks passed

github-actions bot locked and limited conversation to collaborators Jan 13, 2026

		* @param[in] maxRetries Maximum number of retry attempts. Defaults to 25. The function will
		* retry up to this many times with a 100ms delay between attempts.

	err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit);
	err = retryInitialization("dsVideoDeviceInit", dsVideoDeviceInit, 25, true);

RDKEMW-9781: Video format issue on MTK #199

RDKEMW-9781: Video format issue on MTK #199

Uh oh!

Conversation

kiruba115 commented Jan 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

apatel859 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants