Re: [PATCH BlueZ v2] audio/avrcp: Determine Absolute Volume support from feature category 2

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Marijn,

On Tue, Oct 19, 2021 at 2:17 AM Marijn Suijten
<marijn.suijten@somainline.org> wrote:
>
> The AVRCP spec (1.6.2) does not mention anything about a version
> requirement for Absolute Volume, despite this feature only existing
> since spec version 1.4.  Android reports a version of 1.3 [1] for its
> "AVRCP remote" (CT) service and mentions in the comment above it itself
> relies on feature bits rather than the exposed version.  As it stands
> BlueZ requires at least version 1.4 making it unable to communicate
> absolute volume levels with even the most recent Android phones running
> Fluoride (have not checked the version on Gabeldorsche).
>
> The spec states that supporting SetAbsoluteVolume and
> EVENT_VOLUME_CHANGED are mandatory when feature level 2 is declared,
> excluded otherwise.  This feature bit is set on Android and, when used
> by this patch, allows for successfully communicating volume back and
> forth despite the version theoretically being too low.
>
> [1]: https://android.googlesource.com/platform/system/bt/+/android-11.0.0_r28/bta/av/bta_av_main.cc#761

But I don't think this is valid, if EVENT_VOLUME_CHANGED was
introduced in 1.4 it should have been a reserved value in 1.3 so it
cannot be used. So Android should really report 1.4 on its controller
record, have you tried submitting a patch to Android to change it to
report the version as 1.4? Anyway I'd only adopt such a change if we
could identify the remote end is Android otherwise we risk IOP issues
by reporting a value that is considered reserved with other stacks.

> ---
>
> Changes since v1:
> - Use block comment intead of single-line comment.
>
>  profiles/audio/avrcp.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index d3c9cb795..e530eeab4 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -4175,13 +4175,17 @@ static void target_init(struct avrcp *session)
>                                 (1 << AVRCP_EVENT_TRACK_REACHED_END) |
>                                 (1 << AVRCP_EVENT_SETTINGS_CHANGED);
>
> +       /* Remote device supports receiving volume notifications */
> +       if (target->features & AVRCP_FEATURE_CATEGORY_2)
> +               session->supported_events |=
> +                               (1 << AVRCP_EVENT_VOLUME_CHANGED);
> +
>         if (target->version < 0x0104)
>                 return;
>
>         session->supported_events |=
>                                 (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
> -                               (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED) |
> -                               (1 << AVRCP_EVENT_VOLUME_CHANGED);
> +                               (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
>
>         /* Only check capabilities if controller is not supported */
>         if (session->controller == NULL)
> --
> 2.33.1
>


-- 
Luiz Augusto von Dentz