All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player
@ 2021-05-20  4:11 Huang-Huang Bao
  2021-05-20  4:53 ` [BlueZ,v2] " bluez.test.bot
  2021-05-20  5:44 ` [PATCH BlueZ v2] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 4+ messages in thread
From: Huang-Huang Bao @ 2021-05-20  4:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Huang-Huang Bao

v2: fix commit message & code styles

'notify_addressed_player_changed()' expected to be called with
'player->changed_id' set to what 'g_idle_add()' returns.

    player->changed_id = g_idle_add(notify_addressed_player_changed,
                                    player);

And 'avrcp_player_event()' relies on 'player->changed_id' to perform
Addressed Player Changed notification. However,
'avrcp_unregister_player()' calls 'notify_addressed_player_changed()'
without adding it to the main loop and set 'player->changed_id'. To
make 'notify_addressed_player_changed()' can be called without set
'player->changed_id' flag. We add antoher flag
'player->addressed_changing' to indicate addressed player changing.

Fixes https://github.com/bluez/bluez/issues/142
---
 profiles/audio/avrcp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 58d30b24d..5058a6848 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -239,6 +239,7 @@ struct avrcp_player {
 	uint8_t *features;
 	char *path;
 	guint changed_id;
+	bool addressed_changing;

 	struct pending_list_items *p;
 	char *change_path;
@@ -792,7 +793,8 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,

 	DBG("id=%u", id);

-	if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->changed_id) {
+	if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED &&
+			player->addressed_changing) {
 		code = AVC_CTYPE_REJECTED;
 		size = 1;
 		pdu->params[0] = AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
@@ -1794,6 +1796,8 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
 				};
 	uint8_t i;

+	player->addressed_changing = true;
+
 	avrcp_player_event(player, AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);

 	/*
@@ -1804,6 +1808,7 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
 	for (i = 0; i < sizeof(events); i++)
 		avrcp_player_event(player, events[i], NULL);

+	player->addressed_changing = false;
 	player->changed_id = 0;

 	return FALSE;
--
2.31.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [BlueZ,v2] avrcp: Fix unregister AVRCP player
  2021-05-20  4:11 [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player Huang-Huang Bao
@ 2021-05-20  4:53 ` bluez.test.bot
  2021-05-20  5:44 ` [PATCH BlueZ v2] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2021-05-20  4:53 UTC (permalink / raw)
  To: linux-bluetooth, eh5

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=485461

---Test result---

Test Summary:
CheckPatch                    PASS      0.33 seconds
GitLint                       PASS      0.12 seconds
Prep - Setup ELL              PASS      46.41 seconds
Build - Prep                  PASS      0.11 seconds
Build - Configure             PASS      8.40 seconds
Build - Make                  PASS      206.91 seconds
Make Check                    PASS      9.50 seconds
Make Distcheck                PASS      220.80 seconds
Build w/ext ELL - Configure   PASS      7.70 seconds
Build w/ext ELL - Make        PASS      172.24 seconds

Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf

##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint

##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player
  2021-05-20  4:11 [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player Huang-Huang Bao
  2021-05-20  4:53 ` [BlueZ,v2] " bluez.test.bot
@ 2021-05-20  5:44 ` Luiz Augusto von Dentz
  2021-05-20  8:45   ` Huang-Huang Bao
  1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2021-05-20  5:44 UTC (permalink / raw)
  To: Huang-Huang Bao; +Cc: linux-bluetooth

Hi Huang-Huang,

On Wed, May 19, 2021 at 9:14 PM Huang-Huang Bao <eh5@sokka.cn> wrote:
>
> v2: fix commit message & code styles
>
> 'notify_addressed_player_changed()' expected to be called with
> 'player->changed_id' set to what 'g_idle_add()' returns.
>
>     player->changed_id = g_idle_add(notify_addressed_player_changed,
>                                     player);
>
> And 'avrcp_player_event()' relies on 'player->changed_id' to perform
> Addressed Player Changed notification. However,
> 'avrcp_unregister_player()' calls 'notify_addressed_player_changed()'
> without adding it to the main loop and set 'player->changed_id'. To
> make 'notify_addressed_player_changed()' can be called without set
> 'player->changed_id' flag. We add antoher flag
> 'player->addressed_changing' to indicate addressed player changing.
>
> Fixes https://github.com/bluez/bluez/issues/142
> ---
>  profiles/audio/avrcp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 58d30b24d..5058a6848 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -239,6 +239,7 @@ struct avrcp_player {
>         uint8_t *features;
>         char *path;
>         guint changed_id;
> +       bool addressed_changing;
>
>         struct pending_list_items *p;
>         char *change_path;
> @@ -792,7 +793,8 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>
>         DBG("id=%u", id);
>
> -       if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->changed_id) {
> +       if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED &&
> +                       player->addressed_changing) {
>                 code = AVC_CTYPE_REJECTED;
>                 size = 1;
>                 pdu->params[0] = AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
> @@ -1794,6 +1796,8 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
>                                 };
>         uint8_t i;
>
> +       player->addressed_changing = true;

Well we could just set the change_id manually instead since it is
anyway set to 0 in the code below which is equivalent to what you are
doing here.

> +
>         avrcp_player_event(player, AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
>
>         /*
> @@ -1804,6 +1808,7 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
>         for (i = 0; i < sizeof(events); i++)
>                 avrcp_player_event(player, events[i], NULL);
>
> +       player->addressed_changing = false;
>         player->changed_id = 0;
>
>         return FALSE;
> --
> 2.31.1



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player
  2021-05-20  5:44 ` [PATCH BlueZ v2] " Luiz Augusto von Dentz
@ 2021-05-20  8:45   ` Huang-Huang Bao
  0 siblings, 0 replies; 4+ messages in thread
From: Huang-Huang Bao @ 2021-05-20  8:45 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth



On 5/20/21 1:44 PM, Luiz Augusto von Dentz wrote:
> Hi Huang-Huang,
> 
> On Wed, May 19, 2021 at 9:14 PM Huang-Huang Bao <eh5@sokka.cn> wrote:
>>
>> v2: fix commit message & code styles
>>
>> 'notify_addressed_player_changed()' expected to be called with
>> 'player->changed_id' set to what 'g_idle_add()' returns.
>>
>>      player->changed_id = g_idle_add(notify_addressed_player_changed,
>>                                      player);
>>
>> And 'avrcp_player_event()' relies on 'player->changed_id' to perform
>> Addressed Player Changed notification. However,
>> 'avrcp_unregister_player()' calls 'notify_addressed_player_changed()'
>> without adding it to the main loop and set 'player->changed_id'. To
>> make 'notify_addressed_player_changed()' can be called without set
>> 'player->changed_id' flag. We add antoher flag
>> 'player->addressed_changing' to indicate addressed player changing.
>>
>> Fixes https://github.com/bluez/bluez/issues/142
>> ---
>>   profiles/audio/avrcp.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
>> index 58d30b24d..5058a6848 100644
>> --- a/profiles/audio/avrcp.c
>> +++ b/profiles/audio/avrcp.c
>> @@ -239,6 +239,7 @@ struct avrcp_player {
>>          uint8_t *features;
>>          char *path;
>>          guint changed_id;
>> +       bool addressed_changing;
>>
>>          struct pending_list_items *p;
>>          char *change_path;
>> @@ -792,7 +793,8 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>>
>>          DBG("id=%u", id);
>>
>> -       if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED && player->changed_id) {
>> +       if (id != AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED &&
>> +                       player->addressed_changing) {
>>                  code = AVC_CTYPE_REJECTED;
>>                  size = 1;
>>                  pdu->params[0] = AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
>> @@ -1794,6 +1796,8 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
>>                                  };
>>          uint8_t i;
>>
>> +       player->addressed_changing = true;
> 
> Well we could just set the change_id manually instead since it is
> anyway set to 0 in the code below which is equivalent to what you are
> doing here.

Yeah, make since. I will submit a v3 soon.

Regards,
Bao

> 
>> +
>>          avrcp_player_event(player, AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
>>
>>          /*
>> @@ -1804,6 +1808,7 @@ static gboolean notify_addressed_player_changed(gpointer user_data)
>>          for (i = 0; i < sizeof(events); i++)
>>                  avrcp_player_event(player, events[i], NULL);
>>
>> +       player->addressed_changing = false;
>>          player->changed_id = 0;
>>
>>          return FALSE;
>> --
>> 2.31.1
> 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-05-20  8:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20  4:11 [PATCH BlueZ v2] avrcp: Fix unregister AVRCP player Huang-Huang Bao
2021-05-20  4:53 ` [BlueZ,v2] " bluez.test.bot
2021-05-20  5:44 ` [PATCH BlueZ v2] " Luiz Augusto von Dentz
2021-05-20  8:45   ` Huang-Huang Bao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.