sync to develop by santoshcomcast · Pull Request #215 · rdkcentral/devicesettings

santoshcomcast · 2026-02-23T14:16:34Z

No description provided.

1.0.26

* RDKEMW-11232 getAudioFormatApi issue on Xione-UK * Add priority-based audio output port selection Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: apatel859 <48992974+apatel859@users.noreply.github.com>

1.0.27

* RDKEMW-12054: Fix Coverity identified issues

* implement retry logic for dsVideoPortInit Enhance the retry mechanism for AudioPort and VideoDeviceInit as well

.1.0.28

Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

1.0.29

Copilot

Pull request overview

This PR syncs changes from the develop branch, incorporating multiple bug fixes and improvements from versions 1.0.26 through 1.0.29. The changes primarily focus on Coverity issue fixes, thread safety improvements, printf format specifier corrections, build dependency additions, and an audio format API fix.

Changes:

Fixed Coverity-identified issues including format specifiers, member initialization, thread safety, and removed unreachable default cases
Added IARMBus build-time dependency to Makefiles
Implemented retry logic for all device settings initialization functions and improved exception handling
Fixed audio format API to use proper audio port handle instead of NULL

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
CHANGELOG.md	Added changelog entries for versions 1.0.26-1.0.29 documenting bug fixes and feature additions
ds/manager.cpp	Refactored initialization with retry logic for all init functions; added thread safety improvements with needInit flag
ds/exception.hpp	Fixed member initialization order by initializing _err in constructor
ds/videoDevice.cpp	Fixed member initialization by initializing _dfc in constructor
ds/host.cpp	Added getAudioPortHandle() method to select appropriate audio port for format queries
ds/host.hpp	Added declaration for new getAudioPortHandle() method
ds/audioOutputPort.hpp	Added getOutputPortHandle() getter method
ds/audioOutputPort.cpp	Fixed printf format specifiers for intptr_t (%ld with long cast) and uint32_t (%u)
ds/hdmiIn.cpp	Fixed printf format specifier for sizeof() to use %zu
ds/edid-parser.cpp	Removed default cases from switch statements to address Coverity warnings
rpc/srv/dsAudio.c	Fixed thread safety in termination, added cast for bool in printf, improved error logging
rpc/cli/dsVideoPort.c	Fixed printf format specifiers for intptr_t and size_t
sample/testFrontPanel.cpp	Added exception handling around DeInitialize() call
ds/Makefile	Added -lIARMBus linker flag for build-time dependency
rpc/cli/Makefile	Added -lIARMBus linker flag for build-time dependency

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

Copilot · 2026-02-23T14:20:46Z

ds/host.cpp

+            else 
+            {
+                //TV profile with multiple audio output ports
+                // First check the ports which are dynamically conected and finally fallback to SPEAKER0 which is always connected.


The comment has a typo: "conected" should be "connected". This typo appears in the comment explaining the fallback logic for audio ports.

Suggested change

// First check the ports which are dynamically conected and finally fallback to SPEAKER0 which is always connected.

// First check the ports which are dynamically connected and finally fallback to SPEAKER0 which is always connected.

Copilot · 2026-02-23T14:20:46Z

ds/host.cpp

+        {
+            cout << " Exception Thrown in getAudioPortHandle().. returning NULL...: " << e.what() << "\n";
+        }
+


If all audio ports in the priority list fail the enabled/connected check or throw exceptions, the function returns NULL without logging any information about why no valid port was found. This could make it difficult to diagnose issues where audio format queries fail. Consider adding a log statement before the final return to indicate that no valid audio port was found, especially since this is used by getCurrentAudioFormat which may fail silently if passed a NULL handle.

Suggested change

cout << "getAudioPortHandle(): No valid audio port found; returning NULL handle.\n";

Copilot · 2026-02-23T14:20:47Z

ds/edid-parser.cpp

@@ -55,7 +55,6 @@ static void parse_std_timing(unsigned char* bytes, edid_data_t* data_ptr) {
        case 1: v = (h * 3) / 4; break;
        case 2: v = (h * 4) / 5; break;
        case 3: v = (h * 9) / 16; break;


The removal of the default case in this switch statement means that if an unexpected value appears in the 2-bit field (which can only be 0-3), the variable 'v' will remain at 0. While this switch covers all possible values for a 2-bit field, consider whether v=0 is the appropriate behavior if future EDID specifications extend this field or if corrupted EDID data is encountered. The original default case with early return provided explicit handling for invalid data.

Suggested change

case 3: v = (h * 9) / 16; break;

case 3: v = (h * 9) / 16; break;

default:

INT_DEBUG("Unexpected std timing aspect ratio code: 0x%02X\n",

(bytes[idx + 1] & 0xC0) >> 6);

return;

Copilot · 2026-02-23T14:20:47Z

ds/edid-parser.cpp

@@ -381,8 +380,6 @@ static void parse_ext_timing(unsigned char* bytes, edid_data_t* data_ptr) {
                case 6: break;
                // 'Use Extended Tag'
                case 7: parse_extended_db(&bytes[idx], data_ptr); break;


The removal of the default case for unsupported extension tags means that unknown tag values (anything other than 0-7) will now fall through without any logging or error handling. The original code logged "Unsupported extension tag" for unknown values, which was helpful for debugging EDID parsing issues. Consider whether silently ignoring unknown tags is the intended behavior, especially since new EDID extension tags might be added in future specifications.

Suggested change

case 7: parse_extended_db(&bytes[idx], data_ptr); break;

case 7: parse_extended_db(&bytes[idx], data_ptr); break;

// unsupported / unknown extension tag

default:

INT_DEBUG("parse_ext_timing: unsupported extension tag=%d len=%d\n", tag, len);

break;

Copilot · 2026-02-23T14:20:47Z

ds/manager.cpp

+	bool needInit = false;
+
+    {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++;
+	}


The IsInitialized counter is incremented before the initialization attempt, which means if initialization fails and an exception is thrown, the counter is decremented in the catch block. However, this creates a window where IsInitialized is incremented but initialization hasn't occurred yet. If another thread calls Initialize() during this window after the lock is released, it will see IsInitialized > 0 and skip initialization (needInit will be false). Consider moving the IsInitialized++ to after successful initialization completes, or keep it locked until after the try block completes successfully.

Copilot · 2026-02-23T14:20:48Z

ds/manager.cpp

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


The retry logic always retries up to 25 times (26 attempts total) regardless of the error returned. The previous implementation only retried on dsERR_INVALID_STATE errors and would stop immediately on other errors. Now all initialization functions retry on any error, which could mask genuine configuration problems or cause unnecessary delays. For example, if dsAudioPortInit fails due to missing hardware or invalid configuration, the system will still retry 25 times, adding ~2.5 seconds of delay. Consider adding error-specific retry logic or documenting why all errors warrant retries.

Copilot · 2026-02-23T14:20:48Z

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++;
+	}

-	try {
-	    if (0 == IsInitialized) {	
-
-	    	dsError_t err = dsERR_GENERAL;
-	    	unsigned int retryCount = 0;
-	    	// This retry logic will wait for the device manager initialization from the client side
-	    	// until the device manager service initialization is completed. The retry mechanism checks
-	    	// only for dsERR_INVALID_STATE, which is reported if the underlying service is not ready.
-	    	// Once the service is ready, other port initializations can be called directly without any delay.
-	    	// That's why the retry logic is applied only for dsDisplayInit.
-	    	do {
-	    		err = dsDisplayInit();
-	    		printf ("Manager::Initialize: result :%d retryCount :%d\n", err, retryCount);
-	    		if (dsERR_NONE == err) break;
-	    		usleep(100000);
-	    	} while(( dsERR_INVALID_STATE == err) && (retryCount++ < 25));
+    try {
+        if (needInit) {


In the new implementation, the needInit flag is checked outside the lock, allowing initialization to proceed without lock protection. Between releasing the lock at line 153 and entering the try block at line 155, another thread could also determine needInit=true and both would proceed to initialize concurrently, potentially causing race conditions in the underlying initialization functions (dsDisplayInit, dsAudioPortInit, etc.). The original implementation kept the lock held during the entire initialization sequence, preventing this race condition.

Copilot · 2026-02-23T14:20:48Z

ds/host.cpp

+            cout << " Exception Thrown in getAudioPortHandle().. returning NULL...: " << e.what() << "\n";
+        }
+
+        return NULL;


Returning NULL for an intptr_t type is potentially problematic. While NULL typically evaluates to 0, it's defined as a null pointer constant and may not be the most semantically appropriate choice for an integer type. Consider returning 0 explicitly instead of NULL to make the intent clearer that this represents an invalid/error handle value rather than a null pointer.

Suggested change

return NULL;

return 0;

apatel859 and others added 16 commits January 8, 2026 19:54

1.0.26 release change log updates

898b0bb

Merge branch 'release/1.0.26'

a5b7a9f

Merge tag '1.0.26' into develop

62858d9

1.0.26

1.0.27 release change log updates

6594764

Merge branch 'release/1.0.27'

3a73464

Merge tag '1.0.27' into develop

56fabff

1.0.27

RDKEMW-12054: Fix Coverity identified issues (#190)

8f0dd0f

* RDKEMW-12054: Fix Coverity identified issues

RDKEMW-9781: Video format issue on MTK (#199)

f90a986

* implement retry logic for dsVideoPortInit Enhance the retry mechanism for AudioPort and VideoDeviceInit as well

1.0.28 release change log updates

f11f7de

Merge branch 'release/1.0.28'

5b50634

Merge tag '1.0.28' into develop

05c2d95

.1.0.28

RDKEMW-10899: Adding IARMBus dependency during Build Time (#205)

2540d79

Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>

1.0.29 release change log updates

9e5074b

Merge branch 'release/1.0.29'

edc8c70

Merge tag '1.0.29' into develop

ab7a2df

1.0.29

santoshcomcast requested a review from a team as a code owner February 23, 2026 14:16

Copilot AI review requested due to automatic review settings February 23, 2026 14:16

Copilot started reviewing on behalf of santoshcomcast February 23, 2026 14:16 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

sync to develop#215

sync to develop#215
santoshcomcast wants to merge 16 commits intofeature/RDKEMW-8587-dlsymfrom
develop

santoshcomcast commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Copilot AI Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	// First check the ports which are dynamically conected and finally fallback to SPEAKER0 which is always connected.
	// First check the ports which are dynamically connected and finally fallback to SPEAKER0 which is always connected.



	cout << "getAudioPortHandle(): No valid audio port found; returning NULL handle.\n";

-        case 3: v = (h * 9) / 16; break;
+        case 3: v = (h * 9) / 16; break;
+        default:
+            INT_DEBUG("Unexpected std timing aspect ratio code: 0x%02X\n",
+                      (bytes[idx + 1] & 0xC0) >> 6);
+            return;

Comments

Conversation

santoshcomcast commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants