Re: [PATCH v2] transport: Fix set volume failure with invalid device volume

[Jinwang Li <jinwang.li@oss.qualcomm.com>]

Hi Luiz,

On 3/6/2026 11:00 PM, Luiz Augusto von Dentz wrote:
> Hi Jinwang,
> 
> On Thu, Mar 5, 2026 at 3:38 AM Jinwang Li <jinwang.li@oss.qualcomm.com> wrote:
>>
>> When AVRCP is connected earlier than AVDTP, this invalid device volume
>> causes the A2DP sink to reject the peer's registration for the
>> EVENT_VOLUME_CHANGED notification. As a result, subsequent attempts to
>> set the volume fail.
>>
>> Fix this by ensuring a valid device volume is returned when the device
>> volume is invalid, allowing volume registration to succeed.
>>
>> Fixes: fa7828bddd21 ("transport: Fix not being able to initialize volume properly")
>> Signed-off-by: Jinwang Li <jinwang.li@oss.qualcomm.com>
>> ---
>>   profiles/audio/transport.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> Changes from v1:
>>   - Return the max volume from media_transport_get_a2dp_volume()
>>     when there is no A2DP transport and the device volume is invalid.
>>   - Restore the default device volume in device_new().
>>   - Update the commit subject name.
>>   - Link to v1
>>     https://lore.kernel.org/all/20260122063640.310090-1-jinwang.li@oss.qualcomm.com/
>>
>> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
>> index d9feef768..41d8d0a75 100644
>> --- a/profiles/audio/transport.c
>> +++ b/profiles/audio/transport.c
>> @@ -2767,6 +2767,7 @@ static void media_transport_emit_volume(struct media_transport *transport)
>>   int media_transport_get_a2dp_volume(struct btd_device *dev)
>>   {
>>          GSList *l;
>> +       int volume;
>>
>>          if (dev == NULL)
>>                  return -1;
>> @@ -2793,7 +2794,11 @@ int media_transport_get_a2dp_volume(struct btd_device *dev)
>>           * of ordering between AVRCP and A2DP session start. (Note BAP+VCP do
>>           * not have this issue.)
>>           */
>> -       return btd_device_get_volume(dev);
>> +       volume = btd_device_get_volume(dev);
>> +       if (volume < 0)
>> +               volume = 127;
> 
> This doesn't work because the behavior differs depending on the role,
> as a source we need to return -1, so I suggest doing this instead:
> 
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 724b46c592ff..326dfdab7e3d 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -4296,9 +4296,16 @@ static void target_init(struct avrcp *session)
>          if (target->version < 0x0104)
>                  return;
> 
> -       if (avrcp_volume_supported(target))
> +       if (avrcp_volume_supported(target)) {
>                  session->supported_events |=
>                                  (1 << AVRCP_EVENT_VOLUME_CHANGED);
> +               /* Check if transport volume hasn't been initialized then set it
> +                * to max so it works properly if the controller attempts to
> +                * subscribe to AVRCP_EVENT_VOLUME_CHANGED.
> +                */
> +               if (media_transport_get_a2dp_volume(session->dev) < 0)
> +                       media_transport_set_a2dp_volume(session->dev, 127);
> +       }
> 
>          session->supported_events |=
>                                  (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
> 

I agree this is the correct approach, and I’ve verified that it resolves 
the volume registration issue. I’ll send a follow-up patch and include 
the credit.

>> +       return volume;
>>   }
>>
>>   void media_transport_set_a2dp_volume(struct btd_device *dev, int volume)
>> --
>> 2.34.1
>>
> 
>