From: Mikel Astiz <[email protected]>
This patch series includes minor modifications that have been useful while testing the Media API for HFP use-cases.
These three patches were acked by Luiz Augusto von Dentz the 15-12-2011.
The first patch has been removed from the previous proposal, given that it doesn't really solve the underlying problem. According to Luiz (15-12-2011):
"gateway_suspend_stream as it is useless, but I think we should fix it
to work similarly to headset_suspend_stream so we properly wait until
the socket is closed before we unlock and reply, this is necessary in
order to properly synchronize switching profiles otherwise
gateway_suspend_complete maybe called without SCO being completely
disconnected. Also when a fd is passed over to another process calling
close/g_io_channel_shutdown is not enough since this only release our
reference, in this case shutdown must be called."
This issue is not tackled here.
Mikel Astiz (3):
audio: gateway_unlock together with in_use = FALSE
a2dp: return NotSupported error if no server found
audio: do not disconnect on gateway SCO failure
audio/a2dp.c | 2 +-
audio/gateway.c | 2 +-
audio/transport.c | 3 ++-
3 files changed, 4 insertions(+), 3 deletions(-)
--
1.7.6.5
Hi Mikel,
On Tue, Feb 14, 2012, Mikel Astiz wrote:
> From: Mikel Astiz <[email protected]>
>
> This patch series includes minor modifications that have been useful
> while testing the Media API for HFP use-cases.
>
> These three patches were acked by Luiz Augusto von Dentz the 15-12-2011.
>
> The first patch has been removed from the previous proposal, given
> that it doesn't really solve the underlying problem. According to Luiz
> (15-12-2011):
>
> "gateway_suspend_stream as it is useless, but I think we should fix it
> to work similarly to headset_suspend_stream so we properly wait until
> the socket is closed before we unlock and reply, this is necessary in
> order to properly synchronize switching profiles otherwise
> gateway_suspend_complete maybe called without SCO being completely
> disconnected. Also when a fd is passed over to another process calling
> close/g_io_channel_shutdown is not enough since this only release our
> reference, in this case shutdown must be called."
>
> This issue is not tackled here.
>
> Mikel Astiz (3):
> audio: gateway_unlock together with in_use = FALSE
> a2dp: return NotSupported error if no server found
> audio: do not disconnect on gateway SCO failure
>
> audio/a2dp.c | 2 +-
> audio/gateway.c | 2 +-
> audio/transport.c | 3 ++-
> 3 files changed, 4 insertions(+), 3 deletions(-)
All three patches have been applied. Thanks.
Johan
Hi Mikel,
On Tue, Feb 14, 2012 at 11:06 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Failure on BlueZ-initiated SCO requests should not drop the RFCOMM
> connection to the gateway. Instead, considering the stream as suspended
> should be enough.
> ---
> ?audio/gateway.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index bde3e02..163f7b5 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -255,7 +255,7 @@ static void sco_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>
> ? ? ? ?if (err) {
> ? ? ? ? ? ? ? ?error("sco_connect_cb(): %s", err->message);
> - ? ? ? ? ? ? ? gateway_close(dev);
> + ? ? ? ? ? ? ? gateway_suspend_stream(dev);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
>
> --
> 1.7.6.5
Ack
--
Luiz Augusto von Dentz
Hi Mikel,
On Tue, Feb 14, 2012 at 11:06 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> With both sink and sources disabled, the A2DP server is not even
> registered in audio_manager_init. When trying to register the endpoint,
> this should result in the same error as if the server existed but the
> profile was disabled.
> ---
> ?audio/a2dp.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index bbb91ce..623a0bc 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -1589,7 +1589,7 @@ struct a2dp_sep *a2dp_add_sep(const bdaddr_t *src, uint8_t type,
> ? ? ? ?server = find_server(servers, src);
> ? ? ? ?if (server == NULL) {
> ? ? ? ? ? ? ? ?if (err)
> - ? ? ? ? ? ? ? ? ? ? ? *err = -EINVAL;
> + ? ? ? ? ? ? ? ? ? ? ? *err = -EPROTONOSUPPORT;
> ? ? ? ? ? ? ? ?return NULL;
> ? ? ? ?}
>
> --
> 1.7.6.5
Ack
--
Luiz Augusto von Dentz
Hi Mikel,
On Tue, Feb 14, 2012 at 11:06 AM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Calling gateway_unlock seems safer in gateway_suspend_complete, in
> combination with the update of transport->in_use. This avoids the
> transitional state of having the gateway unlocked but the transport
> still in use.
>
> This approach is also more consistent with the way headset and a2dp
> handle it.
> ---
> ?audio/transport.c | ? ?3 ++-
> ?1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/audio/transport.c b/audio/transport.c
> index 7bde32d..2092644 100644
> --- a/audio/transport.c
> +++ b/audio/transport.c
> @@ -510,6 +510,7 @@ static gboolean gateway_suspend_complete(gpointer user_data)
> ?{
> ? ? ? ?struct media_owner *owner = user_data;
> ? ? ? ?struct media_transport *transport = owner->transport;
> + ? ? ? struct audio_device *device = transport->device;
>
> ? ? ? ?/* Release always succeeds */
> ? ? ? ?if (owner->pending) {
> @@ -518,6 +519,7 @@ static gboolean gateway_suspend_complete(gpointer user_data)
> ? ? ? ? ? ? ? ?media_owner_remove(owner);
> ? ? ? ?}
>
> + ? ? ? gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
> ? ? ? ?transport->in_use = FALSE;
> ? ? ? ?media_transport_remove(transport, owner);
> ? ? ? ?return FALSE;
> @@ -536,7 +538,6 @@ static guint suspend_gateway(struct media_transport *transport,
> ? ? ? ?}
>
> ? ? ? ?gateway_suspend_stream(device);
> - ? ? ? gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
> ? ? ? ?g_idle_add(gateway_suspend_complete, owner);
> ? ? ? ?return id++;
> ?}
> --
> 1.7.6.5
Ack
--
Luiz Augusto von Dentz
From: Mikel Astiz <[email protected]>
With both sink and sources disabled, the A2DP server is not even
registered in audio_manager_init. When trying to register the endpoint,
this should result in the same error as if the server existed but the
profile was disabled.
---
audio/a2dp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index bbb91ce..623a0bc 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1589,7 +1589,7 @@ struct a2dp_sep *a2dp_add_sep(const bdaddr_t *src, uint8_t type,
server = find_server(servers, src);
if (server == NULL) {
if (err)
- *err = -EINVAL;
+ *err = -EPROTONOSUPPORT;
return NULL;
}
--
1.7.6.5
From: Mikel Astiz <[email protected]>
Failure on BlueZ-initiated SCO requests should not drop the RFCOMM
connection to the gateway. Instead, considering the stream as suspended
should be enough.
---
audio/gateway.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/gateway.c b/audio/gateway.c
index bde3e02..163f7b5 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -255,7 +255,7 @@ static void sco_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
if (err) {
error("sco_connect_cb(): %s", err->message);
- gateway_close(dev);
+ gateway_suspend_stream(dev);
return;
}
--
1.7.6.5
From: Mikel Astiz <[email protected]>
Calling gateway_unlock seems safer in gateway_suspend_complete, in
combination with the update of transport->in_use. This avoids the
transitional state of having the gateway unlocked but the transport
still in use.
This approach is also more consistent with the way headset and a2dp
handle it.
---
audio/transport.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 7bde32d..2092644 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -510,6 +510,7 @@ static gboolean gateway_suspend_complete(gpointer user_data)
{
struct media_owner *owner = user_data;
struct media_transport *transport = owner->transport;
+ struct audio_device *device = transport->device;
/* Release always succeeds */
if (owner->pending) {
@@ -518,6 +519,7 @@ static gboolean gateway_suspend_complete(gpointer user_data)
media_owner_remove(owner);
}
+ gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
transport->in_use = FALSE;
media_transport_remove(transport, owner);
return FALSE;
@@ -536,7 +538,6 @@ static guint suspend_gateway(struct media_transport *transport,
}
gateway_suspend_stream(device);
- gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
g_idle_add(gateway_suspend_complete, owner);
return id++;
}
--
1.7.6.5