[PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

By the time the profile is registered it is not really possible to
tell which role of AVRCP should be connected, currently this cause
a problem with headsets that normally are controllers/sink but since
it normally also has target record for features related to things like
volume control the target profile is also probed and as it currently
has the auto_connect set it would lead to the wrong profile to start
connecting.
---
 profiles/audio/manager.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index 15226e4..1518e5d 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -376,7 +376,6 @@ static struct btd_profile avrcp_target_profile = {
 	.device_probe	= avrcp_probe,
 	.device_remove	= audio_remove,
 
-	.auto_connect	= true,
 	.connect	= avrcp_target_connect,
 	.disconnect	= avrcp_target_disconnect,
 
-- 
1.8.1.4

[PATCH BlueZ 2/3] audio: Make use of .device_probe in all AVRCP profile drivers

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

We should not rely on the order of the profile driver registration nor
recheck if the UUID match every time.
---
 profiles/audio/control.c | 36 ++++++++++++++++++++++++++++++++----
 profiles/audio/control.h |  4 +++-
 profiles/audio/manager.c | 27 ++++++++++++++++++++-------
 3 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/profiles/audio/control.c b/profiles/audio/control.c
index cdba385..f520e49 100644
--- a/profiles/audio/control.c
+++ b/profiles/audio/control.c
@@ -294,11 +294,13 @@ void control_update(struct control *control, struct btd_service *service)
 		control->remote = btd_service_ref(service);
 }
 
-struct control *control_init(struct audio_device *dev,
-						struct btd_service *service)
+static struct control *control_init(struct audio_device *dev)
 {
 	struct control *control;
 
+	if (dev->control != NULL)
+		return dev->control;
+
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
 					device_get_path(dev->btd_dev),
 					AUDIO_CONTROL_INTERFACE,
@@ -312,13 +314,39 @@ struct control *control_init(struct audio_device *dev,
 
 	control = g_new0(struct control, 1);
 
-	control_update(control, service);
-
 	control->avctp_id = avctp_add_state_cb(dev, state_changed);
 
 	return control;
 }
 
+struct control *control_init_target(struct audio_device *dev,
+						struct btd_service *service)
+{
+	struct control *control;
+
+	control = control_init(dev);
+	if (control == NULL)
+		return NULL;
+
+	control->target = btd_service_ref(service);
+
+	return control;
+}
+
+struct control *control_init_remote(struct audio_device *dev,
+						struct btd_service *service)
+{
+	struct control *control;
+
+	control = control_init(dev);
+	if (control == NULL)
+		return NULL;
+
+	control->remote = btd_service_ref(service);
+
+	return control;
+}
+
 gboolean control_is_active(struct audio_device *dev)
 {
 	struct control *control = dev->control;
diff --git a/profiles/audio/control.h b/profiles/audio/control.h
index 0a0f208..369cca6 100644
--- a/profiles/audio/control.h
+++ b/profiles/audio/control.h
@@ -26,7 +26,9 @@
 
 struct btd_service;
 
-struct control *control_init(struct audio_device *dev,
+struct control *control_init_target(struct audio_device *dev,
+						struct btd_service *service);
+struct control *control_init_remote(struct audio_device *dev,
 						struct btd_service *service);
 void control_update(struct control *control, struct btd_service *service);
 void control_unregister(struct audio_device *dev);
diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index 1518e5d..7f02fbd 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -135,7 +135,7 @@ static int a2dp_sink_probe(struct btd_service *service)
 	return 0;
 }
 
-static int avrcp_probe(struct btd_service *service)
+static int avrcp_target_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
 	struct audio_device *audio_dev;
@@ -146,10 +146,7 @@ static int avrcp_probe(struct btd_service *service)
 		return -1;
 	}
 
-	if (audio_dev->control)
-		control_update(audio_dev->control, service);
-	else
-		audio_dev->control = control_init(audio_dev, service);
+	audio_dev->control = control_init_target(audio_dev, service);
 
 	if (audio_dev->sink && sink_is_active(audio_dev))
 		avrcp_connect(audio_dev);
@@ -157,6 +154,22 @@ static int avrcp_probe(struct btd_service *service)
 	return 0;
 }
 
+static int avrcp_remote_probe(struct btd_service *service)
+{
+	struct btd_device *device = btd_service_get_device(service);
+	struct audio_device *audio_dev;
+
+	audio_dev = get_audio_dev(device);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	audio_dev->control = control_init_remote(audio_dev, service);
+
+	return 0;
+}
+
 static int a2dp_source_connect(struct btd_service *service)
 {
 	struct btd_device *dev = btd_service_get_device(service);
@@ -373,7 +386,7 @@ static struct btd_profile avrcp_target_profile = {
 	.name		= "audio-avrcp-target",
 
 	.remote_uuid	= AVRCP_TARGET_UUID,
-	.device_probe	= avrcp_probe,
+	.device_probe	= avrcp_target_probe,
 	.device_remove	= audio_remove,
 
 	.connect	= avrcp_target_connect,
@@ -387,7 +400,7 @@ static struct btd_profile avrcp_remote_profile = {
 	.name		= "audio-avrcp-control",
 
 	.remote_uuid	= AVRCP_REMOTE_UUID,
-	.device_probe	= avrcp_probe,
+	.device_probe	= avrcp_remote_probe,
 	.device_remove	= audio_remove,
 
 	.adapter_probe	= avrcp_remote_server_probe,
-- 
1.8.1.4

[PATCH BlueZ 3/3] audio/control: Remove control_update

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 profiles/audio/control.c | 12 ------------
 profiles/audio/control.h |  1 -
 2 files changed, 13 deletions(-)

diff --git a/profiles/audio/control.c b/profiles/audio/control.c
index f520e49..c33dcad 100644
--- a/profiles/audio/control.c
+++ b/profiles/audio/control.c
@@ -282,18 +282,6 @@ void control_unregister(struct audio_device *dev)
 						AUDIO_CONTROL_INTERFACE);
 }
 
-void control_update(struct control *control, struct btd_service *service)
-{
-	struct btd_profile *p = btd_service_get_profile(service);
-	const char *uuid = p->remote_uuid;
-
-	if (!control->target && bt_uuid_strcmp(uuid, AVRCP_TARGET_UUID) == 0)
-		control->target = btd_service_ref(service);
-	else if (!control->remote &&
-				bt_uuid_strcmp(uuid, AVRCP_REMOTE_UUID) == 0)
-		control->remote = btd_service_ref(service);
-}
-
 static struct control *control_init(struct audio_device *dev)
 {
 	struct control *control;
diff --git a/profiles/audio/control.h b/profiles/audio/control.h
index 369cca6..af6893b 100644
--- a/profiles/audio/control.h
+++ b/profiles/audio/control.h
@@ -30,7 +30,6 @@ struct control *control_init_target(struct audio_device *dev,
 						struct btd_service *service);
 struct control *control_init_remote(struct audio_device *dev,
 						struct btd_service *service);
-void control_update(struct control *control, struct btd_service *service);
 void control_unregister(struct audio_device *dev);
 gboolean control_is_active(struct audio_device *dev);
 
-- 
1.8.1.4

Re: [PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[Luiz Augusto von Dentz]

Hi,

On Thu, May 23, 2013 at 11:58 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> By the time the profile is registered it is not really possible to
> tell which role of AVRCP should be connected, currently this cause
> a problem with headsets that normally are controllers/sink but since
> it normally also has target record for features related to things like
> volume control the target profile is also probed and as it currently
> has the auto_connect set it would lead to the wrong profile to start
> connecting.
> ---
>  profiles/audio/manager.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
> index 15226e4..1518e5d 100644
> --- a/profiles/audio/manager.c
> +++ b/profiles/audio/manager.c
> @@ -376,7 +376,6 @@ static struct btd_profile avrcp_target_profile = {
>         .device_probe   = avrcp_probe,
>         .device_remove  = audio_remove,
>
> -       .auto_connect   = true,
>         .connect        = avrcp_target_connect,
>         .disconnect     = avrcp_target_disconnect,
>
> --
> 1.8.1.4

This set is now pushed.


--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[Mikel Astiz]

Hi Luis,

On Fri, May 24, 2013 at 2:21 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Thu, May 23, 2013 at 11:58 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> By the time the profile is registered it is not really possible to
>> tell which role of AVRCP should be connected, currently this cause
>> a problem with headsets that normally are controllers/sink but since
>> it normally also has target record for features related to things like
>> volume control the target profile is also probed and as it currently
>> has the auto_connect set it would lead to the wrong profile to start
>> connecting.
>> ---
>>  profiles/audio/manager.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
>> index 15226e4..1518e5d 100644
>> --- a/profiles/audio/manager.c
>> +++ b/profiles/audio/manager.c
>> @@ -376,7 +376,6 @@ static struct btd_profile avrcp_target_profile = {
>>         .device_probe   = avrcp_probe,
>>         .device_remove  = audio_remove,
>>
>> -       .auto_connect   = true,
>>         .connect        = avrcp_target_connect,
>>         .disconnect     = avrcp_target_disconnect,
>>
>> --
>> 1.8.1.4
>
> This set is now pushed.

What I don't quite get is how exactly BlueZ would ever connect AVRCP.
Let's say both ends are BlueZ 5. Would we trigger the connection once
A2DP gets connected? I'm not seeing where such a policy is currently
implemented.

Cheers,
Mikel

Re: [PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[Luiz Augusto von Dentz]

Hi Mikel,

On Thu, May 23, 2013 at 11:54 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> Hi Luis,
>
> On Fri, May 24, 2013 at 2:21 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi,
>>
>> On Thu, May 23, 2013 at 11:58 AM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>
>>> By the time the profile is registered it is not really possible to
>>> tell which role of AVRCP should be connected, currently this cause
>>> a problem with headsets that normally are controllers/sink but since
>>> it normally also has target record for features related to things like
>>> volume control the target profile is also probed and as it currently
>>> has the auto_connect set it would lead to the wrong profile to start
>>> connecting.
>>> ---
>>>  profiles/audio/manager.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
>>> index 15226e4..1518e5d 100644
>>> --- a/profiles/audio/manager.c
>>> +++ b/profiles/audio/manager.c
>>> @@ -376,7 +376,6 @@ static struct btd_profile avrcp_target_profile = {
>>>         .device_probe   = avrcp_probe,
>>>         .device_remove  = audio_remove,
>>>
>>> -       .auto_connect   = true,
>>>         .connect        = avrcp_target_connect,
>>>         .disconnect     = avrcp_target_disconnect,
>>>
>>> --
>>> 1.8.1.4
>>
>> This set is now pushed.
>
> What I don't quite get is how exactly BlueZ would ever connect AVRCP.
> Let's say both ends are BlueZ 5. Would we trigger the connection once
> A2DP gets connected? I'm not seeing where such a policy is currently
> implemented.

It is implemented in profiles/audio/device.c:device_avdtp_cb so it
should connect just fine in BlueZ against BlueZ scenario, it might
actually connect both sink and source.


--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/3] audio: Remove auto_connect flag from audio-avrcp-target

[Lucas De Marchi]

On Fri, May 24, 2013 at 11:29 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Mikel,
>
> On Thu, May 23, 2013 at 11:54 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
>> Hi Luis,
>>
>> On Fri, May 24, 2013 at 2:21 AM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi,
>>>
>>> On Thu, May 23, 2013 at 11:58 AM, Luiz Augusto von Dentz
>>> <luiz.dentz@gmail.com> wrote:
>>>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>>>
>>>> By the time the profile is registered it is not really possible to
>>>> tell which role of AVRCP should be connected, currently this cause
>>>> a problem with headsets that normally are controllers/sink but since
>>>> it normally also has target record for features related to things like
>>>> volume control the target profile is also probed and as it currently
>>>> has the auto_connect set it would lead to the wrong profile to start
>>>> connecting.
>>>> ---
>>>>  profiles/audio/manager.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
>>>> index 15226e4..1518e5d 100644
>>>> --- a/profiles/audio/manager.c
>>>> +++ b/profiles/audio/manager.c
>>>> @@ -376,7 +376,6 @@ static struct btd_profile avrcp_target_profile = {
>>>>         .device_probe   = avrcp_probe,
>>>>         .device_remove  = audio_remove,
>>>>
>>>> -       .auto_connect   = true,
>>>>         .connect        = avrcp_target_connect,
>>>>         .disconnect     = avrcp_target_disconnect,
>>>>
>>>> --
>>>> 1.8.1.4
>>>
>>> This set is now pushed.
>>
>> What I don't quite get is how exactly BlueZ would ever connect AVRCP.
>> Let's say both ends are BlueZ 5. Would we trigger the connection once
>> A2DP gets connected? I'm not seeing where such a policy is currently
>> implemented.
>
> It is implemented in profiles/audio/device.c:device_avdtp_cb so it
> should connect just fine in BlueZ against BlueZ scenario, it might
> actually connect both sink and source.

I think you meant TG and CT, right?

Lucas De Marchi