* [PATCH BlueZ 1/2] a2dp: Store last used endpoint
@ 2019-04-24 8:39 Luiz Augusto von Dentz
2019-04-24 8:39 ` [PATCH BlueZ 2/2] doc/settings-storage: Document LastUsed Endpoints entry Luiz Augusto von Dentz
2019-04-24 9:42 ` [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
0 siblings, 2 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-24 8:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This stores the last used endpoint whenever it is considered open and
then prefer to use it when attempting to reconnect.
---
profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
1 file changed, 91 insertions(+), 13 deletions(-)
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 8f141739c..78b02dc84 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -147,6 +147,7 @@ struct a2dp_channel {
unsigned int auth_id;
struct avdtp *session;
struct queue *seps;
+ struct a2dp_remote_sep *last_used;
};
static GSList *servers = NULL;
@@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
return TRUE;
}
+static bool match_remote_sep(const void *data, const void *user_data)
+{
+ const struct a2dp_remote_sep *sep = data;
+ const struct avdtp_remote_sep *rsep = user_data;
+
+ return sep->sep == rsep;
+}
+
+static void store_last_used(struct a2dp_channel *chan,
+ struct avdtp_remote_sep *rsep)
+{
+ GKeyFile *key_file;
+ char filename[PATH_MAX];
+ char dst_addr[18];
+ char value[4];
+ char *data;
+ size_t len = 0;
+
+ ba2str(device_get_address(chan->device), dst_addr);
+
+ snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
+ btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
+ dst_addr);
+ key_file = g_key_file_new();
+ g_key_file_load_from_file(key_file, filename, 0, NULL);
+
+ sprintf(value, "%02hhx", avdtp_get_seid(rsep));
+
+ g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
+
+ data = g_key_file_to_data(key_file, &len, NULL);
+ g_file_set_contents(filename, data, len, NULL);
+
+ g_free(data);
+ g_key_file_free(key_file);
+}
+
+static void update_last_used(struct a2dp_channel *chan,
+ struct avdtp_stream *stream)
+{
+ struct avdtp_remote_sep *rsep;
+ struct a2dp_remote_sep *sep;
+
+ rsep = avdtp_stream_get_remote_sep(stream);
+
+ /* Update last used */
+ sep = queue_find(chan->seps, match_remote_sep, rsep);
+ if (chan->last_used == sep)
+ return;
+
+ chan->last_used = sep;
+ store_last_used(chan, rsep);
+}
+
static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
struct avdtp_stream *stream, struct avdtp_error *err,
void *user_data)
@@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
setup->err = err;
if (setup->start)
finalize_resume(setup);
- }
+ } else if (setup->chan)
+ update_last_used(setup->chan, stream);
finalize_config(setup);
@@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
return TRUE;
}
-static bool match_remote_sep(const void *data, const void *user_data)
-{
- const struct a2dp_remote_sep *sep = data;
- const struct avdtp_remote_sep *rsep = user_data;
-
- return sep->sep == rsep;
-}
-
static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
struct a2dp_sep *sep)
{
@@ -1791,19 +1839,28 @@ done:
queue_push_tail(chan->seps, sep);
}
+static bool match_seid(const void *data, const void *user_data)
+{
+ const struct a2dp_remote_sep *sep = data;
+ const uint8_t *seid = user_data;
+
+ return avdtp_get_seid(sep->sep) == *seid;
+}
+
static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
char **seids)
{
struct avdtp_remote_sep *sep;
+ uint8_t seid;
+ char *value;
if (!seids)
return;
for (; *seids; seids++) {
- uint8_t seid;
uint8_t type;
uint8_t codec;
- char *value, caps[256];
+ char caps[256];
uint8_t data[128];
int i, size;
GSList *l = NULL;
@@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
register_remote_sep(sep, chan);
}
+
+ value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
+ if (!value)
+ return;
+
+ if (sscanf(value, "%02hhx", &seid) != 1)
+ return;
+
+ chan->last_used = queue_find(chan->seps, match_seid, &seid);
}
static void load_remote_seps(struct a2dp_channel *chan)
@@ -2355,8 +2421,12 @@ done:
static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
const char *sender)
{
+ struct a2dp_sep *first = NULL;
+ struct a2dp_channel *chan = find_channel(session);
+
for (; list; list = list->next) {
struct a2dp_sep *sep = list->data;
+ struct avdtp_remote_sep *rsep;
/* Use sender's endpoint if available */
if (sender) {
@@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
continue;
}
- if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
+ rsep = avdtp_find_remote_sep(session, sep->lsep);
+ if (!rsep)
continue;
+ /* Locate last used if set */
+ if (chan->last_used) {
+ if (chan->last_used->sep == rsep)
+ return sep;
+ first = sep;
+ }
+
return sep;
}
- return NULL;
+ return first;
}
static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH BlueZ 2/2] doc/settings-storage: Document LastUsed Endpoints entry
2019-04-24 8:39 [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
@ 2019-04-24 8:39 ` Luiz Augusto von Dentz
2019-04-24 9:42 ` [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
1 sibling, 0 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-24 8:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Document the use of LastUsed entry inside Endpoints group.
---
doc/settings-storage.txt | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/doc/settings-storage.txt b/doc/settings-storage.txt
index 44b0b4961..f595f9817 100644
--- a/doc/settings-storage.txt
+++ b/doc/settings-storage.txt
@@ -169,7 +169,9 @@ In "Attributes" group GATT database is stored using attribute handle as key
all data required to re-create given attribute. ":" is used to separate fields.
In "Endpoints" group A2DP remote endpoints are stored using the seid as key
-(hexadecimal format) and ":" is used to separate fields.
+(hexadecimal format) and ":" is used to separate fields. It may also contain
+an entry which key is set to "LastUsed" which represented the last endpoint
+used.
[General] group contains:
@@ -220,6 +222,9 @@ Sample Attributes section:
followed by codec type and its
capabilies as hexadecimal encoded
string.
+ LastUsed:<xx> String LastUsed has one field which is the
+ endpoint ID as hexadecimal encoded
+ string.
Info file format
================
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 8:39 [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
2019-04-24 8:39 ` [PATCH BlueZ 2/2] doc/settings-storage: Document LastUsed Endpoints entry Luiz Augusto von Dentz
@ 2019-04-24 9:42 ` Luiz Augusto von Dentz
2019-04-24 16:57 ` Pali Rohár
2019-04-24 17:01 ` Pali Rohár
1 sibling, 2 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-24 9:42 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pali Rohár
Hi Pali,
On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This stores the last used endpoint whenever it is considered open and
> then prefer to use it when attempting to reconnect.
> ---
> profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 91 insertions(+), 13 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index 8f141739c..78b02dc84 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -147,6 +147,7 @@ struct a2dp_channel {
> unsigned int auth_id;
> struct avdtp *session;
> struct queue *seps;
> + struct a2dp_remote_sep *last_used;
> };
>
> static GSList *servers = NULL;
> @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> return TRUE;
> }
>
> +static bool match_remote_sep(const void *data, const void *user_data)
> +{
> + const struct a2dp_remote_sep *sep = data;
> + const struct avdtp_remote_sep *rsep = user_data;
> +
> + return sep->sep == rsep;
> +}
> +
> +static void store_last_used(struct a2dp_channel *chan,
> + struct avdtp_remote_sep *rsep)
> +{
> + GKeyFile *key_file;
> + char filename[PATH_MAX];
> + char dst_addr[18];
> + char value[4];
> + char *data;
> + size_t len = 0;
> +
> + ba2str(device_get_address(chan->device), dst_addr);
> +
> + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> + dst_addr);
> + key_file = g_key_file_new();
> + g_key_file_load_from_file(key_file, filename, 0, NULL);
> +
> + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> +
> + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> +
> + data = g_key_file_to_data(key_file, &len, NULL);
> + g_file_set_contents(filename, data, len, NULL);
> +
> + g_free(data);
> + g_key_file_free(key_file);
> +}
> +
> +static void update_last_used(struct a2dp_channel *chan,
> + struct avdtp_stream *stream)
> +{
> + struct avdtp_remote_sep *rsep;
> + struct a2dp_remote_sep *sep;
> +
> + rsep = avdtp_stream_get_remote_sep(stream);
> +
> + /* Update last used */
> + sep = queue_find(chan->seps, match_remote_sep, rsep);
> + if (chan->last_used == sep)
> + return;
> +
> + chan->last_used = sep;
> + store_last_used(chan, rsep);
> +}
> +
> static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> struct avdtp_stream *stream, struct avdtp_error *err,
> void *user_data)
> @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> setup->err = err;
> if (setup->start)
> finalize_resume(setup);
> - }
> + } else if (setup->chan)
> + update_last_used(setup->chan, stream);
>
> finalize_config(setup);
>
> @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> return TRUE;
> }
>
> -static bool match_remote_sep(const void *data, const void *user_data)
> -{
> - const struct a2dp_remote_sep *sep = data;
> - const struct avdtp_remote_sep *rsep = user_data;
> -
> - return sep->sep == rsep;
> -}
> -
> static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> struct a2dp_sep *sep)
> {
> @@ -1791,19 +1839,28 @@ done:
> queue_push_tail(chan->seps, sep);
> }
>
> +static bool match_seid(const void *data, const void *user_data)
> +{
> + const struct a2dp_remote_sep *sep = data;
> + const uint8_t *seid = user_data;
> +
> + return avdtp_get_seid(sep->sep) == *seid;
> +}
> +
> static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> char **seids)
> {
> struct avdtp_remote_sep *sep;
> + uint8_t seid;
> + char *value;
>
> if (!seids)
> return;
>
> for (; *seids; seids++) {
> - uint8_t seid;
> uint8_t type;
> uint8_t codec;
> - char *value, caps[256];
> + char caps[256];
> uint8_t data[128];
> int i, size;
> GSList *l = NULL;
> @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
>
> register_remote_sep(sep, chan);
> }
> +
> + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> + if (!value)
> + return;
> +
> + if (sscanf(value, "%02hhx", &seid) != 1)
> + return;
> +
> + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> }
>
> static void load_remote_seps(struct a2dp_channel *chan)
> @@ -2355,8 +2421,12 @@ done:
> static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> const char *sender)
> {
> + struct a2dp_sep *first = NULL;
> + struct a2dp_channel *chan = find_channel(session);
> +
> for (; list; list = list->next) {
> struct a2dp_sep *sep = list->data;
> + struct avdtp_remote_sep *rsep;
>
> /* Use sender's endpoint if available */
> if (sender) {
> @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> continue;
> }
>
> - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> + rsep = avdtp_find_remote_sep(session, sep->lsep);
> + if (!rsep)
> continue;
>
> + /* Locate last used if set */
> + if (chan->last_used) {
> + if (chan->last_used->sep == rsep)
> + return sep;
> + first = sep;
> + }
> +
> return sep;
>
> }
>
> - return NULL;
> + return first;
> }
>
> static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> --
> 2.20.1
Can you give this a try, it should make the daemon remember what was
the last endpoint used (locally the remote selection we cannot really
control).
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 9:42 ` [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
@ 2019-04-24 16:57 ` Pali Rohár
2019-04-25 8:00 ` Luiz Augusto von Dentz
` (3 more replies)
2019-04-24 17:01 ` Pali Rohár
1 sibling, 4 replies; 14+ messages in thread
From: Pali Rohár @ 2019-04-24 16:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 8193 bytes --]
On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This stores the last used endpoint whenever it is considered open and
> > then prefer to use it when attempting to reconnect.
> > ---
> > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 91 insertions(+), 13 deletions(-)
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index 8f141739c..78b02dc84 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > unsigned int auth_id;
> > struct avdtp *session;
> > struct queue *seps;
> > + struct a2dp_remote_sep *last_used;
> > };
> >
> > static GSList *servers = NULL;
> > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > return TRUE;
> > }
> >
> > +static bool match_remote_sep(const void *data, const void *user_data)
> > +{
> > + const struct a2dp_remote_sep *sep = data;
> > + const struct avdtp_remote_sep *rsep = user_data;
> > +
> > + return sep->sep == rsep;
> > +}
> > +
> > +static void store_last_used(struct a2dp_channel *chan,
> > + struct avdtp_remote_sep *rsep)
> > +{
> > + GKeyFile *key_file;
> > + char filename[PATH_MAX];
> > + char dst_addr[18];
> > + char value[4];
> > + char *data;
> > + size_t len = 0;
> > +
> > + ba2str(device_get_address(chan->device), dst_addr);
> > +
> > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > + dst_addr);
> > + key_file = g_key_file_new();
> > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > +
> > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > +
> > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > +
> > + data = g_key_file_to_data(key_file, &len, NULL);
> > + g_file_set_contents(filename, data, len, NULL);
> > +
> > + g_free(data);
> > + g_key_file_free(key_file);
> > +}
> > +
> > +static void update_last_used(struct a2dp_channel *chan,
> > + struct avdtp_stream *stream)
> > +{
> > + struct avdtp_remote_sep *rsep;
> > + struct a2dp_remote_sep *sep;
> > +
> > + rsep = avdtp_stream_get_remote_sep(stream);
> > +
> > + /* Update last used */
> > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > + if (chan->last_used == sep)
> > + return;
> > +
> > + chan->last_used = sep;
> > + store_last_used(chan, rsep);
> > +}
> > +
> > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > struct avdtp_stream *stream, struct avdtp_error *err,
> > void *user_data)
> > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > setup->err = err;
> > if (setup->start)
> > finalize_resume(setup);
> > - }
> > + } else if (setup->chan)
> > + update_last_used(setup->chan, stream);
> >
> > finalize_config(setup);
> >
> > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > return TRUE;
> > }
> >
> > -static bool match_remote_sep(const void *data, const void *user_data)
> > -{
> > - const struct a2dp_remote_sep *sep = data;
> > - const struct avdtp_remote_sep *rsep = user_data;
> > -
> > - return sep->sep == rsep;
> > -}
> > -
> > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > struct a2dp_sep *sep)
> > {
> > @@ -1791,19 +1839,28 @@ done:
> > queue_push_tail(chan->seps, sep);
> > }
> >
> > +static bool match_seid(const void *data, const void *user_data)
> > +{
> > + const struct a2dp_remote_sep *sep = data;
> > + const uint8_t *seid = user_data;
> > +
> > + return avdtp_get_seid(sep->sep) == *seid;
> > +}
> > +
> > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > char **seids)
> > {
> > struct avdtp_remote_sep *sep;
> > + uint8_t seid;
> > + char *value;
> >
> > if (!seids)
> > return;
> >
> > for (; *seids; seids++) {
> > - uint8_t seid;
> > uint8_t type;
> > uint8_t codec;
> > - char *value, caps[256];
> > + char caps[256];
> > uint8_t data[128];
> > int i, size;
> > GSList *l = NULL;
> > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> >
> > register_remote_sep(sep, chan);
> > }
> > +
> > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > + if (!value)
> > + return;
> > +
> > + if (sscanf(value, "%02hhx", &seid) != 1)
> > + return;
> > +
> > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > }
> >
> > static void load_remote_seps(struct a2dp_channel *chan)
> > @@ -2355,8 +2421,12 @@ done:
> > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > const char *sender)
> > {
> > + struct a2dp_sep *first = NULL;
> > + struct a2dp_channel *chan = find_channel(session);
> > +
> > for (; list; list = list->next) {
> > struct a2dp_sep *sep = list->data;
> > + struct avdtp_remote_sep *rsep;
> >
> > /* Use sender's endpoint if available */
> > if (sender) {
> > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > continue;
> > }
> >
> > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > + if (!rsep)
> > continue;
> >
> > + /* Locate last used if set */
> > + if (chan->last_used) {
> > + if (chan->last_used->sep == rsep)
> > + return sep;
> > + first = sep;
> > + }
> > +
> > return sep;
> >
> > }
> >
> > - return NULL;
> > + return first;
> > }
> >
> > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > --
> > 2.20.1
>
> Can you give this a try, it should make the daemon remember what was
> the last endpoint used (locally the remote selection we cannot really
> control).
Great, I will try it. Remote selection is done by remote device so this
really cannot be controlled.
I have there another suggestion for improvement. bluez support
registering more endpoints for one codec. I'm going to use it for
registering one SBC endpoint with fixed bitpool as UHQ profile and
another with lower bitpool values. To distinguish in pulseaudio between
low and high quality of SBC. Seems that bluez sometimes try to only
select only one endpoint, even there are more and do not try another if
first one selection fails (e.g. because remote capability of bitpool is
too low and our local selection needs higher). Could be bluez modified
to try another local endpoint if there are more registered until some
accept capabilities?
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 9:42 ` [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
2019-04-24 16:57 ` Pali Rohár
@ 2019-04-24 17:01 ` Pali Rohár
2019-04-24 18:26 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 14+ messages in thread
From: Pali Rohár @ 2019-04-24 17:01 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]
On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > @@ -2355,8 +2421,12 @@ done:
> > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > const char *sender)
> > {
> > + struct a2dp_sep *first = NULL;
> > + struct a2dp_channel *chan = find_channel(session);
> > +
> > for (; list; list = list->next) {
> > struct a2dp_sep *sep = list->data;
> > + struct avdtp_remote_sep *rsep;
> >
> > /* Use sender's endpoint if available */
> > if (sender) {
> > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > continue;
> > }
> >
> > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > + if (!rsep)
> > continue;
> >
> > + /* Locate last used if set */
> > + if (chan->last_used) {
> > + if (chan->last_used->sep == rsep)
> > + return sep;
> > + first = sep;
This assignment is useless as 2 lines below you are returning sep from
function. So assignment of sep into first does nothing.
> > + }
> > +
> > return sep;
> >
> > }
> >
> > - return NULL;
> > + return first;
> > }
> >
> > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 17:01 ` Pali Rohár
@ 2019-04-24 18:26 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-24 18:26 UTC (permalink / raw)
To: Pali Rohár; +Cc: linux-bluetooth
Hi Pali,
On Wed, Apr 24, 2019 at 8:01 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > > @@ -2355,8 +2421,12 @@ done:
> > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > const char *sender)
> > > {
> > > + struct a2dp_sep *first = NULL;
> > > + struct a2dp_channel *chan = find_channel(session);
> > > +
> > > for (; list; list = list->next) {
> > > struct a2dp_sep *sep = list->data;
> > > + struct avdtp_remote_sep *rsep;
> > >
> > > /* Use sender's endpoint if available */
> > > if (sender) {
> > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > continue;
> > > }
> > >
> > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > + if (!rsep)
> > > continue;
> > >
> > > + /* Locate last used if set */
> > > + if (chan->last_used) {
> > > + if (chan->last_used->sep == rsep)
> > > + return sep;
> > > + first = sep;
>
> This assignment is useless as 2 lines below you are returning sep from
> function. So assignment of sep into first does nothing.
Right, there was supposed to be a continue at this point.
> > > + }
> > > +
> > > return sep;
> > >
> > > }
> > >
> > > - return NULL;
> > > + return first;
> > > }
> > >
> > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
>
> --
> Pali Rohár
> pali.rohar@gmail.com
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 16:57 ` Pali Rohár
@ 2019-04-25 8:00 ` Luiz Augusto von Dentz
2019-04-28 18:53 ` Pali Rohár
2019-05-02 21:14 ` Pali Rohár
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-25 8:00 UTC (permalink / raw)
To: Pali Rohár; +Cc: linux-bluetooth
Hi Pali,
On Wed, Apr 24, 2019 at 7:57 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This stores the last used endpoint whenever it is considered open and
> > > then prefer to use it when attempting to reconnect.
> > > ---
> > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > index 8f141739c..78b02dc84 100644
> > > --- a/profiles/audio/a2dp.c
> > > +++ b/profiles/audio/a2dp.c
> > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > unsigned int auth_id;
> > > struct avdtp *session;
> > > struct queue *seps;
> > > + struct a2dp_remote_sep *last_used;
> > > };
> > >
> > > static GSList *servers = NULL;
> > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const struct avdtp_remote_sep *rsep = user_data;
> > > +
> > > + return sep->sep == rsep;
> > > +}
> > > +
> > > +static void store_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_remote_sep *rsep)
> > > +{
> > > + GKeyFile *key_file;
> > > + char filename[PATH_MAX];
> > > + char dst_addr[18];
> > > + char value[4];
> > > + char *data;
> > > + size_t len = 0;
> > > +
> > > + ba2str(device_get_address(chan->device), dst_addr);
> > > +
> > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > + dst_addr);
> > > + key_file = g_key_file_new();
> > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > +
> > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > +
> > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > +
> > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > + g_file_set_contents(filename, data, len, NULL);
> > > +
> > > + g_free(data);
> > > + g_key_file_free(key_file);
> > > +}
> > > +
> > > +static void update_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_stream *stream)
> > > +{
> > > + struct avdtp_remote_sep *rsep;
> > > + struct a2dp_remote_sep *sep;
> > > +
> > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > +
> > > + /* Update last used */
> > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > + if (chan->last_used == sep)
> > > + return;
> > > +
> > > + chan->last_used = sep;
> > > + store_last_used(chan, rsep);
> > > +}
> > > +
> > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > void *user_data)
> > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > setup->err = err;
> > > if (setup->start)
> > > finalize_resume(setup);
> > > - }
> > > + } else if (setup->chan)
> > > + update_last_used(setup->chan, stream);
> > >
> > > finalize_config(setup);
> > >
> > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > -{
> > > - const struct a2dp_remote_sep *sep = data;
> > > - const struct avdtp_remote_sep *rsep = user_data;
> > > -
> > > - return sep->sep == rsep;
> > > -}
> > > -
> > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > struct a2dp_sep *sep)
> > > {
> > > @@ -1791,19 +1839,28 @@ done:
> > > queue_push_tail(chan->seps, sep);
> > > }
> > >
> > > +static bool match_seid(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const uint8_t *seid = user_data;
> > > +
> > > + return avdtp_get_seid(sep->sep) == *seid;
> > > +}
> > > +
> > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > char **seids)
> > > {
> > > struct avdtp_remote_sep *sep;
> > > + uint8_t seid;
> > > + char *value;
> > >
> > > if (!seids)
> > > return;
> > >
> > > for (; *seids; seids++) {
> > > - uint8_t seid;
> > > uint8_t type;
> > > uint8_t codec;
> > > - char *value, caps[256];
> > > + char caps[256];
> > > uint8_t data[128];
> > > int i, size;
> > > GSList *l = NULL;
> > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > >
> > > register_remote_sep(sep, chan);
> > > }
> > > +
> > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > + if (!value)
> > > + return;
> > > +
> > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > + return;
> > > +
> > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > }
> > >
> > > static void load_remote_seps(struct a2dp_channel *chan)
> > > @@ -2355,8 +2421,12 @@ done:
> > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > const char *sender)
> > > {
> > > + struct a2dp_sep *first = NULL;
> > > + struct a2dp_channel *chan = find_channel(session);
> > > +
> > > for (; list; list = list->next) {
> > > struct a2dp_sep *sep = list->data;
> > > + struct avdtp_remote_sep *rsep;
> > >
> > > /* Use sender's endpoint if available */
> > > if (sender) {
> > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > continue;
> > > }
> > >
> > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > + if (!rsep)
> > > continue;
> > >
> > > + /* Locate last used if set */
> > > + if (chan->last_used) {
> > > + if (chan->last_used->sep == rsep)
> > > + return sep;
> > > + first = sep;
> > > + }
> > > +
> > > return sep;
> > >
> > > }
> > >
> > > - return NULL;
> > > + return first;
> > > }
> > >
> > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > --
> > > 2.20.1
> >
> > Can you give this a try, it should make the daemon remember what was
> > the last endpoint used (locally the remote selection we cannot really
> > control).
>
> Great, I will try it. Remote selection is done by remote device so this
> really cannot be controlled.
>
> I have there another suggestion for improvement. bluez support
> registering more endpoints for one codec. I'm going to use it for
> registering one SBC endpoint with fixed bitpool as UHQ profile and
> another with lower bitpool values. To distinguish in pulseaudio between
> low and high quality of SBC. Seems that bluez sometimes try to only
> select only one endpoint, even there are more and do not try another if
> first one selection fails (e.g. because remote capability of bitpool is
> too low and our local selection needs higher). Could be bluez modified
> to try another local endpoint if there are more registered until some
> accept capabilities?
Well that was actually one of the reason Ive recommended not
registering more than one endpoint for the same codec, we should
perhaps fix that since those endpoints can belong to different
processes but if we do insist to have PA with multiple SBC endpoints
that means it would not be backward compatible since it may try to
configure with UHQ as it should be higher priority, besides having a
higher fixed range means we can no longer reduce the quality at
runtime, but perhaps there is a way to make it work if PA detects it
has multiple endpoints that could be used it could just switch to
normal quality if UHQ cannot be used, this is actually beneficial in
terms of latency as well since we don't have extra D-Bus round trips
asking different endpoints.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-25 8:00 ` Luiz Augusto von Dentz
@ 2019-04-28 18:53 ` Pali Rohár
2019-04-28 19:15 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 14+ messages in thread
From: Pali Rohár @ 2019-04-28 18:53 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 10879 bytes --]
On Thursday 25 April 2019 11:00:07 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Wed, Apr 24, 2019 at 7:57 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This stores the last used endpoint whenever it is considered open and
> > > > then prefer to use it when attempting to reconnect.
> > > > ---
> > > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > > index 8f141739c..78b02dc84 100644
> > > > --- a/profiles/audio/a2dp.c
> > > > +++ b/profiles/audio/a2dp.c
> > > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > > unsigned int auth_id;
> > > > struct avdtp *session;
> > > > struct queue *seps;
> > > > + struct a2dp_remote_sep *last_used;
> > > > };
> > > >
> > > > static GSList *servers = NULL;
> > > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > return TRUE;
> > > > }
> > > >
> > > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > > +{
> > > > + const struct a2dp_remote_sep *sep = data;
> > > > + const struct avdtp_remote_sep *rsep = user_data;
> > > > +
> > > > + return sep->sep == rsep;
> > > > +}
> > > > +
> > > > +static void store_last_used(struct a2dp_channel *chan,
> > > > + struct avdtp_remote_sep *rsep)
> > > > +{
> > > > + GKeyFile *key_file;
> > > > + char filename[PATH_MAX];
> > > > + char dst_addr[18];
> > > > + char value[4];
> > > > + char *data;
> > > > + size_t len = 0;
> > > > +
> > > > + ba2str(device_get_address(chan->device), dst_addr);
> > > > +
> > > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > > + dst_addr);
> > > > + key_file = g_key_file_new();
> > > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > > +
> > > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > > +
> > > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > > +
> > > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > > + g_file_set_contents(filename, data, len, NULL);
> > > > +
> > > > + g_free(data);
> > > > + g_key_file_free(key_file);
> > > > +}
> > > > +
> > > > +static void update_last_used(struct a2dp_channel *chan,
> > > > + struct avdtp_stream *stream)
> > > > +{
> > > > + struct avdtp_remote_sep *rsep;
> > > > + struct a2dp_remote_sep *sep;
> > > > +
> > > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > > +
> > > > + /* Update last used */
> > > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > > + if (chan->last_used == sep)
> > > > + return;
> > > > +
> > > > + chan->last_used = sep;
> > > > + store_last_used(chan, rsep);
> > > > +}
> > > > +
> > > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > > void *user_data)
> > > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > setup->err = err;
> > > > if (setup->start)
> > > > finalize_resume(setup);
> > > > - }
> > > > + } else if (setup->chan)
> > > > + update_last_used(setup->chan, stream);
> > > >
> > > > finalize_config(setup);
> > > >
> > > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > return TRUE;
> > > > }
> > > >
> > > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > > -{
> > > > - const struct a2dp_remote_sep *sep = data;
> > > > - const struct avdtp_remote_sep *rsep = user_data;
> > > > -
> > > > - return sep->sep == rsep;
> > > > -}
> > > > -
> > > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > > struct a2dp_sep *sep)
> > > > {
> > > > @@ -1791,19 +1839,28 @@ done:
> > > > queue_push_tail(chan->seps, sep);
> > > > }
> > > >
> > > > +static bool match_seid(const void *data, const void *user_data)
> > > > +{
> > > > + const struct a2dp_remote_sep *sep = data;
> > > > + const uint8_t *seid = user_data;
> > > > +
> > > > + return avdtp_get_seid(sep->sep) == *seid;
> > > > +}
> > > > +
> > > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > char **seids)
> > > > {
> > > > struct avdtp_remote_sep *sep;
> > > > + uint8_t seid;
> > > > + char *value;
> > > >
> > > > if (!seids)
> > > > return;
> > > >
> > > > for (; *seids; seids++) {
> > > > - uint8_t seid;
> > > > uint8_t type;
> > > > uint8_t codec;
> > > > - char *value, caps[256];
> > > > + char caps[256];
> > > > uint8_t data[128];
> > > > int i, size;
> > > > GSList *l = NULL;
> > > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > >
> > > > register_remote_sep(sep, chan);
> > > > }
> > > > +
> > > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > > + if (!value)
> > > > + return;
> > > > +
> > > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > > + return;
> > > > +
> > > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > > }
> > > >
> > > > static void load_remote_seps(struct a2dp_channel *chan)
> > > > @@ -2355,8 +2421,12 @@ done:
> > > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > const char *sender)
> > > > {
> > > > + struct a2dp_sep *first = NULL;
> > > > + struct a2dp_channel *chan = find_channel(session);
> > > > +
> > > > for (; list; list = list->next) {
> > > > struct a2dp_sep *sep = list->data;
> > > > + struct avdtp_remote_sep *rsep;
> > > >
> > > > /* Use sender's endpoint if available */
> > > > if (sender) {
> > > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > continue;
> > > > }
> > > >
> > > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > > + if (!rsep)
> > > > continue;
> > > >
> > > > + /* Locate last used if set */
> > > > + if (chan->last_used) {
> > > > + if (chan->last_used->sep == rsep)
> > > > + return sep;
> > > > + first = sep;
> > > > + }
> > > > +
> > > > return sep;
> > > >
> > > > }
> > > >
> > > > - return NULL;
> > > > + return first;
> > > > }
> > > >
> > > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > > --
> > > > 2.20.1
> > >
> > > Can you give this a try, it should make the daemon remember what was
> > > the last endpoint used (locally the remote selection we cannot really
> > > control).
> >
> > Great, I will try it. Remote selection is done by remote device so this
> > really cannot be controlled.
> >
> > I have there another suggestion for improvement. bluez support
> > registering more endpoints for one codec. I'm going to use it for
> > registering one SBC endpoint with fixed bitpool as UHQ profile and
> > another with lower bitpool values. To distinguish in pulseaudio between
> > low and high quality of SBC. Seems that bluez sometimes try to only
> > select only one endpoint, even there are more and do not try another if
> > first one selection fails (e.g. because remote capability of bitpool is
> > too low and our local selection needs higher). Could be bluez modified
> > to try another local endpoint if there are more registered until some
> > accept capabilities?
>
> Well that was actually one of the reason Ive recommended not
> registering more than one endpoint for the same codec, we should
> perhaps fix that since those endpoints can belong to different
> processes but if we do insist to have PA with multiple SBC endpoints
> that means it would not be backward compatible since it may try to
> configure with UHQ as it should be higher priority, besides having a
> higher fixed range means we can no longer reduce the quality at
> runtime, but perhaps there is a way to make it work if PA detects it
> has multiple endpoints that could be used it could just switch to
> normal quality if UHQ cannot be used, this is actually beneficial in
> terms of latency as well since we don't have extra D-Bus round trips
> asking different endpoints.
Registering multiple endpoints is the only way how to enforce SBC UHQ.
Most of headset supports only SBC bitpools 1-53, therefore for SBC UHQ
is needed Dualchannel mode, not Joint Stereo.
So I see there only one backward-compatible implementation: Runtime
check of bluez version (or capability or whatever). Also this is
probably also useful/needed to check if bluez supports API for changing
codec.
So... can you implement that "fallback" mechanisms of trying to use all
possible endpoints until one succeed; and provide ability for
applications to detect this functionality is supported?
Application (like pulseaudio) would register multiple endpoints only in
the case then bluez declares its functionality to be code backward
compatible. And if case it is not supported then application would not
support fixed profiles like SBC UHQ.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-28 18:53 ` Pali Rohár
@ 2019-04-28 19:15 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-28 19:15 UTC (permalink / raw)
To: Pali Rohár; +Cc: linux-bluetooth
Hi Pali,
On Sun, Apr 28, 2019 at 9:53 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Thursday 25 April 2019 11:00:07 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Wed, Apr 24, 2019 at 7:57 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > >
> > > > > This stores the last used endpoint whenever it is considered open and
> > > > > then prefer to use it when attempting to reconnect.
> > > > > ---
> > > > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > > > index 8f141739c..78b02dc84 100644
> > > > > --- a/profiles/audio/a2dp.c
> > > > > +++ b/profiles/audio/a2dp.c
> > > > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > > > unsigned int auth_id;
> > > > > struct avdtp *session;
> > > > > struct queue *seps;
> > > > > + struct a2dp_remote_sep *last_used;
> > > > > };
> > > > >
> > > > > static GSList *servers = NULL;
> > > > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > return TRUE;
> > > > > }
> > > > >
> > > > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > > > +{
> > > > > + const struct a2dp_remote_sep *sep = data;
> > > > > + const struct avdtp_remote_sep *rsep = user_data;
> > > > > +
> > > > > + return sep->sep == rsep;
> > > > > +}
> > > > > +
> > > > > +static void store_last_used(struct a2dp_channel *chan,
> > > > > + struct avdtp_remote_sep *rsep)
> > > > > +{
> > > > > + GKeyFile *key_file;
> > > > > + char filename[PATH_MAX];
> > > > > + char dst_addr[18];
> > > > > + char value[4];
> > > > > + char *data;
> > > > > + size_t len = 0;
> > > > > +
> > > > > + ba2str(device_get_address(chan->device), dst_addr);
> > > > > +
> > > > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > > > + dst_addr);
> > > > > + key_file = g_key_file_new();
> > > > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > > > +
> > > > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > > > +
> > > > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > > > +
> > > > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > > > + g_file_set_contents(filename, data, len, NULL);
> > > > > +
> > > > > + g_free(data);
> > > > > + g_key_file_free(key_file);
> > > > > +}
> > > > > +
> > > > > +static void update_last_used(struct a2dp_channel *chan,
> > > > > + struct avdtp_stream *stream)
> > > > > +{
> > > > > + struct avdtp_remote_sep *rsep;
> > > > > + struct a2dp_remote_sep *sep;
> > > > > +
> > > > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > > > +
> > > > > + /* Update last used */
> > > > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > > > + if (chan->last_used == sep)
> > > > > + return;
> > > > > +
> > > > > + chan->last_used = sep;
> > > > > + store_last_used(chan, rsep);
> > > > > +}
> > > > > +
> > > > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > > > void *user_data)
> > > > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > setup->err = err;
> > > > > if (setup->start)
> > > > > finalize_resume(setup);
> > > > > - }
> > > > > + } else if (setup->chan)
> > > > > + update_last_used(setup->chan, stream);
> > > > >
> > > > > finalize_config(setup);
> > > > >
> > > > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > return TRUE;
> > > > > }
> > > > >
> > > > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > > > -{
> > > > > - const struct a2dp_remote_sep *sep = data;
> > > > > - const struct avdtp_remote_sep *rsep = user_data;
> > > > > -
> > > > > - return sep->sep == rsep;
> > > > > -}
> > > > > -
> > > > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > > > struct a2dp_sep *sep)
> > > > > {
> > > > > @@ -1791,19 +1839,28 @@ done:
> > > > > queue_push_tail(chan->seps, sep);
> > > > > }
> > > > >
> > > > > +static bool match_seid(const void *data, const void *user_data)
> > > > > +{
> > > > > + const struct a2dp_remote_sep *sep = data;
> > > > > + const uint8_t *seid = user_data;
> > > > > +
> > > > > + return avdtp_get_seid(sep->sep) == *seid;
> > > > > +}
> > > > > +
> > > > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > > char **seids)
> > > > > {
> > > > > struct avdtp_remote_sep *sep;
> > > > > + uint8_t seid;
> > > > > + char *value;
> > > > >
> > > > > if (!seids)
> > > > > return;
> > > > >
> > > > > for (; *seids; seids++) {
> > > > > - uint8_t seid;
> > > > > uint8_t type;
> > > > > uint8_t codec;
> > > > > - char *value, caps[256];
> > > > > + char caps[256];
> > > > > uint8_t data[128];
> > > > > int i, size;
> > > > > GSList *l = NULL;
> > > > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > >
> > > > > register_remote_sep(sep, chan);
> > > > > }
> > > > > +
> > > > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > > > + if (!value)
> > > > > + return;
> > > > > +
> > > > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > > > + return;
> > > > > +
> > > > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > > > }
> > > > >
> > > > > static void load_remote_seps(struct a2dp_channel *chan)
> > > > > @@ -2355,8 +2421,12 @@ done:
> > > > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > > const char *sender)
> > > > > {
> > > > > + struct a2dp_sep *first = NULL;
> > > > > + struct a2dp_channel *chan = find_channel(session);
> > > > > +
> > > > > for (; list; list = list->next) {
> > > > > struct a2dp_sep *sep = list->data;
> > > > > + struct avdtp_remote_sep *rsep;
> > > > >
> > > > > /* Use sender's endpoint if available */
> > > > > if (sender) {
> > > > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > > continue;
> > > > > }
> > > > >
> > > > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > > > + if (!rsep)
> > > > > continue;
> > > > >
> > > > > + /* Locate last used if set */
> > > > > + if (chan->last_used) {
> > > > > + if (chan->last_used->sep == rsep)
> > > > > + return sep;
> > > > > + first = sep;
> > > > > + }
> > > > > +
> > > > > return sep;
> > > > >
> > > > > }
> > > > >
> > > > > - return NULL;
> > > > > + return first;
> > > > > }
> > > > >
> > > > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > > > --
> > > > > 2.20.1
> > > >
> > > > Can you give this a try, it should make the daemon remember what was
> > > > the last endpoint used (locally the remote selection we cannot really
> > > > control).
> > >
> > > Great, I will try it. Remote selection is done by remote device so this
> > > really cannot be controlled.
> > >
> > > I have there another suggestion for improvement. bluez support
> > > registering more endpoints for one codec. I'm going to use it for
> > > registering one SBC endpoint with fixed bitpool as UHQ profile and
> > > another with lower bitpool values. To distinguish in pulseaudio between
> > > low and high quality of SBC. Seems that bluez sometimes try to only
> > > select only one endpoint, even there are more and do not try another if
> > > first one selection fails (e.g. because remote capability of bitpool is
> > > too low and our local selection needs higher). Could be bluez modified
> > > to try another local endpoint if there are more registered until some
> > > accept capabilities?
> >
> > Well that was actually one of the reason Ive recommended not
> > registering more than one endpoint for the same codec, we should
> > perhaps fix that since those endpoints can belong to different
> > processes but if we do insist to have PA with multiple SBC endpoints
> > that means it would not be backward compatible since it may try to
> > configure with UHQ as it should be higher priority, besides having a
> > higher fixed range means we can no longer reduce the quality at
> > runtime, but perhaps there is a way to make it work if PA detects it
> > has multiple endpoints that could be used it could just switch to
> > normal quality if UHQ cannot be used, this is actually beneficial in
> > terms of latency as well since we don't have extra D-Bus round trips
> > asking different endpoints.
>
> Registering multiple endpoints is the only way how to enforce SBC UHQ.
> Most of headset supports only SBC bitpools 1-53, therefore for SBC UHQ
> is needed Dualchannel mode, not Joint Stereo.
Sure, but that is just one extra endpoint, not the 4 or so you have
currently in your set.
> So I see there only one backward-compatible implementation: Runtime
> check of bluez version (or capability or whatever). Also this is
> probably also useful/needed to check if bluez supports API for changing
> codec.
>
> So... can you implement that "fallback" mechanisms of trying to use all
> possible endpoints until one succeed; and provide ability for
> applications to detect this functionality is supported?
I will have a fix for that, but cycling thru that many endpoints might
not be the best experience, Id really just leave 2 endpoints for SBC,
one for dual channel and another for the rest.
> Application (like pulseaudio) would register multiple endpoints only in
> the case then bluez declares its functionality to be code backward
> compatible. And if case it is not supported then application would not
> support fixed profiles like SBC UHQ.
Well, going with 76 bitpool is in fact supported by early versions of
BlueZ, thus why I was suggesting to register just 2 SBC endpoints as
explained above since you can order register the joint-stereo before
the dual-band that should not be affected by the lack of retrying
logic.
> --
> Pali Rohár
> pali.rohar@gmail.com
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 16:57 ` Pali Rohár
2019-04-25 8:00 ` Luiz Augusto von Dentz
@ 2019-05-02 21:14 ` Pali Rohár
2019-05-03 10:07 ` Pali Rohár
2019-05-03 10:20 ` Pali Rohár
3 siblings, 0 replies; 14+ messages in thread
From: Pali Rohár @ 2019-05-02 21:14 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 8237 bytes --]
On Wednesday 24 April 2019 18:57:21 Pali Rohár wrote:
> On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This stores the last used endpoint whenever it is considered open and
> > > then prefer to use it when attempting to reconnect.
> > > ---
> > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > index 8f141739c..78b02dc84 100644
> > > --- a/profiles/audio/a2dp.c
> > > +++ b/profiles/audio/a2dp.c
> > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > unsigned int auth_id;
> > > struct avdtp *session;
> > > struct queue *seps;
> > > + struct a2dp_remote_sep *last_used;
> > > };
> > >
> > > static GSList *servers = NULL;
> > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const struct avdtp_remote_sep *rsep = user_data;
> > > +
> > > + return sep->sep == rsep;
> > > +}
> > > +
> > > +static void store_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_remote_sep *rsep)
> > > +{
> > > + GKeyFile *key_file;
> > > + char filename[PATH_MAX];
> > > + char dst_addr[18];
> > > + char value[4];
> > > + char *data;
> > > + size_t len = 0;
> > > +
> > > + ba2str(device_get_address(chan->device), dst_addr);
> > > +
> > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > + dst_addr);
> > > + key_file = g_key_file_new();
> > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > +
> > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > +
> > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > +
> > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > + g_file_set_contents(filename, data, len, NULL);
> > > +
> > > + g_free(data);
> > > + g_key_file_free(key_file);
> > > +}
> > > +
> > > +static void update_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_stream *stream)
> > > +{
> > > + struct avdtp_remote_sep *rsep;
> > > + struct a2dp_remote_sep *sep;
> > > +
> > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > +
> > > + /* Update last used */
> > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > + if (chan->last_used == sep)
> > > + return;
> > > +
> > > + chan->last_used = sep;
> > > + store_last_used(chan, rsep);
> > > +}
> > > +
> > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > void *user_data)
> > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > setup->err = err;
> > > if (setup->start)
> > > finalize_resume(setup);
> > > - }
> > > + } else if (setup->chan)
> > > + update_last_used(setup->chan, stream);
> > >
> > > finalize_config(setup);
> > >
> > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > -{
> > > - const struct a2dp_remote_sep *sep = data;
> > > - const struct avdtp_remote_sep *rsep = user_data;
> > > -
> > > - return sep->sep == rsep;
> > > -}
> > > -
> > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > struct a2dp_sep *sep)
> > > {
> > > @@ -1791,19 +1839,28 @@ done:
> > > queue_push_tail(chan->seps, sep);
> > > }
> > >
> > > +static bool match_seid(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const uint8_t *seid = user_data;
> > > +
> > > + return avdtp_get_seid(sep->sep) == *seid;
> > > +}
> > > +
> > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > char **seids)
> > > {
> > > struct avdtp_remote_sep *sep;
> > > + uint8_t seid;
> > > + char *value;
> > >
> > > if (!seids)
> > > return;
> > >
> > > for (; *seids; seids++) {
> > > - uint8_t seid;
> > > uint8_t type;
> > > uint8_t codec;
> > > - char *value, caps[256];
> > > + char caps[256];
> > > uint8_t data[128];
> > > int i, size;
> > > GSList *l = NULL;
> > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > >
> > > register_remote_sep(sep, chan);
> > > }
> > > +
> > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > + if (!value)
> > > + return;
> > > +
> > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > + return;
> > > +
> > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > }
> > >
> > > static void load_remote_seps(struct a2dp_channel *chan)
> > > @@ -2355,8 +2421,12 @@ done:
> > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > const char *sender)
> > > {
> > > + struct a2dp_sep *first = NULL;
> > > + struct a2dp_channel *chan = find_channel(session);
> > > +
> > > for (; list; list = list->next) {
> > > struct a2dp_sep *sep = list->data;
> > > + struct avdtp_remote_sep *rsep;
> > >
> > > /* Use sender's endpoint if available */
> > > if (sender) {
> > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > continue;
> > > }
> > >
> > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > + if (!rsep)
> > > continue;
> > >
> > > + /* Locate last used if set */
> > > + if (chan->last_used) {
> > > + if (chan->last_used->sep == rsep)
> > > + return sep;
> > > + first = sep;
> > > + }
> > > +
> > > return sep;
> > >
> > > }
> > >
> > > - return NULL;
> > > + return first;
> > > }
> > >
> > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > --
> > > 2.20.1
> >
> > Can you give this a try, it should make the daemon remember what was
> > the last endpoint used (locally the remote selection we cannot really
> > control).
>
> Great, I will try it.
Seems that it does not work as expected when there are more SEPs for one
codec.
Before bluez chose first registered endpoint which matched codec. Now in
git master is bluez choosing last registered endpoint which matched
codec. So before it was SBC at highest quality, now it is SBC at lowest
quality.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 16:57 ` Pali Rohár
2019-04-25 8:00 ` Luiz Augusto von Dentz
2019-05-02 21:14 ` Pali Rohár
@ 2019-05-03 10:07 ` Pali Rohár
2019-05-03 10:18 ` Luiz Augusto von Dentz
2019-05-03 10:20 ` Pali Rohár
3 siblings, 1 reply; 14+ messages in thread
From: Pali Rohár @ 2019-05-03 10:07 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 8387 bytes --]
On Wednesday 24 April 2019 18:57:21 Pali Rohár wrote:
> On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This stores the last used endpoint whenever it is considered open and
> > > then prefer to use it when attempting to reconnect.
> > > ---
> > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > index 8f141739c..78b02dc84 100644
> > > --- a/profiles/audio/a2dp.c
> > > +++ b/profiles/audio/a2dp.c
> > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > unsigned int auth_id;
> > > struct avdtp *session;
> > > struct queue *seps;
> > > + struct a2dp_remote_sep *last_used;
> > > };
> > >
> > > static GSList *servers = NULL;
> > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const struct avdtp_remote_sep *rsep = user_data;
> > > +
> > > + return sep->sep == rsep;
> > > +}
> > > +
> > > +static void store_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_remote_sep *rsep)
> > > +{
> > > + GKeyFile *key_file;
> > > + char filename[PATH_MAX];
> > > + char dst_addr[18];
> > > + char value[4];
> > > + char *data;
> > > + size_t len = 0;
> > > +
> > > + ba2str(device_get_address(chan->device), dst_addr);
> > > +
> > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > + dst_addr);
> > > + key_file = g_key_file_new();
> > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > +
> > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > +
> > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > +
> > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > + g_file_set_contents(filename, data, len, NULL);
> > > +
> > > + g_free(data);
> > > + g_key_file_free(key_file);
> > > +}
> > > +
> > > +static void update_last_used(struct a2dp_channel *chan,
> > > + struct avdtp_stream *stream)
> > > +{
> > > + struct avdtp_remote_sep *rsep;
> > > + struct a2dp_remote_sep *sep;
> > > +
> > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > +
> > > + /* Update last used */
> > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > + if (chan->last_used == sep)
> > > + return;
> > > +
> > > + chan->last_used = sep;
> > > + store_last_used(chan, rsep);
> > > +}
> > > +
> > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > void *user_data)
> > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > setup->err = err;
> > > if (setup->start)
> > > finalize_resume(setup);
> > > - }
> > > + } else if (setup->chan)
> > > + update_last_used(setup->chan, stream);
> > >
> > > finalize_config(setup);
> > >
> > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > return TRUE;
> > > }
> > >
> > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > -{
> > > - const struct a2dp_remote_sep *sep = data;
> > > - const struct avdtp_remote_sep *rsep = user_data;
> > > -
> > > - return sep->sep == rsep;
> > > -}
> > > -
> > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > struct a2dp_sep *sep)
> > > {
> > > @@ -1791,19 +1839,28 @@ done:
> > > queue_push_tail(chan->seps, sep);
> > > }
> > >
> > > +static bool match_seid(const void *data, const void *user_data)
> > > +{
> > > + const struct a2dp_remote_sep *sep = data;
> > > + const uint8_t *seid = user_data;
> > > +
> > > + return avdtp_get_seid(sep->sep) == *seid;
> > > +}
> > > +
> > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > char **seids)
> > > {
> > > struct avdtp_remote_sep *sep;
> > > + uint8_t seid;
> > > + char *value;
> > >
> > > if (!seids)
> > > return;
> > >
> > > for (; *seids; seids++) {
> > > - uint8_t seid;
> > > uint8_t type;
> > > uint8_t codec;
> > > - char *value, caps[256];
> > > + char caps[256];
> > > uint8_t data[128];
> > > int i, size;
> > > GSList *l = NULL;
> > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > >
> > > register_remote_sep(sep, chan);
> > > }
> > > +
> > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > + if (!value)
> > > + return;
> > > +
> > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > + return;
> > > +
> > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > }
> > >
> > > static void load_remote_seps(struct a2dp_channel *chan)
> > > @@ -2355,8 +2421,12 @@ done:
> > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > const char *sender)
> > > {
> > > + struct a2dp_sep *first = NULL;
> > > + struct a2dp_channel *chan = find_channel(session);
> > > +
> > > for (; list; list = list->next) {
> > > struct a2dp_sep *sep = list->data;
> > > + struct avdtp_remote_sep *rsep;
> > >
> > > /* Use sender's endpoint if available */
> > > if (sender) {
> > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > continue;
> > > }
> > >
> > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > + if (!rsep)
> > > continue;
> > >
> > > + /* Locate last used if set */
> > > + if (chan->last_used) {
> > > + if (chan->last_used->sep == rsep)
> > > + return sep;
> > > + first = sep;
> > > + }
> > > +
> > > return sep;
> > >
> > > }
> > >
> > > - return NULL;
> > > + return first;
> > > }
> > >
> > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > --
> > > 2.20.1
> >
> > Can you give this a try, it should make the daemon remember what was
> > the last endpoint used (locally the remote selection we cannot really
> > control).
>
> Great, I will try it.
Now I run 'sudo grep LastUsed -r /var/lib/bluetooth/' and see that this
properly is stored only for one device. So seems that bluetoothd does
not always stores LastUsed information.
E.g. when I called and successfully connected device via command
$ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.ConnectProfile 0000110a-0000-1000-8000-00805f9b34fb
and later disconnected, I have not found anything via above grep.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-05-03 10:07 ` Pali Rohár
@ 2019-05-03 10:18 ` Luiz Augusto von Dentz
2019-05-03 10:24 ` Pali Rohár
0 siblings, 1 reply; 14+ messages in thread
From: Luiz Augusto von Dentz @ 2019-05-03 10:18 UTC (permalink / raw)
To: Pali Rohár; +Cc: linux-bluetooth
Hi Pali,
On Fri, May 3, 2019 at 1:07 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Wednesday 24 April 2019 18:57:21 Pali Rohár wrote:
> > On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This stores the last used endpoint whenever it is considered open and
> > > > then prefer to use it when attempting to reconnect.
> > > > ---
> > > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > > index 8f141739c..78b02dc84 100644
> > > > --- a/profiles/audio/a2dp.c
> > > > +++ b/profiles/audio/a2dp.c
> > > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > > unsigned int auth_id;
> > > > struct avdtp *session;
> > > > struct queue *seps;
> > > > + struct a2dp_remote_sep *last_used;
> > > > };
> > > >
> > > > static GSList *servers = NULL;
> > > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > return TRUE;
> > > > }
> > > >
> > > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > > +{
> > > > + const struct a2dp_remote_sep *sep = data;
> > > > + const struct avdtp_remote_sep *rsep = user_data;
> > > > +
> > > > + return sep->sep == rsep;
> > > > +}
> > > > +
> > > > +static void store_last_used(struct a2dp_channel *chan,
> > > > + struct avdtp_remote_sep *rsep)
> > > > +{
> > > > + GKeyFile *key_file;
> > > > + char filename[PATH_MAX];
> > > > + char dst_addr[18];
> > > > + char value[4];
> > > > + char *data;
> > > > + size_t len = 0;
> > > > +
> > > > + ba2str(device_get_address(chan->device), dst_addr);
> > > > +
> > > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > > + dst_addr);
> > > > + key_file = g_key_file_new();
> > > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > > +
> > > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > > +
> > > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > > +
> > > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > > + g_file_set_contents(filename, data, len, NULL);
> > > > +
> > > > + g_free(data);
> > > > + g_key_file_free(key_file);
> > > > +}
> > > > +
> > > > +static void update_last_used(struct a2dp_channel *chan,
> > > > + struct avdtp_stream *stream)
> > > > +{
> > > > + struct avdtp_remote_sep *rsep;
> > > > + struct a2dp_remote_sep *sep;
> > > > +
> > > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > > +
> > > > + /* Update last used */
> > > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > > + if (chan->last_used == sep)
> > > > + return;
> > > > +
> > > > + chan->last_used = sep;
> > > > + store_last_used(chan, rsep);
> > > > +}
> > > > +
> > > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > > void *user_data)
> > > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > setup->err = err;
> > > > if (setup->start)
> > > > finalize_resume(setup);
> > > > - }
> > > > + } else if (setup->chan)
> > > > + update_last_used(setup->chan, stream);
> > > >
> > > > finalize_config(setup);
> > > >
> > > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > return TRUE;
> > > > }
> > > >
> > > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > > -{
> > > > - const struct a2dp_remote_sep *sep = data;
> > > > - const struct avdtp_remote_sep *rsep = user_data;
> > > > -
> > > > - return sep->sep == rsep;
> > > > -}
> > > > -
> > > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > > struct a2dp_sep *sep)
> > > > {
> > > > @@ -1791,19 +1839,28 @@ done:
> > > > queue_push_tail(chan->seps, sep);
> > > > }
> > > >
> > > > +static bool match_seid(const void *data, const void *user_data)
> > > > +{
> > > > + const struct a2dp_remote_sep *sep = data;
> > > > + const uint8_t *seid = user_data;
> > > > +
> > > > + return avdtp_get_seid(sep->sep) == *seid;
> > > > +}
> > > > +
> > > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > char **seids)
> > > > {
> > > > struct avdtp_remote_sep *sep;
> > > > + uint8_t seid;
> > > > + char *value;
> > > >
> > > > if (!seids)
> > > > return;
> > > >
> > > > for (; *seids; seids++) {
> > > > - uint8_t seid;
> > > > uint8_t type;
> > > > uint8_t codec;
> > > > - char *value, caps[256];
> > > > + char caps[256];
> > > > uint8_t data[128];
> > > > int i, size;
> > > > GSList *l = NULL;
> > > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > >
> > > > register_remote_sep(sep, chan);
> > > > }
> > > > +
> > > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > > + if (!value)
> > > > + return;
> > > > +
> > > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > > + return;
> > > > +
> > > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > > }
> > > >
> > > > static void load_remote_seps(struct a2dp_channel *chan)
> > > > @@ -2355,8 +2421,12 @@ done:
> > > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > const char *sender)
> > > > {
> > > > + struct a2dp_sep *first = NULL;
> > > > + struct a2dp_channel *chan = find_channel(session);
> > > > +
> > > > for (; list; list = list->next) {
> > > > struct a2dp_sep *sep = list->data;
> > > > + struct avdtp_remote_sep *rsep;
> > > >
> > > > /* Use sender's endpoint if available */
> > > > if (sender) {
> > > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > continue;
> > > > }
> > > >
> > > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > > + if (!rsep)
> > > > continue;
> > > >
> > > > + /* Locate last used if set */
> > > > + if (chan->last_used) {
> > > > + if (chan->last_used->sep == rsep)
> > > > + return sep;
> > > > + first = sep;
> > > > + }
> > > > +
> > > > return sep;
> > > >
> > > > }
> > > >
> > > > - return NULL;
> > > > + return first;
> > > > }
> > > >
> > > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > > --
> > > > 2.20.1
> > >
> > > Can you give this a try, it should make the daemon remember what was
> > > the last endpoint used (locally the remote selection we cannot really
> > > control).
> >
> > Great, I will try it.
>
> Now I run 'sudo grep LastUsed -r /var/lib/bluetooth/' and see that this
> properly is stored only for one device. So seems that bluetoothd does
> not always stores LastUsed information.
>
> E.g. when I called and successfully connected device via command
>
> $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.ConnectProfile 0000110a-0000-1000-8000-00805f9b34fb
>
> and later disconnected, I have not found anything via above grep.
Is this with the latest set? Were there any failures in the configuration?
(note that you need su/root to list /var/lib/bluetooth/)
/var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/08:DF:1F:D9:2A:93:43:LastUsed=01
/var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/B4:CD:27:F0:8D:0A:60:LastUsed=04
/var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/94:20:53:2E:08:CE:24:LastUsed=05
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-04-24 16:57 ` Pali Rohár
` (2 preceding siblings ...)
2019-05-03 10:07 ` Pali Rohár
@ 2019-05-03 10:20 ` Pali Rohár
3 siblings, 0 replies; 14+ messages in thread
From: Pali Rohár @ 2019-05-03 10:20 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]
And this patch has another problem. If remote device supports only one
remote endpoint with SBC codec and pulseaudio export two endpoints for
SBC codec (one low quality and one high quality) then into LastUsed
property is stored identification of remote endpoint. Not local
pulseaudio endpoint. So information if was activated low quality or high
quality is lost. I have there Nokia N900 device which has problems with
SBC at High Quality. It is unable to send any audio output in HQ.
And due to above problem, I need to use only low quality endpoint, but
because bluez does not remember which one was chosen, it seems to choose
high quality and nothing is working.
This is just example that some codecs or endpoints may be broken by
remote device, or remote device's SEP is not compatible with user
application (pulseaudio) for whatever reason.
So it is really needed to have API: "use this local endpoint for
creating new A2DP connection".
Without it we would have broken applications.
PS: A2DP SBC implementation on Nokia N900 seems to be broken, when it
sees SEP with dual channel SBC it sends nonsense SBC configuration (min
bitpool >> max bitpool) which is really not allowed by SBC UHQ in
pulseaudio. So this is one real example of existing broken
implementation which can be fixed only on client's pulseaudio side.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH BlueZ 1/2] a2dp: Store last used endpoint
2019-05-03 10:18 ` Luiz Augusto von Dentz
@ 2019-05-03 10:24 ` Pali Rohár
0 siblings, 0 replies; 14+ messages in thread
From: Pali Rohár @ 2019-05-03 10:24 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 10014 bytes --]
On Friday 03 May 2019 13:18:10 Luiz Augusto von Dentz wrote:
> Hi Pali,
>
> On Fri, May 3, 2019 at 1:07 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Wednesday 24 April 2019 18:57:21 Pali Rohár wrote:
> > > On Wednesday 24 April 2019 12:42:22 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Wed, Apr 24, 2019 at 11:39 AM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > >
> > > > > This stores the last used endpoint whenever it is considered open and
> > > > > then prefer to use it when attempting to reconnect.
> > > > > ---
> > > > > profiles/audio/a2dp.c | 104 ++++++++++++++++++++++++++++++++++++------
> > > > > 1 file changed, 91 insertions(+), 13 deletions(-)
> > > > >
> > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > > > > index 8f141739c..78b02dc84 100644
> > > > > --- a/profiles/audio/a2dp.c
> > > > > +++ b/profiles/audio/a2dp.c
> > > > > @@ -147,6 +147,7 @@ struct a2dp_channel {
> > > > > unsigned int auth_id;
> > > > > struct avdtp *session;
> > > > > struct queue *seps;
> > > > > + struct a2dp_remote_sep *last_used;
> > > > > };
> > > > >
> > > > > static GSList *servers = NULL;
> > > > > @@ -860,6 +861,60 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > return TRUE;
> > > > > }
> > > > >
> > > > > +static bool match_remote_sep(const void *data, const void *user_data)
> > > > > +{
> > > > > + const struct a2dp_remote_sep *sep = data;
> > > > > + const struct avdtp_remote_sep *rsep = user_data;
> > > > > +
> > > > > + return sep->sep == rsep;
> > > > > +}
> > > > > +
> > > > > +static void store_last_used(struct a2dp_channel *chan,
> > > > > + struct avdtp_remote_sep *rsep)
> > > > > +{
> > > > > + GKeyFile *key_file;
> > > > > + char filename[PATH_MAX];
> > > > > + char dst_addr[18];
> > > > > + char value[4];
> > > > > + char *data;
> > > > > + size_t len = 0;
> > > > > +
> > > > > + ba2str(device_get_address(chan->device), dst_addr);
> > > > > +
> > > > > + snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
> > > > > + btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
> > > > > + dst_addr);
> > > > > + key_file = g_key_file_new();
> > > > > + g_key_file_load_from_file(key_file, filename, 0, NULL);
> > > > > +
> > > > > + sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> > > > > +
> > > > > + g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
> > > > > +
> > > > > + data = g_key_file_to_data(key_file, &len, NULL);
> > > > > + g_file_set_contents(filename, data, len, NULL);
> > > > > +
> > > > > + g_free(data);
> > > > > + g_key_file_free(key_file);
> > > > > +}
> > > > > +
> > > > > +static void update_last_used(struct a2dp_channel *chan,
> > > > > + struct avdtp_stream *stream)
> > > > > +{
> > > > > + struct avdtp_remote_sep *rsep;
> > > > > + struct a2dp_remote_sep *sep;
> > > > > +
> > > > > + rsep = avdtp_stream_get_remote_sep(stream);
> > > > > +
> > > > > + /* Update last used */
> > > > > + sep = queue_find(chan->seps, match_remote_sep, rsep);
> > > > > + if (chan->last_used == sep)
> > > > > + return;
> > > > > +
> > > > > + chan->last_used = sep;
> > > > > + store_last_used(chan, rsep);
> > > > > +}
> > > > > +
> > > > > static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > struct avdtp_stream *stream, struct avdtp_error *err,
> > > > > void *user_data)
> > > > > @@ -884,7 +939,8 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > setup->err = err;
> > > > > if (setup->start)
> > > > > finalize_resume(setup);
> > > > > - }
> > > > > + } else if (setup->chan)
> > > > > + update_last_used(setup->chan, stream);
> > > > >
> > > > > finalize_config(setup);
> > > > >
> > > > > @@ -1077,14 +1133,6 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> > > > > return TRUE;
> > > > > }
> > > > >
> > > > > -static bool match_remote_sep(const void *data, const void *user_data)
> > > > > -{
> > > > > - const struct a2dp_remote_sep *sep = data;
> > > > > - const struct avdtp_remote_sep *rsep = user_data;
> > > > > -
> > > > > - return sep->sep == rsep;
> > > > > -}
> > > > > -
> > > > > static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
> > > > > struct a2dp_sep *sep)
> > > > > {
> > > > > @@ -1791,19 +1839,28 @@ done:
> > > > > queue_push_tail(chan->seps, sep);
> > > > > }
> > > > >
> > > > > +static bool match_seid(const void *data, const void *user_data)
> > > > > +{
> > > > > + const struct a2dp_remote_sep *sep = data;
> > > > > + const uint8_t *seid = user_data;
> > > > > +
> > > > > + return avdtp_get_seid(sep->sep) == *seid;
> > > > > +}
> > > > > +
> > > > > static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > > char **seids)
> > > > > {
> > > > > struct avdtp_remote_sep *sep;
> > > > > + uint8_t seid;
> > > > > + char *value;
> > > > >
> > > > > if (!seids)
> > > > > return;
> > > > >
> > > > > for (; *seids; seids++) {
> > > > > - uint8_t seid;
> > > > > uint8_t type;
> > > > > uint8_t codec;
> > > > > - char *value, caps[256];
> > > > > + char caps[256];
> > > > > uint8_t data[128];
> > > > > int i, size;
> > > > > GSList *l = NULL;
> > > > > @@ -1847,6 +1904,15 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> > > > >
> > > > > register_remote_sep(sep, chan);
> > > > > }
> > > > > +
> > > > > + value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> > > > > + if (!value)
> > > > > + return;
> > > > > +
> > > > > + if (sscanf(value, "%02hhx", &seid) != 1)
> > > > > + return;
> > > > > +
> > > > > + chan->last_used = queue_find(chan->seps, match_seid, &seid);
> > > > > }
> > > > >
> > > > > static void load_remote_seps(struct a2dp_channel *chan)
> > > > > @@ -2355,8 +2421,12 @@ done:
> > > > > static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > > const char *sender)
> > > > > {
> > > > > + struct a2dp_sep *first = NULL;
> > > > > + struct a2dp_channel *chan = find_channel(session);
> > > > > +
> > > > > for (; list; list = list->next) {
> > > > > struct a2dp_sep *sep = list->data;
> > > > > + struct avdtp_remote_sep *rsep;
> > > > >
> > > > > /* Use sender's endpoint if available */
> > > > > if (sender) {
> > > > > @@ -2370,14 +2440,22 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> > > > > continue;
> > > > > }
> > > > >
> > > > > - if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> > > > > + rsep = avdtp_find_remote_sep(session, sep->lsep);
> > > > > + if (!rsep)
> > > > > continue;
> > > > >
> > > > > + /* Locate last used if set */
> > > > > + if (chan->last_used) {
> > > > > + if (chan->last_used->sep == rsep)
> > > > > + return sep;
> > > > > + first = sep;
> > > > > + }
> > > > > +
> > > > > return sep;
> > > > >
> > > > > }
> > > > >
> > > > > - return NULL;
> > > > > + return first;
> > > > > }
> > > > >
> > > > > static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> > > > > --
> > > > > 2.20.1
> > > >
> > > > Can you give this a try, it should make the daemon remember what was
> > > > the last endpoint used (locally the remote selection we cannot really
> > > > control).
> > >
> > > Great, I will try it.
> >
> > Now I run 'sudo grep LastUsed -r /var/lib/bluetooth/' and see that this
> > properly is stored only for one device. So seems that bluetoothd does
> > not always stores LastUsed information.
> >
> > E.g. when I called and successfully connected device via command
> >
> > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.ConnectProfile 0000110a-0000-1000-8000-00805f9b34fb
> >
> > and later disconnected, I have not found anything via above grep.
>
> Is this with the latest set?
git master +
wget -q 'https://lore.kernel.org/linux-bluetooth/20190503084547.15743-1-luiz.dentz@gmail.com/t.mbox.gz' -O - | gunzip | git am
> Were there any failures in the configuration?
No, at least do not see it in console.
> (note that you need su/root to list /var/lib/bluetooth/)
I know, that is why sudo grep
> /var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/08:DF:1F:D9:2A:93:43:LastUsed=01
> /var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/B4:CD:27:F0:8D:0A:60:LastUsed=04
> /var/lib/bluetooth/B8:8A:60:D8:17:D7/cache/94:20:53:2E:08:CE:24:LastUsed=05
>
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-05-03 10:24 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24 8:39 [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
2019-04-24 8:39 ` [PATCH BlueZ 2/2] doc/settings-storage: Document LastUsed Endpoints entry Luiz Augusto von Dentz
2019-04-24 9:42 ` [PATCH BlueZ 1/2] a2dp: Store last used endpoint Luiz Augusto von Dentz
2019-04-24 16:57 ` Pali Rohár
2019-04-25 8:00 ` Luiz Augusto von Dentz
2019-04-28 18:53 ` Pali Rohár
2019-04-28 19:15 ` Luiz Augusto von Dentz
2019-05-02 21:14 ` Pali Rohár
2019-05-03 10:07 ` Pali Rohár
2019-05-03 10:18 ` Luiz Augusto von Dentz
2019-05-03 10:24 ` Pali Rohár
2019-05-03 10:20 ` Pali Rohár
2019-04-24 17:01 ` Pali Rohár
2019-04-24 18:26 ` Luiz Augusto von Dentz
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.