feat: add per-input volume control and fader level to audiomixer (#396)#399
feat: add per-input volume control and fader level to audiomixer (#396)#399
Conversation
|
b'## Copyright scan failure |
There was a problem hiding this comment.
Pull request overview
Adds missing per-input audio level controls to the AudioMixer HAL AIDL surface (per dsAudio.h gap analysis), plus a new mixer-level fader property intended for main/associated balance (e.g., audio description).
Changes:
- Added
setInputVolume()/getInputVolume()toIAudioMixerControllerfor per-input volume (0–100). - Added
Property.FADER_LEVEL(0–100) to represent main vs associated balance. - Updated
hfp-audiomixer.yamlreference profile to declareFADER_LEVELsupport for mixer resourceid: 0.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| audiomixer/current/com/rdk/hal/audiomixer/IAudioMixerController.aidl | Adds per-input volume setter/getter methods with AIDL doc/comments. |
| audiomixer/current/com/rdk/hal/audiomixer/Property.aidl | Introduces new FADER_LEVEL mixer property with documentation. |
| audiomixer/current/hfp-audiomixer.yaml | Declares FADER_LEVEL in mixer 0 supported properties list. |
| * in the HAL Feature Profile (hfp-audiomixer.yaml). Middleware must | ||
| * query capabilities before using this property. |
There was a problem hiding this comment.
The doc says “Middleware must query capabilities before using this property”, but IAudioMixer.getCapabilities() returns Capabilities which currently doesn’t include any field that advertises supported mixer-level Property keys. This makes the guidance ambiguous/misleading for API consumers. Please clarify the wording (e.g., refer explicitly to the HFP supportedProperties list) or expose supported mixer properties via the AIDL capabilities so clients can actually query them at runtime.
| * in the HAL Feature Profile (hfp-audiomixer.yaml). Middleware must | |
| * query capabilities before using this property. | |
| * in the HAL Feature Profile (hfp-audiomixer.yaml), for example via the | |
| * HFP supportedProperties list. Middleware must verify that this property | |
| * is declared there before using it, as mixer property support is not | |
| * currently advertised through the AIDL Capabilities object. |
| * @brief Sets the volume level for a specific mixer input. | ||
| * @details Allows per-input attenuation for use cases such as TTS overlay, | ||
| * system sound mixing, or audio description level adjustment. | ||
| * |
There was a problem hiding this comment.
New documentation adds an @details tag. Per the project’s AIDL/Doxygen conventions, @details should be avoided (detailed text should follow @brief without a tag). Please remove @details here and keep the detailed description as plain comment text under @brief.
| * @param[in] inputIndex Mixer input index (0-based, matching Capabilities.inputs). | ||
| * @param[in] volume Volume level in range 0..100 (0 = silent, 100 = full). | ||
| * | ||
| * @returns true if the volume was set, false if the input index is invalid. |
There was a problem hiding this comment.
For boolean-returning methods, other comments in this file use @returns + @retval entries to document each outcome (e.g., setInputRouting). Consider switching this block to that pattern so the success/failure semantics are unambiguous and consistent.
| * @returns true if the volume was set, false if the input index is invalid. | |
| * @returns Success status. | |
| * @retval true The volume was successfully set for the specified input. | |
| * @retval false The input index is invalid. |
|
b'## WARNING: A Copyright scan failure has been waived A prior failure has been upvoted
|
Summary
Adds two mechanisms for per-input audio level control on the AudioMixer, addressing gaps identified from the dsAudio.h gap analysis (raised by @BatthalaH).
Per-input volume methods (IAudioMixerController)
setInputVolume(int inputIndex, int volume)— set volume (0-100) for a specific mixer inputgetInputVolume(int inputIndex)— query current volume for an inputCovers: TTS attenuation, system sound levels, audio description level adjustment.
Fader property (Property.aidl)
Property.FADER_LEVEL = 5— main/associated audio balance (0=main only, 50=equal, 100=assoc only)Platform-dependent — must be declared in
hfp-audiomixer.yaml. Reference HFP updated for mixer instance 0.Files changed
IAudioMixerController.aidl— addedsetInputVolume()andgetInputVolume()methodsProperty.aidl— addedFADER_LEVELpropertyhfp-audiomixer.yaml— addedFADER_LEVELto mixer 0 supported propertiesCloses #396
See also: discussion #398 (output port lifecycle rationale)