Hi Marijn,
On Fri, Mar 10, 2023 at 4:39 PM Marijn Suijten
<marijn.suijten@xxxxxxxxxxxxxx> wrote:
>
> Restrict the use of SetAbsoluteVolume and EVENT_VOLUME_CHANGED to peers
> with at least AVRCP version 1.4 and AVRCP_FEATURE_CATEGORY_2 on their
> respective target or controller profiles.
Hmm, couldn't this actually make things even worse since we now are
checking the category as well? I know this is by the spec but as we
already are experiencing some stacks tend to deviate a lot from the
spec, perhaps it would have been better to introduce another config
option e.g. VolumeCategory = true so people can roll back to the old
behavior if their devices don't work well with this changes.
> ---
> profiles/audio/avrcp.c | 39 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 5e6322916..c16f9cfef 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -1757,6 +1757,16 @@ static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
> if (len != 1)
> goto err;
>
> + /**
> + * The controller on the remote end is only allowed to call SetAbsoluteVolume
> + * on our target if it's at least version 1.4 and a category-2 device.
> + */
> + if (!session->target || session->target->version < 0x0104 ||
> + !(session->target->features & AVRCP_FEATURE_CATEGORY_2)) {
> + error("Remote SetAbsoluteVolume rejected from non-category-2 peer");
> + goto err;
> + }
> +
> volume = pdu->params[0] & 0x7F;
>
> media_transport_update_device_volume(session->dev, volume);
> @@ -3728,6 +3738,16 @@ static void avrcp_volume_changed(struct avrcp *session,
> struct avrcp_player *player = target_get_player(session);
> int8_t volume;
>
> + /**
> + * The target on the remote end is only allowed to reply to EVENT_VOLUME_CHANGED
> + * on our controller if it's at least version 1.4 and a category-2 device.
> + */
> + if (!session->controller || session->controller->version < 0x0104 ||
> + !(session->controller->features & AVRCP_FEATURE_CATEGORY_2)) {
> + error("Remote EVENT_VOLUME_CHANGED rejected from non-category-2 peer");
> + return;
> + }
> +
> volume = pdu->params[1] & 0x7F;
>
> /* Always attempt to update the transport volume */
> @@ -3981,7 +4001,7 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn, uint8_t code,
> case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> case AVRCP_EVENT_UIDS_CHANGED:
> case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
> - /* These events above requires a player */
> + /* These events above require a player */
> if (!session->controller ||
> !session->controller->player)
> break;
> @@ -4154,10 +4174,13 @@ static void target_init(struct avrcp *session)
> if (target->version < 0x0104)
> return;
>
> + if (target->features & AVRCP_FEATURE_CATEGORY_2)
> + session->supported_events |=
> + (1 << AVRCP_EVENT_VOLUME_CHANGED);
> +
> 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)
> @@ -4572,8 +4595,11 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
> return -ENOTCONN;
>
> if (notify) {
> - if (!session->target)
> + if (!session->target || session->target->version < 0x0104 ||
> + !(session->target->features & AVRCP_FEATURE_CATEGORY_2)) {
> + error("Can't send EVENT_VOLUME_CHANGED to non-category-2 peer");
> return -ENOTSUP;
> + }
> return avrcp_event(session, AVRCP_EVENT_VOLUME_CHANGED,
> &volume);
> }
> @@ -4583,8 +4609,11 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
> AVRCP_EVENT_VOLUME_CHANGED))
> return -ENOTSUP;
> } else {
> - if (!session->controller || session->controller->version < 0x0104)
> + if (!session->controller || session->controller->version < 0x0104 ||
> + !(session->controller->features & AVRCP_FEATURE_CATEGORY_2)) {
> + error("Can't send SetAbsoluteVolume to non-category-2 peer");
> return -ENOTSUP;
> + }
> }
>
> memset(buf, 0, sizeof(buf));
> --
> 2.39.2
>
--
Luiz Augusto von Dentz
