Skip to content

RDKEVD-6226: Cello - no-spdif output #226

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.

Sign up for GitHub

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

Closed
yeshwanth-nagaswamy wants to merge 1 commit into from
+38 −35
Closed

RDKEVD-6226: Cello - no-spdif output #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions ds/audioOutputPortConfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ List<AudioOutputPortType> AudioOutputPortConfig::getSupportedTypes()
{
List<AudioOutputPortType> supportedTypes;
for (std::vector<AudioOutputPortType>::const_iterator it = _aPortTypes.begin(); it != _aPortTypes.end(); it++) {
printf("\nYESH: supportedTypes _aPortTypes *it %s enabled %d", it->toString().c_str(), it->isEnabled());
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This printf looks like temporary debug instrumentation inside a frequently-called API (getSupportedTypes). It will spam stdout and bypass the component’s log controls. Please remove it or convert to INT_DEBUG (and consider gating behind DS_LOG_LEVEL / a runtime flag).

Suggested change
printf("\nYESH: supportedTypes _aPortTypes *it %s enabled %d", it->toString().c_str(), it->isEnabled());

Copilot uses AI. Check for mistakes.
if (it->isEnabled()) {
supportedTypes.push_back(*it);
}
Expand All @@ -114,57 +115,57 @@ List<AudioOutputPortType> AudioOutputPortConfig::getSupportedTypes()
void dumpconfig(audioConfigs_t *config)
{
if (nullptr == config) {
INT_ERROR("Audio config is NULL");
printf("\nAudio config is NULL");
return;
}
if ( -1 == access("/opt/dsMgrDumpDeviceConfigs", F_OK) ) {
INT_INFO("Dumping of Device configs is disabled");
printf("\nDumping of Device configs is disabled");
return;
Comment on lines 115 to 123
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumpconfig/load() switched from INT_INFO/INT_ERROR to printf, which bypasses the logging framework and can produce a lot of unstructured stdout output. Prefer the INT_* macros so logs retain level + file/line/function context and can be redirected via DS_RegisterForLog().

Copilot uses AI. Check for mistakes.
}

int configSize = -1, portSize = -1;
INT_INFO("\n=============== Starting to Dump Audio Configs ===============\n");
printf("\n\n=============== Starting to Dump Audio Configs ===============\n");
if( nullptr != config->pKConfigs )
{
configSize = (config->pKConfigSize) ? *(config->pKConfigSize) : -1;

for (int i = 0; i < configSize; i++) {
const dsAudioTypeConfig_t *typeCfg = &(config->pKConfigs[i]);
INT_INFO("typeCfg->typeId = %d", typeCfg->typeId);
INT_INFO("typeCfg->name = %s", typeCfg->name);
INT_INFO("typeCfg->numSupportedEncodings = %zu", typeCfg->numSupportedEncodings);
INT_INFO("typeCfg->numSupportedCompressions = %zu", typeCfg->numSupportedCompressions);
INT_INFO("typeCfg->numSupportedStereoModes = %zu", typeCfg->numSupportedStereoModes);
printf("\ntypeCfg->typeId = %d", typeCfg->typeId);
printf("\ntypeCfg->name = %s", typeCfg->name);
printf("\ntypeCfg->numSupportedEncodings = %zu", typeCfg->numSupportedEncodings);
printf("\ntypeCfg->numSupportedCompressions = %zu", typeCfg->numSupportedCompressions);
printf("\ntypeCfg->numSupportedStereoModes = %zu", typeCfg->numSupportedStereoModes);
}
Comment on lines 126 to 139
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These printf calls in dumpconfig() produce multi-line output but don’t consistently include trailing newlines, which can interleave poorly with other output. If you keep this, ensure each record ends with a newline (or keep the existing INT_INFO calls which already end with "\n" in many cases).

Copilot uses AI. Check for mistakes.
}
else
{
INT_ERROR("kAudioConfigs is NULL");
printf("\nkAudioConfigs is NULL");
}

if( nullptr != config->pKPorts )
{
portSize = (config->pKPortSize) ? *(config->pKPortSize) : -1;
for (int i = 0; i < portSize; i++) {
const dsAudioPortConfig_t *portCfg = &(config->pKPorts[i]);
INT_INFO("portCfg->id.type = %d", portCfg->id.type);
INT_INFO("portCfg->id.index = %d", portCfg->id.index);
printf("\nportCfg->id.type = %d", portCfg->id.type);
printf("\nportCfg->id.index = %d", portCfg->id.index);
}
}
else
{
INT_ERROR("kAudioPorts is NULL");
printf("\nkAudioPorts is NULL");
}

INT_INFO("\n=============== Dump Audio Configs done ===============\n");
printf("\n\n=============== Dump Audio Configs done ===============\n");
}

void AudioOutputPortConfig::load(audioConfigs_t* dynamicAudioConfigs)
{
int configSize = -1, portSize = -1;
audioConfigs_t configuration = {0};

INT_INFO("Enter function");
printf("\nEnter function");
try {
/*
* Load Constants First.
Expand All @@ -188,7 +189,7 @@ void AudioOutputPortConfig::load(audioConfigs_t* dynamicAudioConfigs)

}

INT_INFO("Using '%s' config", dynamicAudioConfigs ? "dynamic" : "static");
printf("\nUsing '%s' config", dynamicAudioConfigs ? "dynamic" : "static");
if ( nullptr != dynamicAudioConfigs )
{
configuration = *dynamicAudioConfigs;
Expand All @@ -204,7 +205,7 @@ void AudioOutputPortConfig::load(audioConfigs_t* dynamicAudioConfigs)
configuration.pKPortSize = &portSize;
}

INT_INFO("Audio Config[%p] ConfigSize[%d] Ports[%p] PortSize[%d]",
printf("\nAudio Config[%p] ConfigSize[%d] Ports[%p] PortSize[%d]",
configuration.pKConfigs,
configSize,
configuration.pKPorts,
Expand Down Expand Up @@ -250,16 +251,16 @@ void AudioOutputPortConfig::load(audioConfigs_t* dynamicAudioConfigs)
_aPorts.push_back(AudioOutputPort((portCfg->id.type), portCfg->id.index, i));
_aPortTypes.at(portCfg->id.type).addPort(_aPorts.at(i));
}
INT_INFO("Audio Configs loaded successfully");
printf("\nAudio Configs loaded successfully");
}
else {
INT_ERROR("Audio Configs loading failed");
printf("\nAudio Configs loading failed");
}
}
catch(const Exception &e) {
throw e;
}
INT_INFO("Exit function");
printf("\nExit function");
}

void AudioOutputPortConfig::release()
Expand Down
4 changes: 2 additions & 2 deletions ds/include/dslogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ static inline const char* fileName(const char* path) {
}

#ifndef DS_LOG_LEVEL
#define DS_LOG_LEVEL ERROR_LEVEL
#define DS_LOG_LEVEL DEBUG_LEVEL
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DS_LOG_LEVEL default is changed from ERROR_LEVEL to DEBUG_LEVEL, enabling INT_DEBUG logs by default across the entire component. This can significantly increase log volume and affect performance/operations in production builds. Consider keeping the default at ERROR_LEVEL (or making the higher verbosity conditional on a build flag/runtime setting).

Suggested change
#define DS_LOG_LEVEL DEBUG_LEVEL
#define DS_LOG_LEVEL ERROR_LEVEL

Copilot uses AI. Check for mistakes.
#endif

#define INT_INFO(FORMAT, ...) ds_log(INFO_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )
#define INT_INFO(FORMAT, ...) ds_log(DEBUG_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INT_INFO is redefined to log at DEBUG_LEVEL rather than INFO_LEVEL. This changes the meaning of existing INT_INFO callsites (they’ll appear as DEBUG in log output), making filtering/triage harder. Keep INT_INFO mapped to INFO_LEVEL and use INT_DEBUG for debug-level messages.

Suggested change
#define INT_INFO(FORMAT, ...) ds_log(DEBUG_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )
#define INT_INFO(FORMAT, ...) ds_log(INFO_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )

Copilot uses AI. Check for mistakes.
#define INT_WARN(FORMAT, ...) ds_log(WARN_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )
#define INT_ERROR(FORMAT, ...) ds_log(ERROR_LEVEL, fileName(__FILE__), __LINE__, __FUNCTION__, FORMAT, ##__VA_ARGS__ )

Expand Down
28 changes: 14 additions & 14 deletions ds/manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,22 @@ bool LoadDLSymbols(void* pDLHandle, const dlSymbolLookup* symbols, int numberOfS
int currentSymbols = 0;
bool isAllSymbolsLoaded = false;
if ((nullptr == pDLHandle) || (nullptr == symbols)) {
INT_ERROR("Invalid DL Handle or symbolsPtr");
printf("Invalid DL Handle or symbolsPtr");
}
else {
INT_INFO("numberOfSymbols = %d",numberOfSymbols);
printf("numberOfSymbols = %d",numberOfSymbols);
for (int i = 0; i < numberOfSymbols; i++) {
if (( nullptr == symbols[i].dataptr) || ( nullptr == symbols[i].name)) {
INT_ERROR("Invalid symbol entry at index [%d]", i);
printf("Invalid symbol entry at index [%d]", i);
continue;
}
*(symbols[i].dataptr) = dlsym(pDLHandle, symbols[i].name);
if (nullptr == *(symbols[i].dataptr)) {
INT_ERROR("[%s] is not defined", symbols[i].name);
printf("[%s] is not defined", symbols[i].name);
}
else {
currentSymbols++;
INT_INFO("[%s] is defined and loaded, data[%p]", symbols[i].name, *(symbols[i].dataptr));
printf("[%s] is defined and loaded, data[%p]", symbols[i].name, *(symbols[i].dataptr));
}
Comment on lines 73 to 92
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes replace the existing INT_* logging (which includes file/line/function and routes to stderr / registered callback) with raw printf calls. This loses context, bypasses log-level controls, and writes to stdout without newlines in several cases (likely causing buffering / hard-to-read logs). Prefer using INT_INFO/INT_WARN/INT_ERROR (or at least fprintf(stderr, ...) with newlines) to match the rest of the module’s logging behavior.

Copilot uses AI. Check for mistakes.
}
isAllSymbolsLoaded = (numberOfSymbols) ? (currentSymbols == numberOfSymbols) : false;
Expand All @@ -101,7 +101,7 @@ void loadDeviceCapabilities(unsigned int capabilityType)
void* pDLHandle = nullptr;
bool isSymbolsLoaded = false;

INT_INFO("Entering capabilityType = 0x%08X", capabilityType);
printf("Entering capabilityType = 0x%08X", capabilityType);
dlerror(); // clear old error
pDLHandle = dlopen(RDK_DSHAL_NAME, RTLD_LAZY);

Expand All @@ -110,10 +110,10 @@ void loadDeviceCapabilities(unsigned int capabilityType)
INT_WARN("Failed to dlopen '%s': %s - Falling back to static configurations",
RDK_DSHAL_NAME, dlError ? dlError : "Unknown error");
} else {
INT_INFO("Successfully opened dynamic library '%s'", RDK_DSHAL_NAME);
printf("Successfully opened dynamic library '%s'", RDK_DSHAL_NAME);
}

INT_INFO("DL Instance '%s'", (nullptr == pDLHandle ? "NULL" : "Valid"));
printf("DL Instance '%s'", (nullptr == pDLHandle ? "NULL" : "Valid"));

Comment on lines 103 to 117
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printf() calls here omit a trailing newline, so messages may not flush promptly and can get concatenated. If you keep printf, add "\n" (and consider stderr), but ideally use the existing INT_* macros for consistent formatting and routing.

Copilot uses AI. Check for mistakes.
// Audio Port Config
if (DEVICE_CAPABILITY_AUDIO_PORT & capabilityType) {
Expand Down Expand Up @@ -187,7 +187,7 @@ void loadDeviceCapabilities(unsigned int capabilityType)
dlclose(pDLHandle);
pDLHandle = nullptr;
}
INT_INFO("Exiting ...");
printf("Exiting ...");
}

Manager::Manager() {
Expand Down Expand Up @@ -306,16 +306,16 @@ void Manager::Initialize()
throw e;
}

INT_INFO("Exiting ... with thread id %lu",pthread_self());
printf("Exiting ... with thread id %lu",pthread_self());
}

void Manager::load()
{
INT_INFO("Enter function");
printf("Enter function");
loadDeviceCapabilities( device::DEVICE_CAPABILITY_VIDEO_PORT |
device::DEVICE_CAPABILITY_AUDIO_PORT |
device::DEVICE_CAPABILITY_VIDEO_DEVICE);
INT_INFO("Exit function");
printf("Exit function");
}

/**
Expand All @@ -340,7 +340,7 @@ void Manager::load()
void Manager::DeInitialize()
{
{std::lock_guard<std::mutex> lock(gManagerInitMutex);
INT_INFO("Entering ... count %d with thread id %lu",IsInitialized,pthread_self());
printf("Entering ... count %d with thread id %lu",IsInitialized,pthread_self());
if(IsInitialized>0)IsInitialized--;
if (0 == IsInitialized) {

Expand All @@ -354,7 +354,7 @@ void Manager::DeInitialize()
dsDisplayTerm();
}
}
INT_INFO("Exiting ... with thread %lu",pthread_self());
printf("Exiting ... with thread %lu",pthread_self());
}

}
Expand Down
2 changes: 2 additions & 0 deletions rpc/srv/dsAudio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,15 @@ void AudioConfigInit()
dllib = dlopen(RDK_DSHAL_NAME, RTLD_LAZY);
if (dllib) {
dsSetAudioLevelFunc = (dsSetAudioLevel_t) dlsym(dllib, "dsSetAudioLevel");
INT_DEBUG("YESH: dsSetAudioLevelFunc %d \r\n", dsSetAudioLevelFunc);
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log prints a function pointer using "%d". This is undefined/incorrect on most platforms (and will typically warn/fail on 64-bit) because function pointers must be printed with "%p" (casting to (void*) is common). Also consider removing the temporary "YESH" tag from the message to avoid shipping debug-only strings.

Suggested change
INT_DEBUG("YESH: dsSetAudioLevelFunc %d \r\n", dsSetAudioLevelFunc);
INT_DEBUG("dsSetAudioLevelFunc %p \r\n", (void *)dsSetAudioLevelFunc);

Copilot uses AI. Check for mistakes.
if (dsSetAudioLevelFunc) {
INT_DEBUG("dsSetAudioLevel_t(int, float ) is defined and loaded\r\n");
std::string _AudioLevel("0");
float m_audioLevel = 0;
//SPDIF init
handle = 0;
if(dsGetAudioPort(dsAUDIOPORT_TYPE_SPDIF,0,&handle) == dsERR_NONE) {
INT_DEBUG("YESH: dsGetAudioPort of dsAUDIOPORT_TYPE_SPDIF handle received \r\n");
try {
_AudioLevel = device::HostPersistence::getInstance().getProperty("SPDIF0.audio.Level");
}
Expand Down
Loading