Hi Archie,
On Tue, Aug 4, 2020 at 1:26 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
>
> Hi Bluez maintainers,
>
> Could you take a look at this fix?
> Thank you!
Has there been any new version? It looks like CI has caught some
problems with it or that has been resolved by V2?
> Regards,
> Archie
>
>
> On Wed, 8 Jul 2020 at 12:19, Howard Chung <howardchung@xxxxxxxxxx> wrote:
> >
> > According to AVRCP 1.6.2 spec section 6.7.2 table 6.39, all player
> > application settings should be returned to the CT and let CT to
> > determine which settings have changed. Currently bluez only returns
> > the changed attribute instead. This patch also addresses a potential
> > issue on which the number of application settings mismatches with
> > the actual number returned.
> >
> > Reviewed-by: Archie Pusaka <apusaka@xxxxxxxxxxxx>
> > ---
> >
> > Changes in v2:
> > - Fixed unused variables
> >
> > profiles/audio/avrcp.c | 71 +++++++++++++++++++-----------------------
> > 1 file changed, 32 insertions(+), 39 deletions(-)
> >
> > diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> > index e2428250e..a4de7530e 100644
> > --- a/profiles/audio/avrcp.c
> > +++ b/profiles/audio/avrcp.c
> > @@ -369,6 +369,7 @@ static uint32_t company_ids[] = {
> > };
> >
> > static void avrcp_register_notification(struct avrcp *session, uint8_t event);
> > +static GList *player_list_settings(struct avrcp_player *player);
> >
> > static sdp_record_t *avrcp_ct_record(void)
> > {
> > @@ -743,6 +744,35 @@ static int play_status_to_val(const char *status)
> > return -EINVAL;
> > }
> >
> > +static uint16_t player_settings_changed(struct avrcp_player *player,
> > + struct avrcp_header *pdu)
> > +{
> > + GList *settings = player_list_settings(player);
> > + int size = 2;
> > +
> > + for (; settings; settings = settings->next) {
> > + const char *key = settings->data;
> > + int attr;
> > + int val;
> > +
> > + attr = attr_to_val(key);
> > + if (attr < 0)
> > + continue;
> > +
> > + val = player_get_setting(player, attr);
> > + if (val < 0)
> > + continue;
> > +
> > + pdu->params[size++] = attr;
> > + pdu->params[size++] = val;
> > + }
> > +
> > + g_list_free(settings);
> > +
> > + pdu->params[1] = (size - 2) >> 1;
> > + return size;
> > +}
> > +
> > void avrcp_player_event(struct avrcp_player *player, uint8_t id,
> > const void *data)
> > {
> > @@ -751,8 +781,6 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
> > uint8_t code;
> > uint16_t size;
> > GSList *l;
> > - int attr;
> > - int val;
> >
> > if (player->sessions == NULL)
> > return;
> > @@ -791,19 +819,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
> > size = 1;
> > break;
> > case AVRCP_EVENT_SETTINGS_CHANGED:
> > - size = 2;
> > - pdu->params[1] = 1;
> > -
> > - attr = attr_to_val(data);
> > - if (attr < 0)
> > - return;
> > -
> > - val = player_get_setting(player, attr);
> > - if (val < 0)
> > - return;
> > -
> > - pdu->params[size++] = attr;
> > - pdu->params[size++] = val;
> > + size = player_settings_changed(player, pdu);
> > break;
> > case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> > size = 5;
> > @@ -1595,7 +1611,6 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
> > struct btd_device *dev = session->dev;
> > uint16_t len = ntohs(pdu->params_len);
> > uint64_t uid;
> > - GList *settings;
> >
> > /*
> > * 1 byte for EventID, 4 bytes for Playback interval but the latest
> > @@ -1626,29 +1641,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
> > len = 1;
> > break;
> > case AVRCP_EVENT_SETTINGS_CHANGED:
> > - len = 1;
> > - settings = player_list_settings(player);
> > -
> > - pdu->params[len++] = g_list_length(settings);
> > - for (; settings; settings = settings->next) {
> > - const char *key = settings->data;
> > - int attr;
> > - int val;
> > -
> > - attr = attr_to_val(key);
> > - if (attr < 0)
> > - continue;
> > -
> > - val = player_get_setting(player, attr);
> > - if (val < 0)
> > - continue;
> > -
> > - pdu->params[len++] = attr;
> > - pdu->params[len++] = val;
> > - }
> > -
> > - g_list_free(settings);
> > -
> > + len = player_settings_changed(player, pdu);
> > break;
> > case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> > len = 5;
> > --
> > 2.27.0.383.g050319c2ae-goog
> >
--
Luiz Augusto von Dentz
