* [PATCH BlueZ v0 2/4] adapter: Remove dead code from adapter_emit_device_found()
From: Syam Sidhardhan @ 2012-09-14 16:10 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1347639011-22924-1-git-send-email-s.syam@samsung.com>
---
src/adapter.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 1c13b2d..facfa23 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2725,7 +2725,7 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
struct remote_dev_info *dev)
{
struct btd_device *device;
- char peer_addr[18], local_addr[18];
+ char peer_addr[18];
const char *icon, *paddr = peer_addr;
dbus_bool_t paired = FALSE, trusted = FALSE;
dbus_int16_t rssi = dev->rssi;
@@ -2733,7 +2733,6 @@ void adapter_emit_device_found(struct btd_adapter *adapter,
size_t uuid_count;
ba2str(&dev->bdaddr, peer_addr);
- ba2str(&adapter->bdaddr, local_addr);
device = adapter_find_device(adapter, paddr);
if (device) {
--
1.7.4.1
^ permalink raw reply related
* [PATCH BlueZ v0 1/4] device: Remove dead code from get_properties()
From: Syam Sidhardhan @ 2012-09-14 16:10 UTC (permalink / raw)
To: linux-bluetooth
---
src/device.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index 49021e4..77215f1 100644
--- a/src/device.c
+++ b/src/device.c
@@ -318,7 +318,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
DBusMessageIter iter;
DBusMessageIter dict;
bdaddr_t src;
- char name[MAX_NAME_LENGTH + 1], srcaddr[18], dstaddr[18];
+ char name[MAX_NAME_LENGTH + 1], dstaddr[18];
char **str;
const char *ptr, *icon = NULL;
dbus_bool_t boolean;
@@ -348,7 +348,6 @@ static DBusMessage *get_properties(DBusConnection *conn,
ptr = NULL;
memset(name, 0, sizeof(name));
adapter_get_address(adapter, &src);
- ba2str(&src, srcaddr);
ptr = device->name;
dict_append_entry(&dict, "Name", DBUS_TYPE_STRING, &ptr);
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCHv4 04/17] Bluetooth: AMP: Use HCI cmd to Read Loc AMP Assoc
From: Mat Martineau @ 2012-09-14 15:42 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth, gustavo
In-Reply-To: <20120914065807.GB7483@aemeltch-MOBL1>
Hi Andrei -
On Fri, 14 Sep 2012, Andrei Emeltchenko wrote:
> Hi Mat,
>
> On Thu, Sep 13, 2012 at 08:28:06AM -0700, Mat Martineau wrote:
>>> When receiving A2MP Get AMP Assoc Request execute Read Local AMP Assoc
>>> HCI command to AMP controller. If the AMP Assoc data is larger then it
>>> can fit to HCI event only fragment is read. When all fragments are read
>>> send A2MP Get AMP Assoc Response.
>>>
>>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ...
>
>>> +void a2mp_send_getampassoc_rsp(struct hci_dev *hdev, u8 status)
>>> +{
>>> + struct amp_mgr *mgr;
>>> + struct amp_assoc *loc_assoc = &hdev->loc_assoc;
>>> + struct a2mp_amp_assoc_rsp *rsp;
>>> + size_t len;
>>> +
>>> + mgr = amp_mgr_lookup_by_state(READ_LOC_AMP_ASSOC);
>>
>> What if multiple amp managers are in the READ_LOC_AMP_ASSOC state?
>> Is that possible if multiple remote devices send GETAMPASSOC at the
>> same time?
>
> In this very unrealistic case I still do not see a problem. All those HCI
> requests would be serialized and response would be sent for all AMP
> managers.
I agree that it won't be a common problem - but with millions of
instances of the Linux kernel out there, if simultaneous requests
*can* happen, they *will* happen.
It's mostly the reading of fragments I'm concerned about. If multiple
HCI_OP_READ_LOCAL_AMP_ASSOC commands are queued up to send to the AMP
controller, and any of those involve reading fragments, the
hdev->loc_assoc would be corrupted or you could get a buffer overrun
from the memcpy's in hci_cc_read_local_amp_assoc.
An easy fix is to respond with an error status to any GETAMPASSOC
commands that arrive while an earlier GETAMPASSOC is being processed.
--
Mat Martineau
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
^ permalink raw reply
* [PATCH BlueZ] hog: Fix segfault when discovering descriptors
From: João Paulo Rechi Vita @ 2012-09-14 13:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: João Paulo Rechi Vita
Invalid memory was being accessed when discovering the External Report
Reference descriptor.
---
profiles/input/hog_device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/input/hog_device.c b/profiles/input/hog_device.c
index 664c52b..03af931 100644
--- a/profiles/input/hog_device.c
+++ b/profiles/input/hog_device.c
@@ -479,7 +479,7 @@ static void char_discovered_cb(GSList *chars, guint8 status, gpointer user_data)
} else if (bt_uuid_cmp(&uuid, &report_map_uuid) == 0) {
gatt_read_char(hogdev->attrib, chr->value_handle, 0,
report_map_read_cb, hogdev);
- discover_descriptor(hogdev->attrib, chr, next, NULL);
+ discover_descriptor(hogdev->attrib, chr, next, hogdev);
} else if (bt_uuid_cmp(&uuid, &info_uuid) == 0)
info_handle = chr->value_handle;
else if (bt_uuid_cmp(&uuid, &proto_mode_uuid) == 0)
--
1.7.11.4
^ permalink raw reply related
* [RFC v5 4/4] media: Extend media API with optional acquire
From: Mikel Astiz @ 2012-09-14 12:56 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347627360-13997-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Acquiring a transport is needed in two different situations: either
we are initiating the audio stream locally, or the remote side initiated
it and thus we are just reacting. In the second case, we would expect
the stream is already available, and otherwise the operation should
fail. This means the media API needs to be extended in order to make
this difference.
This issue is specially relevant in the case of SCO, because the current
approach is racy. With HFP, for example (say BlueZ has the HS role), the
following race condition could be met:
1. Phone has an incoming call and thus starts in-band ringing.
2. SCO connection is accepted and stablished by BlueZ.
3. Gateway interface state is changed to Playing.
4. Exactly afterwards, the user routes the audio to the phone, to have
a private conversation. So the SCO link is closed.
5. In parallel, PulseAudio sees the transition to Playing, and acquires
the transport.
6. BlueZ receives an Acquire() request, but SCO is down. So it tries to
reconnect the SCO link.
The last step is an undesired behavior (the audio is routed back to the
car). BlueZ should be smart enough to know that the SCO connection
shouldn't be reestablished, but this is only possible if the endpoint
provides additional information in the media API.
Note that the API change introduced by this patch is backward
compatible, meaning that older versions of BlueZ will just ignore the
flag. So clients are encouraged to use it without necessarily adding a
dependency to newer versions of BlueZ.
---
audio/transport.c | 4 ++++
doc/media-api.txt | 11 +++++++++++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 4653cc7..cc9c56f 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -807,6 +807,10 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
if (lock == 0)
return btd_error_invalid_args(msg);
+ if (transport->state != TRANSPORT_STATE_PENDING &&
+ g_strstr_len(accesstype, -1, "?") != NULL)
+ return btd_error_failed(msg, "Transport not playing");
+
if (media_transport_acquire(transport, lock) == FALSE)
return btd_error_not_authorized(msg);
diff --git a/doc/media-api.txt b/doc/media-api.txt
index da8ef53..b9802d9 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -282,6 +282,17 @@ Methods dict GetProperties()
"rw": Read and write access
+ The accesstype string can also be combined with a "?"
+ suffix, which will make the request optional. This
+ typically means the transport will only be acquired if
+ it is already available (remote-initiated), but
+ otherwise no request will be sent to the remote side.
+ In this last case the function will fail. Note that,
+ due to compatibility issues with older versions of
+ BlueZ, clients are encouraged to use exactly the same
+ accesstype for Release(), matching the string provided
+ to Acquire().
+
void Release(string accesstype)
Releases file descriptor.
--
1.7.7.6
^ permalink raw reply related
* [RFC v5 3/4] media: Automatically release transport when HUP
From: Mikel Astiz @ 2012-09-14 12:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347627360-13997-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
When the remote end suspends the audio stream, release the transport
automatically without waiting until the clients call Release().
This affects the D-Bus API since clients will get an error when trying
to release the transport afterwards.
However, this should have no real impact, since most clients (i.e.
PulseAudio) would just log some error trace but otherwise ignore the
issue.
---
audio/transport.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 742cc72..4653cc7 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -60,7 +60,7 @@ typedef enum {
TRANSPORT_STATE_IDLE, /* Not acquired and suspended */
TRANSPORT_STATE_PENDING, /* Playing but not acquired */
TRANSPORT_STATE_REQUESTING, /* Acquire in progress */
- TRANSPORT_STATE_ACTIVE, /* Acquired (not necessarily playing) */
+ TRANSPORT_STATE_ACTIVE, /* Acquired and playing */
TRANSPORT_STATE_SUSPENDING, /* Release in progress */
} transport_state_t;
@@ -1178,6 +1178,18 @@ static void transport_update_playing(struct media_transport *transport,
if (playing == FALSE) {
if (transport->state == TRANSPORT_STATE_PENDING)
transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ else if (transport->state == TRANSPORT_STATE_ACTIVE) {
+ /* Remove all owners */
+ while (transport->owners != NULL) {
+ struct media_owner *owner;
+
+ owner = transport->owners->data;
+ media_transport_remove(transport, owner);
+ }
+
+ /* Suspend so that the locks get released */
+ transport->suspend(transport, NULL);
+ }
} else if (transport->state == TRANSPORT_STATE_IDLE)
transport_set_state(transport, TRANSPORT_STATE_PENDING);
}
--
1.7.7.6
^ permalink raw reply related
* [RFC v5 2/4] media: Expose transport state in D-Bus
From: Mikel Astiz @ 2012-09-14 12:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347627360-13997-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Extend the Media API to expose the transport state in D-Bus, as a
property of the transport. This way the clients do not have to find
out which is the corresponding profile-specific interface for the
transport.
Additionally, this state along with the automatic release of transports
will allow clients to avoid the "optional release" or "accept remote
release" race condition. For example, with HSP/HFP profiles, the problem
is the following:
1. User suspends SCO in the remote end.
2. BlueZ signals the Playing->Connected state change in D-Bus.
3. Exactly afterwards, the user resumes SCO in the remote end.
4. In parallel, PulseAudio sees the aforementioned transition to
Connected, and thus releases the transport.
5. BlueZ receives a Release() request while SCO is up. So the audio
stream will be suspended.
The last step is an undesired behavior since the user explicitly wanted
to route the audio stream through Bluetooth.
The issue is difficult to reproduce but it can easily be solved by
exposing the transport state in D-Bus.
---
audio/transport.c | 29 +++++++++++++++++++++++++++++
doc/media-api.txt | 8 ++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index e0402ad..742cc72 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -155,6 +155,22 @@ static transport_lock_t str2lock(const char *str)
return lock;
}
+static const char *state2str(transport_state_t state)
+{
+ switch (state) {
+ case TRANSPORT_STATE_IDLE:
+ case TRANSPORT_STATE_REQUESTING:
+ return "idle";
+ case TRANSPORT_STATE_PENDING:
+ return "pending";
+ case TRANSPORT_STATE_ACTIVE:
+ case TRANSPORT_STATE_SUSPENDING:
+ return "active";
+ }
+
+ return NULL;
+}
+
static gboolean state_in_use(transport_state_t state)
{
switch (state) {
@@ -174,6 +190,7 @@ static void transport_set_state(struct media_transport *transport,
transport_state_t state)
{
transport_state_t old_state = transport->state;
+ const char *str;
if (old_state == state)
return;
@@ -182,6 +199,13 @@ static void transport_set_state(struct media_transport *transport,
DBG("State changed %s: %s -> %s", transport->path, str_state[old_state],
str_state[state]);
+
+ str = state2str(state);
+
+ if (g_strcmp0(str, state2str(old_state)) != 0)
+ emit_property_changed(transport->conn, transport->path,
+ MEDIA_TRANSPORT_INTERFACE, "State",
+ DBUS_TYPE_STRING, &str);
}
void media_transport_destroy(struct media_transport *transport)
@@ -1017,6 +1041,7 @@ void transport_get_properties(struct media_transport *transport,
DBusMessageIter dict;
const char *uuid;
uint8_t codec;
+ const char *state;
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -1036,6 +1061,10 @@ void transport_get_properties(struct media_transport *transport,
dict_append_array(&dict, "Configuration", DBUS_TYPE_BYTE,
&transport->configuration, transport->size);
+ /* State */
+ state = state2str(transport->state);
+ dict_append_entry(&dict, "State", DBUS_TYPE_STRING, &state);
+
if (transport->get_properties)
transport->get_properties(transport, &dict);
diff --git a/doc/media-api.txt b/doc/media-api.txt
index e5eeaa0..da8ef53 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -317,6 +317,14 @@ Properties object Device [readonly]
Configuration blob, it is used as it is so the size and
byte order must match.
+ string State [readonly]
+
+ Indicates the state of the transport. Possible
+ values are:
+ "idle": not streaming
+ "pending": streaming but not acquired
+ "active": streaming and acquired
+
uint16 Delay [readwrite]
Optional. Transport delay in 1/10 of millisecond, this
--
1.7.7.6
^ permalink raw reply related
* [RFC v5 1/4] media: Split transport state based on playing flag
From: Mikel Astiz @ 2012-09-14 12:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347627360-13997-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Split the transport states (formerly in_use) into more specific states
where the stream state (playing or suspended) is explicitly represented,
along with the transitional states (locally initiated suspend and
resume).
TRANSPORT_STATE_ACTIVE is an exception since it also includes the state
where the transport is acquired, but the audio was later suspended (not
released yet though).
---
audio/transport.c | 68 ++++++++++++++++++++++++++++++++++++++++++++--------
1 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 3026022..e0402ad 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -57,13 +57,19 @@ typedef enum {
} transport_lock_t;
typedef enum {
- TRANSPORT_STATE_IDLE, /* Not acquired */
- TRANSPORT_STATE_ACQUIRED, /* Acquired (not necessarily playing) */
+ TRANSPORT_STATE_IDLE, /* Not acquired and suspended */
+ TRANSPORT_STATE_PENDING, /* Playing but not acquired */
+ TRANSPORT_STATE_REQUESTING, /* Acquire in progress */
+ TRANSPORT_STATE_ACTIVE, /* Acquired (not necessarily playing) */
+ TRANSPORT_STATE_SUSPENDING, /* Release in progress */
} transport_state_t;
static char *str_state[] = {
"TRANSPORT_STATE_IDLE",
- "TRANSPORT_STATE_ACQUIRED",
+ "TRANSPORT_STATE_PENDING",
+ "TRANSPORT_STATE_REQUESTING",
+ "TRANSPORT_STATE_ACTIVE",
+ "TRANSPORT_STATE_SUSPENDING",
};
struct media_request {
@@ -153,8 +159,11 @@ static gboolean state_in_use(transport_state_t state)
{
switch (state) {
case TRANSPORT_STATE_IDLE:
+ case TRANSPORT_STATE_PENDING:
return FALSE;
- case TRANSPORT_STATE_ACQUIRED:
+ case TRANSPORT_STATE_REQUESTING:
+ case TRANSPORT_STATE_ACTIVE:
+ case TRANSPORT_STATE_SUSPENDING:
return TRUE;
}
@@ -356,6 +365,8 @@ static void a2dp_resume_complete(struct avdtp *session,
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -382,7 +393,8 @@ static guint resume_a2dp(struct media_transport *transport,
if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACQUIRED);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return a2dp_resume(a2dp->session, sep, a2dp_resume_complete, owner);
@@ -417,7 +429,12 @@ static guint suspend_a2dp(struct media_transport *transport,
if (!owner) {
a2dp_sep_unlock(sep, a2dp->session);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (a2dp_sep_is_playing(sep))
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -468,6 +485,8 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -486,7 +505,8 @@ static guint resume_headset(struct media_transport *transport,
HEADSET_LOCK_WRITE) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACQUIRED);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return headset_request_stream(device, headset_resume_complete,
@@ -516,8 +536,15 @@ static guint suspend_headset(struct media_transport *transport,
struct audio_device *device = transport->device;
if (!owner) {
+ headset_state_t state = headset_get_state(device);
+
headset_unlock(device, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (state == HEADSET_STATE_PLAYING)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -574,6 +601,8 @@ static void gateway_resume_complete(struct audio_device *dev, GError *err,
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -592,7 +621,8 @@ static guint resume_gateway(struct media_transport *transport,
GATEWAY_LOCK_WRITE) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACQUIRED);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return gateway_request_stream(device, gateway_resume_complete,
@@ -625,8 +655,15 @@ static guint suspend_gateway(struct media_transport *transport,
static int id = 1;
if (!owner) {
+ gateway_state_t state = gateway_get_state(device);
+
gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (state == GATEWAY_STATE_PLAYING)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -806,6 +843,8 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
return btd_error_in_progress(msg);
}
+ transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
+
id = transport->suspend(transport, owner);
if (id == 0) {
media_transport_remove(transport, owner);
@@ -1104,7 +1143,14 @@ static void headset_nrec_changed(struct audio_device *dev, gboolean nrec,
static void transport_update_playing(struct media_transport *transport,
gboolean playing)
{
- DBG("%s Playing=%d", transport->path, playing);
+ DBG("%s State=%s Playing=%d", transport->path,
+ str_state[transport->state], playing);
+
+ if (playing == FALSE) {
+ if (transport->state == TRANSPORT_STATE_PENDING)
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ } else if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
}
static void headset_state_changed(struct audio_device *dev,
--
1.7.7.6
^ permalink raw reply related
* [RFC v5 0/4] Optional acquire in Media API and related
From: Mikel Astiz @ 2012-09-14 12:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Same as v4 but rebased on the latest pushed patches.
>From original patch:
This patch reopens the discussion started by the thread "when is acquire
ok to call". The race condition seems to be real (even thought difficult
to reproduce), and I couldn't think of any approach to solve this
without altering the Media API.
Mikel Astiz (4):
media: Split transport state based on playing flag
media: Expose transport state in D-Bus
media: Automatically release transport when HUP
media: Extend media API with optional acquire
audio/transport.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++-----
doc/media-api.txt | 19 +++++++++
2 files changed, 121 insertions(+), 11 deletions(-)
--
1.7.7.6
^ permalink raw reply
* Re: [RFC v4 0/9] Optional acquire in Media API and related
From: Mikel Astiz @ 2012-09-14 12:40 UTC (permalink / raw)
To: Maxim Levitsky; +Cc: linux-bluetooth, Mikel Astiz
In-Reply-To: <1347624979.8292.1.camel@maxim-laptop>
Hi Maxim,
On Fri, Sep 14, 2012 at 2:16 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> On Fri, 2012-09-14 at 14:03 +0200, Mikel Astiz wrote:
>> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>>
>> Similar patchset as v3 with the integration of Luiz's feedback, including the following changes:
>> - Patches 7 and 8 swapped
>> - State "acquired" renamed to "active" (both in D-Bus and in the code)
>> - TRANSPORT_STATE_POST_ACQUIRE (which was transitional) dropped
>>
>> From original patch:
>>
>> This patch reopens the discussion started by the thread "when is acquire
>> ok to call". The race condition seems to be real (even thought difficult
>> to reproduce), and I couldn't think of any approach to solve this
>> without altering the Media API.
> Will this fix the issue with 2 a2dp headsets unable currently to connect
> at same time? Any idea if this is API limitation, PA bug or something
> else?
No, I'm afraid this patchset is unrelated to your issue.
Cheers,
Mikel
^ permalink raw reply
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: David Dillow @ 2012-09-14 12:17 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: linux-bluetooth, David Herrmann, Bastien Nocera, Antonio Ospite
In-Reply-To: <CABBYNZKD9ZEOoGfJF3Mf4-=CSYL8rev+HbL_X7KdV6oMe+0ibQ@mail.gmail.com>
On Fri, 2012-09-14 at 13:42 +0300, Luiz Augusto von Dentz wrote:
> Hi David,
>
> On Fri, Sep 14, 2012 at 6:04 AM, David Dillow <dave@thedillows.org> wrote:
> > Antonio, if you could test this with the real remote you have, I'd be
> > very appreciative.
> >
> > David, could you point me at the controls for the 10min idle-disconnect?
>
> I suppose we can just continue using input.conf:IdleTimeout (which
> passed via ioctl in the struct hidp_connadd_req.idle_to) or do you
> think this would be useful for other transports, at least for USB it
> does not make much sense since the cable will stay connected.
I'm agreed that it doesn't make much sense for USB. I would like to find
a way to keep the adapter running -- it seems I don't get the first key
press that the adapter woke up for. This would need to be optional, as I
see this draining the batteries on the real remote, but my adapter is
hardwired and it'd be nice to avoid the initial delay/lost press.
Of course solving the initial press would be handy for everyone,
assuming it isn't a Harmony-only issue.
> > auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
> > though it looks like the Harmony adapter is initiating the disconnect --
> > this is likely an area for improvement with the Sony remote.
>
> Btw, it would probably be a good to add a proper defines for 0x0306
> e.g. USB_DEVICE_ID_SONY_PS3_REMOTE and
> USB_DEVICE_ID_LOGITECH_HARMONY_ADAPTER
Heh, I thought you might say that. I personally prefer the numbers ala
the current style for PCI(e) drivers -- and took the prior existence of
the numbers-only entries as permission -- but I'll defer to the wishes
of the subsystem.
Thanks,
Dave
^ permalink raw reply
* Re: [RFC v4 0/9] Optional acquire in Media API and related
From: Maxim Levitsky @ 2012-09-14 12:16 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
On Fri, 2012-09-14 at 14:03 +0200, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> Similar patchset as v3 with the integration of Luiz's feedback, including the following changes:
> - Patches 7 and 8 swapped
> - State "acquired" renamed to "active" (both in D-Bus and in the code)
> - TRANSPORT_STATE_POST_ACQUIRE (which was transitional) dropped
>
> From original patch:
>
> This patch reopens the discussion started by the thread "when is acquire
> ok to call". The race condition seems to be real (even thought difficult
> to reproduce), and I couldn't think of any approach to solve this
> without altering the Media API.
Will this fix the issue with 2 a2dp headsets unable currently to connect
at same time? Any idea if this is API limitation, PA bug or something
else?
Best regards,
Maxim Levitsky
^ permalink raw reply
* Re: [RFC v3 0/9] Optional acquire in Media API and related
From: Luiz Augusto von Dentz @ 2012-09-14 12:11 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
In-Reply-To: <1347031233-21621-1-git-send-email-mikel.astiz.oss@gmail.com>
Hi Mikel,
On Fri, Sep 7, 2012 at 6:20 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> This patchset addresses the same problems as before (optional acquire race condition, see patch v3 9/9) along with the symmetric "optional release" problem as described in patch v3 8/9. The solution in this second case consists of exposing the transport state in D-Bus, as discussed in the IRC.
>
> From original patch:
>
> This patch reopens the discussion started by the thread "when is acquire
> ok to call". The race condition seems to be real (even thought difficult
> to reproduce), and I couldn't think of any approach to solve this
> without altering the Media API.
>
> Mikel Astiz (9):
> media: Fix accesstype comparison
> media: Add a2dp_sep_is_playing() to internal API
> media: Add gateway_get_state() to internal API
> media: Replace transport->in_use flag with state
> media: Watch interface state changes in transport
> media: Split transport state based on playing flag
> media: Automatically release transport when HUP
> media: Expose transport state in D-Bus
> media: Extend media API with optional acquire
>
> audio/a2dp.c | 8 +
> audio/a2dp.h | 1 +
> audio/gateway.c | 7 +
> audio/gateway.h | 1 +
> audio/transport.c | 390 +++++++++++++++++++++++++++++++++++++++++++----------
> doc/media-api.txt | 19 +++
> 6 files changed, 356 insertions(+), 70 deletions(-)
>
> --
> 1.7.7.6
Patches 1-5 are now upstream, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [RFC v4 9/9] media: Extend media API with optional acquire
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Acquiring a transport is needed in two different situations: either
we are initiating the audio stream locally, or the remote side initiated
it and thus we are just reacting. In the second case, we would expect
the stream is already available, and otherwise the operation should
fail. This means the media API needs to be extended in order to make
this difference.
This issue is specially relevant in the case of SCO, because the current
approach is racy. With HFP, for example (say BlueZ has the HS role), the
following race condition could be met:
1. Phone has an incoming call and thus starts in-band ringing.
2. SCO connection is accepted and stablished by BlueZ.
3. Gateway interface state is changed to Playing.
4. Exactly afterwards, the user routes the audio to the phone, to have
a private conversation. So the SCO link is closed.
5. In parallel, PulseAudio sees the transition to Playing, and acquires
the transport.
6. BlueZ receives an Acquire() request, but SCO is down. So it tries to
reconnect the SCO link.
The last step is an undesired behavior (the audio is routed back to the
car). BlueZ should be smart enough to know that the SCO connection
shouldn't be reestablished, but this is only possible if the endpoint
provides additional information in the media API.
Note that the API change introduced by this patch is backward
compatible, meaning that older versions of BlueZ will just ignore the
flag. So clients are encouraged to use it without necessarily adding a
dependency to newer versions of BlueZ.
---
audio/transport.c | 4 ++++
doc/media-api.txt | 11 +++++++++++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 4653cc7..cc9c56f 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -807,6 +807,10 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
if (lock == 0)
return btd_error_invalid_args(msg);
+ if (transport->state != TRANSPORT_STATE_PENDING &&
+ g_strstr_len(accesstype, -1, "?") != NULL)
+ return btd_error_failed(msg, "Transport not playing");
+
if (media_transport_acquire(transport, lock) == FALSE)
return btd_error_not_authorized(msg);
diff --git a/doc/media-api.txt b/doc/media-api.txt
index da8ef53..b9802d9 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -282,6 +282,17 @@ Methods dict GetProperties()
"rw": Read and write access
+ The accesstype string can also be combined with a "?"
+ suffix, which will make the request optional. This
+ typically means the transport will only be acquired if
+ it is already available (remote-initiated), but
+ otherwise no request will be sent to the remote side.
+ In this last case the function will fail. Note that,
+ due to compatibility issues with older versions of
+ BlueZ, clients are encouraged to use exactly the same
+ accesstype for Release(), matching the string provided
+ to Acquire().
+
void Release(string accesstype)
Releases file descriptor.
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 8/9] media: Automatically release transport when HUP
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
When the remote end suspends the audio stream, release the transport
automatically without waiting until the clients call Release().
This affects the D-Bus API since clients will get an error when trying
to release the transport afterwards.
However, this should have no real impact, since most clients (i.e.
PulseAudio) would just log some error trace but otherwise ignore the
issue.
---
audio/transport.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 742cc72..4653cc7 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -60,7 +60,7 @@ typedef enum {
TRANSPORT_STATE_IDLE, /* Not acquired and suspended */
TRANSPORT_STATE_PENDING, /* Playing but not acquired */
TRANSPORT_STATE_REQUESTING, /* Acquire in progress */
- TRANSPORT_STATE_ACTIVE, /* Acquired (not necessarily playing) */
+ TRANSPORT_STATE_ACTIVE, /* Acquired and playing */
TRANSPORT_STATE_SUSPENDING, /* Release in progress */
} transport_state_t;
@@ -1178,6 +1178,18 @@ static void transport_update_playing(struct media_transport *transport,
if (playing == FALSE) {
if (transport->state == TRANSPORT_STATE_PENDING)
transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ else if (transport->state == TRANSPORT_STATE_ACTIVE) {
+ /* Remove all owners */
+ while (transport->owners != NULL) {
+ struct media_owner *owner;
+
+ owner = transport->owners->data;
+ media_transport_remove(transport, owner);
+ }
+
+ /* Suspend so that the locks get released */
+ transport->suspend(transport, NULL);
+ }
} else if (transport->state == TRANSPORT_STATE_IDLE)
transport_set_state(transport, TRANSPORT_STATE_PENDING);
}
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 7/9] media: Expose transport state in D-Bus
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Extend the Media API to expose the transport state in D-Bus, as a
property of the transport. This way the clients do not have to find
out which is the corresponding profile-specific interface for the
transport.
Additionally, this state along with the automatic release of transports
will allow clients to avoid the "optional release" or "accept remote
release" race condition. For example, with HSP/HFP profiles, the problem
is the following:
1. User suspends SCO in the remote end.
2. BlueZ signals the Playing->Connected state change in D-Bus.
3. Exactly afterwards, the user resumes SCO in the remote end.
4. In parallel, PulseAudio sees the aforementioned transition to
Connected, and thus releases the transport.
5. BlueZ receives a Release() request while SCO is up. So the audio
stream will be suspended.
The last step is an undesired behavior since the user explicitly wanted
to route the audio stream through Bluetooth.
The issue is difficult to reproduce but it can easily be solved by
exposing the transport state in D-Bus.
---
audio/transport.c | 29 +++++++++++++++++++++++++++++
doc/media-api.txt | 8 ++++++++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index e0402ad..742cc72 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -155,6 +155,22 @@ static transport_lock_t str2lock(const char *str)
return lock;
}
+static const char *state2str(transport_state_t state)
+{
+ switch (state) {
+ case TRANSPORT_STATE_IDLE:
+ case TRANSPORT_STATE_REQUESTING:
+ return "idle";
+ case TRANSPORT_STATE_PENDING:
+ return "pending";
+ case TRANSPORT_STATE_ACTIVE:
+ case TRANSPORT_STATE_SUSPENDING:
+ return "active";
+ }
+
+ return NULL;
+}
+
static gboolean state_in_use(transport_state_t state)
{
switch (state) {
@@ -174,6 +190,7 @@ static void transport_set_state(struct media_transport *transport,
transport_state_t state)
{
transport_state_t old_state = transport->state;
+ const char *str;
if (old_state == state)
return;
@@ -182,6 +199,13 @@ static void transport_set_state(struct media_transport *transport,
DBG("State changed %s: %s -> %s", transport->path, str_state[old_state],
str_state[state]);
+
+ str = state2str(state);
+
+ if (g_strcmp0(str, state2str(old_state)) != 0)
+ emit_property_changed(transport->conn, transport->path,
+ MEDIA_TRANSPORT_INTERFACE, "State",
+ DBUS_TYPE_STRING, &str);
}
void media_transport_destroy(struct media_transport *transport)
@@ -1017,6 +1041,7 @@ void transport_get_properties(struct media_transport *transport,
DBusMessageIter dict;
const char *uuid;
uint8_t codec;
+ const char *state;
dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
@@ -1036,6 +1061,10 @@ void transport_get_properties(struct media_transport *transport,
dict_append_array(&dict, "Configuration", DBUS_TYPE_BYTE,
&transport->configuration, transport->size);
+ /* State */
+ state = state2str(transport->state);
+ dict_append_entry(&dict, "State", DBUS_TYPE_STRING, &state);
+
if (transport->get_properties)
transport->get_properties(transport, &dict);
diff --git a/doc/media-api.txt b/doc/media-api.txt
index e5eeaa0..da8ef53 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -317,6 +317,14 @@ Properties object Device [readonly]
Configuration blob, it is used as it is so the size and
byte order must match.
+ string State [readonly]
+
+ Indicates the state of the transport. Possible
+ values are:
+ "idle": not streaming
+ "pending": streaming but not acquired
+ "active": streaming and acquired
+
uint16 Delay [readwrite]
Optional. Transport delay in 1/10 of millisecond, this
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 6/9] media: Split transport state based on playing flag
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Split the transport states (formerly in_use) into more specific states
where the stream state (playing or suspended) is explicitly represented,
along with the transitional states (locally initiated suspend and
resume).
TRANSPORT_STATE_ACTIVE is an exception since it also includes the state
where the transport is acquired, but the audio was later suspended (not
released yet though).
---
audio/transport.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 639c87f..e0402ad 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -57,13 +57,19 @@ typedef enum {
} transport_lock_t;
typedef enum {
- TRANSPORT_STATE_IDLE, /* Not acquired */
+ TRANSPORT_STATE_IDLE, /* Not acquired and suspended */
+ TRANSPORT_STATE_PENDING, /* Playing but not acquired */
+ TRANSPORT_STATE_REQUESTING, /* Acquire in progress */
TRANSPORT_STATE_ACTIVE, /* Acquired (not necessarily playing) */
+ TRANSPORT_STATE_SUSPENDING, /* Release in progress */
} transport_state_t;
static char *str_state[] = {
"TRANSPORT_STATE_IDLE",
+ "TRANSPORT_STATE_PENDING",
+ "TRANSPORT_STATE_REQUESTING",
"TRANSPORT_STATE_ACTIVE",
+ "TRANSPORT_STATE_SUSPENDING",
};
struct media_request {
@@ -153,8 +159,11 @@ static gboolean state_in_use(transport_state_t state)
{
switch (state) {
case TRANSPORT_STATE_IDLE:
+ case TRANSPORT_STATE_PENDING:
return FALSE;
+ case TRANSPORT_STATE_REQUESTING:
case TRANSPORT_STATE_ACTIVE:
+ case TRANSPORT_STATE_SUSPENDING:
return TRUE;
}
@@ -356,6 +365,8 @@ static void a2dp_resume_complete(struct avdtp *session,
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -382,7 +393,8 @@ static guint resume_a2dp(struct media_transport *transport,
if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return a2dp_resume(a2dp->session, sep, a2dp_resume_complete, owner);
@@ -417,7 +429,12 @@ static guint suspend_a2dp(struct media_transport *transport,
if (!owner) {
a2dp_sep_unlock(sep, a2dp->session);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (a2dp_sep_is_playing(sep))
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -468,6 +485,8 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -486,7 +505,8 @@ static guint resume_headset(struct media_transport *transport,
HEADSET_LOCK_WRITE) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return headset_request_stream(device, headset_resume_complete,
@@ -516,8 +536,15 @@ static guint suspend_headset(struct media_transport *transport,
struct audio_device *device = transport->device;
if (!owner) {
+ headset_state_t state = headset_get_state(device);
+
headset_unlock(device, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (state == HEADSET_STATE_PLAYING)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -574,6 +601,8 @@ static void gateway_resume_complete(struct audio_device *dev, GError *err,
media_owner_remove(owner);
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
return;
fail:
@@ -592,7 +621,8 @@ static guint resume_gateway(struct media_transport *transport,
GATEWAY_LOCK_WRITE) == FALSE)
return 0;
- transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+ if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_REQUESTING);
done:
return gateway_request_stream(device, gateway_resume_complete,
@@ -625,8 +655,15 @@ static guint suspend_gateway(struct media_transport *transport,
static int id = 1;
if (!owner) {
+ gateway_state_t state = gateway_get_state(device);
+
gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
- transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
+ if (state == GATEWAY_STATE_PLAYING)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
+ else
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+
return 0;
}
@@ -806,6 +843,8 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
return btd_error_in_progress(msg);
}
+ transport_set_state(transport, TRANSPORT_STATE_SUSPENDING);
+
id = transport->suspend(transport, owner);
if (id == 0) {
media_transport_remove(transport, owner);
@@ -1104,7 +1143,14 @@ static void headset_nrec_changed(struct audio_device *dev, gboolean nrec,
static void transport_update_playing(struct media_transport *transport,
gboolean playing)
{
- DBG("%s Playing=%d", transport->path, playing);
+ DBG("%s State=%s Playing=%d", transport->path,
+ str_state[transport->state], playing);
+
+ if (playing == FALSE) {
+ if (transport->state == TRANSPORT_STATE_PENDING)
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
+ } else if (transport->state == TRANSPORT_STATE_IDLE)
+ transport_set_state(transport, TRANSPORT_STATE_PENDING);
}
static void headset_state_changed(struct audio_device *dev,
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 5/9] media: Watch interface state changes in transport
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Install watches to keep track whether the audio is streaming or not.
This should be relevant if the transport needs to reflect this state.
---
audio/transport.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 103 insertions(+), 0 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 410601f..639c87f 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -45,6 +45,8 @@
#include "a2dp.h"
#include "headset.h"
#include "gateway.h"
+#include "sink.h"
+#include "source.h"
#include "avrcp.h"
#define MEDIA_TRANSPORT_INTERFACE "org.bluez.MediaTransport"
@@ -101,6 +103,10 @@ struct media_transport {
uint16_t omtu; /* Transport output mtu */
transport_lock_t lock;
transport_state_t state;
+ guint hs_watch;
+ guint ag_watch;
+ guint source_watch;
+ guint sink_watch;
guint (*resume) (struct media_transport *transport,
struct media_owner *owner);
guint (*suspend) (struct media_transport *transport,
@@ -173,6 +179,18 @@ void media_transport_destroy(struct media_transport *transport)
{
char *path;
+ if (transport->hs_watch)
+ headset_remove_state_cb(transport->hs_watch);
+
+ if (transport->ag_watch)
+ gateway_remove_state_cb(transport->ag_watch);
+
+ if (transport->sink_watch)
+ sink_remove_state_cb(transport->sink_watch);
+
+ if (transport->source_watch)
+ source_remove_state_cb(transport->source_watch);
+
path = g_strdup(transport->path);
g_dbus_unregister_interface(transport->conn, path,
MEDIA_TRANSPORT_INTERFACE);
@@ -1083,6 +1101,76 @@ static void headset_nrec_changed(struct audio_device *dev, gboolean nrec,
DBUS_TYPE_BOOLEAN, &nrec);
}
+static void transport_update_playing(struct media_transport *transport,
+ gboolean playing)
+{
+ DBG("%s Playing=%d", transport->path, playing);
+}
+
+static void headset_state_changed(struct audio_device *dev,
+ headset_state_t old_state,
+ headset_state_t new_state,
+ void *user_data)
+{
+ struct media_transport *transport = user_data;
+
+ if (dev != transport->device)
+ return;
+
+ if (new_state == HEADSET_STATE_PLAYING)
+ transport_update_playing(transport, TRUE);
+ else
+ transport_update_playing(transport, FALSE);
+}
+
+static void gateway_state_changed(struct audio_device *dev,
+ gateway_state_t old_state,
+ gateway_state_t new_state,
+ void *user_data)
+{
+ struct media_transport *transport = user_data;
+
+ if (dev != transport->device)
+ return;
+
+ if (new_state == GATEWAY_STATE_PLAYING)
+ transport_update_playing(transport, TRUE);
+ else
+ transport_update_playing(transport, FALSE);
+}
+
+static void sink_state_changed(struct audio_device *dev,
+ sink_state_t old_state,
+ sink_state_t new_state,
+ void *user_data)
+{
+ struct media_transport *transport = user_data;
+
+ if (dev != transport->device)
+ return;
+
+ if (new_state == SINK_STATE_PLAYING)
+ transport_update_playing(transport, TRUE);
+ else
+ transport_update_playing(transport, FALSE);
+}
+
+static void source_state_changed(struct audio_device *dev,
+ source_state_t old_state,
+ source_state_t new_state,
+ void *user_data)
+{
+ struct media_transport *transport = user_data;
+
+ if (dev != transport->device)
+ return;
+
+ if (new_state == SOURCE_STATE_PLAYING)
+ transport_update_playing(transport, TRUE);
+ else
+ transport_update_playing(transport, FALSE);
+}
+
struct media_transport *media_transport_create(DBusConnection *conn,
struct media_endpoint *endpoint,
struct audio_device *device,
@@ -1118,6 +1206,15 @@ struct media_transport *media_transport_create(DBusConnection *conn,
transport->set_property = set_property_a2dp;
transport->data = a2dp;
transport->destroy = destroy_a2dp;
+
+ if (strcasecmp(uuid, A2DP_SOURCE_UUID) == 0)
+ transport->sink_watch = sink_add_state_cb(
+ sink_state_changed,
+ transport);
+ else
+ transport->source_watch = source_add_state_cb(
+ source_state_changed,
+ transport);
} else if (strcasecmp(uuid, HFP_AG_UUID) == 0 ||
strcasecmp(uuid, HSP_AG_UUID) == 0) {
struct headset_transport *headset;
@@ -1135,6 +1232,9 @@ struct media_transport *media_transport_create(DBusConnection *conn,
transport->set_property = set_property_headset;
transport->data = headset;
transport->destroy = destroy_headset;
+ transport->hs_watch = headset_add_state_cb(
+ headset_state_changed,
+ transport);
} else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
strcasecmp(uuid, HSP_HS_UUID) == 0) {
transport->resume = resume_gateway;
@@ -1142,6 +1242,9 @@ struct media_transport *media_transport_create(DBusConnection *conn,
transport->cancel = cancel_gateway;
transport->get_properties = get_properties_gateway;
transport->set_property = set_property_gateway;
+ transport->ag_watch = gateway_add_state_cb(
+ gateway_state_changed,
+ transport);
} else
goto fail;
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 4/9] media: Replace transport->in_use flag with state
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Refactor the code to use a enum type to represent the transport state.
This should scale better when additional states need to be represented.
A helper function has been added to help track the mapping between the
enum type and the old in_use flag.
---
audio/transport.c | 77 ++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 58 insertions(+), 19 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index d418878..410601f 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -54,6 +54,16 @@ typedef enum {
TRANSPORT_LOCK_WRITE = 1 << 1,
} transport_lock_t;
+typedef enum {
+ TRANSPORT_STATE_IDLE, /* Not acquired */
+ TRANSPORT_STATE_ACTIVE, /* Acquired (not necessarily playing) */
+} transport_state_t;
+
+static char *str_state[] = {
+ "TRANSPORT_STATE_IDLE",
+ "TRANSPORT_STATE_ACTIVE",
+};
+
struct media_request {
DBusMessage *msg;
guint id;
@@ -90,7 +100,7 @@ struct media_transport {
uint16_t imtu; /* Transport input mtu */
uint16_t omtu; /* Transport output mtu */
transport_lock_t lock;
- gboolean in_use;
+ transport_state_t state;
guint (*resume) (struct media_transport *transport,
struct media_owner *owner);
guint (*suspend) (struct media_transport *transport,
@@ -133,6 +143,32 @@ static transport_lock_t str2lock(const char *str)
return lock;
}
+static gboolean state_in_use(transport_state_t state)
+{
+ switch (state) {
+ case TRANSPORT_STATE_IDLE:
+ return FALSE;
+ case TRANSPORT_STATE_ACTIVE:
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
+static void transport_set_state(struct media_transport *transport,
+ transport_state_t state)
+{
+ transport_state_t old_state = transport->state;
+
+ if (old_state == state)
+ return;
+
+ transport->state = state;
+
+ DBG("State changed %s: %s -> %s", transport->path, str_state[old_state],
+ str_state[state]);
+}
+
void media_transport_destroy(struct media_transport *transport)
{
char *path;
@@ -240,7 +276,7 @@ static void media_transport_remove(struct media_transport *transport,
media_owner_free(owner);
/* Suspend if there is no longer any owner */
- if (transport->owners == NULL && transport->in_use)
+ if (transport->owners == NULL && state_in_use(transport->state))
transport->suspend(transport, NULL);
}
@@ -322,13 +358,14 @@ static guint resume_a2dp(struct media_transport *transport,
return 0;
}
- if (transport->in_use == TRUE)
+ if (state_in_use(transport->state))
goto done;
- transport->in_use = a2dp_sep_lock(sep, a2dp->session);
- if (transport->in_use == FALSE)
+ if (a2dp_sep_lock(sep, a2dp->session) == FALSE)
return 0;
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
done:
return a2dp_resume(a2dp->session, sep, a2dp_resume_complete, owner);
}
@@ -349,7 +386,7 @@ static void a2dp_suspend_complete(struct avdtp *session,
}
a2dp_sep_unlock(sep, a2dp->session);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
media_transport_remove(transport, owner);
}
@@ -362,7 +399,7 @@ static guint suspend_a2dp(struct media_transport *transport,
if (!owner) {
a2dp_sep_unlock(sep, a2dp->session);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
return 0;
}
@@ -424,14 +461,15 @@ static guint resume_headset(struct media_transport *transport,
{
struct audio_device *device = transport->device;
- if (transport->in_use == TRUE)
+ if (state_in_use(transport->state))
goto done;
- transport->in_use = headset_lock(device, HEADSET_LOCK_READ |
- HEADSET_LOCK_WRITE);
- if (transport->in_use == FALSE)
+ if (headset_lock(device, HEADSET_LOCK_READ |
+ HEADSET_LOCK_WRITE) == FALSE)
return 0;
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
done:
return headset_request_stream(device, headset_resume_complete,
owner);
@@ -450,7 +488,7 @@ static void headset_suspend_complete(struct audio_device *dev, void *user_data)
}
headset_unlock(dev, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
media_transport_remove(transport, owner);
}
@@ -461,7 +499,7 @@ static guint suspend_headset(struct media_transport *transport,
if (!owner) {
headset_unlock(device, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
return 0;
}
@@ -529,14 +567,15 @@ static guint resume_gateway(struct media_transport *transport,
{
struct audio_device *device = transport->device;
- if (transport->in_use == TRUE)
+ if (state_in_use(transport->state))
goto done;
- transport->in_use = gateway_lock(device, GATEWAY_LOCK_READ |
- GATEWAY_LOCK_WRITE);
- if (transport->in_use == FALSE)
+ if (gateway_lock(device, GATEWAY_LOCK_READ |
+ GATEWAY_LOCK_WRITE) == FALSE)
return 0;
+ transport_set_state(transport, TRANSPORT_STATE_ACTIVE);
+
done:
return gateway_request_stream(device, gateway_resume_complete,
owner);
@@ -556,7 +595,7 @@ static gboolean gateway_suspend_complete(gpointer user_data)
}
gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
media_transport_remove(transport, owner);
return FALSE;
}
@@ -569,7 +608,7 @@ static guint suspend_gateway(struct media_transport *transport,
if (!owner) {
gateway_unlock(device, GATEWAY_LOCK_READ | GATEWAY_LOCK_WRITE);
- transport->in_use = FALSE;
+ transport_set_state(transport, TRANSPORT_STATE_IDLE);
return 0;
}
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 3/9] media: Add gateway_get_state() to internal API
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Expose the state of the gateway interface in the internal API.
---
audio/gateway.c | 7 +++++++
audio/gateway.h | 1 +
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/audio/gateway.c b/audio/gateway.c
index 0603f12..53094af 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -787,6 +787,13 @@ gboolean gateway_is_active(struct audio_device *dev)
return FALSE;
}
+gateway_state_t gateway_get_state(struct audio_device *dev)
+{
+ struct gateway *gw = dev->gateway;
+
+ return gw->state;
+}
+
int gateway_connect_rfcomm(struct audio_device *dev, GIOChannel *io)
{
if (!io)
diff --git a/audio/gateway.h b/audio/gateway.h
index 0893962..d87d76a 100644
--- a/audio/gateway.h
+++ b/audio/gateway.h
@@ -59,6 +59,7 @@ void gateway_unregister(struct audio_device *dev);
struct gateway *gateway_init(struct audio_device *dev);
gboolean gateway_is_active(struct audio_device *dev);
gboolean gateway_is_connected(struct audio_device *dev);
+gateway_state_t gateway_get_state(struct audio_device *dev);
int gateway_connect_rfcomm(struct audio_device *dev, GIOChannel *io);
int gateway_connect_sco(struct audio_device *dev, GIOChannel *chan);
void gateway_start_service(struct audio_device *device);
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 2/9] media: Add a2dp_sep_is_playing() to internal API
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Add this function to expose whether the local SEP is streaming or not.
---
audio/a2dp.c | 8 ++++++++
audio/a2dp.h | 1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/audio/a2dp.c b/audio/a2dp.c
index a9546b7..64b37e7 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1833,6 +1833,14 @@ gboolean a2dp_sep_unlock(struct a2dp_sep *sep, struct avdtp *session)
return TRUE;
}
+gboolean a2dp_sep_is_playing(struct a2dp_sep *sep)
+{
+ if (avdtp_sep_get_state(sep->lsep) == AVDTP_STATE_STREAMING)
+ return TRUE;
+ else
+ return FALSE;
+}
+
gboolean a2dp_sep_get_lock(struct a2dp_sep *sep)
{
return sep->locked;
diff --git a/audio/a2dp.h b/audio/a2dp.h
index 27b4a57..deab3b8 100644
--- a/audio/a2dp.h
+++ b/audio/a2dp.h
@@ -90,6 +90,7 @@ gboolean a2dp_cancel(struct audio_device *dev, unsigned int id);
gboolean a2dp_sep_lock(struct a2dp_sep *sep, struct avdtp *session);
gboolean a2dp_sep_unlock(struct a2dp_sep *sep, struct avdtp *session);
gboolean a2dp_sep_get_lock(struct a2dp_sep *sep);
+gboolean a2dp_sep_is_playing(struct a2dp_sep *sep);
struct avdtp_stream *a2dp_sep_get_stream(struct a2dp_sep *sep);
struct a2dp_sep *a2dp_get_sep(struct avdtp *session,
struct avdtp_stream *stream);
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 1/9] media: Fix accesstype comparison
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1347624196-11281-1-git-send-email-mikel.astiz.oss@gmail.com>
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Replace the string representation of the accesstype with a conventional
binary representation. This makes the code simpler and more efficient.
This also fixes a minor bug in the Release() D-Bus method, where the
string comparison was used to see whether the owner should be removed. A
client acquiring with "rw" and releasing with "wr" would lead to the
inconsistent state of having a released transport with an owner with no
accesstype. Partial releases can also get affected by this bug since the
released character (partial accesstype) got replaced by a whitespace.
Additionally, this approach is more robust in case new flags are added
in the future.
---
audio/transport.c | 119 ++++++++++++++++++++++++++++++-----------------------
1 files changed, 68 insertions(+), 51 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index d40c92d..d418878 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -49,6 +49,11 @@
#define MEDIA_TRANSPORT_INTERFACE "org.bluez.MediaTransport"
+typedef enum {
+ TRANSPORT_LOCK_READ = 1,
+ TRANSPORT_LOCK_WRITE = 1 << 1,
+} transport_lock_t;
+
struct media_request {
DBusMessage *msg;
guint id;
@@ -58,7 +63,7 @@ struct media_owner {
struct media_transport *transport;
struct media_request *pending;
char *name;
- char *accesstype;
+ transport_lock_t lock;
guint watch;
};
@@ -84,8 +89,7 @@ struct media_transport {
int fd; /* Transport file descriptor */
uint16_t imtu; /* Transport input mtu */
uint16_t omtu; /* Transport output mtu */
- gboolean read_lock;
- gboolean write_lock;
+ transport_lock_t lock;
gboolean in_use;
guint (*resume) (struct media_transport *transport,
struct media_owner *owner);
@@ -104,6 +108,31 @@ struct media_transport {
void *data;
};
+static const char *lock2str(transport_lock_t lock)
+{
+ if (lock == 0)
+ return "";
+ else if (lock == TRANSPORT_LOCK_READ)
+ return "r";
+ else if (lock == TRANSPORT_LOCK_WRITE)
+ return "w";
+ else
+ return "rw";
+}
+
+static transport_lock_t str2lock(const char *str)
+{
+ transport_lock_t lock = 0;
+
+ if (g_strstr_len(str, -1, "r") != NULL)
+ lock |= TRANSPORT_LOCK_READ;
+
+ if (g_strstr_len(str, -1, "w") != NULL)
+ lock |= TRANSPORT_LOCK_WRITE;
+
+ return lock;
+}
+
void media_transport_destroy(struct media_transport *transport)
{
char *path;
@@ -148,17 +177,15 @@ static void media_request_reply(struct media_request *req,
}
static gboolean media_transport_release(struct media_transport *transport,
- const char *accesstype)
+ transport_lock_t lock)
{
- if (g_strstr_len(accesstype, -1, "r") != NULL) {
- transport->read_lock = FALSE;
+ transport->lock &= ~lock;
+
+ if (lock & TRANSPORT_LOCK_READ)
DBG("Transport %s: read lock released", transport->path);
- }
- if (g_strstr_len(accesstype, -1, "w") != NULL) {
- transport->write_lock = FALSE;
+ if (lock & TRANSPORT_LOCK_WRITE)
DBG("Transport %s: write lock released", transport->path);
- }
return TRUE;
}
@@ -191,7 +218,6 @@ static void media_owner_free(struct media_owner *owner)
media_owner_remove(owner);
g_free(owner->name);
- g_free(owner->accesstype);
g_free(owner);
}
@@ -200,7 +226,7 @@ static void media_transport_remove(struct media_transport *transport,
{
DBG("Transport %s Owner %s", transport->path, owner->name);
- media_transport_release(transport, owner->accesstype);
+ media_transport_release(transport, owner->lock);
/* Reply if owner has a pending request */
if (owner->pending)
@@ -260,10 +286,10 @@ static void a2dp_resume_complete(struct avdtp *session,
media_transport_set_fd(transport, fd, imtu, omtu);
- if (g_strstr_len(owner->accesstype, -1, "r") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_READ) == 0)
imtu = 0;
- if (g_strstr_len(owner->accesstype, -1, "w") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_WRITE) == 0)
omtu = 0;
ret = g_dbus_send_reply(transport->conn, req->msg,
@@ -371,10 +397,10 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
media_transport_set_fd(transport, fd, imtu, omtu);
- if (g_strstr_len(owner->accesstype, -1, "r") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_READ) == 0)
imtu = 0;
- if (g_strstr_len(owner->accesstype, -1, "w") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_WRITE) == 0)
omtu = 0;
ret = g_dbus_send_reply(transport->conn, req->msg,
@@ -476,10 +502,10 @@ static void gateway_resume_complete(struct audio_device *dev, GError *err,
media_transport_set_fd(transport, fd, imtu, omtu);
- if (g_strstr_len(owner->accesstype, -1, "r") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_READ) == 0)
imtu = 0;
- if (g_strstr_len(owner->accesstype, -1, "w") == NULL)
+ if ((owner->lock & TRANSPORT_LOCK_WRITE) == 0)
omtu = 0;
ret = g_dbus_send_reply(transport->conn, req->msg,
@@ -569,35 +595,18 @@ static void media_owner_exit(DBusConnection *connection, void *user_data)
}
static gboolean media_transport_acquire(struct media_transport *transport,
- const char *accesstype)
+ transport_lock_t lock)
{
- gboolean read_lock = FALSE, write_lock = FALSE;
-
- if (g_strstr_len(accesstype, -1, "r") != NULL) {
- if (transport->read_lock == TRUE)
- return FALSE;
- read_lock = TRUE;
- }
-
- if (g_strstr_len(accesstype, -1, "w") != NULL) {
- if (transport->write_lock == TRUE)
- return FALSE;
- write_lock = TRUE;
- }
-
- /* Check invalid accesstype */
- if (read_lock == FALSE && write_lock == FALSE)
+ if (transport->lock & lock)
return FALSE;
- if (read_lock) {
- transport->read_lock = read_lock;
+ transport->lock |= lock;
+
+ if (lock & TRANSPORT_LOCK_READ)
DBG("Transport %s: read lock acquired", transport->path);
- }
- if (write_lock) {
- transport->write_lock = write_lock;
+ if (lock & TRANSPORT_LOCK_WRITE)
DBG("Transport %s: write lock acquired", transport->path);
- }
return TRUE;
@@ -616,16 +625,16 @@ static void media_transport_add(struct media_transport *transport,
static struct media_owner *media_owner_create(DBusConnection *conn,
DBusMessage *msg,
- const char *accesstype)
+ transport_lock_t lock)
{
struct media_owner *owner;
owner = g_new0(struct media_owner, 1);
owner->name = g_strdup(dbus_message_get_sender(msg));
- owner->accesstype = g_strdup(accesstype);
+ owner->lock = lock;
DBG("Owner created: sender=%s accesstype=%s", owner->name,
- accesstype);
+ lock2str(lock));
return owner;
}
@@ -662,6 +671,7 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
struct media_owner *owner;
struct media_request *req;
const char *accesstype, *sender;
+ transport_lock_t lock;
guint id;
if (!dbus_message_get_args(msg, NULL,
@@ -675,13 +685,17 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
if (owner != NULL)
return btd_error_not_authorized(msg);
- if (media_transport_acquire(transport, accesstype) == FALSE)
+ lock = str2lock(accesstype);
+ if (lock == 0)
+ return btd_error_invalid_args(msg);
+
+ if (media_transport_acquire(transport, lock) == FALSE)
return btd_error_not_authorized(msg);
- owner = media_owner_create(conn, msg, accesstype);
+ owner = media_owner_create(conn, msg, lock);
id = transport->resume(transport, owner);
if (id == 0) {
- media_transport_release(transport, accesstype);
+ media_transport_release(transport, lock);
media_owner_free(owner);
return btd_error_not_authorized(msg);
}
@@ -699,6 +713,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
struct media_transport *transport = data;
struct media_owner *owner;
const char *accesstype, *sender;
+ transport_lock_t lock;
struct media_request *req;
if (!dbus_message_get_args(msg, NULL,
@@ -712,7 +727,9 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
if (owner == NULL)
return btd_error_not_authorized(msg);
- if (g_strcmp0(owner->accesstype, accesstype) == 0) {
+ lock = str2lock(accesstype);
+
+ if (owner->lock == lock) {
guint id;
/* Not the last owner, no need to suspend */
@@ -742,9 +759,9 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
media_owner_add(owner, req);
return NULL;
- } else if (g_strstr_len(owner->accesstype, -1, accesstype) != NULL) {
- media_transport_release(transport, accesstype);
- g_strdelimit(owner->accesstype, accesstype, ' ');
+ } else if ((owner->lock & lock) == lock) {
+ media_transport_release(transport, lock);
+ owner->lock &= ~lock;
} else
return btd_error_not_authorized(msg);
--
1.7.7.6
^ permalink raw reply related
* [RFC v4 0/9] Optional acquire in Media API and related
From: Mikel Astiz @ 2012-09-14 12:03 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
Similar patchset as v3 with the integration of Luiz's feedback, including the following changes:
- Patches 7 and 8 swapped
- State "acquired" renamed to "active" (both in D-Bus and in the code)
- TRANSPORT_STATE_POST_ACQUIRE (which was transitional) dropped
>From original patch:
This patch reopens the discussion started by the thread "when is acquire
ok to call". The race condition seems to be real (even thought difficult
to reproduce), and I couldn't think of any approach to solve this
without altering the Media API.
Mikel Astiz (9):
media: Fix accesstype comparison
media: Add a2dp_sep_is_playing() to internal API
media: Add gateway_get_state() to internal API
media: Replace transport->in_use flag with state
media: Watch interface state changes in transport
media: Split transport state based on playing flag
media: Expose transport state in D-Bus
media: Automatically release transport when HUP
media: Extend media API with optional acquire
audio/a2dp.c | 8 +
audio/a2dp.h | 1 +
audio/gateway.c | 7 +
audio/gateway.h | 1 +
audio/transport.c | 390 +++++++++++++++++++++++++++++++++++++++++++----------
doc/media-api.txt | 19 +++
6 files changed, 356 insertions(+), 70 deletions(-)
--
1.7.7.6
^ permalink raw reply
* [PATCH v0] media: Fix infinite loop due to release_endpoint()
From: Mikel Astiz @ 2012-09-14 11:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Mikel Astiz
From: Mikel Astiz <mikel.astiz@bmw-carit.de>
release_endpoint() cannot succeed unless all transports are released
first. For example a2dp_remove_sep() will ignore the call if the SEP is
locked, leading to an infinite loop in path_free(), which expects to
successfully release and remove the endpoint in each call to
release_endpoint().
This issue can easily be reproduced by shutting bluetoothd daemon
during A2DP streaming (tested in sink role).
---
audio/media.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index b0ea4e9..e4a7684 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -454,6 +454,8 @@ static void release_endpoint(struct media_endpoint *endpoint)
if (endpoint->watch == 0)
goto done;
+ clear_endpoint(endpoint);
+
msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
MEDIA_ENDPOINT_INTERFACE,
"Release");
@@ -615,8 +617,6 @@ static void a2dp_destroy_endpoint(void *user_data)
{
struct media_endpoint *endpoint = user_data;
- clear_endpoint(endpoint);
-
endpoint->sep = NULL;
release_endpoint(endpoint);
}
--
1.7.7.6
^ permalink raw reply related
* Re: [KERNEL PATCH] HID: Add support for Sony BD Remote
From: Luiz Augusto von Dentz @ 2012-09-14 10:42 UTC (permalink / raw)
To: David Dillow
Cc: linux-bluetooth, David Herrmann, Bastien Nocera, Antonio Ospite
In-Reply-To: <1347591892.6145.14.camel@obelisk.thedillows.org>
Hi David,
On Fri, Sep 14, 2012 at 6:04 AM, David Dillow <dave@thedillows.org> wrote:
> We also gain support for the Logitech Harmony Adapter for PS3.
>
> Signed-off-by: David Dillow <dave@thedillows.org>
> --
> drivers/hid/Kconfig | 7 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ps3remote.c | 325 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 335 insertions(+), 0 deletions(-)
>
> On Thu, 2012-09-13 at 00:36 +0200, Antonio Ospite wrote:
> On Wed, 12 Sep 2012 22:26:04 +0200 David Herrmann wrote:
>> > So if the devices only need some short setup-command during
>> > initialization and then they send HID reports whenever a button is
>> > pressed, then everything should be very easy to get working. Even
>> > auto-reconnect and 10min idle-disconnect are _very_ easy to get
>> > working in the kernel with the current HID infrastructure.
>>
>> I too will take a look during the week-end about writing a kernel
>> driver, if it's not that much of an effort I might take the project
>> (no promises yet). I have a PS3 BD remote Bastien donated.
>
> Here's a first pass at a kernel driver to support the BD remote and
> Harmony PS adapter. I've tested it on Fedora 16 with single keypresses
> only, as I cannot send multiple simultaneous keypresses with the
> Harmony. This patch is against kernel.org's bluetooth.git as of an hour
> ago, but should apply to other versions without much fuss.
Great that you are looking into it.
> Antonio, if you could test this with the real remote you have, I'd be
> very appreciative.
>
> David, could you point me at the controls for the 10min idle-disconnect?
I suppose we can just continue using input.conf:IdleTimeout (which
passed via ioctl in the struct hidp_connadd_req.idle_to) or do you
think this would be useful for other transports, at least for USB it
does not make much sense since the cable will stay connected.
> auto-connect and idle-disconnect seem to be working on my Fedora 16 box,
> though it looks like the Harmony adapter is initiating the disconnect --
> this is likely an area for improvement with the Sony remote.
Btw, it would probably be a good to add a proper defines for 0x0306
e.g. USB_DEVICE_ID_SONY_PS3_REMOTE and
USB_DEVICE_ID_LOGITECH_HARMONY_ADAPTER
--
Luiz Augusto von Dentz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox