Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH v4 00/16] mSBC tests
From: Siarhei Siamashka @ 2012-11-14 19:57 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: frederic.dalleau
In-Reply-To: <1352904655.20338.11.camel@aeonflux>

On Wed, 14 Nov 2012 23:50:55 +0900
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Fred,
> 
> > > v4 integrate latest comments from Siarhei:
> > >   - remove state->pending field from sbc_encoder_state
> > >   - add armv6 and iwmmxt primitives
> > >   - use simd primitive instead of neon
> > >   - reorder patches so that the SBC_MSBC flag is exposed to users only when
> > > implementation is complete.
> > 
> > Any feedback ?
> 
> beside a tiny cosmetic comment, I have nothing. However I am not the
> expert here. If Siarhei looks over this and is fine with it, I will be
> as well.

Hi,

My biggest concern is the way how we handle the mSBC API/ABI extension.
Because once the new version of sbc library is out, we can't
(easily/painlessly) change it any more. Everything else is fixable.

Anyway, as far as I can see, there are no regressions in the existing
SBC functionality, which is the most important requirement.

I personally would prefer a bit more verbose commit messages. But let
me know if these demands are unreasonable :)

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* Re: [PATCH v4 05/16] sbc: Add mmx primitive for 1b 8s analysis
From: Siarhei Siamashka @ 2012-11-14 20:23 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A3BC30.8060209@linux.intel.com>

On Wed, 14 Nov 2012 16:43:44 +0100
Frédéric Dalleau  <frederic.dalleau@linux.intel.com> wrote:

> Hi,
> 
> Since I'm gonna resend a new series, I'll comment myself ;)
> 
> On 10/30/2012 10:39 AM, Frédéric Dalleau wrote:
> > +static inline void sbc_analyze_1b_8s_mmx(struct sbc_encoder_state *state,
> > +		int16_t *x, int32_t *out, int out_stride)
> > +{
> > +	if (state->odd)
> > +		sbc_analyze_eight_mmx(x, out, analysis_consts_fixed8_simd_odd);
> > +	else
> > +		sbc_analyze_eight_mmx(x, out, analysis_consts_fixed8_simd_even);
> > +
> > +	state->odd = !state->odd;
> > +
> > +	__asm__ volatile ("emms\n");
> > +}
> > +
> 
> One thing bother me about this patch : the emms instruction is called
> after every block, instead of every four blocks until now. I have very
> little knowledge about this, but I read that emms instruction is
> somewhat expensive.
> Some quick tests haven't shown differences, but it is possible to add a
> post analyze callback to overcome this. In this case, emms instruction
> could be run every 15 blocks or whatever is defined.

The EMMS instruction must be used after the use of MMX instructions,
otherwise the subsequent floating point calculations are broken.

As part of calling conventions, FPU state must be clear after returning
from any function:
    http://www.agner.org/optimize/calling_conventions.pdf
It means that normally every MMX function needs to execute EMMS
instruction before returning. We were already cutting the corners a bit
by putting MMX code into static inline functions which don't have
EMMS themselves. But using the post analyze callback would be really
wrong as that's going to explicitly cross function boundaries with
inconsistent FPU state.

> 
> Is it worth it?

If benchmarks do not show a significant performance drop, then it does
not really matter. A minor performance regression is fine, as long as
the MMX code is still significantly faster than C.

Nowadays using SSE2 is a much better idea. And SSE2 does not suffer
from EMMS-alike warts.

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* Re: [RFCv1 1/3] Bluetooth: Refactor locking in amp_physical_cfm
From: Marcel Holtmann @ 2012-11-14 23:04 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1352907572-7113-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

> Remove locking from l2cap_physical_cfm and lock chan inside
> amp_physical_cfm.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  include/net/bluetooth/l2cap.h |    2 +-
>  net/bluetooth/amp.c           |    6 +++++-
>  net/bluetooth/l2cap_core.c    |    7 ++-----
>  3 files changed, 8 insertions(+), 7 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [RFCv1 3/3] Bluetooth: trivial: Use __constant for constants
From: Marcel Holtmann @ 2012-11-14 23:07 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1352907572-7113-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/l2cap_core.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: [RFCv1 2/3] Bluetooth: Disable FCS only for new HS channels
From: Marcel Holtmann @ 2012-11-14 23:08 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1352907572-7113-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

> Set chan->fcs to L2CAP_FCS_NONE only for new L2CAP channels
> (not moved). Other side can still request to use FCS.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/amp.c        |    1 -
>  net/bluetooth/l2cap_core.c |    2 ++
>  2 files changed, 2 insertions(+), 1 deletion(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply

* Re: CSA2:  User space aspect
From: Marcel Holtmann @ 2012-11-14 23:17 UTC (permalink / raw)
  To: Michael Knudsen; +Cc: linux-bluetooth
In-Reply-To: <50A3B0D5.9030107@samsung.com>

Hi Michael,

> > If anyone else has an ideas or opinions about this, please speak up,
> > otherwise I'll try coming up with an interface specification with
> > more details.
> 
> This diff shows the direction I'm heading:
> 
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index 1e35c43..a565a4d 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -51,6 +51,42 @@ struct sco_conninfo {
>  	__u8  dev_class[3];
>  };
>  
> +/* Audio format setting */
> +#define SCO_HOST_FORMAT	0x04
> +#define SCO_AIR_FORMAT	0x05
> +
> +#define SCO_FORMAT_ULAW		0x00
> +#define SCO_FORMAT_ALAW		0x01
> +#define SCO_FORMAT_CVSD		0x02
> +#define SCO_FORMAT_TRANSPARENT	0x03 
> +#define SCO_FORMAT_PCM		0x05 
> +#define SCO_FORMAT_MSBC		0x05 
> +#define SCO_FORMAT_VENDOR	0xff
> +struct sco_format_vendor {
> +	__u16 vendor;
> +	__u16 codec;
> +};
> +
> +struct sco_format {
> +	__u8                     in_format;
> +	struct sco_format_vendor in_vendor;
> +
> +	__u8                     out_format;
> +	struct sco_format_vendor out_vendor;
> +};
> +
> +#define SCO_CODECS		0x06
> +struct sco_codecs {
> +	__u8  count;
> +	__u8 *codec;
> +};
> +
> +#define SCO_CODECS_VENDOR	0x07
> +struct sco_codecs_vendor {
> +	__u8                      count;
> +	struct sco_format_vendor *format;
> +};
> +
>  /* ---- SCO connections ---- */
>  struct sco_conn {
>  	struct hci_conn	*hcon;
> @@ -74,6 +110,8 @@ struct sco_pinfo {
>  	struct bt_sock	bt;
>  	__u32		flags;
>  	struct sco_conn	*conn;
> +	struct sco_format host_format;
> +	struct sco_format air_format;
>  };
>  
>  #endif /* __SCO_H */
> 
> Basically, this adds four socket options (I'll do the audio path
> stuff as well once this is done):
> 
> 	SCO_AIR_FORMAT
> 	SCO_HOST_FORMAT
> 	SCO_CODECS (ro)
> 	SCO_CODECS_VENDOR (ro)
> 
> The SCO_CODECS ones provide the application with a list of codecs
> supported by the hdev as indicated in the HCI_Read_Local_Supported_Codecs
> command response, and if the hdev does not support this command a
> default of linear PCM, CVSD, and transparent will be provided.

please to not attempt to use socket options as ioctl. They are called
options for a reason.

Getting the list of supported codecs should be done via mgmt interface
command and it should be only done once.

> Because the result length is variable, the idea is that the application-
> provided structure is modified by the kernel to hold the actual number
> of results so the application can allocate a buffer accordingly, e.g.:
> 
> 	struct sco_codecs sc;
> 	
> 	memset(&sc, 0, sizeof(sc));
> 	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);
> 
> 	sk.codecs = malloc(sk.count);
> 	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);

We are not doing that. I have no intention to map kernel memory to
userspace memory and back with socket options.

> The SCO_*_FORMAT ones allows the application to set the parameters that
> are to be used on host-controller and controller-controller paths.  While
> the spec requires the pairs (host input/output, air input/output) to be
> identical, I don't see a reason to enforce this in the API, thus all are
> set independently.
> 
> So, before I spend any more time on this.. comments?

Please ask yourself the question when and how the SCO data from the
socket is actually affected. I still have not seen you provide the
semantics of how the socket would be used afterwards. Especially on
impact for the application establishing the SCO socket.

In a more important question, is this static one time fits all selection
or is this actually to be dynamic on every new connection establishment.

Regards

Marcel



^ permalink raw reply

* Re: [PATCH v4 11/16] sbc: Add SBC_MSBC flag to enable 15 block encoding
From: Marcel Holtmann @ 2012-11-14 23:20 UTC (permalink / raw)
  To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A3B9FC.3070009@linux.intel.com>

Hi Fred,

> > I am debating to actually call this SBC_FLAG_MSBC instead of just
> > SBC_MSBC. Anyone any comments?
> 
> The use of the flag may not be the most elegant API. As an alternative,
> maybe we could have a specific function for msbc initialization. Then,
> instead of sbc_init(), sbc_encode() sbc_decode(), we could have
> msbc_init(), sbc_encode(), sbc_decode().
> The biggest advantage is that parameters like bitpool, subbands,
> allocation, and other parts could be specified inside this function
> instead of being given from the application.

we could do an sbc_init_msbc() function.

As this is the only real external visible API change, we need to be 100%
sure with this one.

Regards

Marcel



^ permalink raw reply

* [PATCH v2 0/8] Adopt btd_profile for A2DP
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

The proposal is to split A2DP roles into different btd_profile instances, in order to expose .connect and .disconnect.

The changes in v2 include:

- Renaming of functions as suggested by Luiz (avoid using local_xxx)
- Merge of source_shutdown and source_disconnect: they were quite similar and having a generalized function seems a better idea (to be discussed if we can get rid of the gboolean shutdown parameter)
- New patches have been added to address the sink part as well (v1 focused on the source role)

Mikel Astiz (8):
  audio: Trivial function renames
  sink: Generalize disconnection function
  source: Add missing code in source_disconnect()
  source: Expose internal connection API
  sink: Expose internal connection API
  audio: Split A2DP into three btd_profile
  source: Add profile .connect and .disconnect
  sink: Add profile .connect and .disconnect

 profiles/audio/a2dp.c    |  88 ++++++++---------
 profiles/audio/a2dp.h    |   4 +-
 profiles/audio/device.c  |   4 +-
 profiles/audio/device.h  |   3 +
 profiles/audio/manager.c | 242 +++++++++++++++++++++++++++++++++++++++++++----
 profiles/audio/sink.c    | 179 ++++++++++++++++++-----------------
 profiles/audio/sink.h    |   4 +-
 profiles/audio/source.c  | 189 ++++++++++++++++++++----------------
 profiles/audio/source.h  |   4 +-
 9 files changed, 480 insertions(+), 237 deletions(-)

-- 
1.7.11.7


^ permalink raw reply

* [PATCH v2 1/8] audio: Trivial function renames
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Rename functions to be consistent with the rest of the code.
---
 profiles/audio/device.c |  4 ++--
 profiles/audio/sink.c   | 10 +++++-----
 profiles/audio/sink.h   |  2 +-
 profiles/audio/source.c | 10 +++++-----
 profiles/audio/source.h |  2 +-
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/profiles/audio/device.c b/profiles/audio/device.c
index df57d81..811ed75 100644
--- a/profiles/audio/device.c
+++ b/profiles/audio/device.c
@@ -203,7 +203,7 @@ static void disconnect_cb(struct btd_device *btd_dev, gboolean removal,
 		avrcp_disconnect(dev);
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_shutdown(dev->sink);
+		sink_disconnect(dev->sink);
 	else
 		priv->disconnecting = FALSE;
 }
@@ -390,7 +390,7 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
 	}
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_shutdown(dev->sink);
+		sink_disconnect(dev->sink);
 	else {
 		dbus_message_unref(priv->dc_req);
 		priv->dc_req = NULL;
diff --git a/profiles/audio/sink.c b/profiles/audio/sink.c
index 2e63579..e769917 100644
--- a/profiles/audio/sink.c
+++ b/profiles/audio/sink.c
@@ -390,7 +390,7 @@ gboolean sink_setup_stream(struct sink *sink, struct avdtp *session)
 	return TRUE;
 }
 
-static DBusMessage *sink_connect(DBusConnection *conn,
+static DBusMessage *connect_sink(DBusConnection *conn,
 				DBusMessage *msg, void *data)
 {
 	struct audio_device *dev = data;
@@ -423,7 +423,7 @@ static DBusMessage *sink_connect(DBusConnection *conn,
 	return NULL;
 }
 
-static DBusMessage *sink_disconnect(DBusConnection *conn,
+static DBusMessage *disconnect_sink(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
 	struct audio_device *device = data;
@@ -489,8 +489,8 @@ static DBusMessage *sink_get_properties(DBusConnection *conn,
 }
 
 static const GDBusMethodTable sink_methods[] = {
-	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, sink_connect) },
-	{ GDBUS_ASYNC_METHOD("Disconnect", NULL, NULL, sink_disconnect) },
+	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, connect_sink) },
+	{ GDBUS_ASYNC_METHOD("Disconnect", NULL, NULL, disconnect_sink) },
 	{ GDBUS_METHOD("GetProperties",
 				NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
 				sink_get_properties) },
@@ -604,7 +604,7 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-gboolean sink_shutdown(struct sink *sink)
+gboolean sink_disconnect(struct sink *sink)
 {
 	if (!sink->session)
 		return FALSE;
diff --git a/profiles/audio/sink.h b/profiles/audio/sink.h
index 4fea5ba..edac364 100644
--- a/profiles/audio/sink.h
+++ b/profiles/audio/sink.h
@@ -46,4 +46,4 @@ sink_state_t sink_get_state(struct audio_device *dev);
 gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean sink_setup_stream(struct sink *sink, struct avdtp *session);
-gboolean sink_shutdown(struct sink *sink);
+gboolean sink_disconnect(struct sink *sink);
diff --git a/profiles/audio/source.c b/profiles/audio/source.c
index 1d0c74a..38cb822 100644
--- a/profiles/audio/source.c
+++ b/profiles/audio/source.c
@@ -379,7 +379,7 @@ gboolean source_setup_stream(struct source *source, struct avdtp *session)
 	return TRUE;
 }
 
-static DBusMessage *source_connect(DBusConnection *conn,
+static DBusMessage *connect_source(DBusConnection *conn,
 				DBusMessage *msg, void *data)
 {
 	struct audio_device *dev = data;
@@ -412,7 +412,7 @@ static DBusMessage *source_connect(DBusConnection *conn,
 	return NULL;
 }
 
-static DBusMessage *source_disconnect(DBusConnection *conn,
+static DBusMessage *disconnect_source(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
 	struct audio_device *device = data;
@@ -478,8 +478,8 @@ static DBusMessage *source_get_properties(DBusConnection *conn,
 }
 
 static const GDBusMethodTable source_methods[] = {
-	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, source_connect) },
-	{ GDBUS_ASYNC_METHOD("Disconnect", NULL, NULL, source_disconnect) },
+	{ GDBUS_ASYNC_METHOD("Connect", NULL, NULL, connect_source) },
+	{ GDBUS_ASYNC_METHOD("Disconnect", NULL, NULL, disconnect_source) },
 	{ GDBUS_METHOD("GetProperties",
 				NULL, GDBUS_ARGS({ "properties", "a{sv}" }),
 				source_get_properties) },
@@ -593,7 +593,7 @@ gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-gboolean source_shutdown(struct source *source)
+gboolean source_disconnect(struct source *source)
 {
 	if (!source->stream)
 		return FALSE;
diff --git a/profiles/audio/source.h b/profiles/audio/source.h
index 695bded..49a8d64 100644
--- a/profiles/audio/source.h
+++ b/profiles/audio/source.h
@@ -47,4 +47,4 @@ source_state_t source_get_state(struct audio_device *dev);
 gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean source_setup_stream(struct source *source, struct avdtp *session);
-gboolean source_shutdown(struct source *source);
+gboolean source_disconnect(struct source *source);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 2/8] sink: Generalize disconnection function
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Extend the function for a more general usage other than full device
disconnection. Besides, return error code instead of a boolean.
---
 profiles/audio/device.c |  4 ++--
 profiles/audio/sink.c   | 20 ++++++++++----------
 profiles/audio/sink.h   |  2 +-
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/profiles/audio/device.c b/profiles/audio/device.c
index 811ed75..09a1ed7 100644
--- a/profiles/audio/device.c
+++ b/profiles/audio/device.c
@@ -203,7 +203,7 @@ static void disconnect_cb(struct btd_device *btd_dev, gboolean removal,
 		avrcp_disconnect(dev);
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_disconnect(dev->sink);
+		sink_disconnect(dev, TRUE);
 	else
 		priv->disconnecting = FALSE;
 }
@@ -390,7 +390,7 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
 	}
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_disconnect(dev->sink);
+		sink_disconnect(dev, TRUE);
 	else {
 		dbus_message_unref(priv->dc_req);
 		priv->dc_req = NULL;
diff --git a/profiles/audio/sink.c b/profiles/audio/sink.c
index e769917..7a08960 100644
--- a/profiles/audio/sink.c
+++ b/profiles/audio/sink.c
@@ -604,12 +604,15 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-gboolean sink_disconnect(struct sink *sink)
+int sink_disconnect(struct audio_device *dev, gboolean shutdown)
 {
+	struct sink *sink = dev->sink;
+
 	if (!sink->session)
-		return FALSE;
+		return -ENOTCONN;
 
-	avdtp_set_device_disconnect(sink->session, TRUE);
+	if (shutdown)
+		avdtp_set_device_disconnect(sink->session, TRUE);
 
 	/* cancel pending connect */
 	if (sink->connect) {
@@ -623,20 +626,17 @@ gboolean sink_disconnect(struct sink *sink)
 		avdtp_unref(sink->session);
 		sink->session = NULL;
 
-		return TRUE;
+		return 0;
 	}
 
 	/* disconnect already ongoing */
 	if (sink->disconnect)
-		return TRUE;
+		return -EBUSY;
 
 	if (!sink->stream)
-		return FALSE;
-
-	if (avdtp_close(sink->session, sink->stream, FALSE) < 0)
-		return FALSE;
+		return -ENOTCONN;
 
-	return TRUE;
+	return avdtp_close(sink->session, sink->stream, FALSE);
 }
 
 unsigned int sink_add_state_cb(sink_state_cb cb, void *user_data)
diff --git a/profiles/audio/sink.h b/profiles/audio/sink.h
index edac364..426d83f 100644
--- a/profiles/audio/sink.h
+++ b/profiles/audio/sink.h
@@ -46,4 +46,4 @@ sink_state_t sink_get_state(struct audio_device *dev);
 gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean sink_setup_stream(struct sink *sink, struct avdtp *session);
-gboolean sink_disconnect(struct sink *sink);
+int sink_disconnect(struct audio_device *dev, gboolean shutdown);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 3/8] source: Add missing code in source_disconnect()
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Use the implementation in sink_disconnect() as a template to implement
the missing code in source_disconnect(). Both functions should be
equivalent.
---
 profiles/audio/source.c | 37 +++++++++++++++++++++++++++++++------
 profiles/audio/source.h |  2 +-
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/profiles/audio/source.c b/profiles/audio/source.c
index 38cb822..9981d79 100644
--- a/profiles/audio/source.c
+++ b/profiles/audio/source.c
@@ -593,15 +593,40 @@ gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-gboolean source_disconnect(struct source *source)
+int source_disconnect(struct audio_device *dev, gboolean shutdown)
 {
-	if (!source->stream)
-		return FALSE;
+	struct source *source = dev->source;
 
-	if (avdtp_close(source->session, source->stream, FALSE) < 0)
-		return FALSE;
+	if (!source->session)
+		return -ENOTCONN;
 
-	return TRUE;
+	if (shutdown)
+		avdtp_set_device_disconnect(source->session, TRUE);
+
+	/* cancel pending connect */
+	if (source->connect) {
+		struct pending_request *pending = source->connect;
+
+		if (pending->msg)
+			error_failed(pending->msg, "Stream setup failed");
+
+		pending_request_free(source->dev, pending);
+		source->connect = NULL;
+
+		avdtp_unref(source->session);
+		source->session = NULL;
+
+		return 0;
+	}
+
+	/* disconnect already ongoing */
+	if (source->disconnect)
+		return -EBUSY;
+
+	if (!source->stream)
+		return -ENOTCONN;
+
+	return avdtp_close(source->session, source->stream, FALSE);
 }
 
 unsigned int source_add_state_cb(source_state_cb cb, void *user_data)
diff --git a/profiles/audio/source.h b/profiles/audio/source.h
index 49a8d64..3406754 100644
--- a/profiles/audio/source.h
+++ b/profiles/audio/source.h
@@ -47,4 +47,4 @@ source_state_t source_get_state(struct audio_device *dev);
 gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean source_setup_stream(struct source *source, struct avdtp *session);
-gboolean source_disconnect(struct source *source);
+int source_disconnect(struct audio_device *dev, gboolean shutdown);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 4/8] source: Expose internal connection API
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Separate the D-Bus code from the internal connection handling code,
exposing an internal API in case some internal codepath/plugin is
interested in using it.
---
 profiles/audio/device.h |   3 +
 profiles/audio/source.c | 160 ++++++++++++++++++++++++------------------------
 profiles/audio/source.h |   4 +-
 3 files changed, 87 insertions(+), 80 deletions(-)

diff --git a/profiles/audio/device.h b/profiles/audio/device.h
index f0d178d..0801cda 100644
--- a/profiles/audio/device.h
+++ b/profiles/audio/device.h
@@ -22,6 +22,7 @@
  *
  */
 
+struct audio_device;
 struct source;
 struct control;
 struct target;
@@ -30,6 +31,8 @@ struct headset;
 struct gateway;
 struct dev_priv;
 
+typedef void (*audio_device_cb) (struct audio_device *dev, int err, void *data);
+
 struct audio_device {
 	struct btd_device *btd_dev;
 
diff --git a/profiles/audio/source.c b/profiles/audio/source.c
index 9981d79..163f5b9 100644
--- a/profiles/audio/source.c
+++ b/profiles/audio/source.c
@@ -53,7 +53,8 @@
 #define STREAM_SETUP_RETRY_TIMER 2
 
 struct pending_request {
-	DBusMessage *msg;
+	audio_device_cb cb;
+	void *cb_data;
 	unsigned int id;
 };
 
@@ -152,10 +153,12 @@ static void avdtp_state_callback(struct audio_device *dev,
 }
 
 static void pending_request_free(struct audio_device *dev,
-					struct pending_request *pending)
+					struct pending_request *pending,
+					int err)
 {
-	if (pending->msg)
-		dbus_message_unref(pending->msg);
+	if (pending->cb)
+		pending->cb(dev, err, pending->cb_data);
+
 	if (pending->id)
 		a2dp_cancel(dev, pending->id);
 
@@ -177,15 +180,12 @@ static void stream_state_changed(struct avdtp_stream *stream,
 	switch (new_state) {
 	case AVDTP_STATE_IDLE:
 		if (source->disconnect) {
-			DBusMessage *reply;
 			struct pending_request *p;
 
 			p = source->disconnect;
 			source->disconnect = NULL;
 
-			reply = dbus_message_new_method_return(p->msg);
-			g_dbus_send_message(btd_get_dbus_connection(), reply);
-			pending_request_free(dev, p);
+			pending_request_free(dev, p, 0);
 		}
 
 		if (source->session) {
@@ -211,35 +211,24 @@ static void stream_state_changed(struct avdtp_stream *stream,
 	source->stream_state = new_state;
 }
 
-static void error_failed(DBusMessage *msg,
-							const char *desc)
-{
-	DBusMessage *reply = btd_error_failed(msg, desc);
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
-}
-
 static gboolean stream_setup_retry(gpointer user_data)
 {
 	struct source *source = user_data;
 	struct pending_request *pending = source->connect;
+	int err;
 
 	source->retry_id = 0;
 
 	if (source->stream_state >= AVDTP_STATE_OPEN) {
 		DBG("Stream successfully created, after XCASE connect:connect");
-		if (pending->msg) {
-			DBusMessage *reply;
-			reply = dbus_message_new_method_return(pending->msg);
-			g_dbus_send_message(btd_get_dbus_connection(), reply);
-		}
+		err = 0;
 	} else {
 		DBG("Stream setup failed, after XCASE connect:connect");
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
+		err = -EIO;
 	}
 
 	source->connect = NULL;
-	pending_request_free(source->dev, pending);
+	pending_request_free(source->dev, pending, err);
 
 	return FALSE;
 }
@@ -258,14 +247,8 @@ static void stream_setup_complete(struct avdtp *session, struct a2dp_sep *sep,
 	if (stream) {
 		DBG("Stream successfully created");
 
-		if (pending->msg) {
-			DBusMessage *reply;
-			reply = dbus_message_new_method_return(pending->msg);
-			g_dbus_send_message(btd_get_dbus_connection(), reply);
-		}
-
 		source->connect = NULL;
-		pending_request_free(source->dev, pending);
+		pending_request_free(source->dev, pending, 0);
 
 		return;
 	}
@@ -279,10 +262,8 @@ static void stream_setup_complete(struct avdtp *session, struct a2dp_sep *sep,
 							stream_setup_retry,
 							source);
 	} else {
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
 		source->connect = NULL;
-		pending_request_free(source->dev, pending);
+		pending_request_free(source->dev, pending, -EIO);
 		DBG("Stream setup failed : %s", avdtp_strerror(err));
 	}
 }
@@ -309,9 +290,7 @@ static void select_complete(struct avdtp *session, struct a2dp_sep *sep,
 	return;
 
 failed:
-	if (pending->msg)
-		error_failed(pending->msg, "Stream setup failed");
-	pending_request_free(source->dev, pending);
+	pending_request_free(source->dev, pending, -EIO);
 	source->connect = NULL;
 	avdtp_unref(source->session);
 	source->session = NULL;
@@ -352,9 +331,7 @@ static void discovery_complete(struct avdtp *session, GSList *seps, struct avdtp
 	return;
 
 failed:
-	if (pending->msg)
-		error_failed(pending->msg, "Stream setup failed");
-	pending_request_free(source->dev, pending);
+	pending_request_free(source->dev, pending, -EIO);
 	source->connect = NULL;
 	avdtp_unref(source->session);
 	source->session = NULL;
@@ -379,69 +356,84 @@ gboolean source_setup_stream(struct source *source, struct avdtp *session)
 	return TRUE;
 }
 
+static void generic_cb(struct audio_device *dev, int err, void *data)
+{
+	DBusMessage *msg = data;
+	DBusMessage *reply;
+
+	if (err < 0) {
+		reply = btd_error_failed(msg, strerror(-err));
+		g_dbus_send_message(btd_get_dbus_connection(), reply);
+		dbus_message_unref(msg);
+		return;
+	}
+
+	g_dbus_send_reply(btd_get_dbus_connection(), msg, DBUS_TYPE_INVALID);
+
+	dbus_message_unref(msg);
+}
+
 static DBusMessage *connect_source(DBusConnection *conn,
 				DBusMessage *msg, void *data)
 {
 	struct audio_device *dev = data;
+	int err;
+
+	err = source_connect(dev, generic_cb, msg);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	dbus_message_ref(msg);
+
+	return NULL;
+}
+
+int source_connect(struct audio_device *dev, audio_device_cb cb, void *data)
+{
 	struct source *source = dev->source;
 	struct pending_request *pending;
 
 	if (!source->session)
 		source->session = avdtp_get(&dev->src, &dev->dst);
 
-	if (!source->session)
-		return btd_error_failed(msg, "Unable to get a session");
+	if (!source->session) {
+		DBG("Unable to get a session");
+		return -EIO;
+	}
 
 	if (source->connect || source->disconnect)
-		return btd_error_busy(msg);
+		return -EBUSY;
 
 	if (source->stream_state >= AVDTP_STATE_OPEN)
-		return btd_error_already_connected(msg);
+		return -EALREADY;
 
-	if (!source_setup_stream(source, NULL))
-		return btd_error_failed(msg, "Failed to create a stream");
+	if (!source_setup_stream(source, NULL)) {
+		DBG("Failed to create a stream");
+		return -EIO;
+	}
 
 	dev->auto_connect = FALSE;
 
 	pending = source->connect;
-
-	pending->msg = dbus_message_ref(msg);
+	pending->cb = cb;
+	pending->cb_data = data;
 
 	DBG("stream creation in progress");
 
-	return NULL;
+	return 0;
 }
 
 static DBusMessage *disconnect_source(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
-	struct audio_device *device = data;
-	struct source *source = device->source;
-	struct pending_request *pending;
+	struct audio_device *dev = data;
 	int err;
 
-	if (!source->session)
-		return btd_error_not_connected(msg);
-
-	if (source->connect || source->disconnect)
-		return btd_error_busy(msg);
-
-	if (source->stream_state < AVDTP_STATE_OPEN) {
-		DBusMessage *reply = dbus_message_new_method_return(msg);
-		if (!reply)
-			return NULL;
-		avdtp_unref(source->session);
-		source->session = NULL;
-		return reply;
-	}
-
-	err = avdtp_close(source->session, source->stream, FALSE);
+	err = source_disconnect(dev, FALSE, generic_cb, msg);
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	pending = g_new0(struct pending_request, 1);
-	pending->msg = dbus_message_ref(msg);
-	source->disconnect = pending;
+	dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -504,10 +496,10 @@ static void source_free(struct audio_device *dev)
 		avdtp_unref(source->session);
 
 	if (source->connect)
-		pending_request_free(dev, source->connect);
+		pending_request_free(dev, source->connect, -ECANCELED);
 
 	if (source->disconnect)
-		pending_request_free(dev, source->disconnect);
+		pending_request_free(dev, source->disconnect, -ECANCELED);
 
 	if (source->retry_id)
 		g_source_remove(source->retry_id);
@@ -593,9 +585,13 @@ gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-int source_disconnect(struct audio_device *dev, gboolean shutdown)
+int source_disconnect(struct audio_device *dev, gboolean shutdown,
+						audio_device_cb cb, void *data)
 {
-	struct source *source = dev->source;
+	struct audio_device *device = data;
+	struct source *source = device->source;
+	struct pending_request *pending;
+	int err;
 
 	if (!source->session)
 		return -ENOTCONN;
@@ -607,10 +603,7 @@ int source_disconnect(struct audio_device *dev, gboolean shutdown)
 	if (source->connect) {
 		struct pending_request *pending = source->connect;
 
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
-
-		pending_request_free(source->dev, pending);
+		pending_request_free(source->dev, pending, -ECANCELED);
 		source->connect = NULL;
 
 		avdtp_unref(source->session);
@@ -626,7 +619,16 @@ int source_disconnect(struct audio_device *dev, gboolean shutdown)
 	if (!source->stream)
 		return -ENOTCONN;
 
-	return avdtp_close(source->session, source->stream, FALSE);
+	err = avdtp_close(source->session, source->stream, FALSE);
+	if (err < 0)
+		return err;
+
+	pending = g_new0(struct pending_request, 1);
+	pending->cb = cb;
+	pending->cb_data = data;
+	source->disconnect = pending;
+
+	return 0;
 }
 
 unsigned int source_add_state_cb(source_state_cb cb, void *user_data)
diff --git a/profiles/audio/source.h b/profiles/audio/source.h
index 3406754..2d382b5 100644
--- a/profiles/audio/source.h
+++ b/profiles/audio/source.h
@@ -44,7 +44,9 @@ struct source *source_init(struct audio_device *dev);
 void source_unregister(struct audio_device *dev);
 gboolean source_is_active(struct audio_device *dev);
 source_state_t source_get_state(struct audio_device *dev);
+int source_connect(struct audio_device *dev, audio_device_cb cb, void *data);
 gboolean source_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean source_setup_stream(struct source *source, struct avdtp *session);
-int source_disconnect(struct audio_device *dev, gboolean shutdown);
+int source_disconnect(struct audio_device *dev, gboolean shutdown,
+						audio_device_cb cb, void *data);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 5/8] sink: Expose internal connection API
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Separate the D-Bus code from the internal connection handling code,
exposing an internal API in case some internal codepath/plugin is
interested in using it.
---
 profiles/audio/device.c |   4 +-
 profiles/audio/sink.c   | 159 ++++++++++++++++++++++++------------------------
 profiles/audio/sink.h   |   4 +-
 3 files changed, 86 insertions(+), 81 deletions(-)

diff --git a/profiles/audio/device.c b/profiles/audio/device.c
index 09a1ed7..56239a4 100644
--- a/profiles/audio/device.c
+++ b/profiles/audio/device.c
@@ -203,7 +203,7 @@ static void disconnect_cb(struct btd_device *btd_dev, gboolean removal,
 		avrcp_disconnect(dev);
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_disconnect(dev, TRUE);
+		sink_disconnect(dev, TRUE, NULL, NULL);
 	else
 		priv->disconnecting = FALSE;
 }
@@ -390,7 +390,7 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
 	}
 
 	if (dev->sink && priv->sink_state != SINK_STATE_DISCONNECTED)
-		sink_disconnect(dev, TRUE);
+		sink_disconnect(dev, TRUE, NULL, NULL);
 	else {
 		dbus_message_unref(priv->dc_req);
 		priv->dc_req = NULL;
diff --git a/profiles/audio/sink.c b/profiles/audio/sink.c
index 7a08960..c3278cb 100644
--- a/profiles/audio/sink.c
+++ b/profiles/audio/sink.c
@@ -52,7 +52,8 @@
 #define STREAM_SETUP_RETRY_TIMER 2
 
 struct pending_request {
-	DBusMessage *msg;
+	audio_device_cb cb;
+	void *cb_data;
 	unsigned int id;
 };
 
@@ -161,10 +162,12 @@ static void avdtp_state_callback(struct audio_device *dev,
 }
 
 static void pending_request_free(struct audio_device *dev,
-					struct pending_request *pending)
+					struct pending_request *pending,
+					int err)
 {
-	if (pending->msg)
-		dbus_message_unref(pending->msg);
+	if (pending->cb)
+		pending->cb(dev, err, pending->cb_data);
+
 	if (pending->id)
 		a2dp_cancel(dev, pending->id);
 
@@ -177,7 +180,6 @@ static void stream_state_changed(struct avdtp_stream *stream,
 					struct avdtp_error *err,
 					void *user_data)
 {
-	DBusConnection *conn = btd_get_dbus_connection();
 	struct audio_device *dev = user_data;
 	struct sink *sink = dev->sink;
 
@@ -187,15 +189,12 @@ static void stream_state_changed(struct avdtp_stream *stream,
 	switch (new_state) {
 	case AVDTP_STATE_IDLE:
 		if (sink->disconnect) {
-			DBusMessage *reply;
 			struct pending_request *p;
 
 			p = sink->disconnect;
 			sink->disconnect = NULL;
 
-			reply = dbus_message_new_method_return(p->msg);
-			g_dbus_send_message(conn, reply);
-			pending_request_free(dev, p);
+			pending_request_free(dev, p, 0);
 		}
 
 		if (sink->session) {
@@ -221,34 +220,24 @@ static void stream_state_changed(struct avdtp_stream *stream,
 	sink->stream_state = new_state;
 }
 
-static void error_failed(DBusMessage *msg, const char *desc)
-{
-	DBusMessage *reply = btd_error_failed(msg, desc);
-	g_dbus_send_message(btd_get_dbus_connection(), reply);
-}
-
 static gboolean stream_setup_retry(gpointer user_data)
 {
 	struct sink *sink = user_data;
 	struct pending_request *pending = sink->connect;
+	int err;
 
 	sink->retry_id = 0;
 
 	if (sink->stream_state >= AVDTP_STATE_OPEN) {
 		DBG("Stream successfully created, after XCASE connect:connect");
-		if (pending->msg) {
-			DBusMessage *reply;
-			reply = dbus_message_new_method_return(pending->msg);
-			g_dbus_send_message(btd_get_dbus_connection(), reply);
-		}
+		err = 0;
 	} else {
 		DBG("Stream setup failed, after XCASE connect:connect");
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
+		err = -EIO;
 	}
 
 	sink->connect = NULL;
-	pending_request_free(sink->dev, pending);
+	pending_request_free(sink->dev, pending, err);
 
 	return FALSE;
 }
@@ -267,14 +256,8 @@ static void stream_setup_complete(struct avdtp *session, struct a2dp_sep *sep,
 	if (stream) {
 		DBG("Stream successfully created");
 
-		if (pending->msg) {
-			DBusMessage *reply;
-			reply = dbus_message_new_method_return(pending->msg);
-			g_dbus_send_message(btd_get_dbus_connection(), reply);
-		}
-
 		sink->connect = NULL;
-		pending_request_free(sink->dev, pending);
+		pending_request_free(sink->dev, pending, 0);
 
 		return;
 	}
@@ -288,11 +271,8 @@ static void stream_setup_complete(struct avdtp *session, struct a2dp_sep *sep,
 							stream_setup_retry,
 							sink);
 	} else {
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
 		sink->connect = NULL;
-		pending_request_free(sink->dev, pending);
-		DBG("Stream setup failed : %s", avdtp_strerror(err));
+		pending_request_free(sink->dev, pending, -EIO);
 	}
 }
 
@@ -314,9 +294,7 @@ static void select_complete(struct avdtp *session, struct a2dp_sep *sep,
 	return;
 
 failed:
-	if (pending->msg)
-		error_failed(pending->msg, "Stream setup failed");
-	pending_request_free(sink->dev, pending);
+	pending_request_free(sink->dev, pending, -EIO);
 	sink->connect = NULL;
 	avdtp_unref(sink->session);
 	sink->session = NULL;
@@ -363,9 +341,7 @@ static void discovery_complete(struct avdtp *session, GSList *seps, struct avdtp
 	return;
 
 failed:
-	if (pending->msg)
-		error_failed(pending->msg, "Stream setup failed");
-	pending_request_free(sink->dev, pending);
+	pending_request_free(sink->dev, pending, -EIO);
 	sink->connect = NULL;
 	avdtp_unref(sink->session);
 	sink->session = NULL;
@@ -390,69 +366,85 @@ gboolean sink_setup_stream(struct sink *sink, struct avdtp *session)
 	return TRUE;
 }
 
+static void generic_cb(struct audio_device *dev, int err, void *data)
+{
+	DBusMessage *msg = data;
+	DBusMessage *reply;
+
+	if (err < 0) {
+		reply = btd_error_failed(msg, strerror(-err));
+		g_dbus_send_message(btd_get_dbus_connection(), reply);
+		dbus_message_unref(msg);
+		return;
+	}
+
+	g_dbus_send_reply(btd_get_dbus_connection(), msg, DBUS_TYPE_INVALID);
+
+	dbus_message_unref(msg);
+}
+
 static DBusMessage *connect_sink(DBusConnection *conn,
 				DBusMessage *msg, void *data)
 {
 	struct audio_device *dev = data;
+	int err;
+
+	err = sink_connect(dev, generic_cb, msg);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	dbus_message_ref(msg);
+
+	return NULL;
+}
+
+int sink_connect(struct audio_device *dev, audio_device_cb cb, void *data)
+{
 	struct sink *sink = dev->sink;
 	struct pending_request *pending;
 
 	if (!sink->session)
 		sink->session = avdtp_get(&dev->src, &dev->dst);
 
-	if (!sink->session)
-		return btd_error_failed(msg, "Unable to get a session");
+	if (!sink->session) {
+		DBG("Unable to get a session");
+		return -EIO;
+	}
 
 	if (sink->connect || sink->disconnect)
-		return btd_error_busy(msg);
+		return -EBUSY;
 
 	if (sink->stream_state >= AVDTP_STATE_OPEN)
-		return btd_error_already_connected(msg);
+		return -EALREADY;
 
-	if (!sink_setup_stream(sink, NULL))
-		return btd_error_failed(msg, "Failed to create a stream");
+	if (!sink_setup_stream(sink, NULL)) {
+		DBG("Failed to create a stream");
+		return -EIO;
+	}
 
 	dev->auto_connect = FALSE;
 
 	pending = sink->connect;
 
-	pending->msg = dbus_message_ref(msg);
+	pending->cb = cb;
+	pending->cb_data = data;
 
 	DBG("stream creation in progress");
 
-	return NULL;
+	return 0;
 }
 
 static DBusMessage *disconnect_sink(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
-	struct audio_device *device = data;
-	struct sink *sink = device->sink;
-	struct pending_request *pending;
+	struct audio_device *dev = data;
 	int err;
 
-	if (!sink->session)
-		return btd_error_not_connected(msg);
-
-	if (sink->connect || sink->disconnect)
-		return btd_error_busy(msg);
-
-	if (sink->stream_state < AVDTP_STATE_OPEN) {
-		DBusMessage *reply = dbus_message_new_method_return(msg);
-		if (!reply)
-			return NULL;
-		avdtp_unref(sink->session);
-		sink->session = NULL;
-		return reply;
-	}
-
-	err = avdtp_close(sink->session, sink->stream, FALSE);
+	err = sink_disconnect(dev, FALSE, generic_cb, msg);
 	if (err < 0)
 		return btd_error_failed(msg, strerror(-err));
 
-	pending = g_new0(struct pending_request, 1);
-	pending->msg = dbus_message_ref(msg);
-	sink->disconnect = pending;
+	dbus_message_ref(msg);
 
 	return NULL;
 }
@@ -515,10 +507,10 @@ static void sink_free(struct audio_device *dev)
 		avdtp_unref(sink->session);
 
 	if (sink->connect)
-		pending_request_free(dev, sink->connect);
+		pending_request_free(dev, sink->connect, -ECANCELED);
 
 	if (sink->disconnect)
-		pending_request_free(dev, sink->disconnect);
+		pending_request_free(dev, sink->disconnect, -ECANCELED);
 
 	if (sink->retry_id)
 		g_source_remove(sink->retry_id);
@@ -604,9 +596,13 @@ gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 	return TRUE;
 }
 
-int sink_disconnect(struct audio_device *dev, gboolean shutdown)
+int sink_disconnect(struct audio_device *dev, gboolean shutdown,
+						audio_device_cb cb, void *data)
 {
-	struct sink *sink = dev->sink;
+	struct audio_device *device = data;
+	struct sink *sink = device->sink;
+	struct pending_request *pending;
+	int err;
 
 	if (!sink->session)
 		return -ENOTCONN;
@@ -618,9 +614,7 @@ int sink_disconnect(struct audio_device *dev, gboolean shutdown)
 	if (sink->connect) {
 		struct pending_request *pending = sink->connect;
 
-		if (pending->msg)
-			error_failed(pending->msg, "Stream setup failed");
-		pending_request_free(sink->dev, pending);
+		pending_request_free(sink->dev, pending, -ECANCELED);
 		sink->connect = NULL;
 
 		avdtp_unref(sink->session);
@@ -636,7 +630,16 @@ int sink_disconnect(struct audio_device *dev, gboolean shutdown)
 	if (!sink->stream)
 		return -ENOTCONN;
 
-	return avdtp_close(sink->session, sink->stream, FALSE);
+	err = avdtp_close(sink->session, sink->stream, FALSE);
+	if (err < 0)
+		return err;
+
+	pending = g_new0(struct pending_request, 1);
+	pending->cb = cb;
+	pending->cb_data = data;
+	sink->disconnect = pending;
+
+	return 0;
 }
 
 unsigned int sink_add_state_cb(sink_state_cb cb, void *user_data)
diff --git a/profiles/audio/sink.h b/profiles/audio/sink.h
index 426d83f..38098f2 100644
--- a/profiles/audio/sink.h
+++ b/profiles/audio/sink.h
@@ -43,7 +43,9 @@ struct sink *sink_init(struct audio_device *dev);
 void sink_unregister(struct audio_device *dev);
 gboolean sink_is_active(struct audio_device *dev);
 sink_state_t sink_get_state(struct audio_device *dev);
+int sink_connect(struct audio_device *dev, audio_device_cb cb, void *data);
 gboolean sink_new_stream(struct audio_device *dev, struct avdtp *session,
 				struct avdtp_stream *stream);
 gboolean sink_setup_stream(struct sink *sink, struct avdtp *session);
-int sink_disconnect(struct audio_device *dev, gboolean shutdown);
+int sink_disconnect(struct audio_device *dev, gboolean shutdown,
+						audio_device_cb cb, void *data);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 6/8] audio: Split A2DP into three btd_profile
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Merging the three audio profiles (AVDTP, A2DP sink and A2DP source)
into one was convenient in the past was doesn't fit very well the new
btd_profile approach. The split is also more consistent with the
approach of the HFP/HSP profile.
---
 profiles/audio/a2dp.c    |  88 ++++++++++++++++++--------------------
 profiles/audio/a2dp.h    |   4 +-
 profiles/audio/manager.c | 107 ++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 135 insertions(+), 64 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 7799420..6bfc8c2 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1167,73 +1167,67 @@ static struct a2dp_server *find_server(GSList *list, const bdaddr_t *src)
 	return NULL;
 }
 
-int a2dp_register(const bdaddr_t *src, GKeyFile *config)
+int a2dp_server_register(const bdaddr_t *src, GKeyFile *config)
 {
-	gboolean source = TRUE, sink = FALSE;
 	gboolean delay_reporting = FALSE;
-	char *str;
-	GError *err = NULL;
 	struct a2dp_server *server;
+	int av_err;
 
-	if (!config)
-		goto proceed;
+	server = find_server(servers, src);
+	if (server)
+		return 0;
 
-	str = g_key_file_get_string(config, "General", "Enable", &err);
+	server = g_new0(struct a2dp_server, 1);
 
-	if (err) {
-		DBG("audio.conf: %s", err->message);
-		g_clear_error(&err);
-	} else {
-		if (strstr(str, "Sink"))
-			source = TRUE;
-		if (strstr(str, "Source"))
-			sink = TRUE;
-		g_free(str);
+	av_err = avdtp_init(src, config, &server->version);
+	if (av_err < 0) {
+		g_free(server);
+		return av_err;
 	}
 
-	str = g_key_file_get_string(config, "General", "Disable", &err);
+	bacpy(&server->src, src);
+	servers = g_slist_append(servers, server);
 
-	if (err) {
-		DBG("audio.conf: %s", err->message);
-		g_clear_error(&err);
-	} else {
-		if (strstr(str, "Sink"))
-			source = FALSE;
-		if (strstr(str, "Source"))
-			sink = FALSE;
-		g_free(str);
-	}
+	if (config)
+		delay_reporting = g_key_file_get_boolean(config, "A2DP",
+						"DelayReporting", NULL);
 
-proceed:
+	if (delay_reporting)
+		server->version = 0x0103;
+	else
+		server->version = 0x0102;
+
+	return 0;
+}
+
+int a2dp_source_register(const bdaddr_t *src, GKeyFile *config)
+{
+	struct a2dp_server *server;
 
 	server = find_server(servers, src);
 	if (!server) {
-		int av_err;
+		DBG("AVDTP not registered");
+		return -EPROTONOSUPPORT;
 
-		server = g_new0(struct a2dp_server, 1);
+	}
 
-		av_err = avdtp_init(src, config, &server->version);
-		if (av_err < 0) {
-			g_free(server);
-			return av_err;
-		}
+	server->source_enabled = TRUE;
 
-		bacpy(&server->src, src);
-		servers = g_slist_append(servers, server);
-	}
+	return 0;
+}
 
-	if (config)
-		delay_reporting = g_key_file_get_boolean(config, "A2DP",
-						"DelayReporting", NULL);
+int a2dp_sink_register(const bdaddr_t *src, GKeyFile *config)
+{
+	struct a2dp_server *server;
 
-	if (delay_reporting)
-		server->version = 0x0103;
-	else
-		server->version = 0x0102;
+	server = find_server(servers, src);
+	if (!server) {
+		DBG("AVDTP not registered");
+		return -EPROTONOSUPPORT;
 
-	server->source_enabled = source;
+	}
 
-	server->sink_enabled = sink;
+	server->sink_enabled = TRUE;
 
 	return 0;
 }
diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
index 736bc66..c471499 100644
--- a/profiles/audio/a2dp.h
+++ b/profiles/audio/a2dp.h
@@ -64,7 +64,9 @@ typedef void (*a2dp_stream_cb_t) (struct avdtp *session,
 					struct avdtp_error *err,
 					void *user_data);
 
-int a2dp_register(const bdaddr_t *src, GKeyFile *config);
+int a2dp_server_register(const bdaddr_t *src, GKeyFile *config);
+int a2dp_source_register(const bdaddr_t *src, GKeyFile *config);
+int a2dp_sink_register(const bdaddr_t *src, GKeyFile *config);
 void a2dp_unregister(const bdaddr_t *src);
 
 struct a2dp_sep *a2dp_add_sep(const bdaddr_t *src, uint8_t type,
diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index 19aeb24..d7c1717 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -128,7 +128,7 @@ static void audio_remove(struct btd_profile *p, struct btd_device *device)
 	audio_device_unregister(dev);
 }
 
-static int a2dp_probe(struct btd_profile *p, struct btd_device *device,
+static int a2dp_source_probe(struct btd_profile *p, struct btd_device *device,
 								GSList *uuids)
 {
 	struct audio_device *audio_dev;
@@ -139,13 +139,23 @@ static int a2dp_probe(struct btd_profile *p, struct btd_device *device,
 		return -1;
 	}
 
-	if (g_slist_find_custom(uuids, A2DP_SINK_UUID, bt_uuid_strcmp) &&
-						audio_dev->sink == NULL)
-		audio_dev->sink = sink_init(audio_dev);
+	audio_dev->source = source_init(audio_dev);
 
-	if (g_slist_find_custom(uuids, A2DP_SOURCE_UUID, bt_uuid_strcmp) &&
-						audio_dev->source == NULL)
-		audio_dev->source = source_init(audio_dev);
+	return 0;
+}
+
+static int a2dp_sink_probe(struct btd_profile *p, struct btd_device *device,
+								GSList *uuids)
+{
+	struct audio_device *audio_dev;
+
+	audio_dev = get_audio_dev(device);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	audio_dev->sink = sink_init(audio_dev);
 
 	return 0;
 }
@@ -232,7 +242,7 @@ static int a2dp_server_probe(struct btd_profile *p,
 	if (!adp)
 		return -EINVAL;
 
-	err = a2dp_register(adapter_get_address(adapter), config);
+	err = a2dp_server_register(adapter_get_address(adapter), config);
 	if (err < 0)
 		audio_adapter_unref(adp);
 
@@ -255,6 +265,40 @@ static void a2dp_server_remove(struct btd_profile *p,
 	audio_adapter_unref(adp);
 }
 
+static int a2dp_source_server_probe(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	struct audio_adapter *adp;
+	const gchar *path = adapter_get_path(adapter);
+
+	DBG("path %s", path);
+
+	adp = audio_adapter_get(adapter);
+	if (!adp)
+		return -EINVAL;
+
+	audio_adapter_unref(adp); /* Referenced by a2dp server */
+
+	return a2dp_source_register(adapter_get_address(adapter), config);
+}
+
+static int a2dp_sink_server_probe(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	struct audio_adapter *adp;
+	const gchar *path = adapter_get_path(adapter);
+
+	DBG("path %s", path);
+
+	adp = audio_adapter_get(adapter);
+	if (!adp)
+		return -EINVAL;
+
+	audio_adapter_unref(adp); /* Referenced by a2dp server */
+
+	return a2dp_sink_register(adapter_get_address(adapter), config);
+}
+
 static int avrcp_server_probe(struct btd_profile *p,
 						struct btd_adapter *adapter)
 {
@@ -325,19 +369,38 @@ static void media_server_remove(struct btd_adapter *adapter)
 	audio_adapter_unref(adp);
 }
 
-static struct btd_profile a2dp_profile = {
-	.name		= "audio-a2dp",
+static struct btd_profile avdtp_profile = {
+	.name		= "audio-avdtp",
 	.priority	= BTD_PROFILE_PRIORITY_MEDIUM,
 
-	.remote_uuids	= BTD_UUIDS(A2DP_SOURCE_UUID, A2DP_SINK_UUID,
-							ADVANCED_AUDIO_UUID),
-	.device_probe	= a2dp_probe,
-	.device_remove	= audio_remove,
+	.remote_uuids	= BTD_UUIDS(ADVANCED_AUDIO_UUID),
 
 	.adapter_probe	= a2dp_server_probe,
 	.adapter_remove = a2dp_server_remove,
 };
 
+static struct btd_profile a2dp_source_profile = {
+	.name		= "audio-source",
+	.priority	= BTD_PROFILE_PRIORITY_MEDIUM,
+
+	.remote_uuids	= BTD_UUIDS(A2DP_SOURCE_UUID),
+	.device_probe	= a2dp_source_probe,
+	.device_remove	= audio_remove,
+
+	.adapter_probe	= a2dp_source_server_probe,
+};
+
+static struct btd_profile a2dp_sink_profile = {
+	.name		= "audio-sink",
+	.priority	= BTD_PROFILE_PRIORITY_MEDIUM,
+
+	.remote_uuids	= BTD_UUIDS(A2DP_SINK_UUID),
+	.device_probe	= a2dp_sink_probe,
+	.device_remove	= audio_remove,
+
+	.adapter_probe	= a2dp_sink_server_probe,
+};
+
 static struct btd_profile avrcp_profile = {
 	.name		= "audio-avrcp",
 
@@ -409,7 +472,13 @@ int audio_manager_init(GKeyFile *conf)
 
 proceed:
 	if (enabled.source || enabled.sink)
-		btd_profile_register(&a2dp_profile);
+		btd_profile_register(&avdtp_profile);
+
+	if (enabled.source)
+		btd_profile_register(&a2dp_source_profile);
+
+	if (enabled.sink)
+		btd_profile_register(&a2dp_sink_profile);
 
 	if (enabled.control)
 		btd_profile_register(&avrcp_profile);
@@ -426,8 +495,14 @@ void audio_manager_exit(void)
 		config = NULL;
 	}
 
+	if (enabled.source)
+		btd_profile_unregister(&a2dp_source_profile);
+
+	if (enabled.sink)
+		btd_profile_unregister(&a2dp_sink_profile);
+
 	if (enabled.source || enabled.sink)
-		btd_profile_unregister(&a2dp_profile);
+		btd_profile_unregister(&avdtp_profile);
 
 	if (enabled.control)
 		btd_profile_unregister(&avrcp_profile);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 7/8] source: Add profile .connect and .disconnect
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Add the connection and disconnection hooks to the a2dp_source
btd_profile.
---
 profiles/audio/manager.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index d7c1717..64df824 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -80,6 +80,12 @@ struct audio_adapter {
 	gint ref;
 };
 
+struct profile_req {
+	struct btd_device	*device;
+	struct btd_profile	*profile;
+	btd_profile_cb		cb;
+};
+
 static gboolean auto_connect = TRUE;
 static int max_connected_headsets = 1;
 static GKeyFile *config = NULL;
@@ -182,6 +188,79 @@ static int avrcp_probe(struct btd_profile *p, struct btd_device *device,
 	return 0;
 }
 
+static struct profile_req *new_profile_request(struct btd_device *dev,
+						struct btd_profile *profile,
+						btd_profile_cb cb)
+{
+	struct profile_req *req;
+
+	req  = g_new0(struct profile_req, 1);
+	req->device = dev;
+	req->profile = profile;
+	req->cb = cb;
+
+	return req;
+}
+
+static void profile_cb(struct audio_device *dev, int err, void *data)
+{
+	struct profile_req *req = data;
+
+	req->cb(req->profile, req->device, err);
+
+	g_free(req);
+}
+
+static int a2dp_source_connect(struct btd_device *dev,
+						struct btd_profile *profile,
+						btd_profile_cb cb)
+{
+	struct audio_device *audio_dev;
+	struct profile_req *req;
+	int err;
+
+	audio_dev = get_audio_dev(dev);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	req = new_profile_request(dev, profile, cb);
+
+	err = source_connect(audio_dev, profile_cb, req);
+	if (err < 0) {
+		g_free(req);
+		return err;
+	}
+
+	return 0;
+}
+
+static int a2dp_source_disconnect(struct btd_device *dev,
+						struct btd_profile *profile,
+						btd_profile_cb cb)
+{
+	struct audio_device *audio_dev;
+	struct profile_req *req;
+	int err;
+
+	audio_dev = get_audio_dev(dev);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	req = new_profile_request(dev, profile, cb);
+
+	err = source_disconnect(audio_dev, FALSE, profile_cb, req);
+	if (err < 0) {
+		g_free(req);
+		return err;
+	}
+
+	return 0;
+}
+
 static struct audio_adapter *audio_adapter_ref(struct audio_adapter *adp)
 {
 	adp->ref++;
@@ -387,6 +466,9 @@ static struct btd_profile a2dp_source_profile = {
 	.device_probe	= a2dp_source_probe,
 	.device_remove	= audio_remove,
 
+	.connect	= a2dp_source_connect,
+	.disconnect	= a2dp_source_disconnect,
+
 	.adapter_probe	= a2dp_source_server_probe,
 };
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH v2 8/8] sink: Add profile .connect and .disconnect
From: Mikel Astiz @ 2012-11-15  7:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mikel Astiz
In-Reply-To: <1352964460-8531-1-git-send-email-mikel.astiz.oss@gmail.com>

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Add the connection and disconnection hooks to the a2dp_sink btd_profile.
---
 profiles/audio/manager.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/profiles/audio/manager.c b/profiles/audio/manager.c
index 64df824..38e3c2b 100644
--- a/profiles/audio/manager.c
+++ b/profiles/audio/manager.c
@@ -261,6 +261,56 @@ static int a2dp_source_disconnect(struct btd_device *dev,
 	return 0;
 }
 
+static int a2dp_sink_connect(struct btd_device *dev,
+						struct btd_profile *profile,
+						btd_profile_cb cb)
+{
+	struct audio_device *audio_dev;
+	struct profile_req *req;
+	int err;
+
+	audio_dev = get_audio_dev(dev);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	req = new_profile_request(dev, profile, cb);
+
+	err = sink_connect(audio_dev, profile_cb, req);
+	if (err < 0) {
+		g_free(req);
+		return err;
+	}
+
+	return 0;
+}
+
+static int a2dp_sink_disconnect(struct btd_device *dev,
+						struct btd_profile *profile,
+						btd_profile_cb cb)
+{
+	struct audio_device *audio_dev;
+	struct profile_req *req;
+	int err;
+
+	audio_dev = get_audio_dev(dev);
+	if (!audio_dev) {
+		DBG("unable to get a device object");
+		return -1;
+	}
+
+	req = new_profile_request(dev, profile, cb);
+
+	err = sink_disconnect(audio_dev, FALSE, profile_cb, req);
+	if (err < 0) {
+		g_free(req);
+		return err;
+	}
+
+	return 0;
+}
+
 static struct audio_adapter *audio_adapter_ref(struct audio_adapter *adp)
 {
 	adp->ref++;
@@ -480,6 +530,9 @@ static struct btd_profile a2dp_sink_profile = {
 	.device_probe	= a2dp_sink_probe,
 	.device_remove	= audio_remove,
 
+	.connect	= a2dp_sink_connect,
+	.disconnect	= a2dp_sink_disconnect,
+
 	.adapter_probe	= a2dp_sink_server_probe,
 };
 
-- 
1.7.11.7


^ permalink raw reply related

* Re: [PATCH v2 01/20] cyclingspeed: Add CSC profile plugin skeleton
From: Johan Hedberg @ 2012-11-15  9:31 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1352105705-13988-2-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Mon, Nov 05, 2012, Andrzej Kaczmarek wrote:
> This patch adds stub profile driver plugin for CSC profile.
> ---
>  Makefile.am                          |   9 +-
>  lib/uuid.h                           |   2 +
>  profiles/cyclingspeed/cyclingspeed.c | 163 +++++++++++++++++++++++++++++++++++
>  profiles/cyclingspeed/cyclingspeed.h |  26 ++++++
>  profiles/cyclingspeed/main.c         |  53 ++++++++++++
>  profiles/cyclingspeed/manager.c      |  79 +++++++++++++++++
>  profiles/cyclingspeed/manager.h      |  24 ++++++
>  7 files changed, 354 insertions(+), 2 deletions(-)
>  create mode 100644 profiles/cyclingspeed/cyclingspeed.c
>  create mode 100644 profiles/cyclingspeed/cyclingspeed.h
>  create mode 100644 profiles/cyclingspeed/main.c
>  create mode 100644 profiles/cyclingspeed/manager.c
>  create mode 100644 profiles/cyclingspeed/manager.h

You'll need to rebase these against latest git since this one doesn't
apply cleanly (due to Makefile.am) and doesn't compile either (due to
main_opts.gatt_enabled having been removed).

> +struct csc_adapter {
> +	struct btd_adapter	*adapter;
> +	GSList			*devices;	/* list of registered devices */
> +};
> +
> +struct csc {
> +	struct btd_device	*dev;
> +	struct csc_adapter	*cadapter;
> +};

I'm a bit worried that we've failed to create proper internal GATT APIs
if they require this kind of per-profile tracking of devices and
adapters. You can already get a list of btd_device from btd_adapter and
the btd_adapter from a btd_device. Profiles should only need to track a
very minimal amount of context.

> +++ b/profiles/cyclingspeed/main.c
> @@ -0,0 +1,53 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2012 Tieto Poland
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdint.h>
> +#include <glib.h>
> +#include <errno.h>
> +
> +#include "plugin.h"
> +#include "manager.h"
> +#include "hcid.h"
> +#include "log.h"
> +
> +static int cyclingspeed_init(void)
> +{
> +	if (!main_opts.gatt_enabled) {
> +		DBG("GATT is disabled");
> +		return -ENOTSUP;
> +	}
> +
> +	return cyclingspeed_manager_init();
> +}
> +
> +static void cyclingspeed_exit(void)
> +{
> +	cyclingspeed_manager_exit();
> +}
> +
> +BLUETOOTH_PLUGIN_DEFINE(cyclingspeed, VERSION,
> +					BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
> +					cyclingspeed_init, cyclingspeed_exit)

If this is all the main.c file does seems like it should be merged into
manager.c or even cyclingspeed.c.

> +++ b/profiles/cyclingspeed/manager.c
> @@ -0,0 +1,79 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2012 Tieto Poland
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#include <gdbus.h>
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <bluetooth/uuid.h>
> +
> +#include "adapter.h"
> +#include "device.h"
> +#include "profile.h"
> +#include "att.h"
> +#include "gattrib.h"
> +#include "gatt.h"
> +#include "cyclingspeed.h"
> +#include "manager.h"
> +
> +static int csc_adapter_probe(struct btd_profile *p, struct btd_adapter *adapter)
> +{
> +	return csc_adapter_register(adapter);
> +}
> +
> +static void csc_adapter_remove(struct btd_profile *p,
> +						struct btd_adapter *adapter)
> +{
> +	csc_adapter_unregister(adapter);
> +}
> +
> +static int csc_device_probe(struct btd_profile *p,
> +				struct btd_device *device, GSList *uuids)
> +{
> +	return csc_device_register(device);
> +}
> +
> +static void csc_device_remove(struct btd_profile *p,
> +						struct btd_device *device)
> +{
> +	csc_device_unregister(device);
> +}
> +
> +static struct btd_profile cscp_profile = {
> +	.name		= "Cycling Speed and Cadence GATT Driver",
> +	.remote_uuids	= BTD_UUIDS(CYCLING_SC_UUID),
> +
> +	.adapter_probe	= csc_adapter_probe,
> +	.adapter_remove	= csc_adapter_remove,
> +
> +	.device_probe	= csc_device_probe,
> +	.device_remove	= csc_device_remove,
> +};
> +
> +int cyclingspeed_manager_init(void)
> +{
> +	return btd_profile_register(&cscp_profile);
> +}
> +
> +void cyclingspeed_manager_exit(void)
> +{
> +	btd_profile_unregister(&cscp_profile);
> +}

Are these functions ever going to do anything else than make single
Vcalls into cyclingspeed.c? If not it seems to me like all this should
be merged into that file.

Btw, I do realize that other profiles might have similar brain dead
design but that's not a good enough excuse to keep replicating the bad
design. Instead the other profiles should be fixed.

Johan

^ permalink raw reply

* Re: [PATCH v4 09/16] sbc: Use simd primitive if doing msbc on neon
From: Frédéric Dalleau @ 2012-11-15 10:23 UTC (permalink / raw)
  To: Siarhei Siamashka; +Cc: linux-bluetooth
In-Reply-To: <20121114212703.0d8bf8cd@i7>

Hi,
On 11/14/2012 08:27 PM, Siarhei Siamashka wrote:
> On Tue, 30 Oct 2012 10:39:28 +0100
>> +	if (state->increment == 1)
>> +		state->sbc_analyze_8s = sbc_analyze_1b_8s_simd;
>>   #endif
>>   }
>
> This is not enough. As I commented earlier in
>      http://permalink.gmane.org/gmane.linux.bluez.kernel/31567
>
> "neon code also provides optimized "sbc_enc_process_input_*" functions,
> which are not going to work correctly for mSBC:
>
> 	state->sbc_enc_process_input_8s_le = sbc_enc_process_input_8s_le_neon;
> 	state->sbc_enc_process_input_8s_be = sbc_enc_process_input_8s_be_neon;"

Indeed, this is a mistake :
I wanted to use the neon analysis in conjonction with simd input processing.

Instead, the patch would look like this :
+ if (state->increment == 1) {
+ 	state->sbc_enc_process_input_8s_be = 	
+					sbc_enc_process_input_8s_be;
+	state->sbc_enc_process_input_8s_le =
+					sbc_enc_process_input_8s_le;
+ }
Damned it is 82 chars long!

Unfortunately I can't test this one. I can't even build it. I could but 
that's gonna take a lot of time.

Regards,
Frédéric

^ permalink raw reply

* RE: CSA2:  User space aspect
From: Michael Knudsen @ 2012-11-15 11:34 UTC (permalink / raw)
  To: 'Marcel Holtmann'; +Cc: linux-bluetooth
In-Reply-To: <1352935032.20338.19.camel@aeonflux>

> please to not attempt to use socket options as ioctl. They are
> called options for a reason.
> 
> Getting the list of supported codecs should be done via mgmt
> interface command and it should be only done once.

Okay, I'll take a stab at this, then.

I expect that socket options are still fine for actually
setting the configuration? 

> Please ask yourself the question when and how the SCO data from the
> socket is actually affected. I still have not seen you provide the
> semantics of how the socket would be used afterwards. Especially on
> impact for the application establishing the SCO socket.

There are two cases, connect() and accept().

For connect(), this is pretty simple:

	sk = socket(..);
	setsockopt(sk, SOL_SCO, SCO_AIR_FORMAT, ...);
	connect(sk, ..);

For accept(), it is the intention that this will be used in
conjunction with BT_DEFER_SETUP such that we can set the codecs
as per how we just negotiated with the peer:

	sk = socket(..);
	bind(sk, ..);
	setsockopt(sk, SOL_SCO, BT_DEFER_SETUP, 1);
	listen(sk, ..);
	peer = accept(sk, ..);
	setsockopt(peer, SOL_SCO, SCO_AIR_FORMAT, SCO_FORMAT_MSBC);
	recvmsg(peer, ..);

> In a more important question, is this static one time fits all
> selection or is this actually to be dynamic on every new connection
> establishment.

Both (defer/non-defer).  The socket options will set the parameters
that are used by the stack whenever it sets up or accepts a connection.

Did this answer your questions?

-m.



^ permalink raw reply

* [PATCH BlueZ 1/5] core: Fix registering '/org/bluez' path before '/'
From: Luiz Augusto von Dentz @ 2012-11-15 14:05 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This cause problems with ObjectManager being exported in both paths
---
 src/manager.c | 13 ++++++++-----
 src/manager.h |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/manager.c b/src/manager.c
index 07f9482..3088dd9 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -181,14 +181,17 @@ static const GDBusPropertyTable manager_properties[] = {
 	{ }
 };
 
-dbus_bool_t manager_init(const char *path)
+bool manager_init(const char *path)
 {
-	btd_profile_init();
-
-	return g_dbus_register_interface(btd_get_dbus_connection(),
+	if (!g_dbus_register_interface(btd_get_dbus_connection(),
 					"/", MANAGER_INTERFACE,
 					manager_methods, manager_signals,
-					manager_properties, NULL, NULL);
+					manager_properties, NULL, NULL))
+		return false;
+
+	btd_profile_init();
+
+	return true;
 }
 
 static void manager_set_default_adapter(int id)
diff --git a/src/manager.h b/src/manager.h
index 0bb8b2c..4d094b6 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -29,7 +29,7 @@
 
 typedef void (*adapter_cb) (struct btd_adapter *adapter, gpointer user_data);
 
-dbus_bool_t manager_init(const char *path);
+bool manager_init(const char *path);
 void manager_cleanup(const char *path);
 
 const char *manager_get_base_path(void);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 2/5] core: Make exit sequence consistent with init
From: Luiz Augusto von Dentz @ 2012-11-15 14:05 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352988325-10782-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

manager_cleanup should be called after plugin_cleanup on exit as
manager_init is called before plugin_init on the init sequence.
---
 src/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main.c b/src/main.c
index 9ea12df..414849a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -545,12 +545,12 @@ int main(int argc, char *argv[])
 
 	g_source_remove(signal);
 
+	plugin_cleanup();
+
 	manager_cleanup("/");
 
 	rfkill_exit();
 
-	plugin_cleanup();
-
 	stop_sdp_server();
 
 	g_main_loop_unref(event_loop);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 3/5] gdbus: Fix having multiple path exporting ObjectManager
From: Luiz Augusto von Dentz @ 2012-11-15 14:05 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352988325-10782-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

ObjectManager should only be available on the root path so if the
current is a child of the object being registered the root should be
changed.
---
 gdbus/object.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 214fd84..1f0ea39 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -84,6 +84,8 @@ struct property_data {
 	DBusMessage *message;
 };
 
+static struct generic_data *root = NULL;
+
 static gboolean process_changes(gpointer user_data);
 static void process_properties_from_interface(struct generic_data *data,
 						struct interface_data *iface);
@@ -574,11 +576,7 @@ static void emit_interfaces_added(struct generic_data *data)
 	if (parent == NULL)
 		return;
 
-	/* Find root data */
-	while (parent->parent)
-		parent = parent->parent;
-
-	signal = dbus_message_new_signal(parent->path,
+	signal = dbus_message_new_signal(root->path,
 					DBUS_INTERFACE_OBJECT_MANAGER,
 					"InterfacesAdded");
 	if (signal == NULL)
@@ -948,11 +946,7 @@ static void emit_interfaces_removed(struct generic_data *data)
 	if (parent == NULL)
 		return;
 
-	/* Find root data */
-	while (parent->parent)
-		parent = parent->parent;
-
-	signal = dbus_message_new_signal(parent->path,
+	signal = dbus_message_new_signal(root->path,
 					DBUS_INTERFACE_OBJECT_MANAGER,
 					"InterfacesRemoved");
 	if (signal == NULL)
@@ -1008,6 +1002,9 @@ static void generic_unregister(DBusConnection *connection, void *user_data)
 	g_slist_foreach(data->objects, reset_parent, data->parent);
 	g_slist_free(data->objects);
 
+	if (root == data)
+		root = NULL;
+
 	dbus_connection_unref(data->conn);
 	g_free(data->introspect);
 	g_free(data->path);
@@ -1175,6 +1172,20 @@ static void add_interface(struct generic_data *data,
 	data->process_id = g_idle_add(process_changes, data);
 }
 
+static void set_root(struct generic_data *data)
+{
+	if (root != NULL) {
+		data->objects = g_slist_prepend(data->objects, root);
+		root->parent = data;
+		remove_interface(root, DBUS_INTERFACE_OBJECT_MANAGER);
+	}
+
+	add_interface(data, DBUS_INTERFACE_OBJECT_MANAGER,
+					manager_methods, manager_signals,
+					NULL, data, NULL);
+	root = data;
+}
+
 static struct generic_data *object_path_ref(DBusConnection *connection,
 							const char *path)
 {
@@ -1209,9 +1220,7 @@ static struct generic_data *object_path_ref(DBusConnection *connection,
 
 	/* Only root path export ObjectManager interface */
 	if (data->parent == NULL)
-		add_interface(data, DBUS_INTERFACE_OBJECT_MANAGER,
-					manager_methods, manager_signals,
-					NULL, data, NULL);
+		set_root(data);
 
 	add_interface(data, DBUS_INTERFACE_PROPERTIES, properties_methods,
 					properties_signals, NULL, data, NULL);
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 4/5] gdbus: Automatically register  '/' path
From: Luiz Augusto von Dentz @ 2012-11-15 14:05 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352988325-10782-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes g_dbus_setup_bus to automatically register '/' path so
user application that don't export any interface on '/' will have it
registered for ObjectManager.

Note that it is now required to call g_dbus_disconnect before exit.
---
 gdbus/gdbus.h    |  1 +
 gdbus/mainloop.c | 21 +++++++++++++++++----
 gdbus/object.c   |  2 +-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index ba49621..3fbe3bc 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -53,6 +53,7 @@ DBusConnection *g_dbus_setup_bus(DBusBusType type, const char *name,
 
 DBusConnection *g_dbus_setup_private(DBusBusType type, const char *name,
 							DBusError *error);
+void g_dbus_disconnect(DBusConnection *connection);
 
 gboolean g_dbus_request_name(DBusConnection *connection, const char *name,
 							DBusError *error);
diff --git a/gdbus/mainloop.c b/gdbus/mainloop.c
index 099b67f..acea308 100644
--- a/gdbus/mainloop.c
+++ b/gdbus/mainloop.c
@@ -301,12 +301,25 @@ DBusConnection *g_dbus_setup_bus(DBusBusType type, const char *name,
 	if (conn == NULL)
 		return NULL;
 
-	if (setup_bus(conn, name, error) == FALSE) {
-		dbus_connection_unref(conn);
-		return NULL;
-	}
+	if (setup_bus(conn, name, error) == FALSE)
+		goto fail;
+
+	if (!g_dbus_register_interface(conn, "/", NULL, NULL, NULL, NULL, NULL,
+									NULL))
+		goto fail;
 
 	return conn;
+
+fail:
+	dbus_connection_unref(conn);
+	return NULL;
+
+}
+
+void g_dbus_disconnect(DBusConnection *connection)
+{
+	g_dbus_unregister_interface(connection, "/", NULL);
+	dbus_connection_unref(connection);
 }
 
 DBusConnection *g_dbus_setup_private(DBusBusType type, const char *name,
diff --git a/gdbus/object.c b/gdbus/object.c
index 1f0ea39..04941c7 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -615,7 +615,7 @@ static struct interface_data *find_interface(GSList *interfaces,
 
 	for (list = interfaces; list; list = list->next) {
 		struct interface_data *iface = list->data;
-		if (!strcmp(name, iface->name))
+		if (g_strcmp0(name, iface->name) == 0)
 			return iface;
 	}
 
-- 
1.7.11.7


^ permalink raw reply related

* [PATCH BlueZ 5/5] core: Make use of g_dbus_disconnect
From: Luiz Augusto von Dentz @ 2012-11-15 14:05 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1352988325-10782-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 src/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main.c b/src/main.c
index 414849a..c180e09 100644
--- a/src/main.c
+++ b/src/main.c
@@ -383,7 +383,7 @@ static void disconnect_dbus(void)
 
 	set_dbus_connection(NULL);
 
-	dbus_connection_unref(conn);
+	g_dbus_disconnect(conn);
 }
 
 static void disconnected_dbus(DBusConnection *conn, void *data)
-- 
1.7.11.7


^ permalink raw reply related

* Re: [PATCH BlueZ 2/5] core: Make exit sequence consistent with init
From: Johan Hedberg @ 2012-11-15 14:15 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1352988325-10782-2-git-send-email-luiz.dentz@gmail.com>

Hi Luiz,

On Thu, Nov 15, 2012, Luiz Augusto von Dentz wrote:
> manager_cleanup should be called after plugin_cleanup on exit as
> manager_init is called before plugin_init on the init sequence.
> ---
>  src/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Patches 1 and 2 have been applied. For the others I'll wait for some
more feedback (e.g. from Lucas and Marcel).

Johan

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox