* [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
@ 2024-10-26 10:41 Pauli Virtanen
2024-10-26 12:38 ` [BlueZ] " bluez.test.bot
2024-10-28 14:39 ` [PATCH BlueZ] " Luiz Augusto von Dentz
0 siblings, 2 replies; 7+ messages in thread
From: Pauli Virtanen @ 2024-10-26 10:41 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
User can cancel transport acquire by calling Release() while Acquire()
is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
which is not desired.
Fix by instead waiting until START completes and then send SUSPEND. Make
Release() reply only after the whole sequence completes.
Fix also sending error reply to the canceled Acquire() call, which was
missing previously.
---
Notes:
In theory we could also send ABORT and reconfigure the SEP again after
that. It's more hairy though as with how the code currently works, we
may need to complete discovery first. This is a corner case anyway, so
just waiting a bit should be easier.
profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
1 file changed, 110 insertions(+), 42 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 0f7909a94..4d5afe022 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -88,6 +88,9 @@ struct a2dp_transport {
uint16_t delay;
int8_t volume;
guint watch;
+ guint resume_id;
+ gboolean cancel_resume;
+ guint cancel_id;
};
struct bap_transport {
@@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
return a2dp_sep_get_stream(sep);
}
+static void a2dp_suspend_complete(struct avdtp *session, int err,
+ void *user_data)
+{
+ struct media_owner *owner = user_data;
+ struct media_transport *transport = owner->transport;
+ struct a2dp_transport *a2dp = transport->data;
+ struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
+
+ /* Release always succeeds */
+ if (owner->pending) {
+ owner->pending->id = 0;
+ media_request_reply(owner->pending, 0);
+ media_owner_remove(owner);
+ }
+
+ a2dp_sep_unlock(sep, a2dp->session);
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ media_transport_remove_owner(transport);
+}
+
+static guint transport_a2dp_suspend(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ struct a2dp_transport *a2dp = transport->data;
+ struct media_endpoint *endpoint = transport->endpoint;
+ struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
+
+ if (a2dp->cancel_resume)
+ return a2dp->resume_id;
+
+ if (owner != NULL)
+ return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
+ owner);
+
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ a2dp_sep_unlock(sep, a2dp->session);
+
+ return 0;
+}
+
+static gboolean a2dp_cancel_resume_cb(void *user_data)
+{
+ struct media_owner *owner = user_data;
+ struct media_transport *transport = owner->transport;
+ struct a2dp_transport *a2dp = transport->data;
+
+ a2dp->cancel_id = 0;
+ a2dp->cancel_resume = FALSE;
+ owner->pending->id = transport_a2dp_suspend(transport, owner);
+ return FALSE;
+}
+
static void a2dp_resume_complete(struct avdtp *session, int err,
void *user_data)
{
struct media_owner *owner = user_data;
struct media_request *req = owner->pending;
struct media_transport *transport = owner->transport;
+ struct a2dp_transport *a2dp = transport->data;
struct avdtp_stream *stream;
int fd;
uint16_t imtu, omtu;
gboolean ret;
+ a2dp->resume_id = 0;
req->id = 0;
if (err)
goto fail;
+ if (a2dp->cancel_resume) {
+ DBG("cancel resume");
+ a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
+ return;
+ }
+
stream = transport_a2dp_get_stream(transport);
if (stream == NULL)
goto fail;
@@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
guint id;
+ if (a2dp->resume_id || a2dp->cancel_id)
+ return -EBUSY;
+
if (a2dp->session == NULL) {
a2dp->session = a2dp_avdtp_get(transport->device);
if (a2dp->session == NULL)
return 0;
}
- if (state_in_use(transport->state))
- return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
+ if (state_in_use(transport->state)) {
+ id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
owner);
+ a2dp->resume_id = id;
+ return id;
+ }
if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
return 0;
@@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
if (transport->state == TRANSPORT_STATE_IDLE)
transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
+ a2dp->resume_id = id;
return id;
}
-static void a2dp_suspend_complete(struct avdtp *session, int err,
- void *user_data)
-{
- struct media_owner *owner = user_data;
- struct media_transport *transport = owner->transport;
- struct a2dp_transport *a2dp = transport->data;
- struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
-
- /* Release always succeeds */
- if (owner->pending) {
- owner->pending->id = 0;
- media_request_reply(owner->pending, 0);
- media_owner_remove(owner);
- }
-
- a2dp_sep_unlock(sep, a2dp->session);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
- media_transport_remove_owner(transport);
-}
-
-static guint transport_a2dp_suspend(struct media_transport *transport,
- struct media_owner *owner)
-{
- struct a2dp_transport *a2dp = transport->data;
- struct media_endpoint *endpoint = transport->endpoint;
- struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
-
- if (owner != NULL)
- return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
- owner);
-
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
- a2dp_sep_unlock(sep, a2dp->session);
-
- return 0;
-}
-
static void transport_a2dp_cancel(struct media_transport *transport, guint id)
{
+ struct a2dp_transport *a2dp = transport->data;
+
+ if (a2dp->resume_id && a2dp->resume_id == id) {
+ /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
+ * START can be triggered by user via Release(), and it's better
+ * to not drop the A2DP connection then, so we just suspend
+ * after resume completes.
+ */
+ a2dp->cancel_resume = TRUE;
+ return;
+ }
+
a2dp_cancel(id);
}
+static void transport_a2dp_remove_owner(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ struct a2dp_transport *a2dp = transport->data;
+
+ /* Clean up callbacks that refer to the owner */
+
+ if (a2dp->cancel_id) {
+ g_source_remove(a2dp->cancel_id);
+ a2dp->cancel_id = 0;
+ }
+
+ if (a2dp->resume_id) {
+ a2dp_cancel(a2dp->resume_id);
+ a2dp->resume_id = 0;
+ }
+
+ a2dp->cancel_resume = FALSE;
+}
+
static int8_t transport_a2dp_get_volume(struct media_transport *transport)
{
struct a2dp_transport *a2dp = transport->data;
@@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
member = dbus_message_get_member(owner->pending->msg);
/* Cancel Acquire request if that exist */
- if (g_str_equal(member, "Acquire"))
+ if (g_str_equal(member, "Acquire")) {
+ media_request_reply(owner->pending, ECANCELED);
media_owner_remove(owner);
- else
+ } else {
return btd_error_in_progress(msg);
+ }
}
transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
@@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
}
#define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
- TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
+ TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
+ transport_a2dp_remove_owner, _init, \
transport_a2dp_resume, transport_a2dp_suspend, \
transport_a2dp_cancel, NULL, \
transport_a2dp_get_stream, transport_a2dp_get_volume, \
--
2.47.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-26 10:41 [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release() Pauli Virtanen
@ 2024-10-26 12:38 ` bluez.test.bot
2024-10-28 14:39 ` [PATCH BlueZ] " Luiz Augusto von Dentz
1 sibling, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2024-10-26 12:38 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 949 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=903413
---Test result---
Test Summary:
CheckPatch PASS 0.54 seconds
GitLint PASS 0.32 seconds
BuildEll PASS 24.55 seconds
BluezMake PASS 1617.77 seconds
MakeCheck PASS 13.75 seconds
MakeDistcheck PASS 179.49 seconds
CheckValgrind PASS 251.77 seconds
CheckSmatch PASS 354.37 seconds
bluezmakeextell PASS 120.08 seconds
IncrementalBuild PASS 1441.58 seconds
ScanBuild PASS 1020.25 seconds
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-26 10:41 [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release() Pauli Virtanen
2024-10-26 12:38 ` [BlueZ] " bluez.test.bot
@ 2024-10-28 14:39 ` Luiz Augusto von Dentz
2024-10-28 17:48 ` Pauli Virtanen
1 sibling, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-28 14:39 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hi Pauli,
On Sat, Oct 26, 2024 at 6:41 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> User can cancel transport acquire by calling Release() while Acquire()
> is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
> forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
> which is not desired.
>
> Fix by instead waiting until START completes and then send SUSPEND. Make
> Release() reply only after the whole sequence completes.
Hmm, but assumes the client is not reconfiguring to another endpoint
or just giving up on the transport since it doesn't intend to use it
anymore, anyway we can't really send anything else other than abort if
we don't want to wait so I think this would be better to be handled by
the application if it intends to suspend then it should wait acquire
to complete and then release.
> Fix also sending error reply to the canceled Acquire() call, which was
> missing previously.
> ---
>
> Notes:
> In theory we could also send ABORT and reconfigure the SEP again after
> that. It's more hairy though as with how the code currently works, we
> may need to complete discovery first. This is a corner case anyway, so
> just waiting a bit should be easier.
>
> profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
> 1 file changed, 110 insertions(+), 42 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 0f7909a94..4d5afe022 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -88,6 +88,9 @@ struct a2dp_transport {
> uint16_t delay;
> int8_t volume;
> guint watch;
> + guint resume_id;
> + gboolean cancel_resume;
> + guint cancel_id;
> };
>
> struct bap_transport {
> @@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
> return a2dp_sep_get_stream(sep);
> }
>
> +static void a2dp_suspend_complete(struct avdtp *session, int err,
> + void *user_data)
> +{
> + struct media_owner *owner = user_data;
> + struct media_transport *transport = owner->transport;
> + struct a2dp_transport *a2dp = transport->data;
> + struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> +
> + /* Release always succeeds */
> + if (owner->pending) {
> + owner->pending->id = 0;
> + media_request_reply(owner->pending, 0);
> + media_owner_remove(owner);
> + }
> +
> + a2dp_sep_unlock(sep, a2dp->session);
> + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> + media_transport_remove_owner(transport);
> +}
> +
> +static guint transport_a2dp_suspend(struct media_transport *transport,
> + struct media_owner *owner)
> +{
> + struct a2dp_transport *a2dp = transport->data;
> + struct media_endpoint *endpoint = transport->endpoint;
> + struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> +
> + if (a2dp->cancel_resume)
> + return a2dp->resume_id;
> +
> + if (owner != NULL)
> + return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> + owner);
> +
> + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> + a2dp_sep_unlock(sep, a2dp->session);
> +
> + return 0;
> +}
> +
> +static gboolean a2dp_cancel_resume_cb(void *user_data)
> +{
> + struct media_owner *owner = user_data;
> + struct media_transport *transport = owner->transport;
> + struct a2dp_transport *a2dp = transport->data;
> +
> + a2dp->cancel_id = 0;
> + a2dp->cancel_resume = FALSE;
> + owner->pending->id = transport_a2dp_suspend(transport, owner);
> + return FALSE;
> +}
> +
> static void a2dp_resume_complete(struct avdtp *session, int err,
> void *user_data)
> {
> struct media_owner *owner = user_data;
> struct media_request *req = owner->pending;
> struct media_transport *transport = owner->transport;
> + struct a2dp_transport *a2dp = transport->data;
> struct avdtp_stream *stream;
> int fd;
> uint16_t imtu, omtu;
> gboolean ret;
>
> + a2dp->resume_id = 0;
> req->id = 0;
>
> if (err)
> goto fail;
>
> + if (a2dp->cancel_resume) {
> + DBG("cancel resume");
> + a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
> + return;
> + }
> +
> stream = transport_a2dp_get_stream(transport);
> if (stream == NULL)
> goto fail;
> @@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> guint id;
>
> + if (a2dp->resume_id || a2dp->cancel_id)
> + return -EBUSY;
> +
> if (a2dp->session == NULL) {
> a2dp->session = a2dp_avdtp_get(transport->device);
> if (a2dp->session == NULL)
> return 0;
> }
>
> - if (state_in_use(transport->state))
> - return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> + if (state_in_use(transport->state)) {
> + id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> owner);
> + a2dp->resume_id = id;
> + return id;
> + }
>
> if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
> return 0;
> @@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> if (transport->state == TRANSPORT_STATE_IDLE)
> transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
>
> + a2dp->resume_id = id;
> return id;
> }
>
> -static void a2dp_suspend_complete(struct avdtp *session, int err,
> - void *user_data)
> -{
> - struct media_owner *owner = user_data;
> - struct media_transport *transport = owner->transport;
> - struct a2dp_transport *a2dp = transport->data;
> - struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> -
> - /* Release always succeeds */
> - if (owner->pending) {
> - owner->pending->id = 0;
> - media_request_reply(owner->pending, 0);
> - media_owner_remove(owner);
> - }
> -
> - a2dp_sep_unlock(sep, a2dp->session);
> - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> - media_transport_remove_owner(transport);
> -}
> -
> -static guint transport_a2dp_suspend(struct media_transport *transport,
> - struct media_owner *owner)
> -{
> - struct a2dp_transport *a2dp = transport->data;
> - struct media_endpoint *endpoint = transport->endpoint;
> - struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> -
> - if (owner != NULL)
> - return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> - owner);
> -
> - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> - a2dp_sep_unlock(sep, a2dp->session);
> -
> - return 0;
> -}
> -
> static void transport_a2dp_cancel(struct media_transport *transport, guint id)
> {
> + struct a2dp_transport *a2dp = transport->data;
> +
> + if (a2dp->resume_id && a2dp->resume_id == id) {
> + /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
> + * START can be triggered by user via Release(), and it's better
> + * to not drop the A2DP connection then, so we just suspend
> + * after resume completes.
> + */
> + a2dp->cancel_resume = TRUE;
> + return;
> + }
> +
> a2dp_cancel(id);
> }
>
> +static void transport_a2dp_remove_owner(struct media_transport *transport,
> + struct media_owner *owner)
> +{
> + struct a2dp_transport *a2dp = transport->data;
> +
> + /* Clean up callbacks that refer to the owner */
> +
> + if (a2dp->cancel_id) {
> + g_source_remove(a2dp->cancel_id);
> + a2dp->cancel_id = 0;
> + }
> +
> + if (a2dp->resume_id) {
> + a2dp_cancel(a2dp->resume_id);
> + a2dp->resume_id = 0;
> + }
> +
> + a2dp->cancel_resume = FALSE;
> +}
> +
> static int8_t transport_a2dp_get_volume(struct media_transport *transport)
> {
> struct a2dp_transport *a2dp = transport->data;
> @@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
>
> member = dbus_message_get_member(owner->pending->msg);
> /* Cancel Acquire request if that exist */
> - if (g_str_equal(member, "Acquire"))
> + if (g_str_equal(member, "Acquire")) {
> + media_request_reply(owner->pending, ECANCELED);
> media_owner_remove(owner);
> - else
> + } else {
> return btd_error_in_progress(msg);
> + }
> }
>
> transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
> @@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
> }
>
> #define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
> - TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
> + TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
> + transport_a2dp_remove_owner, _init, \
> transport_a2dp_resume, transport_a2dp_suspend, \
> transport_a2dp_cancel, NULL, \
> transport_a2dp_get_stream, transport_a2dp_get_volume, \
> --
> 2.47.0
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-28 14:39 ` [PATCH BlueZ] " Luiz Augusto von Dentz
@ 2024-10-28 17:48 ` Pauli Virtanen
2024-10-30 20:56 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2024-10-28 17:48 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
ma, 2024-10-28 kello 10:39 -0400, Luiz Augusto von Dentz kirjoitti:
> On Sat, Oct 26, 2024 at 6:41 AM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > User can cancel transport acquire by calling Release() while Acquire()
> > is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
> > forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
> > which is not desired.
> >
> > Fix by instead waiting until START completes and then send SUSPEND. Make
> > Release() reply only after the whole sequence completes.
>
> Hmm, but assumes the client is not reconfiguring to another endpoint
> or just giving up on the transport since it doesn't intend to use it
> anymore, anyway we can't really send anything else other than abort if
> we don't want to wait so I think this would be better to be handled by
I guess you have in mind
Acquire(t1)
Release(t1) # canceling acquire
SetConfiguration(ep2)
I don't think this works right in current BlueZ master branch either,
the Release() results to removing the DBus endpoints & AVDTP
disconnect, so nothing to SetConfigure() after abort completes.
SetConfiguration() probably fails also if called while Abort_Cfm is
pending, since it cannot then close the existing stream.
I think (but did not test) that with the patch here, it would end up
doing START -> CLOSE -> reconfigure, so indeed has to wait for START
completion.
> the application if it intends to suspend then it should wait acquire
> to complete and then release.
I think it's a bug in BlueZ that calling Release() on transport result
to disconnect of A2DP, instead of suspend.
I have another version of this patch that does send ABORT, but recalls
it was due to canceled suspend and reconnects the sink/source when
getting to IDLE.
> > Fix also sending error reply to the canceled Acquire() call, which was
> > missing previously.
> > ---
> >
> > Notes:
> > In theory we could also send ABORT and reconfigure the SEP again after
> > that. It's more hairy though as with how the code currently works, we
> > may need to complete discovery first. This is a corner case anyway, so
> > just waiting a bit should be easier.
> >
> > profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
> > 1 file changed, 110 insertions(+), 42 deletions(-)
> >
> > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > index 0f7909a94..4d5afe022 100644
> > --- a/profiles/audio/transport.c
> > +++ b/profiles/audio/transport.c
> > @@ -88,6 +88,9 @@ struct a2dp_transport {
> > uint16_t delay;
> > int8_t volume;
> > guint watch;
> > + guint resume_id;
> > + gboolean cancel_resume;
> > + guint cancel_id;
> > };
> >
> > struct bap_transport {
> > @@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
> > return a2dp_sep_get_stream(sep);
> > }
> >
> > +static void a2dp_suspend_complete(struct avdtp *session, int err,
> > + void *user_data)
> > +{
> > + struct media_owner *owner = user_data;
> > + struct media_transport *transport = owner->transport;
> > + struct a2dp_transport *a2dp = transport->data;
> > + struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > +
> > + /* Release always succeeds */
> > + if (owner->pending) {
> > + owner->pending->id = 0;
> > + media_request_reply(owner->pending, 0);
> > + media_owner_remove(owner);
> > + }
> > +
> > + a2dp_sep_unlock(sep, a2dp->session);
> > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > + media_transport_remove_owner(transport);
> > +}
> > +
> > +static guint transport_a2dp_suspend(struct media_transport *transport,
> > + struct media_owner *owner)
> > +{
> > + struct a2dp_transport *a2dp = transport->data;
> > + struct media_endpoint *endpoint = transport->endpoint;
> > + struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > +
> > + if (a2dp->cancel_resume)
> > + return a2dp->resume_id;
> > +
> > + if (owner != NULL)
> > + return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > + owner);
> > +
> > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > + a2dp_sep_unlock(sep, a2dp->session);
> > +
> > + return 0;
> > +}
> > +
> > +static gboolean a2dp_cancel_resume_cb(void *user_data)
> > +{
> > + struct media_owner *owner = user_data;
> > + struct media_transport *transport = owner->transport;
> > + struct a2dp_transport *a2dp = transport->data;
> > +
> > + a2dp->cancel_id = 0;
> > + a2dp->cancel_resume = FALSE;
> > + owner->pending->id = transport_a2dp_suspend(transport, owner);
> > + return FALSE;
> > +}
> > +
> > static void a2dp_resume_complete(struct avdtp *session, int err,
> > void *user_data)
> > {
> > struct media_owner *owner = user_data;
> > struct media_request *req = owner->pending;
> > struct media_transport *transport = owner->transport;
> > + struct a2dp_transport *a2dp = transport->data;
> > struct avdtp_stream *stream;
> > int fd;
> > uint16_t imtu, omtu;
> > gboolean ret;
> >
> > + a2dp->resume_id = 0;
> > req->id = 0;
> >
> > if (err)
> > goto fail;
> >
> > + if (a2dp->cancel_resume) {
> > + DBG("cancel resume");
> > + a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
> > + return;
> > + }
> > +
> > stream = transport_a2dp_get_stream(transport);
> > if (stream == NULL)
> > goto fail;
> > @@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > guint id;
> >
> > + if (a2dp->resume_id || a2dp->cancel_id)
> > + return -EBUSY;
> > +
> > if (a2dp->session == NULL) {
> > a2dp->session = a2dp_avdtp_get(transport->device);
> > if (a2dp->session == NULL)
> > return 0;
> > }
> >
> > - if (state_in_use(transport->state))
> > - return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > + if (state_in_use(transport->state)) {
> > + id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > owner);
> > + a2dp->resume_id = id;
> > + return id;
> > + }
> >
> > if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
> > return 0;
> > @@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > if (transport->state == TRANSPORT_STATE_IDLE)
> > transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
> >
> > + a2dp->resume_id = id;
> > return id;
> > }
> >
> > -static void a2dp_suspend_complete(struct avdtp *session, int err,
> > - void *user_data)
> > -{
> > - struct media_owner *owner = user_data;
> > - struct media_transport *transport = owner->transport;
> > - struct a2dp_transport *a2dp = transport->data;
> > - struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > -
> > - /* Release always succeeds */
> > - if (owner->pending) {
> > - owner->pending->id = 0;
> > - media_request_reply(owner->pending, 0);
> > - media_owner_remove(owner);
> > - }
> > -
> > - a2dp_sep_unlock(sep, a2dp->session);
> > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > - media_transport_remove_owner(transport);
> > -}
> > -
> > -static guint transport_a2dp_suspend(struct media_transport *transport,
> > - struct media_owner *owner)
> > -{
> > - struct a2dp_transport *a2dp = transport->data;
> > - struct media_endpoint *endpoint = transport->endpoint;
> > - struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > -
> > - if (owner != NULL)
> > - return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > - owner);
> > -
> > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > - a2dp_sep_unlock(sep, a2dp->session);
> > -
> > - return 0;
> > -}
> > -
> > static void transport_a2dp_cancel(struct media_transport *transport, guint id)
> > {
> > + struct a2dp_transport *a2dp = transport->data;
> > +
> > + if (a2dp->resume_id && a2dp->resume_id == id) {
> > + /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
> > + * START can be triggered by user via Release(), and it's better
> > + * to not drop the A2DP connection then, so we just suspend
> > + * after resume completes.
> > + */
> > + a2dp->cancel_resume = TRUE;
> > + return;
> > + }
> > +
> > a2dp_cancel(id);
> > }
> >
> > +static void transport_a2dp_remove_owner(struct media_transport *transport,
> > + struct media_owner *owner)
> > +{
> > + struct a2dp_transport *a2dp = transport->data;
> > +
> > + /* Clean up callbacks that refer to the owner */
> > +
> > + if (a2dp->cancel_id) {
> > + g_source_remove(a2dp->cancel_id);
> > + a2dp->cancel_id = 0;
> > + }
> > +
> > + if (a2dp->resume_id) {
> > + a2dp_cancel(a2dp->resume_id);
> > + a2dp->resume_id = 0;
> > + }
> > +
> > + a2dp->cancel_resume = FALSE;
> > +}
> > +
> > static int8_t transport_a2dp_get_volume(struct media_transport *transport)
> > {
> > struct a2dp_transport *a2dp = transport->data;
> > @@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
> >
> > member = dbus_message_get_member(owner->pending->msg);
> > /* Cancel Acquire request if that exist */
> > - if (g_str_equal(member, "Acquire"))
> > + if (g_str_equal(member, "Acquire")) {
> > + media_request_reply(owner->pending, ECANCELED);
> > media_owner_remove(owner);
> > - else
> > + } else {
> > return btd_error_in_progress(msg);
> > + }
> > }
> >
> > transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
> > @@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
> > }
> >
> > #define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
> > - TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
> > + TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
> > + transport_a2dp_remove_owner, _init, \
> > transport_a2dp_resume, transport_a2dp_suspend, \
> > transport_a2dp_cancel, NULL, \
> > transport_a2dp_get_stream, transport_a2dp_get_volume, \
> > --
> > 2.47.0
> >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-28 17:48 ` Pauli Virtanen
@ 2024-10-30 20:56 ` Luiz Augusto von Dentz
2024-10-30 21:46 ` Pauli Virtanen
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2024-10-30 20:56 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hi Pauli,
On Mon, Oct 28, 2024 at 1:48 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> ma, 2024-10-28 kello 10:39 -0400, Luiz Augusto von Dentz kirjoitti:
> > On Sat, Oct 26, 2024 at 6:41 AM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > User can cancel transport acquire by calling Release() while Acquire()
> > > is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
> > > forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
> > > which is not desired.
> > >
> > > Fix by instead waiting until START completes and then send SUSPEND. Make
> > > Release() reply only after the whole sequence completes.
> >
> > Hmm, but assumes the client is not reconfiguring to another endpoint
> > or just giving up on the transport since it doesn't intend to use it
> > anymore, anyway we can't really send anything else other than abort if
> > we don't want to wait so I think this would be better to be handled by
>
> I guess you have in mind
>
> Acquire(t1)
> Release(t1) # canceling acquire
> SetConfiguration(ep2)
>
> I don't think this works right in current BlueZ master branch either,
> the Release() results to removing the DBus endpoints & AVDTP
> disconnect, so nothing to SetConfigure() after abort completes.
> SetConfiguration() probably fails also if called while Abort_Cfm is
> pending, since it cannot then close the existing stream.
>
> I think (but did not test) that with the patch here, it would end up
> doing START -> CLOSE -> reconfigure, so indeed has to wait for START
> completion.
I guess you are saying that we don't want the stream to transit to
idle since that would cause the AVDTP session to be disconnected? It
seems this is the result of session->dc_timeout being set to 0 on
avdtp_abort, anyway now looking a little more in detail it seems
clearer that the transport methods shouldn't affect the state machine
other than for suspend/resume procedures, that said then we need to
check what happens if those methods are called in quick succession e.g
Acquire -> Release -> Acquire.
> > the application if it intends to suspend then it should wait acquire
> > to complete and then release.
>
> I think it's a bug in BlueZ that calling Release() on transport result
> to disconnect of A2DP, instead of suspend.
>
> I have another version of this patch that does send ABORT, but recalls
> it was due to canceled suspend and reconnects the sink/source when
> getting to IDLE.
>
> > > Fix also sending error reply to the canceled Acquire() call, which was
> > > missing previously.
> > > ---
> > >
> > > Notes:
> > > In theory we could also send ABORT and reconfigure the SEP again after
> > > that. It's more hairy though as with how the code currently works, we
> > > may need to complete discovery first. This is a corner case anyway, so
> > > just waiting a bit should be easier.
> > >
> > > profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
> > > 1 file changed, 110 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > > index 0f7909a94..4d5afe022 100644
> > > --- a/profiles/audio/transport.c
> > > +++ b/profiles/audio/transport.c
> > > @@ -88,6 +88,9 @@ struct a2dp_transport {
> > > uint16_t delay;
> > > int8_t volume;
> > > guint watch;
> > > + guint resume_id;
> > > + gboolean cancel_resume;
> > > + guint cancel_id;
> > > };
> > >
> > > struct bap_transport {
> > > @@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
> > > return a2dp_sep_get_stream(sep);
> > > }
> > >
> > > +static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > + void *user_data)
> > > +{
> > > + struct media_owner *owner = user_data;
> > > + struct media_transport *transport = owner->transport;
> > > + struct a2dp_transport *a2dp = transport->data;
> > > + struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > +
> > > + /* Release always succeeds */
> > > + if (owner->pending) {
> > > + owner->pending->id = 0;
> > > + media_request_reply(owner->pending, 0);
> > > + media_owner_remove(owner);
> > > + }
> > > +
> > > + a2dp_sep_unlock(sep, a2dp->session);
> > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > + media_transport_remove_owner(transport);
> > > +}
> > > +
> > > +static guint transport_a2dp_suspend(struct media_transport *transport,
> > > + struct media_owner *owner)
> > > +{
> > > + struct a2dp_transport *a2dp = transport->data;
> > > + struct media_endpoint *endpoint = transport->endpoint;
> > > + struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > +
> > > + if (a2dp->cancel_resume)
> > > + return a2dp->resume_id;
> > > +
> > > + if (owner != NULL)
> > > + return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > + owner);
> > > +
> > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > + a2dp_sep_unlock(sep, a2dp->session);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static gboolean a2dp_cancel_resume_cb(void *user_data)
> > > +{
> > > + struct media_owner *owner = user_data;
> > > + struct media_transport *transport = owner->transport;
> > > + struct a2dp_transport *a2dp = transport->data;
> > > +
> > > + a2dp->cancel_id = 0;
> > > + a2dp->cancel_resume = FALSE;
> > > + owner->pending->id = transport_a2dp_suspend(transport, owner);
> > > + return FALSE;
> > > +}
> > > +
> > > static void a2dp_resume_complete(struct avdtp *session, int err,
> > > void *user_data)
> > > {
> > > struct media_owner *owner = user_data;
> > > struct media_request *req = owner->pending;
> > > struct media_transport *transport = owner->transport;
> > > + struct a2dp_transport *a2dp = transport->data;
> > > struct avdtp_stream *stream;
> > > int fd;
> > > uint16_t imtu, omtu;
> > > gboolean ret;
> > >
> > > + a2dp->resume_id = 0;
> > > req->id = 0;
> > >
> > > if (err)
> > > goto fail;
> > >
> > > + if (a2dp->cancel_resume) {
> > > + DBG("cancel resume");
> > > + a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
> > > + return;
> > > + }
> > > +
> > > stream = transport_a2dp_get_stream(transport);
> > > if (stream == NULL)
> > > goto fail;
> > > @@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > guint id;
> > >
> > > + if (a2dp->resume_id || a2dp->cancel_id)
> > > + return -EBUSY;
> > > +
> > > if (a2dp->session == NULL) {
> > > a2dp->session = a2dp_avdtp_get(transport->device);
> > > if (a2dp->session == NULL)
> > > return 0;
> > > }
> > >
> > > - if (state_in_use(transport->state))
> > > - return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > + if (state_in_use(transport->state)) {
> > > + id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > owner);
> > > + a2dp->resume_id = id;
> > > + return id;
> > > + }
> > >
> > > if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
> > > return 0;
> > > @@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > if (transport->state == TRANSPORT_STATE_IDLE)
> > > transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
> > >
> > > + a2dp->resume_id = id;
> > > return id;
> > > }
> > >
> > > -static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > - void *user_data)
> > > -{
> > > - struct media_owner *owner = user_data;
> > > - struct media_transport *transport = owner->transport;
> > > - struct a2dp_transport *a2dp = transport->data;
> > > - struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > -
> > > - /* Release always succeeds */
> > > - if (owner->pending) {
> > > - owner->pending->id = 0;
> > > - media_request_reply(owner->pending, 0);
> > > - media_owner_remove(owner);
> > > - }
> > > -
> > > - a2dp_sep_unlock(sep, a2dp->session);
> > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > - media_transport_remove_owner(transport);
> > > -}
> > > -
> > > -static guint transport_a2dp_suspend(struct media_transport *transport,
> > > - struct media_owner *owner)
> > > -{
> > > - struct a2dp_transport *a2dp = transport->data;
> > > - struct media_endpoint *endpoint = transport->endpoint;
> > > - struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > -
> > > - if (owner != NULL)
> > > - return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > - owner);
> > > -
> > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > - a2dp_sep_unlock(sep, a2dp->session);
> > > -
> > > - return 0;
> > > -}
> > > -
> > > static void transport_a2dp_cancel(struct media_transport *transport, guint id)
> > > {
> > > + struct a2dp_transport *a2dp = transport->data;
> > > +
> > > + if (a2dp->resume_id && a2dp->resume_id == id) {
> > > + /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
> > > + * START can be triggered by user via Release(), and it's better
> > > + * to not drop the A2DP connection then, so we just suspend
> > > + * after resume completes.
> > > + */
> > > + a2dp->cancel_resume = TRUE;
> > > + return;
> > > + }
> > > +
> > > a2dp_cancel(id);
> > > }
> > >
> > > +static void transport_a2dp_remove_owner(struct media_transport *transport,
> > > + struct media_owner *owner)
> > > +{
> > > + struct a2dp_transport *a2dp = transport->data;
> > > +
> > > + /* Clean up callbacks that refer to the owner */
> > > +
> > > + if (a2dp->cancel_id) {
> > > + g_source_remove(a2dp->cancel_id);
> > > + a2dp->cancel_id = 0;
> > > + }
> > > +
> > > + if (a2dp->resume_id) {
> > > + a2dp_cancel(a2dp->resume_id);
> > > + a2dp->resume_id = 0;
> > > + }
> > > +
> > > + a2dp->cancel_resume = FALSE;
> > > +}
> > > +
> > > static int8_t transport_a2dp_get_volume(struct media_transport *transport)
> > > {
> > > struct a2dp_transport *a2dp = transport->data;
> > > @@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
> > >
> > > member = dbus_message_get_member(owner->pending->msg);
> > > /* Cancel Acquire request if that exist */
> > > - if (g_str_equal(member, "Acquire"))
> > > + if (g_str_equal(member, "Acquire")) {
> > > + media_request_reply(owner->pending, ECANCELED);
> > > media_owner_remove(owner);
> > > - else
> > > + } else {
> > > return btd_error_in_progress(msg);
> > > + }
> > > }
> > >
> > > transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
> > > @@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
> > > }
> > >
> > > #define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
> > > - TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
> > > + TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
> > > + transport_a2dp_remove_owner, _init, \
> > > transport_a2dp_resume, transport_a2dp_suspend, \
> > > transport_a2dp_cancel, NULL, \
> > > transport_a2dp_get_stream, transport_a2dp_get_volume, \
> > > --
> > > 2.47.0
> > >
> > >
> >
> >
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-30 20:56 ` Luiz Augusto von Dentz
@ 2024-10-30 21:46 ` Pauli Virtanen
2024-10-30 22:03 ` Pauli Virtanen
0 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2024-10-30 21:46 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
ke, 2024-10-30 kello 16:56 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Mon, Oct 28, 2024 at 1:48 PM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Hi Luiz,
> >
> > ma, 2024-10-28 kello 10:39 -0400, Luiz Augusto von Dentz kirjoitti:
> > > On Sat, Oct 26, 2024 at 6:41 AM Pauli Virtanen <pav@iki.fi> wrote:
> > > >
> > > > User can cancel transport acquire by calling Release() while Acquire()
> > > > is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
> > > > forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
> > > > which is not desired.
> > > >
> > > > Fix by instead waiting until START completes and then send SUSPEND. Make
> > > > Release() reply only after the whole sequence completes.
> > >
> > > Hmm, but assumes the client is not reconfiguring to another endpoint
> > > or just giving up on the transport since it doesn't intend to use it
> > > anymore, anyway we can't really send anything else other than abort if
> > > we don't want to wait so I think this would be better to be handled by
> >
> > I guess you have in mind
> >
> > Acquire(t1)
> > Release(t1) # canceling acquire
> > SetConfiguration(ep2)
> >
> > I don't think this works right in current BlueZ master branch either,
> > the Release() results to removing the DBus endpoints & AVDTP
> > disconnect, so nothing to SetConfigure() after abort completes.
> > SetConfiguration() probably fails also if called while Abort_Cfm is
> > pending, since it cannot then close the existing stream.
> >
> > I think (but did not test) that with the patch here, it would end up
> > doing START -> CLOSE -> reconfigure, so indeed has to wait for START
> > completion.
>
> I guess you are saying that we don't want the stream to transit to
> idle since that would cause the AVDTP session to be disconnected? It
> seems this is the result of session->dc_timeout being set to 0 on
> avdtp_abort, anyway now looking a little more in detail it seems
> clearer that the transport methods shouldn't affect the state machine
> other than for suspend/resume procedures, that said then we need to
> check what happens if those methods are called in quick succession e.g
> Acquire -> Release -> Acquire.
It should be possible to make it work so that SetConfiguration() while
either Acquire() or Release() is pending sends ABORT if needed, but
Acquire()+Release() alone only does OPEN->STREAMING->OPEN.
The code that closes/aborts the existing stream in a2dp:a2dp_reconfig()
may need some tuning, I'll need to test this race condition to make
sure.
> > > the application if it intends to suspend then it should wait acquire
> > > to complete and then release.
> >
> > I think it's a bug in BlueZ that calling Release() on transport result
> > to disconnect of A2DP, instead of suspend.
> >
> > I have another version of this patch that does send ABORT, but recalls
> > it was due to canceled suspend and reconnects the sink/source when
> > getting to IDLE.
> >
> > > > Fix also sending error reply to the canceled Acquire() call, which was
> > > > missing previously.
> > > > ---
> > > >
> > > > Notes:
> > > > In theory we could also send ABORT and reconfigure the SEP again after
> > > > that. It's more hairy though as with how the code currently works, we
> > > > may need to complete discovery first. This is a corner case anyway, so
> > > > just waiting a bit should be easier.
> > > >
> > > > profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
> > > > 1 file changed, 110 insertions(+), 42 deletions(-)
> > > >
> > > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > > > index 0f7909a94..4d5afe022 100644
> > > > --- a/profiles/audio/transport.c
> > > > +++ b/profiles/audio/transport.c
> > > > @@ -88,6 +88,9 @@ struct a2dp_transport {
> > > > uint16_t delay;
> > > > int8_t volume;
> > > > guint watch;
> > > > + guint resume_id;
> > > > + gboolean cancel_resume;
> > > > + guint cancel_id;
> > > > };
> > > >
> > > > struct bap_transport {
> > > > @@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
> > > > return a2dp_sep_get_stream(sep);
> > > > }
> > > >
> > > > +static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > > + void *user_data)
> > > > +{
> > > > + struct media_owner *owner = user_data;
> > > > + struct media_transport *transport = owner->transport;
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > + struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > > +
> > > > + /* Release always succeeds */
> > > > + if (owner->pending) {
> > > > + owner->pending->id = 0;
> > > > + media_request_reply(owner->pending, 0);
> > > > + media_owner_remove(owner);
> > > > + }
> > > > +
> > > > + a2dp_sep_unlock(sep, a2dp->session);
> > > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > + media_transport_remove_owner(transport);
> > > > +}
> > > > +
> > > > +static guint transport_a2dp_suspend(struct media_transport *transport,
> > > > + struct media_owner *owner)
> > > > +{
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > + struct media_endpoint *endpoint = transport->endpoint;
> > > > + struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > +
> > > > + if (a2dp->cancel_resume)
> > > > + return a2dp->resume_id;
> > > > +
> > > > + if (owner != NULL)
> > > > + return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > > + owner);
> > > > +
> > > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > + a2dp_sep_unlock(sep, a2dp->session);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static gboolean a2dp_cancel_resume_cb(void *user_data)
> > > > +{
> > > > + struct media_owner *owner = user_data;
> > > > + struct media_transport *transport = owner->transport;
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > +
> > > > + a2dp->cancel_id = 0;
> > > > + a2dp->cancel_resume = FALSE;
> > > > + owner->pending->id = transport_a2dp_suspend(transport, owner);
> > > > + return FALSE;
> > > > +}
> > > > +
> > > > static void a2dp_resume_complete(struct avdtp *session, int err,
> > > > void *user_data)
> > > > {
> > > > struct media_owner *owner = user_data;
> > > > struct media_request *req = owner->pending;
> > > > struct media_transport *transport = owner->transport;
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > struct avdtp_stream *stream;
> > > > int fd;
> > > > uint16_t imtu, omtu;
> > > > gboolean ret;
> > > >
> > > > + a2dp->resume_id = 0;
> > > > req->id = 0;
> > > >
> > > > if (err)
> > > > goto fail;
> > > >
> > > > + if (a2dp->cancel_resume) {
> > > > + DBG("cancel resume");
> > > > + a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
> > > > + return;
> > > > + }
> > > > +
> > > > stream = transport_a2dp_get_stream(transport);
> > > > if (stream == NULL)
> > > > goto fail;
> > > > @@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > > struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > guint id;
> > > >
> > > > + if (a2dp->resume_id || a2dp->cancel_id)
> > > > + return -EBUSY;
> > > > +
> > > > if (a2dp->session == NULL) {
> > > > a2dp->session = a2dp_avdtp_get(transport->device);
> > > > if (a2dp->session == NULL)
> > > > return 0;
> > > > }
> > > >
> > > > - if (state_in_use(transport->state))
> > > > - return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > > + if (state_in_use(transport->state)) {
> > > > + id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > > owner);
> > > > + a2dp->resume_id = id;
> > > > + return id;
> > > > + }
> > > >
> > > > if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
> > > > return 0;
> > > > @@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > > if (transport->state == TRANSPORT_STATE_IDLE)
> > > > transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
> > > >
> > > > + a2dp->resume_id = id;
> > > > return id;
> > > > }
> > > >
> > > > -static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > > - void *user_data)
> > > > -{
> > > > - struct media_owner *owner = user_data;
> > > > - struct media_transport *transport = owner->transport;
> > > > - struct a2dp_transport *a2dp = transport->data;
> > > > - struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > > -
> > > > - /* Release always succeeds */
> > > > - if (owner->pending) {
> > > > - owner->pending->id = 0;
> > > > - media_request_reply(owner->pending, 0);
> > > > - media_owner_remove(owner);
> > > > - }
> > > > -
> > > > - a2dp_sep_unlock(sep, a2dp->session);
> > > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > - media_transport_remove_owner(transport);
> > > > -}
> > > > -
> > > > -static guint transport_a2dp_suspend(struct media_transport *transport,
> > > > - struct media_owner *owner)
> > > > -{
> > > > - struct a2dp_transport *a2dp = transport->data;
> > > > - struct media_endpoint *endpoint = transport->endpoint;
> > > > - struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > -
> > > > - if (owner != NULL)
> > > > - return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > > - owner);
> > > > -
> > > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > - a2dp_sep_unlock(sep, a2dp->session);
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > static void transport_a2dp_cancel(struct media_transport *transport, guint id)
> > > > {
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > +
> > > > + if (a2dp->resume_id && a2dp->resume_id == id) {
> > > > + /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
> > > > + * START can be triggered by user via Release(), and it's better
> > > > + * to not drop the A2DP connection then, so we just suspend
> > > > + * after resume completes.
> > > > + */
> > > > + a2dp->cancel_resume = TRUE;
> > > > + return;
> > > > + }
> > > > +
> > > > a2dp_cancel(id);
> > > > }
> > > >
> > > > +static void transport_a2dp_remove_owner(struct media_transport *transport,
> > > > + struct media_owner *owner)
> > > > +{
> > > > + struct a2dp_transport *a2dp = transport->data;
> > > > +
> > > > + /* Clean up callbacks that refer to the owner */
> > > > +
> > > > + if (a2dp->cancel_id) {
> > > > + g_source_remove(a2dp->cancel_id);
> > > > + a2dp->cancel_id = 0;
> > > > + }
> > > > +
> > > > + if (a2dp->resume_id) {
> > > > + a2dp_cancel(a2dp->resume_id);
> > > > + a2dp->resume_id = 0;
> > > > + }
> > > > +
> > > > + a2dp->cancel_resume = FALSE;
> > > > +}
> > > > +
> > > > static int8_t transport_a2dp_get_volume(struct media_transport *transport)
> > > > {
> > > > struct a2dp_transport *a2dp = transport->data;
> > > > @@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
> > > >
> > > > member = dbus_message_get_member(owner->pending->msg);
> > > > /* Cancel Acquire request if that exist */
> > > > - if (g_str_equal(member, "Acquire"))
> > > > + if (g_str_equal(member, "Acquire")) {
> > > > + media_request_reply(owner->pending, ECANCELED);
> > > > media_owner_remove(owner);
> > > > - else
> > > > + } else {
> > > > return btd_error_in_progress(msg);
> > > > + }
> > > > }
> > > >
> > > > transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
> > > > @@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
> > > > }
> > > >
> > > > #define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
> > > > - TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
> > > > + TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
> > > > + transport_a2dp_remove_owner, _init, \
> > > > transport_a2dp_resume, transport_a2dp_suspend, \
> > > > transport_a2dp_cancel, NULL, \
> > > > transport_a2dp_get_stream, transport_a2dp_get_volume, \
> > > > --
> > > > 2.47.0
> > > >
> > > >
> > >
> > >
> >
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release()
2024-10-30 21:46 ` Pauli Virtanen
@ 2024-10-30 22:03 ` Pauli Virtanen
0 siblings, 0 replies; 7+ messages in thread
From: Pauli Virtanen @ 2024-10-30 22:03 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
ke, 2024-10-30 kello 23:46 +0200, Pauli Virtanen kirjoitti:
> Hi Luiz,
>
> ke, 2024-10-30 kello 16:56 -0400, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Mon, Oct 28, 2024 at 1:48 PM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > Hi Luiz,
> > >
> > > ma, 2024-10-28 kello 10:39 -0400, Luiz Augusto von Dentz kirjoitti:
> > > > On Sat, Oct 26, 2024 at 6:41 AM Pauli Virtanen <pav@iki.fi> wrote:
> > > > >
> > > > > User can cancel transport acquire by calling Release() while Acquire()
> > > > > is in progress. This calls a2dp_cancel() which sends AVDTP_ABORT_CMD,
> > > > > forcing AVDTP state transition to IDLE, and disconnecting A2DP profile,
> > > > > which is not desired.
> > > > >
> > > > > Fix by instead waiting until START completes and then send SUSPEND. Make
> > > > > Release() reply only after the whole sequence completes.
> > > >
> > > > Hmm, but assumes the client is not reconfiguring to another endpoint
> > > > or just giving up on the transport since it doesn't intend to use it
> > > > anymore, anyway we can't really send anything else other than abort if
> > > > we don't want to wait so I think this would be better to be handled by
> > >
> > > I guess you have in mind
> > >
> > > Acquire(t1)
> > > Release(t1) # canceling acquire
> > > SetConfiguration(ep2)
> > >
> > > I don't think this works right in current BlueZ master branch either,
> > > the Release() results to removing the DBus endpoints & AVDTP
> > > disconnect, so nothing to SetConfigure() after abort completes.
> > > SetConfiguration() probably fails also if called while Abort_Cfm is
> > > pending, since it cannot then close the existing stream.
> > >
> > > I think (but did not test) that with the patch here, it would end up
> > > doing START -> CLOSE -> reconfigure, so indeed has to wait for START
> > > completion.
> >
> > I guess you are saying that we don't want the stream to transit to
> > idle since that would cause the AVDTP session to be disconnected? It
> > seems this is the result of session->dc_timeout being set to 0 on
> > avdtp_abort, anyway now looking a little more in detail it seems
> > clearer that the transport methods shouldn't affect the state machine
> > other than for suspend/resume procedures, that said then we need to
> > check what happens if those methods are called in quick succession e.g
> > Acquire -> Release -> Acquire.
>
> It should be possible to make it work so that SetConfiguration() while
> either Acquire() or Release() is pending sends ABORT if needed, but
> Acquire()+Release() alone only does OPEN->STREAMING->OPEN.
>
> The code that closes/aborts the existing stream in a2dp:a2dp_reconfig()
> may need some tuning, I'll need to test this race condition to make
> sure.
For the rapid Acquire() / Release() / Acquire() the intended behavior
probably is:
- while Acquire() call pending:
Release() makes the pending Acquire() return error, and suspends
after pending resume completes
Acquire() returns error
- while Release() call pending:
Release() returns error
Acquire() makes the pending Release() return error, and resumes
after pending suspend completes
so they just control the target state, and the state machine emits
START/SUSPEND as needed to reach the target.
(In this patch pending Release() was supposed to block Acquire() but
maybe we want it to instead work symmetrically like above.)
>
> > > > the application if it intends to suspend then it should wait acquire
> > > > to complete and then release.
> > >
> > > I think it's a bug in BlueZ that calling Release() on transport result
> > > to disconnect of A2DP, instead of suspend.
> > >
> > > I have another version of this patch that does send ABORT, but recalls
> > > it was due to canceled suspend and reconnects the sink/source when
> > > getting to IDLE.
> > >
> > > > > Fix also sending error reply to the canceled Acquire() call, which was
> > > > > missing previously.
> > > > > ---
> > > > >
> > > > > Notes:
> > > > > In theory we could also send ABORT and reconfigure the SEP again after
> > > > > that. It's more hairy though as with how the code currently works, we
> > > > > may need to complete discovery first. This is a corner case anyway, so
> > > > > just waiting a bit should be easier.
> > > > >
> > > > > profiles/audio/transport.c | 152 +++++++++++++++++++++++++++----------
> > > > > 1 file changed, 110 insertions(+), 42 deletions(-)
> > > > >
> > > > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > > > > index 0f7909a94..4d5afe022 100644
> > > > > --- a/profiles/audio/transport.c
> > > > > +++ b/profiles/audio/transport.c
> > > > > @@ -88,6 +88,9 @@ struct a2dp_transport {
> > > > > uint16_t delay;
> > > > > int8_t volume;
> > > > > guint watch;
> > > > > + guint resume_id;
> > > > > + gboolean cancel_resume;
> > > > > + guint cancel_id;
> > > > > };
> > > > >
> > > > > struct bap_transport {
> > > > > @@ -393,22 +396,82 @@ static void *transport_a2dp_get_stream(struct media_transport *transport)
> > > > > return a2dp_sep_get_stream(sep);
> > > > > }
> > > > >
> > > > > +static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > > > + void *user_data)
> > > > > +{
> > > > > + struct media_owner *owner = user_data;
> > > > > + struct media_transport *transport = owner->transport;
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > + struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > > > +
> > > > > + /* Release always succeeds */
> > > > > + if (owner->pending) {
> > > > > + owner->pending->id = 0;
> > > > > + media_request_reply(owner->pending, 0);
> > > > > + media_owner_remove(owner);
> > > > > + }
> > > > > +
> > > > > + a2dp_sep_unlock(sep, a2dp->session);
> > > > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > > + media_transport_remove_owner(transport);
> > > > > +}
> > > > > +
> > > > > +static guint transport_a2dp_suspend(struct media_transport *transport,
> > > > > + struct media_owner *owner)
> > > > > +{
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > + struct media_endpoint *endpoint = transport->endpoint;
> > > > > + struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > > +
> > > > > + if (a2dp->cancel_resume)
> > > > > + return a2dp->resume_id;
> > > > > +
> > > > > + if (owner != NULL)
> > > > > + return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > > > + owner);
> > > > > +
> > > > > + transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > > + a2dp_sep_unlock(sep, a2dp->session);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +static gboolean a2dp_cancel_resume_cb(void *user_data)
> > > > > +{
> > > > > + struct media_owner *owner = user_data;
> > > > > + struct media_transport *transport = owner->transport;
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > +
> > > > > + a2dp->cancel_id = 0;
> > > > > + a2dp->cancel_resume = FALSE;
> > > > > + owner->pending->id = transport_a2dp_suspend(transport, owner);
> > > > > + return FALSE;
> > > > > +}
> > > > > +
> > > > > static void a2dp_resume_complete(struct avdtp *session, int err,
> > > > > void *user_data)
> > > > > {
> > > > > struct media_owner *owner = user_data;
> > > > > struct media_request *req = owner->pending;
> > > > > struct media_transport *transport = owner->transport;
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > struct avdtp_stream *stream;
> > > > > int fd;
> > > > > uint16_t imtu, omtu;
> > > > > gboolean ret;
> > > > >
> > > > > + a2dp->resume_id = 0;
> > > > > req->id = 0;
> > > > >
> > > > > if (err)
> > > > > goto fail;
> > > > >
> > > > > + if (a2dp->cancel_resume) {
> > > > > + DBG("cancel resume");
> > > > > + a2dp->cancel_id = g_idle_add(a2dp_cancel_resume_cb, owner);
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > stream = transport_a2dp_get_stream(transport);
> > > > > if (stream == NULL)
> > > > > goto fail;
> > > > > @@ -445,15 +508,21 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > > > struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > > guint id;
> > > > >
> > > > > + if (a2dp->resume_id || a2dp->cancel_id)
> > > > > + return -EBUSY;
> > > > > +
> > > > > if (a2dp->session == NULL) {
> > > > > a2dp->session = a2dp_avdtp_get(transport->device);
> > > > > if (a2dp->session == NULL)
> > > > > return 0;
> > > > > }
> > > > >
> > > > > - if (state_in_use(transport->state))
> > > > > - return a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > > > + if (state_in_use(transport->state)) {
> > > > > + id = a2dp_resume(a2dp->session, sep, a2dp_resume_complete,
> > > > > owner);
> > > > > + a2dp->resume_id = id;
> > > > > + return id;
> > > > > + }
> > > > >
> > > > > if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
> > > > > return 0;
> > > > > @@ -468,51 +537,47 @@ static guint transport_a2dp_resume(struct media_transport *transport,
> > > > > if (transport->state == TRANSPORT_STATE_IDLE)
> > > > > transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
> > > > >
> > > > > + a2dp->resume_id = id;
> > > > > return id;
> > > > > }
> > > > >
> > > > > -static void a2dp_suspend_complete(struct avdtp *session, int err,
> > > > > - void *user_data)
> > > > > -{
> > > > > - struct media_owner *owner = user_data;
> > > > > - struct media_transport *transport = owner->transport;
> > > > > - struct a2dp_transport *a2dp = transport->data;
> > > > > - struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
> > > > > -
> > > > > - /* Release always succeeds */
> > > > > - if (owner->pending) {
> > > > > - owner->pending->id = 0;
> > > > > - media_request_reply(owner->pending, 0);
> > > > > - media_owner_remove(owner);
> > > > > - }
> > > > > -
> > > > > - a2dp_sep_unlock(sep, a2dp->session);
> > > > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > > - media_transport_remove_owner(transport);
> > > > > -}
> > > > > -
> > > > > -static guint transport_a2dp_suspend(struct media_transport *transport,
> > > > > - struct media_owner *owner)
> > > > > -{
> > > > > - struct a2dp_transport *a2dp = transport->data;
> > > > > - struct media_endpoint *endpoint = transport->endpoint;
> > > > > - struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
> > > > > -
> > > > > - if (owner != NULL)
> > > > > - return a2dp_suspend(a2dp->session, sep, a2dp_suspend_complete,
> > > > > - owner);
> > > > > -
> > > > > - transport_set_state(transport, TRANSPORT_STATE_IDLE);
> > > > > - a2dp_sep_unlock(sep, a2dp->session);
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > -
> > > > > static void transport_a2dp_cancel(struct media_transport *transport, guint id)
> > > > > {
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > +
> > > > > + if (a2dp->resume_id && a2dp->resume_id == id) {
> > > > > + /* a2dp_cancel() results to ABORT->IDLE->disconnect. Canceling
> > > > > + * START can be triggered by user via Release(), and it's better
> > > > > + * to not drop the A2DP connection then, so we just suspend
> > > > > + * after resume completes.
> > > > > + */
> > > > > + a2dp->cancel_resume = TRUE;
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > a2dp_cancel(id);
> > > > > }
> > > > >
> > > > > +static void transport_a2dp_remove_owner(struct media_transport *transport,
> > > > > + struct media_owner *owner)
> > > > > +{
> > > > > + struct a2dp_transport *a2dp = transport->data;
> > > > > +
> > > > > + /* Clean up callbacks that refer to the owner */
> > > > > +
> > > > > + if (a2dp->cancel_id) {
> > > > > + g_source_remove(a2dp->cancel_id);
> > > > > + a2dp->cancel_id = 0;
> > > > > + }
> > > > > +
> > > > > + if (a2dp->resume_id) {
> > > > > + a2dp_cancel(a2dp->resume_id);
> > > > > + a2dp->resume_id = 0;
> > > > > + }
> > > > > +
> > > > > + a2dp->cancel_resume = FALSE;
> > > > > +}
> > > > > +
> > > > > static int8_t transport_a2dp_get_volume(struct media_transport *transport)
> > > > > {
> > > > > struct a2dp_transport *a2dp = transport->data;
> > > > > @@ -773,10 +838,12 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
> > > > >
> > > > > member = dbus_message_get_member(owner->pending->msg);
> > > > > /* Cancel Acquire request if that exist */
> > > > > - if (g_str_equal(member, "Acquire"))
> > > > > + if (g_str_equal(member, "Acquire")) {
> > > > > + media_request_reply(owner->pending, ECANCELED);
> > > > > media_owner_remove(owner);
> > > > > - else
> > > > > + } else {
> > > > > return btd_error_in_progress(msg);
> > > > > + }
> > > > > }
> > > > >
> > > > > transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
> > > > > @@ -2189,7 +2256,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
> > > > > }
> > > > >
> > > > > #define A2DP_OPS(_uuid, _init, _set_volume, _destroy) \
> > > > > - TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, NULL, _init, \
> > > > > + TRANSPORT_OPS(_uuid, transport_a2dp_properties, NULL, \
> > > > > + transport_a2dp_remove_owner, _init, \
> > > > > transport_a2dp_resume, transport_a2dp_suspend, \
> > > > > transport_a2dp_cancel, NULL, \
> > > > > transport_a2dp_get_stream, transport_a2dp_get_volume, \
> > > > > --
> > > > > 2.47.0
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-30 22:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-26 10:41 [PATCH BlueZ] transport: don't disconnect A2DP if canceling Acquire() with Release() Pauli Virtanen
2024-10-26 12:38 ` [BlueZ] " bluez.test.bot
2024-10-28 14:39 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2024-10-28 17:48 ` Pauli Virtanen
2024-10-30 20:56 ` Luiz Augusto von Dentz
2024-10-30 21:46 ` Pauli Virtanen
2024-10-30 22:03 ` Pauli Virtanen
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.