CPESP-7136: DS audio setting for capability - #166
Conversation
Signed-off-by: apatel859 <amit_patel5@comcast.com>
Signed-off-by: gsanto722 <grandhi_santoshkumar@comcast.com>
Signed-off-by: gsanto722 <grandhi_santoshkumar@comcast.com>
Signed-off-by: gsanto722 <grandhi_santoshkumar@comcast.com>
Signed-off-by: gsanto722 <grandhi_santoshkumar@comcast.com>
santoshcomcast
left a comment
santoshcomcast
left a comment
There was a problem hiding this comment.
Addressed all the comments. Please review.
Signed-off-by: gsanto722 <grandhi_santoshkumar@comcast.com>
| * @note The caller is responsible for ensuring the validity of the pointers passed to this function. | ||
| * Ensure that the array pointed to by `kSupportedPortTypes` is large enough to hold all supported port types. | ||
| */ | ||
| dsError_t getSupportedAudioOutputPorts(dsAudioPortType_t* kSupportedPortTypes, int* numAudioPorts); |
There was a problem hiding this comment.
Should we prefix the APIs with ds to maintain consistency ?
| { | ||
| size_t numAudioCapabilities; | ||
| uint16_t audioCapabilities[AUDIO_MAX_CAPABILITIES]; | ||
| size_t numAudioPortTypeSupported; |
There was a problem hiding this comment.
You are querying audio features per port. Why you need these additional things ? We only need to maintain 1 audioFeatures_t structure I think.
| size_t numVideoDeviceCapabilities; | ||
| uint16_t videoDeviceCapabilities[VIDEO_DEVICE_MAX_CAPABILITIES]; | ||
| videoDeviceFeatures_t videoDevFeatures; | ||
| }videoDeviceSupportedFeatures_t; |
There was a problem hiding this comment.
Sorry I don't understand the rationale behind maintaining 2 structures. You are already querying a feature per video device.
Signed-off-by: grandhi_santoshkumar <grandhi_santoshkumar@comcast.com>
| size_t numConnectedVOPs; | ||
| dsVideoPortPortId_t connectedVOPs[dsVIDEOPORT_TYPE_MAX]; | ||
| }dsAudioFeatures_t; | ||
|
|
There was a problem hiding this comment.
Just for consistency, should we move the structure to dsAVDtypes.h ?
| size_t numSupportedDFCs; | ||
| dsVideoZoom_t supportedDFCs[dsVIDEO_ZOOM_MAX]; | ||
| dsVideoZoom_t defaultDFC; | ||
| }dsVideoDeviceFeatures_t; |
There was a problem hiding this comment.
For consistency can we move the structure to : dsVideoDeviceTypes.h ?
| int8_t defaultResIndex; // Index of the default resolution | ||
| size_t numconnectedAOPs; // Number of connected audio output ports | ||
| uint8_t connectedAOP[dsAUDIOPORT_TYPE_MAX]; // Array of connected audio output ports | ||
| } dsVideoPortFeatures_t; |
There was a problem hiding this comment.
For consistency , can we please move the structure to dsAVDtypes.h ?
| * @return dsError_t Returns an error code indicating the success or failure of the operation. | ||
| * Possible values include dsERR_NONE for success or other error codes for failure. | ||
| */ | ||
| dsError_t getSupportedVideoPorts(dsVideoPortType_t* kSupportedVideoPorts, int* numAudioPorts); |
|
@santoshcomcast please raise PR against develop branch and also create a github issue. You can use the CPESP ticket for internal tracking. |
|
Addressed and created new PR from develop. So closed this PR. |
3 participants
No description provided.