RDK-54413: Correct array of pointers usage in GetTVSupported API #6035
Conversation
AVOutput/AVOutputTVHelper.cpp
Outdated
| @@ -266,13 +266,18 @@ namespace Plugin { | |||
| { | |||
| int mode = 0; | |||
| tvDolbyMode_t dolbyModes[tvMode_Max]; | |||
| tvDolbyMode_t *dolbyModesPtr = dolbyModes; // Pointer to statically allocated tvDolbyMode_t array | |||
| tvDolbyMode_t *dolbyModesPtr[tvMode_Max]; | |||
AVOutput/AVOutputTV.cpp
Outdated
| @@ -2423,9 +2426,14 @@ namespace Plugin { | |||
| uint32_t AVOutputTV::getSupportedPictureModes(const JsonObject& parameters, JsonObject& response) | |||
| { | |||
| LOGINFO("Entry\n"); | |||
| pic_modes_t *pictureModes; | |||
| pic_modes_t pictureModes[PIC_MODES_SUPPORTED_MAX]; | |||
| pic_modes_t *pictureModesPtr[PIC_MODES_SUPPORTED_MAX]; | |||
AVOutput/AVOutputTV.cpp
Outdated
| @@ -2204,13 +2204,16 @@ namespace Plugin { | |||
| { | |||
| LOGINFO("Entry\n"); | |||
| tvDolbyMode_t dvModes[tvMode_Max]; | |||
| tvDolbyMode_t *dvModesPtr = dvModes; // Pointer to statically allocated tvDolbyMode_t array | |||
| tvDolbyMode_t *dvModesPtr[tvMode_Max]; | |||
|
|
||
| for (int i = 0; i < tvMode_Max; i++) | ||
| { | ||
| dvModesPtr[i] = &dvModes[i]; |
There was a problem hiding this comment.
Not sure why we are doing this?
In a function like below:
tvError_t GetTVSupportedDolbyVisionModes(tvDolbyMode_t *dvModes , unsigned short *count)
You just need to pass dvModes right which essentially is the pointer to the array.
There was a problem hiding this comment.
The HAL interface expects an array of pointers as its input parameter. While it is technically correct that a pointer to an array would suffice for the functionality of this function, after careful consideration and discussions with the system architects, it has been decided to adhere to the previously agreed-upon open-source HAL API design.
To align with the function declaration, which explicitly expects an array of pointers, the necessary adjustments have been made. Passing the base address of an array would not be appropriate in this context, as it does not match the expected parameter type (i.e., an array of pointers). Therefore, the implementation has been updated to comply with the HAL interface requirements.
There was a problem hiding this comment.
I think you have defined MAX values for both tvMode_Max & PIC_MODES_SUPPORTED_MAX . The use case only needs Pointer to an array of tvDolbyMode_t enums / of pic_modes_t structure. Please see if it's feasible to correct the interface or we really have a Array of Pointers use case where we would like Platform to allocate memory depending on what it supports.
There was a problem hiding this comment.
Merging based on agreement with architects on header : rdkcentral/rdkv-halif-tvsettings#49
GetTVSupportedPictureModes, GetTVSupportedVideoFormats, GetTVSupportedVideoSources, GetTVSupportedVideoSources, GetTVSupportedDolbyVisionModes does not properly use the array of pointers parameter in the HAL. The parameters must be passed according to the interface requirements.