| Message ID | 20230311003826.454858-3-marijn.suijten@somainline.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | audio/avrcp: Determine Absolute Volume support from feature category 2 | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | WARNING:LONG_LINE_COMMENT: line length of 85 exceeds 80 columns #74: FILE: profiles/audio/avrcp.c:1761: + * The controller on the remote end is only allowed to call SetAbsoluteVolume WARNING:LONG_LINE: line length of 82 exceeds 80 columns #78: FILE: profiles/audio/avrcp.c:1765: + !(session->target->features & AVRCP_FEATURE_CATEGORY_2)) { WARNING:LONG_LINE_STRING: line length of 84 exceeds 80 columns #79: FILE: profiles/audio/avrcp.c:1766: + error("Remote SetAbsoluteVolume rejected from non-category-2 peer"); WARNING:LONG_LINE_COMMENT: line length of 88 exceeds 80 columns #91: FILE: profiles/audio/avrcp.c:3742: + * The target on the remote end is only allowed to reply to EVENT_VOLUME_CHANGED WARNING:LONG_LINE_COMMENT: line length of 82 exceeds 80 columns #92: FILE: profiles/audio/avrcp.c:3743: + * on our controller if it's at least version 1.4 and a category-2 device. WARNING:LONG_LINE: line length of 86 exceeds 80 columns #95: FILE: profiles/audio/avrcp.c:3746: + !(session->controller->features & AVRCP_FEATURE_CATEGORY_2)) { WARNING:LONG_LINE_STRING: line length of 87 exceeds 80 columns #96: FILE: profiles/audio/avrcp.c:3747: + error("Remote EVENT_VOLUME_CHANGED rejected from non-category-2 peer"); WARNING:LONG_LINE: line length of 90 exceeds 80 columns #134: FILE: profiles/audio/avrcp.c:4599: + !(session->target->features & AVRCP_FEATURE_CATEGORY_2)) { WARNING:LONG_LINE_STRING: line length of 88 exceeds 80 columns #135: FILE: profiles/audio/avrcp.c:4600: + error("Can't send EVENT_VOLUME_CHANGED to non-category-2 peer"); WARNING:LONG_LINE: line length of 84 exceeds 80 columns #146: FILE: profiles/audio/avrcp.c:4612: + if (!session->controller || session->controller->version < 0x0104 || WARNING:LONG_LINE: line length of 94 exceeds 80 columns #147: FILE: profiles/audio/avrcp.c:4613: + !(session->controller->features & AVRCP_FEATURE_CATEGORY_2)) { WARNING:LONG_LINE_STRING: line length of 85 exceeds 80 columns #148: FILE: profiles/audio/avrcp.c:4614: + error("Can't send SetAbsoluteVolume to non-category-2 peer"); /github/workspace/src/src/13170562.patch total: 0 errors, 12 warnings, 79 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13170562.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (85>80): "[BlueZ,v3,2/3] audio/avrcp: Only allow absolute volume call/event on category-2 peers" |
Hi Marijn, On Fri, Mar 10, 2023 at 4:39 PM Marijn Suijten <marijn.suijten@somainline.org> 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 >
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));