From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Cc: "Pali Rohár" <pali.rohar@gmail.com>
Subject: Re: [PATCH v3 6/6] a2dp: Reword LastUsed
Date: Mon, 6 May 2019 15:46:03 +0300 [thread overview]
Message-ID: <CABBYNZJ4+Be+sAXVAL_96qov5Go3BpRwmJtkdOVXC4apfL2K9A@mail.gmail.com> (raw)
In-Reply-To: <20190506124310.19151-6-luiz.dentz@gmail.com>
Hi Pali,
On Mon, May 6, 2019 at 3:43 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> In addition to storing the remote endpoint also store the local one as
> well so we it can properly be restore.
> ---
> profiles/audio/a2dp.c | 147 +++++++++++++++++++++++++++++++----------
> profiles/audio/avdtp.c | 5 ++
> profiles/audio/avdtp.h | 1 +
> 3 files changed, 119 insertions(+), 34 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index b54c50315..046193688 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -139,6 +139,11 @@ struct a2dp_remote_sep {
> struct avdtp_remote_sep *sep;
> };
>
> +struct a2dp_last_used {
> + struct a2dp_sep *lsep;
> + struct a2dp_remote_sep *rsep;
> +};
> +
> struct a2dp_channel {
> struct a2dp_server *server;
> struct btd_device *device;
> @@ -148,7 +153,7 @@ struct a2dp_channel {
> unsigned int auth_id;
> struct avdtp *session;
> struct queue *seps;
> - struct a2dp_remote_sep *last_used;
> + struct a2dp_last_used *last_used;
> };
>
> static GSList *servers = NULL;
> @@ -214,6 +219,8 @@ static void setup_free(struct a2dp_setup *s)
> g_io_channel_unref(s->io);
> }
>
> + queue_destroy(s->eps, NULL);
> +
> setups = g_slist_remove(setups, s);
> if (s->session)
> avdtp_unref(s->session);
> @@ -844,13 +851,13 @@ static bool match_remote_sep(const void *data, const void *user_data)
> return sep->sep == rsep;
> }
>
> -static void store_last_used(struct a2dp_channel *chan,
> - struct avdtp_remote_sep *rsep)
> +static void store_last_used(struct a2dp_channel *chan, uint8_t lseid,
> + uint8_t rseid)
> {
> GKeyFile *key_file;
> char filename[PATH_MAX];
> char dst_addr[18];
> - char value[4];
> + char value[6];
> char *data;
> size_t len = 0;
>
> @@ -862,7 +869,7 @@ static void store_last_used(struct a2dp_channel *chan,
> key_file = g_key_file_new();
> g_key_file_load_from_file(key_file, filename, 0, NULL);
>
> - sprintf(value, "%02hhx", avdtp_get_seid(rsep));
> + sprintf(value, "%02hhx:%02hhx", lseid, rseid);
>
> g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);
>
> @@ -873,21 +880,38 @@ static void store_last_used(struct a2dp_channel *chan,
> g_key_file_free(key_file);
> }
>
> -static void update_last_used(struct a2dp_channel *chan,
> - struct avdtp_stream *stream)
> +static void add_last_used(struct a2dp_channel *chan, struct a2dp_sep *lsep,
> + struct a2dp_remote_sep *rsep)
> +{
> + if (!chan->last_used)
> + chan->last_used = new0(struct a2dp_last_used, 1);
> +
> + chan->last_used->lsep = lsep;
> + chan->last_used->rsep = rsep;
> +}
> +
> +static void update_last_used(struct a2dp_channel *chan, struct a2dp_sep *lsep,
> + 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)
> + if (!sep) {
> + error("Unable to find remote SEP");
> return;
> + }
> +
> + /* Check if already stored then skip */
> + if (chan->last_used && (chan->last_used->lsep == lsep &&
> + chan->last_used->rsep == sep))
> + return;
> +
> + add_last_used(chan, lsep, sep);
>
> - chan->last_used = sep;
> - store_last_used(chan, rsep);
> + store_last_used(chan, avdtp_sep_get_seid(lsep->lsep),
> + avdtp_get_seid(rsep));
> }
>
> static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> @@ -909,7 +933,7 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> setup->stream = stream;
>
> if (!err && setup->chan)
> - update_last_used(setup->chan, stream);
> + update_last_used(setup->chan, a2dp_sep, stream);
>
> if (setup->reconfigure)
> setup->reconfigure = FALSE;
> @@ -944,7 +968,7 @@ static void open_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
> if (setup->start)
> finalize_resume(setup);
> } else if (setup->chan)
> - update_last_used(setup->chan, stream);
> + update_last_used(setup->chan, a2dp_sep, stream);
>
> finalize_config(setup);
>
> @@ -1483,6 +1507,7 @@ static void channel_free(void *data)
> avdtp_remove_state_cb(chan->state_id);
>
> queue_destroy(chan->seps, remove_remote_sep);
> + free(chan->last_used);
> g_free(chan);
> }
>
> @@ -1631,6 +1656,9 @@ static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
> setup->sep = lsep;
> setup->rsep = rsep;
>
> + g_slist_free_full(setup->caps, g_free);
> + setup->caps = NULL;
> +
> caps_add_codec(&setup->caps, setup->sep->codec, caps, size);
>
> l = avdtp_get_type(rsep->sep) == AVDTP_SEP_TYPE_SINK ?
> @@ -1871,11 +1899,37 @@ static bool match_seid(const void *data, const void *user_data)
> return avdtp_get_seid(sep->sep) == *seid;
> }
>
> +static int match_sep(const void *data, const void *user_data)
> +{
> + struct a2dp_sep *sep = (void *) data;
> + const uint8_t *seid = user_data;
> +
> + return *seid - avdtp_sep_get_seid(sep->lsep);
> +}
> +
> +static struct a2dp_sep *find_sep_by_seid(struct a2dp_server *server,
> + uint8_t seid)
> +{
> + GSList *l;
> +
> + l = g_slist_find_custom(server->sources, &seid, match_sep);
> + if (l)
> + return l->data;
> +
> + l = g_slist_find_custom(server->sinks, &seid, match_sep);
> + if (l)
> + return l->data;
> +
> + return NULL;
> +}
> +
> static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> char **seids)
> {
> - struct avdtp_remote_sep *sep;
> - uint8_t seid;
> + struct a2dp_sep *lsep;
> + struct a2dp_remote_sep *sep;
> + struct avdtp_remote_sep *rsep;
> + uint8_t lseid, rseid;
> char *value;
>
> if (!seids)
> @@ -1884,12 +1938,12 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> for (; *seids; seids++) {
> uint8_t type;
> uint8_t codec;
> + GSList *l = NULL;
> char caps[256];
> uint8_t data[128];
> int i, size;
> - GSList *l = NULL;
>
> - if (sscanf(*seids, "%02hhx", &seid) != 1)
> + if (sscanf(*seids, "%02hhx", &rseid) != 1)
> continue;
>
> value = g_key_file_get_string(key_file, "Endpoints", *seids,
> @@ -1899,7 +1953,7 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
>
> if (sscanf(value, "%02hhx:%02hhx:%s", &type, &codec,
> caps) != 3) {
> - warn("Unable to load Endpoint: seid %u", seid);
> + warn("Unable to load Endpoint: seid %u", rseid);
> g_free(value);
> continue;
> }
> @@ -1908,7 +1962,8 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
> uint8_t *tmp = data + i / 2;
>
> if (sscanf(caps + i, "%02hhx", tmp) != 1) {
> - warn("Unable to load Endpoint: seid %u", seid);
> + warn("Unable to load Endpoint: seid %u", rseid);
> + g_free(value);
> break;
> }
> }
> @@ -1920,23 +1975,39 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
>
> caps_add_codec(&l, codec, data, size / 2);
>
> - sep = avdtp_register_remote_sep(chan->session, seid, type, l);
> - if (!sep) {
> - warn("Unable to register Endpoint: seid %u", seid);
> + rsep = avdtp_register_remote_sep(chan->session, rseid, type, l);
> + if (!rsep) {
> + warn("Unable to register Endpoint: seid %u", rseid);
> continue;
> }
>
> - register_remote_sep(sep, chan);
> + register_remote_sep(rsep, chan);
> }
>
> value = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> if (!value)
> return;
>
> - if (sscanf(value, "%02hhx", &seid) != 1)
> + if (sscanf(value, "%02hhx:%02hhx", &lseid, &rseid) != 2) {
> + warn("Unable to load LastUsed");
> + return;
> + }
> +
> + g_free(value);
> +
> + lsep = find_sep_by_seid(chan->server, lseid);
> + if (!lsep) {
> + warn("Unable to load LastUsed: lseid %u not found", lseid);
> return;
> + }
>
> - chan->last_used = queue_find(chan->seps, match_seid, &seid);
> + sep = queue_find(chan->seps, match_seid, &rseid);
> + if (!sep) {
> + warn("Unable to load LastUsed: rseid %u not found", rseid);
> + return;
> + }
> +
> + add_last_used(chan, lsep, sep);
> }
>
> static void load_remote_seps(struct a2dp_channel *chan)
> @@ -2442,7 +2513,6 @@ static void select_cb(struct a2dp_setup *setup, void *ret, int size)
> setup->sep = queue_pop_head(setup->eps);
> if (!setup->sep) {
> error("Unable to select a valid configuration");
> - queue_destroy(setup->eps, NULL);
> goto done;
> }
>
> @@ -2453,7 +2523,7 @@ static void select_cb(struct a2dp_setup *setup, void *ret, int size)
> err = setup->sep->endpoint->select_configuration(setup->sep,
> codec->data,
> service->length - sizeof(*codec),
> - setup_ref(setup),
> + setup,
> select_cb, setup->sep->user_data);
> if (err == 0)
> return;
> @@ -2493,11 +2563,11 @@ static struct queue *a2dp_find_eps(struct avdtp *session, GSList *list,
> seps = queue_new();
>
> /* Prepend last used so it is preferred over others */
> - if (chan->last_used && chan->last_used->sep == rsep)
> + if (chan->last_used && (chan->last_used->lsep == sep &&
> + chan->last_used->rsep->sep == rsep))
> queue_push_head(seps, sep);
> else
> queue_push_tail(seps, sep);
> -
> }
>
> return seps;
> @@ -2566,11 +2636,18 @@ static void store_remote_seps(struct a2dp_channel *chan)
> key_file = g_key_file_new();
> g_key_file_load_from_file(key_file, filename, 0, NULL);
>
> + data = g_key_file_get_string(key_file, "Endpoints", "LastUsed", NULL);
> +
> /* Remove current endpoints since it might have changed */
> g_key_file_remove_group(key_file, "Endpoints", NULL);
>
> queue_foreach(chan->seps, store_remote_sep, key_file);
>
> + if (data) {
> + g_key_file_set_string(key_file, "Endpoints", "LastUsed", data);
> + g_free(data);
> + }
> +
> data = g_key_file_to_data(key_file, &length, NULL);
> g_file_set_contents(filename, data, length, NULL);
>
> @@ -2656,10 +2733,12 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> codec = (struct avdtp_media_codec_capability *) service->data;
>
> err = setup->sep->endpoint->select_configuration(setup->sep,
> - codec->data,
> - service->length - sizeof(*codec),
> - setup_ref(setup),
> - select_cb, setup->sep->user_data);
> + codec->data,
> + service->length -
> + sizeof(*codec),
> + setup_ref(setup),
> + select_cb,
> + setup->sep->user_data);
> if (err == 0)
> return cb_data->id;
>
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index f4c9a2ed8..91b1e4b96 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -3710,6 +3710,11 @@ avdtp_state_t avdtp_sep_get_state(struct avdtp_local_sep *sep)
> return sep->state;
> }
>
> +uint8_t avdtp_sep_get_seid(struct avdtp_local_sep *sep)
> +{
> + return sep->info.seid;
> +}
> +
> struct btd_adapter *avdtp_get_adapter(struct avdtp *session)
> {
> return device_get_adapter(session->device);
> diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
> index b03ca9030..ad2cb9bcb 100644
> --- a/profiles/audio/avdtp.h
> +++ b/profiles/audio/avdtp.h
> @@ -296,6 +296,7 @@ struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep);
>
> avdtp_state_t avdtp_sep_get_state(struct avdtp_local_sep *sep);
> +uint8_t avdtp_sep_get_seid(struct avdtp_local_sep *sep);
>
> void avdtp_error_init(struct avdtp_error *err, uint8_t type, int id);
> const char *avdtp_strerror(struct avdtp_error *err);
> --
> 2.20.1
I hope this fixes the problems you have been seeing, it should at
least take care of the issues with wrong order of SelectConfiguration
and restoring the exact same endpoint used last time.
--
Luiz Augusto von Dentz
next prev parent reply other threads:[~2019-05-06 12:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-06 12:43 [PATCH v3 1/6] a2dp: Fix crash when endpoint respond with an error Luiz Augusto von Dentz
2019-05-06 12:43 ` [PATCH v3 2/6] a2dp: Try aborting when switching endpoints Luiz Augusto von Dentz
2019-05-06 12:43 ` [PATCH v3 3/6] a2dp: Update last used on open indication Luiz Augusto von Dentz
2019-05-06 12:43 ` [PATCH v3 4/6] a2dp: Fix reconfiguring when there multiple devices connected Luiz Augusto von Dentz
2019-05-06 12:43 ` [PATCH v3 5/6] a2dp: Fix useless statement Luiz Augusto von Dentz
2019-05-06 12:43 ` [PATCH v3 6/6] a2dp: Reword LastUsed Luiz Augusto von Dentz
2019-05-06 12:46 ` Luiz Augusto von Dentz [this message]
2019-05-06 13:02 ` Pali Rohár
2019-05-07 8:52 ` Pali Rohár
2019-05-07 10:13 ` Luiz Augusto von Dentz
2019-05-07 18:57 ` Pali Rohár
2019-06-07 13:04 ` Pali Rohár
2019-06-07 15:18 ` Luiz Augusto von Dentz
2019-06-10 12:35 ` Pali Rohár
2019-06-10 13:02 ` Luiz Augusto von Dentz
2019-05-07 10:17 ` [PATCH v3 1/6] a2dp: Fix crash when endpoint respond with an error Luiz Augusto von Dentz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABBYNZJ4+Be+sAXVAL_96qov5Go3BpRwmJtkdOVXC4apfL2K9A@mail.gmail.com \
--to=luiz.dentz@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=pali.rohar@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).