[PATCH BlueZ] Increase timeout before initiating AVDTP connection

[PATCH BlueZ] Increase timeout before initiating AVDTP connection

[Peter Hurley]

QVZEVFBfQ09OTkVDVF9USU1FT1VUIGNvbnRyb2xzIHRoZSBkZWxheSBiZXR3ZWVuIEhTUC9IRlAN
CmNvbm5lY3Rpb24gZXN0YWJsaXNobWVudCBhbmQgQVZEVFAgc2lnbmFsIGNoYW5uZWwgZXN0YWJs
aXNobWVudC4NClRoZSBvcmlnaW5hbCB2YWx1ZSBvZiAxIHNlYy4gaXMgdG9vIHNob3J0IHRvIGF2
b2lkIHJhY2luZyBmb3IgQVZEVFANCmNvbm5lY3Rpb24gZXN0YWJsaXNobWVudCAoZWcuLCBpZiB0
aGUgZGV2aWNlIGNvbnRpbnVlcyB0byBjb25maWd1cmUNCnRoZSBIRlAgc2VydmljZSBsZXZlbCBj
b25uZWN0aW9uIHdpdGggYWRkJ2wgQVQgY21kcykuDQoNClNvbWUgZGV2aWNlcyBoYXZlIGJyb2tl
biBBVkRUUCBpbXBsZW1lbnRhdGlvbnMgdGhhdCBqdXN0IGNhbm5vdA0KaGFuZGxlIHRoZSByYWNl
IGNvbmRpdGlvbnMgdGhhdCBhcmlzZSBpZiBib3RoIGRldmljZXMgYXJlIGF0dGVtcHRpbmcNCnN0
cmVhbSBlc3RhYmxpc2htZW50IGF0IHRoZSBzYW1lIHRpbWUuIEhvd2V2ZXIsIHRoZXNlIGNvbmRp
dGlvbnMgYXJpc2UNCm9ubHkgd2hlbiB0aGUgcmVtb3RlIGRldmljZSBpcyB0aGUgQUNMIGluaXRp
YXRvciAoYW5kIGluIHByYWN0aWNlLCB0aGUNClJGQ09NTSBpbml0aWF0b3IgYXMgd2VsbCkuICBU
aGlzIGZpeCBidW1wcyBvdXQgdGhlIHRpbWVvdXQgdmFsdWUgb25seQ0Kd2hlbiB0aGUgaGVhZHNl
dCBoYXMgaW5pdGlhdGVkIHRoZSBsaW5rLg0KLS0tDQogYXVkaW8vZGV2aWNlLmMgIHwgICAgOSAr
KysrKysrKy0NCiBhdWRpby9oZWFkc2V0LmMgfCAgIDE2ICsrKysrKysrKysrKysrKysNCiBhdWRp
by9oZWFkc2V0LmggfCAgICAzICsrKw0KIGF1ZGlvL21hbmFnZXIuYyB8ICAgIDEgKw0KIDQgZmls
ZXMgY2hhbmdlZCwgMjggaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdp
dCBhL2F1ZGlvL2RldmljZS5jIGIvYXVkaW8vZGV2aWNlLmMNCmluZGV4IDE4ZTg3M2UuLjBiZDE0
NDMgMTAwNjQ0DQotLS0gYS9hdWRpby9kZXZpY2UuYw0KKysrIGIvYXVkaW8vZGV2aWNlLmMNCkBA
IC02MSw2ICs2MSw3IEBADQogDQogI2RlZmluZSBDT05UUk9MX0NPTk5FQ1RfVElNRU9VVCAyDQog
I2RlZmluZSBBVkRUUF9DT05ORUNUX1RJTUVPVVQgMQ0KKyNkZWZpbmUgQVZEVFBfQ09OTkVDVF9U
SU1FT1VUX0JPT1NUIDENCiAjZGVmaW5lIEhFQURTRVRfQ09OTkVDVF9USU1FT1VUIDENCiANCiB0
eXBlZGVmIGVudW0gew0KQEAgLTMwNSw2ICszMDYsNyBAQCBzdGF0aWMgZ2Jvb2xlYW4gYXZkdHBf
Y29ubmVjdF90aW1lb3V0KGdwb2ludGVyIHVzZXJfZGF0YSkNCiBzdGF0aWMgZ2Jvb2xlYW4gZGV2
aWNlX3NldF9hdmR0cF90aW1lcihzdHJ1Y3QgYXVkaW9fZGV2aWNlICpkZXYpDQogew0KIAlzdHJ1
Y3QgZGV2X3ByaXYgKnByaXYgPSBkZXYtPnByaXY7DQorCWd1aW50IHRpbWVvdXQgPSBBVkRUUF9D
T05ORUNUX1RJTUVPVVQ7DQogDQogCWlmICghZGV2LT5zaW5rKQ0KIAkJcmV0dXJuIEZBTFNFOw0K
QEAgLTMxMiw3ICszMTQsMTIgQEAgc3RhdGljIGdib29sZWFuIGRldmljZV9zZXRfYXZkdHBfdGlt
ZXIoc3RydWN0IGF1ZGlvX2RldmljZSAqZGV2KQ0KIAlpZiAocHJpdi0+YXZkdHBfdGltZXIpDQog
CQlyZXR1cm4gRkFMU0U7DQogDQotCXByaXYtPmF2ZHRwX3RpbWVyID0gZ190aW1lb3V0X2FkZF9z
ZWNvbmRzKEFWRFRQX0NPTk5FQ1RfVElNRU9VVCwNCisJLyogaWYgdGhlIGhlYWRzZXQgaXMgdGhl
IEhTUC9IRlAgUkZDT01NIGluaXRpYXRvciwgZ2l2ZSB0aGUgaGVhZHNldA0KKwkgICB0aW1lIHRv
IGluaXRpYXRlIEFWRFRQIHNpZ25hbGxpbmcgKGFuZCBhdm9pZCBmdXJ0aGVyIHJhY2luZykgKi8N
CisJaWYgKGRldi0+aGVhZHNldCAmJiBnZXRfcmZjb21tX2luaXRpYXRvcihkZXYpKQ0KKwkJdGlt
ZW91dCArPSBBVkRUUF9DT05ORUNUX1RJTUVPVVRfQk9PU1Q7DQorDQorCXByaXYtPmF2ZHRwX3Rp
bWVyID0gZ190aW1lb3V0X2FkZF9zZWNvbmRzKHRpbWVvdXQsDQogCQkJCQkJCWF2ZHRwX2Nvbm5l
Y3RfdGltZW91dCwNCiAJCQkJCQkJZGV2KTsNCiANCmRpZmYgLS1naXQgYS9hdWRpby9oZWFkc2V0
LmMgYi9hdWRpby9oZWFkc2V0LmMNCmluZGV4IDhlNjNhZmMuLjY2YTI4ZjIgMTAwNjQ0DQotLS0g
YS9hdWRpby9oZWFkc2V0LmMNCisrKyBiL2F1ZGlvL2hlYWRzZXQuYw0KQEAgLTE2Nyw2ICsxNjcs
NyBAQCBzdHJ1Y3QgaGVhZHNldCB7DQogDQogCWdib29sZWFuIGhmcF9hY3RpdmU7DQogCWdib29s
ZWFuIHNlYXJjaF9oZnA7DQorCWdib29sZWFuIHJmY29tbV9pbml0aWF0b3I7DQogDQogCWhlYWRz
ZXRfc3RhdGVfdCBzdGF0ZTsNCiAJc3RydWN0IHBlbmRpbmdfY29ubmVjdCAqcGVuZGluZzsNCkBA
IC0xNjMzLDYgKzE2MzQsNyBAQCBzdGF0aWMgaW50IHJmY29tbV9jb25uZWN0KHN0cnVjdCBhdWRp
b19kZXZpY2UgKmRldiwgaGVhZHNldF9zdHJlYW1fY2JfdCBjYiwNCiAJfQ0KIA0KIAlocy0+aGZw
X2FjdGl2ZSA9IGhzLT5oZnBfaGFuZGxlICE9IDAgPyBUUlVFIDogRkFMU0U7DQorCWhzLT5yZmNv
bW1faW5pdGlhdG9yID0gRkFMU0U7DQogDQogCWhlYWRzZXRfc2V0X3N0YXRlKGRldiwgSEVBRFNF
VF9TVEFURV9DT05ORUNUSU5HKTsNCiANCkBAIC0yNDQyLDYgKzI0NDQsMjAgQEAgdm9pZCBzZXRf
aGZwX2FjdGl2ZShzdHJ1Y3QgYXVkaW9fZGV2aWNlICpkZXYsIGdib29sZWFuIGFjdGl2ZSkNCiAJ
aHMtPmhmcF9hY3RpdmUgPSBhY3RpdmU7DQogfQ0KIA0KK2dib29sZWFuIGdldF9yZmNvbW1faW5p
dGlhdG9yKHN0cnVjdCBhdWRpb19kZXZpY2UgKmRldikNCit7DQorCXN0cnVjdCBoZWFkc2V0ICpo
cyA9IGRldi0+aGVhZHNldDsNCisNCisJcmV0dXJuIGhzLT5yZmNvbW1faW5pdGlhdG9yOw0KK30N
CisNCit2b2lkIHNldF9yZmNvbW1faW5pdGlhdG9yKHN0cnVjdCBhdWRpb19kZXZpY2UgKmRldiwg
Z2Jvb2xlYW4gaW5pdGlhdG9yKQ0KK3sNCisJc3RydWN0IGhlYWRzZXQgKmhzID0gZGV2LT5oZWFk
c2V0Ow0KKw0KKwlocy0+cmZjb21tX2luaXRpYXRvciA9IGluaXRpYXRvcjsNCit9DQorDQogR0lP
Q2hhbm5lbCAqaGVhZHNldF9nZXRfcmZjb21tKHN0cnVjdCBhdWRpb19kZXZpY2UgKmRldikNCiB7
DQogCXN0cnVjdCBoZWFkc2V0ICpocyA9IGRldi0+aGVhZHNldDsNCmRpZmYgLS1naXQgYS9hdWRp
by9oZWFkc2V0LmggYi9hdWRpby9oZWFkc2V0LmgNCmluZGV4IDdjZTg4YzguLmYyNzVlMDMgMTAw
NjQ0DQotLS0gYS9hdWRpby9oZWFkc2V0LmgNCisrKyBiL2F1ZGlvL2hlYWRzZXQuaA0KQEAgLTgy
LDYgKzgyLDkgQEAgZ2Jvb2xlYW4gaGVhZHNldF9jYW5jZWxfc3RyZWFtKHN0cnVjdCBhdWRpb19k
ZXZpY2UgKmRldiwgdW5zaWduZWQgaW50IGlkKTsNCiBnYm9vbGVhbiBnZXRfaGZwX2FjdGl2ZShz
dHJ1Y3QgYXVkaW9fZGV2aWNlICpkZXYpOw0KIHZvaWQgc2V0X2hmcF9hY3RpdmUoc3RydWN0IGF1
ZGlvX2RldmljZSAqZGV2LCBnYm9vbGVhbiBhY3RpdmUpOw0KIA0KK2dib29sZWFuIGdldF9yZmNv
bW1faW5pdGlhdG9yKHN0cnVjdCBhdWRpb19kZXZpY2UgKmRldik7DQordm9pZCBzZXRfcmZjb21t
X2luaXRpYXRvcihzdHJ1Y3QgYXVkaW9fZGV2aWNlICpkZXYsIGdib29sZWFuIGluaXRpYXRvcik7
DQorDQogdm9pZCBoZWFkc2V0X3NldF9hdXRob3JpemVkKHN0cnVjdCBhdWRpb19kZXZpY2UgKmRl
dik7DQogaW50IGhlYWRzZXRfY29ubmVjdF9yZmNvbW0oc3RydWN0IGF1ZGlvX2RldmljZSAqZGV2
LCBHSU9DaGFubmVsICpjaGFuKTsNCiBpbnQgaGVhZHNldF9jb25uZWN0X3NjbyhzdHJ1Y3QgYXVk
aW9fZGV2aWNlICpkZXYsIEdJT0NoYW5uZWwgKmlvKTsNCmRpZmYgLS1naXQgYS9hdWRpby9tYW5h
Z2VyLmMgYi9hdWRpby9tYW5hZ2VyLmMNCmluZGV4IDA1Y2E2NTQuLjQwMGExYWMgMTAwNjQ0DQot
LS0gYS9hdWRpby9tYW5hZ2VyLmMNCisrKyBiL2F1ZGlvL21hbmFnZXIuYw0KQEAgLTUwOCw2ICs1
MDgsNyBAQCBzdGF0aWMgdm9pZCBhZ19jb25maXJtKEdJT0NoYW5uZWwgKmNoYW4sIGdwb2ludGVy
IGRhdGEpDQogCX0NCiANCiAJc2V0X2hmcF9hY3RpdmUoZGV2aWNlLCBoZnBfYWN0aXZlKTsNCisJ
c2V0X3JmY29tbV9pbml0aWF0b3IoZGV2aWNlLCBUUlVFKTsNCiANCiAJaWYgKGhlYWRzZXRfY29u
bmVjdF9yZmNvbW0oZGV2aWNlLCBjaGFuKSA8IDApIHsNCiAJCWVycm9yKCJoZWFkc2V0X2Nvbm5l
Y3RfcmZjb21tIGZhaWxlZCIpOw0KLS0gDQoxLjcuNC4xDQoNCg==

Re: [PATCH BlueZ] Increase timeout before initiating AVDTP connection

[Luiz Augusto von Dentz]

Hi Peter,

On Fri, Aug 12, 2011 at 3:56 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> AVDTP_CONNECT_TIMEOUT controls the delay between HSP/HFP
> connection establishment and AVDTP signal channel establishment.
> The original value of 1 sec. is too short to avoid racing for AVDTP
> connection establishment (eg., if the device continues to configure
> the HFP service level connection with add'l AT cmds).
>
> Some devices have broken AVDTP implementations that just cannot
> handle the race conditions that arise if both devices are attempting
> stream establishment at the same time. However, these conditions arise
> only when the remote device is the ACL initiator (and in practice, the
> RFCOMM initiator as well).  This fix bumps out the timeout value only
> when the headset has initiated the link.
> ---
>  audio/device.c  |    9 ++++++++-
>  audio/headset.c |   16 ++++++++++++++++
>  audio/headset.h |    3 +++
>  audio/manager.c |    1 +
>  4 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/audio/device.c b/audio/device.c
> index 18e873e..0bd1443 100644
> --- a/audio/device.c
> +++ b/audio/device.c
> @@ -61,6 +61,7 @@
>
>  #define CONTROL_CONNECT_TIMEOUT 2
>  #define AVDTP_CONNECT_TIMEOUT 1
> +#define AVDTP_CONNECT_TIMEOUT_BOOST 1
>  #define HEADSET_CONNECT_TIMEOUT 1
>
>  typedef enum {
> @@ -305,6 +306,7 @@ static gboolean avdtp_connect_timeout(gpointer user_data)
>  static gboolean device_set_avdtp_timer(struct audio_device *dev)
>  {
>        struct dev_priv *priv = dev->priv;
> +       guint timeout = AVDTP_CONNECT_TIMEOUT;
>
>        if (!dev->sink)
>                return FALSE;
> @@ -312,7 +314,12 @@ static gboolean device_set_avdtp_timer(struct audio_device *dev)
>        if (priv->avdtp_timer)
>                return FALSE;
>
> -       priv->avdtp_timer = g_timeout_add_seconds(AVDTP_CONNECT_TIMEOUT,
> +       /* if the headset is the HSP/HFP RFCOMM initiator, give the headset
> +          time to initiate AVDTP signalling (and avoid further racing) */
> +       if (dev->headset && get_rfcomm_initiator(dev))
> +               timeout += AVDTP_CONNECT_TIMEOUT_BOOST;
> +
> +       priv->avdtp_timer = g_timeout_add_seconds(timeout,
>                                                        avdtp_connect_timeout,
>                                                        dev);
>
> diff --git a/audio/headset.c b/audio/headset.c
> index 8e63afc..66a28f2 100644
> --- a/audio/headset.c
> +++ b/audio/headset.c
> @@ -167,6 +167,7 @@ struct headset {
>
>        gboolean hfp_active;
>        gboolean search_hfp;
> +       gboolean rfcomm_initiator;
>
>        headset_state_t state;
>        struct pending_connect *pending;
> @@ -1633,6 +1634,7 @@ static int rfcomm_connect(struct audio_device *dev, headset_stream_cb_t cb,
>        }
>
>        hs->hfp_active = hs->hfp_handle != 0 ? TRUE : FALSE;
> +       hs->rfcomm_initiator = FALSE;
>
>        headset_set_state(dev, HEADSET_STATE_CONNECTING);
>
> @@ -2442,6 +2444,20 @@ void set_hfp_active(struct audio_device *dev, gboolean active)
>        hs->hfp_active = active;
>  }
>
> +gboolean get_rfcomm_initiator(struct audio_device *dev)
> +{
> +       struct headset *hs = dev->headset;
> +
> +       return hs->rfcomm_initiator;
> +}
> +
> +void set_rfcomm_initiator(struct audio_device *dev, gboolean initiator)
> +{
> +       struct headset *hs = dev->headset;
> +
> +       hs->rfcomm_initiator = initiator;
> +}
> +

I would suggest you to follow the name convention e.g.
headset_get_rfcomm_initiator...

>  GIOChannel *headset_get_rfcomm(struct audio_device *dev)
>  {
>        struct headset *hs = dev->headset;
> diff --git a/audio/headset.h b/audio/headset.h
> index 7ce88c8..f275e03 100644
> --- a/audio/headset.h
> +++ b/audio/headset.h
> @@ -82,6 +82,9 @@ gboolean headset_cancel_stream(struct audio_device *dev, unsigned int id);
>  gboolean get_hfp_active(struct audio_device *dev);
>  void set_hfp_active(struct audio_device *dev, gboolean active);
>
> +gboolean get_rfcomm_initiator(struct audio_device *dev);
> +void set_rfcomm_initiator(struct audio_device *dev, gboolean initiator);
> +

Here too.

-- 
Luiz Augusto von Dentz