Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Jiri Kosina @ 2014-02-04 13:46 UTC (permalink / raw)
  To: David Herrmann
  Cc: Marcel Holtmann, open list:HID CORE LAYER,
	linux-bluetooth@vger.kernel.org development, Gustavo F. Padovan
In-Reply-To: <CANq1E4S++uqMzQue_0jC3N=Lm8Os5V-48Hw5M4vWvM+2Vx91yA@mail.gmail.com>

On Mon, 3 Feb 2014, David Herrmann wrote:

> >> >> Due to various reasons I will not have access to any testing HW for the
> >> >> upcoming 2-3 days, so if you can cook something up in that timeframe, it'd
> >> >> be appreciated.
> >> >>
> >> >> Otherwise I'll be working on it by the end of this week.
> >> >
> >> > David,
> >> >
> >> > just got back to this, finally ... did you have time to work on this at
> >> > all, or should I just start from scratch?
> >>
> >> Sorry, no. Fosdem is this weekend and I needed to get my code ready
> >> for that. But I'll finally have time again next week.
> >
> > Okay, thanks. I then guess we should proceed with this bandaid (double
> > allocation) for 3.14
> 
> It would be really nice if we could get the HIDP patch into 3.14-rc2
> and backported to stable. There have been quite a bunch of reports now
> and I dislike adding a GFP_ATOMIC allocation in HID core. 

I agree with David; Gustavo, what is your take on this, please?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] unit/avctp: Add connection establishment dummy tests
From: Luiz Augusto von Dentz @ 2014-02-04 13:46 UTC (permalink / raw)
  To: Andrei Emeltchenko, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140204115347.GL2930@aemeltch-MOBL1>

Hi Andrei,

On Tue, Feb 4, 2014 at 1:53 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> On Mon, Feb 03, 2014 at 02:23:18PM +0200, Andrei Emeltchenko wrote:
>> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> ping
>
>>
>> Add some tests checking that L2CAP connection is established, so they
>> are basically dummy tests.
>> ---
>>  unit/test-avctp.c | 30 +++++++++++++++++++++++++++---
>>  1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/unit/test-avctp.c b/unit/test-avctp.c
>> index 041e0c0..c0d16a4 100644
>> --- a/unit/test-avctp.c
>> +++ b/unit/test-avctp.c
>> @@ -220,10 +220,8 @@ static struct context *create_context(uint16_t version, gconstpointer data)
>>       return context;
>>  }
>>
>> -static void execute_context(struct context *context)
>> +static void destroy_context(struct context *context)
>>  {
>> -     g_main_loop_run(context->main_loop);
>> -
>>       if (context->source > 0)
>>               g_source_remove(context->source);
>>
>> @@ -235,6 +233,13 @@ static void execute_context(struct context *context)
>>       g_free(context);
>>  }
>>
>> +static void execute_context(struct context *context)
>> +{
>> +     g_main_loop_run(context->main_loop);
>> +
>> +     destroy_context(context);
>> +}
>> +
>>  static void test_client(gconstpointer data)
>>  {
>>       struct context *context = create_context(0x0100, data);
>> @@ -253,6 +258,13 @@ static void test_server(gconstpointer data)
>>       execute_context(context);
>>  }
>>
>> +static void test_dummy(gconstpointer data)
>> +{
>> +     struct context *context = create_context(0x0100, data);
>> +
>> +     destroy_context(context);
>> +}
>> +
>>  int main(int argc, char *argv[])
>>  {
>>       g_test_init(&argc, &argv, NULL);
>> @@ -260,6 +272,18 @@ int main(int argc, char *argv[])
>>       if (g_test_verbose())
>>               __btd_log_init("*", 0);
>>
>> +     /* Connection Channel Management tests */
>> +
>> +     /*
>> +      * Tests are checking that IUT is able to request establishing
>> +      * channels, since we already have connection through socketpair
>> +      * the tests are dummy.
>> +      */
>> +     define_test("/TP/CCM/BV-01-C", test_dummy, raw_pdu(0x00));
>> +     define_test("/TP/CCM/BV-02-C", test_dummy, raw_pdu(0x00));
>> +     define_test("/TP/CCM/BV-03-C", test_dummy, raw_pdu(0x00));
>> +     define_test("/TP/CCM/BV-04-C", test_dummy, raw_pdu(0x00));
>> +
>>       define_test("/TP/NFR/BV-01-C", test_client,
>>                               raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00));
>>
>> --
>> 1.8.3.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --

Applied, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] unit/avctp: Add test TP/NFR/BV-03-C
From: Luiz Augusto von Dentz @ 2014-02-04 13:45 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <1391514294-25337-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Tue, Feb 4, 2014 at 1:44 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> The test verifies that IUT (TG) correctly reports the parameters of the
> incoming command in the handler callback.
> ---
>  unit/test-avctp.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/unit/test-avctp.c b/unit/test-avctp.c
> index c0d16a4..6d7e34a 100644
> --- a/unit/test-avctp.c
> +++ b/unit/test-avctp.c
> @@ -249,10 +249,35 @@ static void test_client(gconstpointer data)
>         execute_context(context);
>  }
>
> +static size_t handler(struct avctp *session,
> +                                       uint8_t transaction, uint8_t *code,
> +                                       uint8_t *subunit, uint8_t *operands,
> +                                       size_t operand_count, void *user_data)
> +{
> +       DBG("transaction %d code %d subunit %d operand_count %zu",
> +               transaction, *code, *subunit, operand_count);
> +
> +       g_assert_cmpint(transaction, ==, 0);
> +       g_assert_cmpint(*code, ==, 0);
> +       g_assert_cmpint(*subunit, ==, 0);
> +       g_assert_cmpint(operand_count, ==, 0);
> +
> +       return operand_count;
> +}
> +
>  static void test_server(gconstpointer data)
>  {
>         struct context *context = create_context(0x0100, data);
>
> +       if (g_str_equal(context->data->test_name, "/TP/NFR/BV-03-C")) {
> +               int ret;
> +
> +               ret = avctp_register_pdu_handler(context->session, 0x00,
> +                                                               handler, NULL);
> +               DBG("ret %d", ret);
> +               g_assert_cmpint(ret, !=, 0);
> +       }
> +
>         g_idle_add(send_pdu, context);
>
>         execute_context(context);
> @@ -284,6 +309,8 @@ int main(int argc, char *argv[])
>         define_test("/TP/CCM/BV-03-C", test_dummy, raw_pdu(0x00));
>         define_test("/TP/CCM/BV-04-C", test_dummy, raw_pdu(0x00));
>
> +       /* Non-Fragmented Messages tests */
> +
>         define_test("/TP/NFR/BV-01-C", test_client,
>                                 raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00));
>
> @@ -291,6 +318,10 @@ int main(int argc, char *argv[])
>                                 raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
>                                 raw_pdu(0x02, 0x11, 0x0e, 0x0a, 0x00, 0x00));
>
> +       define_test("/TP/NFR/BV-03-C", test_server,
> +                               raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
> +                               raw_pdu(0x02, 0x11, 0x0e, 0x00, 0x00, 0x00));
> +
>         define_test("/TP/NFR/BI-01-C", test_server,
>                                 raw_pdu(0x00, 0xff, 0xff, 0x00, 0x00, 0x00),
>                                 raw_pdu(0x03, 0xff, 0xff));
> --
> 1.8.3.2
>
> --

Applied, thanks.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* [PATCH v3] android/hal-audio: Add simple downmix to mono
From: Andrzej Kaczmarek @ 2014-02-04 13:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek

This patch adds simple downmix support from stereo to mono in order to
support mono channel mode as it's mandatory for SBC codec. It uses
simple (L+R)/2 calculation which should be good enough.
---
 android/hal-audio.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index efdf823..f90265b 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -36,9 +36,12 @@
 #include "hal-log.h"
 #include "hal-msg.h"
 #include "../profiles/audio/a2dp-codecs.h"
+#include "../src/shared/util.h"
 
 #define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
 
+#define FIXED_BUFFER_SIZE (20 * 512)
+
 #define MAX_FRAMES_IN_PAYLOAD 15
 
 static const uint8_t a2dp_src_uuid[] = {
@@ -220,6 +223,8 @@ struct a2dp_stream_out {
 	struct audio_endpoint *ep;
 	enum a2dp_state_t audio_state;
 	struct audio_input_config cfg;
+
+	uint8_t *downmix_buf;
 };
 
 struct a2dp_audio_dev {
@@ -230,7 +235,8 @@ struct a2dp_audio_dev {
 static const a2dp_sbc_t sbc_presets[] = {
 	{
 		.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
-		.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
+		.channel_mode = SBC_CHANNEL_MODE_MONO |
+				SBC_CHANNEL_MODE_DUAL_CHANNEL |
 				SBC_CHANNEL_MODE_STEREO |
 				SBC_CHANNEL_MODE_JOINT_STEREO,
 		.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
@@ -826,6 +832,21 @@ static void unregister_endpoints(void)
 	}
 }
 
+static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
+								size_t bytes)
+{
+	const int16_t *input = (const void *) buffer;
+	int16_t *output = (void *) out->downmix_buf;
+	size_t i;
+
+	for (i = 0; i < bytes / 2; i++) {
+		int16_t l = le16_to_cpu(get_unaligned(&input[i * 2]));
+		int16_t r = le16_to_cpu(get_unaligned(&input[i * 2 + 1]));
+
+		put_unaligned(cpu_to_le16((l + r) / 2), &output[i]);
+	}
+}
+
 static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 								size_t bytes)
 {
@@ -853,6 +874,18 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		return -1;
 	}
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		if (!out->downmix_buf) {
+			error("audio: downmix buffer not initialized");
+			return -1;
+		}
+
+		downmix_to_mono(out, buffer, bytes);
+
+		return out->ep->codec->write_data(out->ep->codec_data,
+				out->downmix_buf, bytes / 2, out->ep->fd) * 2;
+	}
+
 	return out->ep->codec->write_data(out->ep->codec_data, buffer,
 							bytes, out->ep->fd);
 }
@@ -890,16 +923,18 @@ static size_t out_get_buffer_size(const struct audio_stream *stream)
 	 * use magic value here and out_write code takes care of splitting
 	 * input buffer into multiple media packets.
 	 */
-	return 20 * 512;
+	return FIXED_BUFFER_SIZE;
 }
 
 static uint32_t out_get_channels(const struct audio_stream *stream)
 {
-	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
-
 	DBG("");
 
-	return out->cfg.channels;
+	/* AudioFlinger can only provide stereo stream, so we return it here and
+	 * later we'll downmix this to mono in case codec requires it
+	 */
+
+	return AUDIO_CHANNEL_OUT_STEREO;
 }
 
 static audio_format_t out_get_format(const struct audio_stream *stream)
@@ -1212,6 +1247,12 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 
 	free(preset);
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
+		if (!out->downmix_buf)
+			goto fail;
+	}
+
 	*stream_out = &out->stream;
 	a2dp_dev->out = out;
 
@@ -1230,6 +1271,7 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 					struct audio_stream_out *stream)
 {
 	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
+	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
 	struct audio_endpoint *ep = a2dp_dev->out->ep;
 
 	DBG("");
@@ -1243,6 +1285,8 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 	ep->codec->cleanup(ep->codec_data);
 	ep->codec_data = NULL;
 
+	free(out->downmix_buf);
+
 	free(stream);
 	a2dp_dev->out = NULL;
 }
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH v2] android/hal-audio: Add simple downmix to mono
From: Andrzej Kaczmarek @ 2014-02-04 13:16 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek

This patch adds simple downmix support from stereo to mono in order to
support mono channel mode as it's mandatory for SBC codec. It uses
simple (L+R)/2 calculation which should be good enough.
---
 android/hal-audio.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 5 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index efdf823..cef70aa 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -36,9 +36,12 @@
 #include "hal-log.h"
 #include "hal-msg.h"
 #include "../profiles/audio/a2dp-codecs.h"
+#include "../src/shared/util.h"
 
 #define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
 
+#define FIXED_BUFFER_SIZE (20 * 512)
+
 #define MAX_FRAMES_IN_PAYLOAD 15
 
 static const uint8_t a2dp_src_uuid[] = {
@@ -220,6 +223,8 @@ struct a2dp_stream_out {
 	struct audio_endpoint *ep;
 	enum a2dp_state_t audio_state;
 	struct audio_input_config cfg;
+
+	uint8_t *downmix_buf;
 };
 
 struct a2dp_audio_dev {
@@ -230,7 +235,8 @@ struct a2dp_audio_dev {
 static const a2dp_sbc_t sbc_presets[] = {
 	{
 		.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
-		.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
+		.channel_mode = SBC_CHANNEL_MODE_MONO |
+				SBC_CHANNEL_MODE_DUAL_CHANNEL |
 				SBC_CHANNEL_MODE_STEREO |
 				SBC_CHANNEL_MODE_JOINT_STEREO,
 		.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
@@ -826,6 +832,21 @@ static void unregister_endpoints(void)
 	}
 }
 
+static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
+								size_t bytes)
+{
+	size_t i;
+	int16_t *input = (int16_t *) buffer;
+	int16_t *output = (int16_t *) out->downmix_buf;
+
+	for (i = 0; i < bytes / 2; i++) {
+		int16_t l = le16_to_cpu(get_unaligned(&input[i * 2]));
+		int16_t r = le16_to_cpu(get_unaligned(&input[i * 2 + 1]));
+
+		output[i] = (l + r) / 2;
+	}
+}
+
 static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 								size_t bytes)
 {
@@ -853,6 +874,18 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		return -1;
 	}
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		if (!out->downmix_buf) {
+			error("audio: downmix buffer not initialized");
+			return -1;
+		}
+
+		downmix_to_mono(out, buffer, bytes);
+
+		return out->ep->codec->write_data(out->ep->codec_data,
+				out->downmix_buf, bytes / 2, out->ep->fd) * 2;
+	}
+
 	return out->ep->codec->write_data(out->ep->codec_data, buffer,
 							bytes, out->ep->fd);
 }
@@ -890,16 +923,18 @@ static size_t out_get_buffer_size(const struct audio_stream *stream)
 	 * use magic value here and out_write code takes care of splitting
 	 * input buffer into multiple media packets.
 	 */
-	return 20 * 512;
+	return FIXED_BUFFER_SIZE;
 }
 
 static uint32_t out_get_channels(const struct audio_stream *stream)
 {
-	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
-
 	DBG("");
 
-	return out->cfg.channels;
+	/* AudioFlinger can only provide stereo stream, so we return it here and
+	 * later we'll downmix this to mono in case codec requires it
+	 */
+
+	return AUDIO_CHANNEL_OUT_STEREO;
 }
 
 static audio_format_t out_get_format(const struct audio_stream *stream)
@@ -1212,6 +1247,12 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 
 	free(preset);
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
+		if (!out->downmix_buf)
+			goto fail;
+	}
+
 	*stream_out = &out->stream;
 	a2dp_dev->out = out;
 
@@ -1230,6 +1271,7 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 					struct audio_stream_out *stream)
 {
 	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
+	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
 	struct audio_endpoint *ep = a2dp_dev->out->ep;
 
 	DBG("");
@@ -1243,6 +1285,8 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 	ep->codec->cleanup(ep->codec_data);
 	ep->codec_data = NULL;
 
+	free(out->downmix_buf);
+
 	free(stream);
 	a2dp_dev->out = NULL;
 }
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation
From: Andrei Emeltchenko @ 2014-02-04 13:05 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKTZ4WMTp26ajSOxqkNDj4D9--L2=V6YvG6QzuyqrufZQ@mail.gmail.com>

Hi Luiz,

On Tue, Feb 04, 2014 at 02:49:02PM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Tue, Feb 4, 2014 at 1:55 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote:
> >> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >>
> >> There no need to calculate those values
> >
> > ping
> >
> >> ---
> >>  profiles/audio/avrcp.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> >> index fa5adab..2e1a940 100644
> >> --- a/profiles/audio/avrcp.c
> >> +++ b/profiles/audio/avrcp.c
> >> @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event)
> >>       pdu->params[0] = event;
> >>       pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH);
> >>
> >> -     length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> >> +     length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH;
> >>
> >>       avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY,
> >>                                       AVC_SUBUNIT_PANEL, buf, length,
> >> @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session)
> >>       pdu->params[0] = CAP_EVENTS_SUPPORTED;
> >>       pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
> >>
> >> -     length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> >> +     length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH;
> >>
> >>       avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
> >>                                       AVC_SUBUNIT_PANEL, buf, length,
> >> --
> >> 1.8.3.2
> 
> I will leave this as it for now, we will probably create a avrcp_send
> helper to have this in common place.

OK, though those 

a = ntohs(b);
c = htons(a);

looks IMO really ugly

Best regards 
Andrei Emeltchenko 

^ permalink raw reply

* Re: [PATCHv2] android/avrcp: Decouple AVRCP logic from btio
From: Andrei Emeltchenko @ 2014-02-04 13:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKLwPzEsLCLDGr6cMgYQRobcfQ7W_EqqUpSSvBp4=hGVg@mail.gmail.com>

Hi Luiz,

On Tue, Feb 04, 2014 at 02:50:11PM +0200, Luiz Augusto von Dentz wrote:
> Hi Andrei,
> 
> On Tue, Feb 4, 2014 at 1:56 PM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > On Mon, Feb 03, 2014 at 11:16:17AM +0200, Andrei Emeltchenko wrote:
> >> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> >>
> >> The patch makes AVRCP to be channel-agnostic so that it might be used in
> >> unit tests. The idea is that all AVRCP logic would come to avrcp-lib and
> >> channel stuff got to avrcp.
> 
> I think we discussed this offline and decided not to have a separate
> file for now.

So is the idea is to have unit-avrcp to be compiled with btio, etc? 
I will then just to link everything what android/avrcp needs.

Best regards 
Andrei Emeltchenko 


^ permalink raw reply

* Re: [PATCHv2] android/avrcp: Decouple AVRCP logic from btio
From: Luiz Augusto von Dentz @ 2014-02-04 12:50 UTC (permalink / raw)
  To: Andrei Emeltchenko, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140204115622.GN2930@aemeltch-MOBL1>

Hi Andrei,

On Tue, Feb 4, 2014 at 1:56 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> On Mon, Feb 03, 2014 at 11:16:17AM +0200, Andrei Emeltchenko wrote:
>> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>>
>> The patch makes AVRCP to be channel-agnostic so that it might be used in
>> unit tests. The idea is that all AVRCP logic would come to avrcp-lib and
>> channel stuff got to avrcp.
>
> ping
>
>> ---
>>  android/Android.mk  |  1 +
>>  android/Makefile.am |  1 +
>>  android/avrcp-lib.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  android/avrcp-lib.h | 32 ++++++++++++++++++++
>>  android/avrcp.c     | 44 ++-------------------------
>>  5 files changed, 122 insertions(+), 41 deletions(-)
>>  create mode 100644 android/avrcp-lib.c
>>  create mode 100644 android/avrcp-lib.h
>>
>> diff --git a/android/Android.mk b/android/Android.mk
>> index 09ed32d..2772b58 100644
>> --- a/android/Android.mk
>> +++ b/android/Android.mk
>> @@ -30,6 +30,7 @@ LOCAL_SRC_FILES := \
>>       bluez/android/avdtp.c \
>>       bluez/android/a2dp.c \
>>       bluez/android/avctp.c \
>> +     bluez/android/avrcp-lib.c \
>>       bluez/android/avrcp.c \
>>       bluez/android/pan.c \
>>       bluez/src/log.c \
>> diff --git a/android/Makefile.am b/android/Makefile.am
>> index e065c0c..29cbf79 100644
>> --- a/android/Makefile.am
>> +++ b/android/Makefile.am
>> @@ -34,6 +34,7 @@ android_bluetoothd_SOURCES = android/main.c \
>>                               android/avdtp.h android/avdtp.c \
>>                               android/a2dp.h android/a2dp.c \
>>                               android/avctp.h android/avctp.c \
>> +                             android/avrcp-lib.h android/avrcp-lib.c \
>>                               android/avrcp.h android/avrcp.c \
>>                               android/socket.h android/socket.c \
>>                               android/pan.h android/pan.c \
>> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
>> new file mode 100644
>> index 0000000..a4bfe6d
>> --- /dev/null
>> +++ b/android/avrcp-lib.c
>> @@ -0,0 +1,85 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
>> + *
>> + *
>> + *  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 <stdbool.h>
>> +#include <glib.h>
>> +
>> +#include "lib/bluetooth.h"
>> +
>> +#include "src/log.h"
>> +
>> +#include "avctp.h"
>> +#include "avrcp-lib.h"
>> +
>> +static GSList *devices = NULL;
>> +
>> +void avrcp_device_free(void *data)
>> +{
>> +     struct avrcp_device *dev = data;
>> +
>> +     if (dev->session)
>> +             avctp_shutdown(dev->session);
>> +
>> +     devices = g_slist_remove(devices, dev);
>> +     g_free(dev);
>> +}
>> +
>> +void avrcp_free_all(void)
>> +{
>> +     g_slist_free_full(devices, avrcp_device_free);
>> +     devices = NULL;
>> +}
>> +
>> +struct avrcp_device *avrcp_device_new(const bdaddr_t *dst)
>> +{
>> +     struct avrcp_device *dev;
>> +
>> +     dev = g_new0(struct avrcp_device, 1);
>> +     bacpy(&dev->dst, dst);
>> +     devices = g_slist_prepend(devices, dev);
>> +
>> +     return dev;
>> +}
>> +
>> +static int device_cmp(gconstpointer s, gconstpointer user_data)
>> +{
>> +     const struct avrcp_device *dev = s;
>> +     const bdaddr_t *dst = user_data;
>> +
>> +     return bacmp(&dev->dst, dst);
>> +}
>> +
>> +struct avrcp_device *avrcp_find(bdaddr_t *dst)
>> +{
>> +     GSList *l;
>> +
>> +     l = g_slist_find_custom(devices, dst, device_cmp);
>> +     if (!l)
>> +             return NULL;
>> +
>> +     return l->data;
>> +}
>> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
>> new file mode 100644
>> index 0000000..a7213fb
>> --- /dev/null
>> +++ b/android/avrcp-lib.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
>> + *
>> + *
>> + *  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
>> + *
>> + */
>> +
>> +struct avrcp_device {
>> +     bdaddr_t        dst;
>> +     struct avctp    *session;
>> +};
>> +
>> +struct avrcp_device *avrcp_device_new(const bdaddr_t *dst);
>> +void avrcp_device_free(void *data);
>> +void avrcp_free_all(void);
>> +struct avrcp_device *avrcp_find(bdaddr_t *dst);
>> diff --git a/android/avrcp.c b/android/avrcp.c
>> index ef833df..ff923cb 100644
>> --- a/android/avrcp.c
>> +++ b/android/avrcp.c
>> @@ -38,6 +38,7 @@
>>  #include "hal-msg.h"
>>  #include "ipc.h"
>>  #include "avctp.h"
>> +#include "avrcp-lib.h"
>>
>>  #define L2CAP_PSM_AVCTP 0x17
>>
>> @@ -48,14 +49,8 @@
>>
>>  static bdaddr_t adapter_addr;
>>  static uint32_t record_id = 0;
>> -static GSList *devices = NULL;
>>  static GIOChannel *server = NULL;
>>
>> -struct avrcp_device {
>> -     bdaddr_t        dst;
>> -     struct avctp    *session;
>> -};
>> -
>>  static const struct ipc_handler cmd_handlers[] = {
>>  };
>>
>> @@ -127,36 +122,6 @@ static sdp_record_t *avrcp_record(void)
>>       return record;
>>  }
>>
>> -static void avrcp_device_free(void *data)
>> -{
>> -     struct avrcp_device *dev = data;
>> -
>> -     if (dev->session)
>> -             avctp_shutdown(dev->session);
>> -
>> -     devices = g_slist_remove(devices, dev);
>> -     g_free(dev);
>> -}
>> -
>> -static struct avrcp_device *avrcp_device_new(const bdaddr_t *dst)
>> -{
>> -     struct avrcp_device *dev;
>> -
>> -     dev = g_new0(struct avrcp_device, 1);
>> -     bacpy(&dev->dst, dst);
>> -     devices = g_slist_prepend(devices, dev);
>> -
>> -     return dev;
>> -}
>> -
>> -static int device_cmp(gconstpointer s, gconstpointer user_data)
>> -{
>> -     const struct avrcp_device *dev = s;
>> -     const bdaddr_t *dst = user_data;
>> -
>> -     return bacmp(&dev->dst, dst);
>> -}
>> -
>>  static void disconnect_cb(void *data)
>>  {
>>       struct avrcp_device *dev = data;
>> @@ -175,7 +140,6 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>>       char address[18];
>>       uint16_t imtu, omtu;
>>       GError *gerr = NULL;
>> -     GSList *l;
>>       int fd;
>>
>>       if (err) {
>> @@ -199,8 +163,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>>       ba2str(&dst, address);
>>       DBG("Incoming connection from %s", address);
>>
>> -     l = g_slist_find_custom(devices, &dst, device_cmp);
>> -     if (l) {
>> +     if (avrcp_find(&dst)) {
>>               error("Unexpected connection");
>>               return;
>>       }
>> @@ -274,8 +237,7 @@ void bt_avrcp_unregister(void)
>>  {
>>       DBG("");
>>
>> -     g_slist_free_full(devices, avrcp_device_free);
>> -     devices = NULL;
>> +     avrcp_free_all();
>>
>>       ipc_unregister(HAL_SERVICE_ID_AVRCP);
>>
>> --
>> 1.8.3.2

I think we discussed this offline and decided not to have a separate
file for now.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation
From: Luiz Augusto von Dentz @ 2014-02-04 12:49 UTC (permalink / raw)
  To: Andrei Emeltchenko, linux-bluetooth@vger.kernel.org
In-Reply-To: <20140204115549.GM2930@aemeltch-MOBL1>

Hi Andrei,

On Tue, Feb 4, 2014 at 1:55 PM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote:
>> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>>
>> There no need to calculate those values
>
> ping
>
>> ---
>>  profiles/audio/avrcp.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
>> index fa5adab..2e1a940 100644
>> --- a/profiles/audio/avrcp.c
>> +++ b/profiles/audio/avrcp.c
>> @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event)
>>       pdu->params[0] = event;
>>       pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH);
>>
>> -     length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
>> +     length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH;
>>
>>       avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY,
>>                                       AVC_SUBUNIT_PANEL, buf, length,
>> @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session)
>>       pdu->params[0] = CAP_EVENTS_SUPPORTED;
>>       pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
>>
>> -     length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
>> +     length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH;
>>
>>       avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
>>                                       AVC_SUBUNIT_PANEL, buf, length,
>> --
>> 1.8.3.2

I will leave this as it for now, we will probably create a avrcp_send
helper to have this in common place.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH] android/hal-audio: Add simple downmix to mono
From: Luiz Augusto von Dentz @ 2014-02-04 12:44 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Andrzej Kaczmarek, linux-bluetooth@vger.kernel.org
In-Reply-To: <9469082.LestBIZf2H@uw000953>

Hi Andrzej,

On Tue, Feb 4, 2014 at 1:49 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Andrzej,
>
> On Tuesday 04 of February 2014 12:37:22 Andrzej Kaczmarek wrote:
>> This patch adds simple downmix support from stereo to mono in order to
>> support mono channel mode as it's mandatory for SBC codec. It uses
>> simple (L+R)/2 calculation which should be good enough.

Not following this one, doesn't AudioFlinger supports mono?

^ permalink raw reply

* Re: [PATCHv2] android/avrcp: Decouple AVRCP logic from btio
From: Andrei Emeltchenko @ 2014-02-04 11:56 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391418977-24811-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

On Mon, Feb 03, 2014 at 11:16:17AM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> The patch makes AVRCP to be channel-agnostic so that it might be used in
> unit tests. The idea is that all AVRCP logic would come to avrcp-lib and
> channel stuff got to avrcp.

ping

> ---
>  android/Android.mk  |  1 +
>  android/Makefile.am |  1 +
>  android/avrcp-lib.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  android/avrcp-lib.h | 32 ++++++++++++++++++++
>  android/avrcp.c     | 44 ++-------------------------
>  5 files changed, 122 insertions(+), 41 deletions(-)
>  create mode 100644 android/avrcp-lib.c
>  create mode 100644 android/avrcp-lib.h
> 
> diff --git a/android/Android.mk b/android/Android.mk
> index 09ed32d..2772b58 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -30,6 +30,7 @@ LOCAL_SRC_FILES := \
>  	bluez/android/avdtp.c \
>  	bluez/android/a2dp.c \
>  	bluez/android/avctp.c \
> +	bluez/android/avrcp-lib.c \
>  	bluez/android/avrcp.c \
>  	bluez/android/pan.c \
>  	bluez/src/log.c \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index e065c0c..29cbf79 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -34,6 +34,7 @@ android_bluetoothd_SOURCES = android/main.c \
>  				android/avdtp.h android/avdtp.c \
>  				android/a2dp.h android/a2dp.c \
>  				android/avctp.h android/avctp.c \
> +				android/avrcp-lib.h android/avrcp-lib.c \
>  				android/avrcp.h android/avrcp.c \
>  				android/socket.h android/socket.c \
>  				android/pan.h android/pan.c \
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> new file mode 100644
> index 0000000..a4bfe6d
> --- /dev/null
> +++ b/android/avrcp-lib.c
> @@ -0,0 +1,85 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> + *
> + *
> + *  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 <stdbool.h>
> +#include <glib.h>
> +
> +#include "lib/bluetooth.h"
> +
> +#include "src/log.h"
> +
> +#include "avctp.h"
> +#include "avrcp-lib.h"
> +
> +static GSList *devices = NULL;
> +
> +void avrcp_device_free(void *data)
> +{
> +	struct avrcp_device *dev = data;
> +
> +	if (dev->session)
> +		avctp_shutdown(dev->session);
> +
> +	devices = g_slist_remove(devices, dev);
> +	g_free(dev);
> +}
> +
> +void avrcp_free_all(void)
> +{
> +	g_slist_free_full(devices, avrcp_device_free);
> +	devices = NULL;
> +}
> +
> +struct avrcp_device *avrcp_device_new(const bdaddr_t *dst)
> +{
> +	struct avrcp_device *dev;
> +
> +	dev = g_new0(struct avrcp_device, 1);
> +	bacpy(&dev->dst, dst);
> +	devices = g_slist_prepend(devices, dev);
> +
> +	return dev;
> +}
> +
> +static int device_cmp(gconstpointer s, gconstpointer user_data)
> +{
> +	const struct avrcp_device *dev = s;
> +	const bdaddr_t *dst = user_data;
> +
> +	return bacmp(&dev->dst, dst);
> +}
> +
> +struct avrcp_device *avrcp_find(bdaddr_t *dst)
> +{
> +	GSList *l;
> +
> +	l = g_slist_find_custom(devices, dst, device_cmp);
> +	if (!l)
> +		return NULL;
> +
> +	return l->data;
> +}
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> new file mode 100644
> index 0000000..a7213fb
> --- /dev/null
> +++ b/android/avrcp-lib.h
> @@ -0,0 +1,32 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2014  Intel Corporation. All rights reserved.
> + *
> + *
> + *  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
> + *
> + */
> +
> +struct avrcp_device {
> +	bdaddr_t	dst;
> +	struct avctp	*session;
> +};
> +
> +struct avrcp_device *avrcp_device_new(const bdaddr_t *dst);
> +void avrcp_device_free(void *data);
> +void avrcp_free_all(void);
> +struct avrcp_device *avrcp_find(bdaddr_t *dst);
> diff --git a/android/avrcp.c b/android/avrcp.c
> index ef833df..ff923cb 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -38,6 +38,7 @@
>  #include "hal-msg.h"
>  #include "ipc.h"
>  #include "avctp.h"
> +#include "avrcp-lib.h"
>  
>  #define L2CAP_PSM_AVCTP 0x17
>  
> @@ -48,14 +49,8 @@
>  
>  static bdaddr_t adapter_addr;
>  static uint32_t record_id = 0;
> -static GSList *devices = NULL;
>  static GIOChannel *server = NULL;
>  
> -struct avrcp_device {
> -	bdaddr_t	dst;
> -	struct avctp	*session;
> -};
> -
>  static const struct ipc_handler cmd_handlers[] = {
>  };
>  
> @@ -127,36 +122,6 @@ static sdp_record_t *avrcp_record(void)
>  	return record;
>  }
>  
> -static void avrcp_device_free(void *data)
> -{
> -	struct avrcp_device *dev = data;
> -
> -	if (dev->session)
> -		avctp_shutdown(dev->session);
> -
> -	devices = g_slist_remove(devices, dev);
> -	g_free(dev);
> -}
> -
> -static struct avrcp_device *avrcp_device_new(const bdaddr_t *dst)
> -{
> -	struct avrcp_device *dev;
> -
> -	dev = g_new0(struct avrcp_device, 1);
> -	bacpy(&dev->dst, dst);
> -	devices = g_slist_prepend(devices, dev);
> -
> -	return dev;
> -}
> -
> -static int device_cmp(gconstpointer s, gconstpointer user_data)
> -{
> -	const struct avrcp_device *dev = s;
> -	const bdaddr_t *dst = user_data;
> -
> -	return bacmp(&dev->dst, dst);
> -}
> -
>  static void disconnect_cb(void *data)
>  {
>  	struct avrcp_device *dev = data;
> @@ -175,7 +140,6 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>  	char address[18];
>  	uint16_t imtu, omtu;
>  	GError *gerr = NULL;
> -	GSList *l;
>  	int fd;
>  
>  	if (err) {
> @@ -199,8 +163,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
>  	ba2str(&dst, address);
>  	DBG("Incoming connection from %s", address);
>  
> -	l = g_slist_find_custom(devices, &dst, device_cmp);
> -	if (l) {
> +	if (avrcp_find(&dst)) {
>  		error("Unexpected connection");
>  		return;
>  	}
> @@ -274,8 +237,7 @@ void bt_avrcp_unregister(void)
>  {
>  	DBG("");
>  
> -	g_slist_free_full(devices, avrcp_device_free);
> -	devices = NULL;
> +	avrcp_free_all();
>  
>  	ipc_unregister(HAL_SERVICE_ID_AVRCP);
>  
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/3] avrcp: Avoid unneeded calculation
From: Andrei Emeltchenko @ 2014-02-04 11:55 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391098376-26834-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

On Thu, Jan 30, 2014 at 06:12:54PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> There no need to calculate those values

ping

> ---
>  profiles/audio/avrcp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index fa5adab..2e1a940 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -3177,7 +3177,7 @@ static void avrcp_register_notification(struct avrcp *session, uint8_t event)
>  	pdu->params[0] = event;
>  	pdu->params_len = htons(AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH);
>  
> -	length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> +	length = AVRCP_HEADER_LENGTH + AVRCP_REGISTER_NOTIFICATION_PARAM_LENGTH;
>  
>  	avctp_send_vendordep_req(session->conn, AVC_CTYPE_NOTIFY,
>  					AVC_SUBUNIT_PANEL, buf, length,
> @@ -3250,7 +3250,7 @@ static void avrcp_get_capabilities(struct avrcp *session)
>  	pdu->params[0] = CAP_EVENTS_SUPPORTED;
>  	pdu->params_len = htons(AVRCP_GET_CAPABILITIES_PARAM_LENGTH);
>  
> -	length = AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
> +	length = AVRCP_HEADER_LENGTH + AVRCP_GET_CAPABILITIES_PARAM_LENGTH;
>  
>  	avctp_send_vendordep_req(session->conn, AVC_CTYPE_STATUS,
>  					AVC_SUBUNIT_PANEL, buf, length,
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] unit/avctp: Add connection establishment dummy tests
From: Andrei Emeltchenko @ 2014-02-04 11:53 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1391430198-26823-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

On Mon, Feb 03, 2014 at 02:23:18PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

ping

> 
> Add some tests checking that L2CAP connection is established, so they
> are basically dummy tests.
> ---
>  unit/test-avctp.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/unit/test-avctp.c b/unit/test-avctp.c
> index 041e0c0..c0d16a4 100644
> --- a/unit/test-avctp.c
> +++ b/unit/test-avctp.c
> @@ -220,10 +220,8 @@ static struct context *create_context(uint16_t version, gconstpointer data)
>  	return context;
>  }
>  
> -static void execute_context(struct context *context)
> +static void destroy_context(struct context *context)
>  {
> -	g_main_loop_run(context->main_loop);
> -
>  	if (context->source > 0)
>  		g_source_remove(context->source);
>  
> @@ -235,6 +233,13 @@ static void execute_context(struct context *context)
>  	g_free(context);
>  }
>  
> +static void execute_context(struct context *context)
> +{
> +	g_main_loop_run(context->main_loop);
> +
> +	destroy_context(context);
> +}
> +
>  static void test_client(gconstpointer data)
>  {
>  	struct context *context = create_context(0x0100, data);
> @@ -253,6 +258,13 @@ static void test_server(gconstpointer data)
>  	execute_context(context);
>  }
>  
> +static void test_dummy(gconstpointer data)
> +{
> +	struct context *context = create_context(0x0100, data);
> +
> +	destroy_context(context);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	g_test_init(&argc, &argv, NULL);
> @@ -260,6 +272,18 @@ int main(int argc, char *argv[])
>  	if (g_test_verbose())
>  		__btd_log_init("*", 0);
>  
> +	/* Connection Channel Management tests */
> +
> +	/*
> +	 * Tests are checking that IUT is able to request establishing
> +	 * channels, since we already have connection through socketpair
> +	 * the tests are dummy.
> +	 */
> +	define_test("/TP/CCM/BV-01-C", test_dummy, raw_pdu(0x00));
> +	define_test("/TP/CCM/BV-02-C", test_dummy, raw_pdu(0x00));
> +	define_test("/TP/CCM/BV-03-C", test_dummy, raw_pdu(0x00));
> +	define_test("/TP/CCM/BV-04-C", test_dummy, raw_pdu(0x00));
> +
>  	define_test("/TP/NFR/BV-01-C", test_client,
>  				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00));
>  
> -- 
> 1.8.3.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] android/hal-audio: Add simple downmix to mono
From: Szymon Janc @ 2014-02-04 11:49 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1391513842-9548-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Tuesday 04 of February 2014 12:37:22 Andrzej Kaczmarek wrote:
> This patch adds simple downmix support from stereo to mono in order to
> support mono channel mode as it's mandatory for SBC codec. It uses
> simple (L+R)/2 calculation which should be good enough.
> ---
>  android/hal-audio.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index efdf823..b5b75d3 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -39,6 +39,8 @@
>  
>  #define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
>  
> +#define FIXED_BUFFER_SIZE (20 * 512)
> +
>  #define MAX_FRAMES_IN_PAYLOAD 15
>  
>  static const uint8_t a2dp_src_uuid[] = {
> @@ -220,6 +222,8 @@ struct a2dp_stream_out {
>  	struct audio_endpoint *ep;
>  	enum a2dp_state_t audio_state;
>  	struct audio_input_config cfg;
> +
> +	uint8_t *downmix_buf;
>  };
>  
>  struct a2dp_audio_dev {
> @@ -230,7 +234,8 @@ struct a2dp_audio_dev {
>  static const a2dp_sbc_t sbc_presets[] = {
>  	{
>  		.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
> -		.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
> +		.channel_mode = SBC_CHANNEL_MODE_MONO |
> +				SBC_CHANNEL_MODE_DUAL_CHANNEL |
>  				SBC_CHANNEL_MODE_STEREO |
>  				SBC_CHANNEL_MODE_JOINT_STEREO,
>  		.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
> @@ -826,6 +831,19 @@ static void unregister_endpoints(void)
>  	}
>  }
>  
> +static void prepare_downmix_buf(struct a2dp_stream_out *out,
> +					const uint8_t *buffer, size_t bytes)

I'd name it something like downmix_to_mono() or mono_downmix().

> +{
> +	size_t i;
> +	int16_t *input = (int16_t *) buffer;
> +	int16_t *output = (int16_t *) out->downmix_buf;

This might cause unaligned memory access.

> +
> +	for (i = 0; i < bytes / 2; i++) {
> +		int sum = ((int) input[i * 2] + input[i * 2 + 1]);
> +		output[i] = sum >> 1;

I'd just divide by 2 here.

> +	}
> +}
> +
>  static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>  								size_t bytes)
>  {
> @@ -853,6 +871,18 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>  		return -1;
>  	}
>  
> +	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
> +		if (!out->downmix_buf) {
> +			error("audio: downmix buffer not initialized");
> +			return -1;
> +		}
> +
> +		prepare_downmix_buf(out, buffer, bytes);
> +
> +		return out->ep->codec->write_data(out->ep->codec_data,
> +				out->downmix_buf, bytes / 2, out->ep->fd) * 2;
> +	}
> +
>  	return out->ep->codec->write_data(out->ep->codec_data, buffer,
>  							bytes, out->ep->fd);
>  }
> @@ -890,16 +920,18 @@ static size_t out_get_buffer_size(const struct audio_stream *stream)
>  	 * use magic value here and out_write code takes care of splitting
>  	 * input buffer into multiple media packets.
>  	 */
> -	return 20 * 512;
> +	return FIXED_BUFFER_SIZE;
>  }
>  
>  static uint32_t out_get_channels(const struct audio_stream *stream)
>  {
> -	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
> -
>  	DBG("");
>  
> -	return out->cfg.channels;
> +	/* AudioFlinger can only provide stereo stream, so we return it here and
> +	 * later we'll downmix this to mono in case codec requires it
> +	 */
> +
> +	return AUDIO_CHANNEL_OUT_STEREO;
>  }
>  
>  static audio_format_t out_get_format(const struct audio_stream *stream)
> @@ -1212,6 +1244,12 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>  
>  	free(preset);
>  
> +	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
> +		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
> +		if (!out->downmix_buf)
> +			goto fail;
> +	}
> +
>  	*stream_out = &out->stream;
>  	a2dp_dev->out = out;
>  
> @@ -1230,6 +1268,7 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
>  					struct audio_stream_out *stream)
>  {
>  	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
> +	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
>  	struct audio_endpoint *ep = a2dp_dev->out->ep;
>  
>  	DBG("");
> @@ -1243,6 +1282,8 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
>  	ep->codec->cleanup(ep->codec_data);
>  	ep->codec_data = NULL;
>  
> +	free(out->downmix_buf);
> +
>  	free(stream);
>  	a2dp_dev->out = NULL;
>  }
> 

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* [PATCH] unit/avctp: Add test TP/NFR/BV-03-C
From: Andrei Emeltchenko @ 2014-02-04 11:44 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

The test verifies that IUT (TG) correctly reports the parameters of the
incoming command in the handler callback.
---
 unit/test-avctp.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/unit/test-avctp.c b/unit/test-avctp.c
index c0d16a4..6d7e34a 100644
--- a/unit/test-avctp.c
+++ b/unit/test-avctp.c
@@ -249,10 +249,35 @@ static void test_client(gconstpointer data)
 	execute_context(context);
 }
 
+static size_t handler(struct avctp *session,
+					uint8_t transaction, uint8_t *code,
+					uint8_t *subunit, uint8_t *operands,
+					size_t operand_count, void *user_data)
+{
+	DBG("transaction %d code %d subunit %d operand_count %zu",
+		transaction, *code, *subunit, operand_count);
+
+	g_assert_cmpint(transaction, ==, 0);
+	g_assert_cmpint(*code, ==, 0);
+	g_assert_cmpint(*subunit, ==, 0);
+	g_assert_cmpint(operand_count, ==, 0);
+
+	return operand_count;
+}
+
 static void test_server(gconstpointer data)
 {
 	struct context *context = create_context(0x0100, data);
 
+	if (g_str_equal(context->data->test_name, "/TP/NFR/BV-03-C")) {
+		int ret;
+
+		ret = avctp_register_pdu_handler(context->session, 0x00,
+								handler, NULL);
+		DBG("ret %d", ret);
+		g_assert_cmpint(ret, !=, 0);
+	}
+
 	g_idle_add(send_pdu, context);
 
 	execute_context(context);
@@ -284,6 +309,8 @@ int main(int argc, char *argv[])
 	define_test("/TP/CCM/BV-03-C", test_dummy, raw_pdu(0x00));
 	define_test("/TP/CCM/BV-04-C", test_dummy, raw_pdu(0x00));
 
+	/* Non-Fragmented Messages tests */
+
 	define_test("/TP/NFR/BV-01-C", test_client,
 				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00));
 
@@ -291,6 +318,10 @@ int main(int argc, char *argv[])
 				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
 				raw_pdu(0x02, 0x11, 0x0e, 0x0a, 0x00, 0x00));
 
+	define_test("/TP/NFR/BV-03-C", test_server,
+				raw_pdu(0x00, 0x11, 0x0e, 0x00, 0x00, 0x00),
+				raw_pdu(0x02, 0x11, 0x0e, 0x00, 0x00, 0x00));
+
 	define_test("/TP/NFR/BI-01-C", test_server,
 				raw_pdu(0x00, 0xff, 0xff, 0x00, 0x00, 0x00),
 				raw_pdu(0x03, 0xff, 0xff));
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/hal-audio: Add simple downmix to mono
From: Andrzej Kaczmarek @ 2014-02-04 11:37 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek

This patch adds simple downmix support from stereo to mono in order to
support mono channel mode as it's mandatory for SBC codec. It uses
simple (L+R)/2 calculation which should be good enough.
---
 android/hal-audio.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index efdf823..b5b75d3 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -39,6 +39,8 @@
 
 #define FIXED_A2DP_PLAYBACK_LATENCY_MS 25
 
+#define FIXED_BUFFER_SIZE (20 * 512)
+
 #define MAX_FRAMES_IN_PAYLOAD 15
 
 static const uint8_t a2dp_src_uuid[] = {
@@ -220,6 +222,8 @@ struct a2dp_stream_out {
 	struct audio_endpoint *ep;
 	enum a2dp_state_t audio_state;
 	struct audio_input_config cfg;
+
+	uint8_t *downmix_buf;
 };
 
 struct a2dp_audio_dev {
@@ -230,7 +234,8 @@ struct a2dp_audio_dev {
 static const a2dp_sbc_t sbc_presets[] = {
 	{
 		.frequency = SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000,
-		.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL |
+		.channel_mode = SBC_CHANNEL_MODE_MONO |
+				SBC_CHANNEL_MODE_DUAL_CHANNEL |
 				SBC_CHANNEL_MODE_STEREO |
 				SBC_CHANNEL_MODE_JOINT_STEREO,
 		.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8,
@@ -826,6 +831,19 @@ static void unregister_endpoints(void)
 	}
 }
 
+static void prepare_downmix_buf(struct a2dp_stream_out *out,
+					const uint8_t *buffer, size_t bytes)
+{
+	size_t i;
+	int16_t *input = (int16_t *) buffer;
+	int16_t *output = (int16_t *) out->downmix_buf;
+
+	for (i = 0; i < bytes / 2; i++) {
+		int sum = ((int) input[i * 2] + input[i * 2 + 1]);
+		output[i] = sum >> 1;
+	}
+}
+
 static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 								size_t bytes)
 {
@@ -853,6 +871,18 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		return -1;
 	}
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		if (!out->downmix_buf) {
+			error("audio: downmix buffer not initialized");
+			return -1;
+		}
+
+		prepare_downmix_buf(out, buffer, bytes);
+
+		return out->ep->codec->write_data(out->ep->codec_data,
+				out->downmix_buf, bytes / 2, out->ep->fd) * 2;
+	}
+
 	return out->ep->codec->write_data(out->ep->codec_data, buffer,
 							bytes, out->ep->fd);
 }
@@ -890,16 +920,18 @@ static size_t out_get_buffer_size(const struct audio_stream *stream)
 	 * use magic value here and out_write code takes care of splitting
 	 * input buffer into multiple media packets.
 	 */
-	return 20 * 512;
+	return FIXED_BUFFER_SIZE;
 }
 
 static uint32_t out_get_channels(const struct audio_stream *stream)
 {
-	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
-
 	DBG("");
 
-	return out->cfg.channels;
+	/* AudioFlinger can only provide stereo stream, so we return it here and
+	 * later we'll downmix this to mono in case codec requires it
+	 */
+
+	return AUDIO_CHANNEL_OUT_STEREO;
 }
 
 static audio_format_t out_get_format(const struct audio_stream *stream)
@@ -1212,6 +1244,12 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 
 	free(preset);
 
+	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
+		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
+		if (!out->downmix_buf)
+			goto fail;
+	}
+
 	*stream_out = &out->stream;
 	a2dp_dev->out = out;
 
@@ -1230,6 +1268,7 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 					struct audio_stream_out *stream)
 {
 	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
+	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
 	struct audio_endpoint *ep = a2dp_dev->out->ep;
 
 	DBG("");
@@ -1243,6 +1282,8 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 	ep->codec->cleanup(ep->codec_data);
 	ep->codec_data = NULL;
 
+	free(out->downmix_buf);
+
 	free(stream);
 	a2dp_dev->out = NULL;
 }
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH] avctp: Fix memory leak
From: Andrei Emeltchenko @ 2014-02-04  9:50 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Unregister pdu hanlders in avctp_disconnected()
---
 profiles/audio/avctp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/profiles/audio/avctp.c b/profiles/audio/avctp.c
index a77aa9d..a28cf71 100644
--- a/profiles/audio/avctp.c
+++ b/profiles/audio/avctp.c
@@ -496,6 +496,15 @@ static void avctp_disconnected(struct avctp *session)
 	if (!session)
 		return;
 
+	if (session->passthrough_id > 0)
+		avctp_unregister_pdu_handler(session->passthrough_id);
+
+	if (session->unit_id > 0)
+		avctp_unregister_pdu_handler(session->unit_id);
+
+	if (session->subunit_id > 0)
+		avctp_unregister_pdu_handler(session->subunit_id);
+
 	if (session->browsing)
 		avctp_channel_destroy(session->browsing);
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/avctp: Fix memory leak: unregister pdu handler
From: Andrei Emeltchenko @ 2014-02-04  9:28 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Unregister pdu handlers on shutdown.
---
 android/avctp.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/android/avctp.c b/android/avctp.c
index 8f35403..8681cb0 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -1462,6 +1462,15 @@ void avctp_shutdown(struct avctp *session)
 	if (session->browsing)
 		avctp_channel_destroy(session->browsing);
 
+	if (session->passthrough_id > 0)
+		avctp_unregister_pdu_handler(session, session->passthrough_id);
+
+	if (session->unit_id > 0)
+		avctp_unregister_pdu_handler(session, session->unit_id);
+
+	if (session->subunit_id > 0)
+		avctp_unregister_pdu_handler(session, session->subunit_id);
+
 	if (session->control)
 		avctp_channel_destroy(session->control);
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/pts: Update PICS and PTS for AVCTP
From: Sebastian Chlad @ 2014-02-04  9:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sebastian Chlad

Since we do not support AVCTP fragmentation for now we shell set PICS
settings for AVCTP accordingly as well as set respective PTS test cases
as N/A
---
 android/pics-avctp.txt | 2 +-
 android/pts-avctp.txt  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/android/pics-avctp.txt b/android/pics-avctp.txt
index 939ffdb..269bf08 100644
--- a/android/pics-avctp.txt
+++ b/android/pics-avctp.txt
@@ -58,7 +58,7 @@ TSPC_AVCTP_2_14	False		Support for multiple AVCTP channel establishment
 -------------------------------------------------------------------------------
 Parameter Name	Selected	Description
 -------------------------------------------------------------------------------
-TSPC_AVCTP_3_1	True (*)	Message fragmentation (O)
+TSPC_AVCTP_3_1	False		Message fragmentation (O)
 TSPC_AVCTP_3_2	True		Transaction label management (M)
 TSPC_AVCTP_3_3	True		Packet type field management (M)
 TSPC_AVCTP_3_4	True		Message type field management (M)
diff --git a/android/pts-avctp.txt b/android/pts-avctp.txt
index 4090ec0..72a6373 100644
--- a/android/pts-avctp.txt
+++ b/android/pts-avctp.txt
@@ -37,6 +37,6 @@ TC_TG_CCM_BV_04_C	PASS	avtest --device hci0 --avctp --send x <bdaddr>
 TC_TG_NFR_BV_02_C	PASS	avtest --device hci0 --avctp --send x <bdaddr>
 TC_TG_NFR_BV_03_C	PASS	avtest --device hci0 --avctp --send x <bdaddr>
 TC_TG_NFR_BI_01_C	PASS
-TC_TG_FRA_BV_02_C	FAIL
-TC_TG_FRA_BV_03_C	INC
+TC_TG_FRA_BV_02_C	N/A	Fragmentation not supported
+TC_TG_FRA_BV_03_C	N/A	Fragmentation not supported
 -------------------------------------------------------------------------------
-- 
1.8.3.2

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related

* Re: How do you install bluez for development?
From: Alejandro Exojo @ 2014-02-04  7:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <20140203183606.GA12511@molly>

2014-02-03 Vinicius Costa Gomes <vcgomes@gmail.com>:
>> Feb 03 17:14:38 PC-MW03 systemd[1]: Starting Bluetooth service...
>> Feb 03 17:14:38 PC-MW03 bluetoothd[22571]: Bluetooth daemon 5.14
>> Feb 03 17:14:38 PC-MW03 systemd[1]: Started Bluetooth service.
>> Feb 03 17:14:38 PC-MW03 systemd[1]: Starting Bluetooth.
>> Feb 03 17:14:38 PC-MW03 systemd[1]: Reached target Bluetooth.
>> Feb 03 17:14:38 PC-MW03 bluetoothd[22571]: Failed to access management interface
>
> Two probable causes, your kernel is older than 3.4, or the user that is running
> bluetoothd doesn't have the CAP_NET_ADMIN capability.

True! I was running 3.2. I've installed a more recent one, and I got
it working. Thank you very much, I would not have thought of the
kernel at all.

How come that at least 3.4 is needed?

Thank you again.

Cheers.
-- 
Alejandro Exojo Piqueras

ModpoW, S.L.
Technova LaSalle | Sant Joan de la Salle 42 | 08022 Barcelona | www.modpow.es

^ permalink raw reply

* Re: [RFC v7 08/11] Bluetooth: Temporarily stop background scanning on discovery
From: Marcel Holtmann @ 2014-02-04  4:25 UTC (permalink / raw)
  To: Andre Guedes; +Cc: BlueZ development
In-Reply-To: <1391446588-18729-8-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> If the user sends a mgmt start discovery command while the background
> scanning is running, we should temporarily stop it. Once the discovery
> finishes, we start the background scanning again.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_core.c |  2 ++
> net/bluetooth/mgmt.c     | 12 ++++++++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 388a453..222bd07 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1609,6 +1609,8 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
> 
> 	switch (state) {
> 	case DISCOVERY_STOPPED:
> +		hci_update_background_scan(hdev);
> +
> 		if (hdev->discovery.state != DISCOVERY_STARTING)
> 			mgmt_discovering(hdev, 0);
> 		break;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index ce7ef33..83af8de 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3319,11 +3319,15 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
> 			goto failed;
> 		}
> 
> +		/* If controller is scanning, it means the background scanning
> +		 * is running. Thus, we should temporarily stop it in order to
> +		 * set the discovery scanning parameters.
> +		 */
> 		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
> -			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
> -					 MGMT_STATUS_BUSY);
> -			mgmt_pending_remove(cmd);
> -			goto failed;
> +			memset(&enable_cp, 0, sizeof(enable_cp));
> +			enable_cp.enable = LE_SCAN_DISABLE;
> +			hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE,
> +				    sizeof(enable_cp), &enable_cp);
> 		}

is the start_discovery protected enough by itself so that we do not accidentally stop some discovery that got triggered which is actually not a background scan.

Regards

Marcel


^ permalink raw reply

* Re: [RFC v7 07/11] Bluetooth: Re-enable background scan in case of error
From: Marcel Holtmann @ 2014-02-04  4:10 UTC (permalink / raw)
  To: Andre Guedes; +Cc: BlueZ development
In-Reply-To: <1391446588-18729-7-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> Since we temporarily stop the background scanning in favor of
> connection, we should re-enable it in case something goes wrong
> with connection establishment. So this patch adds a hci_update_
> background_scan() call in fail_conn_attempt() and hci_le_conn_
> complete_evt() error flow.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c  | 2 ++
> net/bluetooth/hci_event.c | 1 +
> 2 files changed, 3 insertions(+)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 127465f..2ef29c7 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -527,6 +527,8 @@ static void le_conn_failed(struct hci_conn *conn, u8 status)
> 	hci_proto_connect_cfm(conn, status);
> 
> 	hci_conn_del(conn);
> +
> +	hci_update_background_scan(hdev);
> }
> 
> static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ef95030..8de51b1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -3606,6 +3606,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
> 		hci_proto_connect_cfm(conn, ev->status);
> 		conn->state = BT_CLOSED;
> 		hci_conn_del(conn);
> +		hci_update_background_scan(hdev);
> 		goto unlock;

please fold this patch into the one that does the disabling.

Regards

Marcel


^ permalink raw reply

* Re: [RFC v7 03/11] Bluetooth: Stop scanning on LE connection
From: Marcel Holtmann @ 2014-02-04  4:08 UTC (permalink / raw)
  To: Andre Guedes; +Cc: BlueZ development
In-Reply-To: <1391446588-18729-3-git-send-email-andre.guedes@openbossa.org>

Hi Andrei,

> Some LE controllers don't support scanning and creating a connection
> at the same time. So we should always stop scanning in order to
> establish the connection.
> 
> Since we may prematurely stop the discovery procedure in favor of
> the connection establishment, we should also cancel hdev->le_scan_
> disable delayed work and set the discovery state to DISCOVERY_STOPPED.
> 
> This change does a small improvement since it is not mandatory the
> user stops scanning before connecting anymore. Moreover, this change
> is required by upcoming LE auto connection mechanism in order to work
> properly with controllers that don't support background scanning and
> connection establishment at the same time.
> 
> In future, we might want to do a small optimization by checking if
> controller is able to scan and connect at the same time. For now,
> we want the simplest approach so we always stop scanning (even if
> the controller is able to carry out both operations).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_conn.c    | 84 +++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 83 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 352d3d7..a0d5262 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -352,6 +352,7 @@ enum {
> 
> /* ---- HCI Error Codes ---- */
> #define HCI_ERROR_AUTH_FAILURE		0x05
> +#define HCI_ERROR_MEMORY_EXCEEDED	0x07
> #define HCI_ERROR_CONNECTION_TIMEOUT	0x08
> #define HCI_ERROR_REJ_BAD_ADDR		0x0f
> #define HCI_ERROR_REMOTE_USER_TERM	0x13
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 6797292..63c1e4f 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -583,11 +583,71 @@ static int hci_create_le_conn(struct hci_conn *conn)
> 	return 0;
> }
> 
> +static void create_le_conn_req(struct hci_request *req, struct hci_conn *conn)
> +{
> +	struct hci_cp_le_create_conn cp;
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	memset(&cp, 0, sizeof(cp));
> +	cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
> +	cp.scan_window = cpu_to_le16(hdev->le_scan_window);
> +	bacpy(&cp.peer_addr, &conn->dst);
> +	cp.peer_addr_type = conn->dst_type;
> +	cp.own_address_type = conn->src_type;
> +	cp.conn_interval_min = cpu_to_le16(conn->le_conn_min_interval);
> +	cp.conn_interval_max = cpu_to_le16(conn->le_conn_max_interval);
> +	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
> +	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
> +	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
> +
> +	hci_req_add(req, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
> +}
> +
> +static void stop_scan_complete(struct hci_dev *hdev, u8 status)
> +{
> +	struct hci_request req;
> +	struct hci_conn *conn;
> +	int err;
> +
> +	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> +	if (!conn)
> +		return;
> +
> +	if (status) {
> +		BT_DBG("HCI request failed to stop scanning: status 0x%2.2x",
> +		       status);
> +
> +		hci_dev_lock(hdev);
> +		le_conn_failed(conn, status);
> +		hci_dev_unlock(hdev);
> +		return;
> +	}
> +
> +	/* Since we may have prematurely stopped discovery procedure, we should
> +	 * update discovery state.
> +	 */
> +	cancel_delayed_work(&hdev->le_scan_disable);
> +	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> +
> +	hci_req_init(&req, hdev);
> +
> +	create_le_conn_req(&req, conn);
> +
> +	err = hci_req_run(&req, create_le_conn_complete);
> +	if (err) {
> +		hci_dev_lock(hdev);
> +		le_conn_failed(conn, HCI_ERROR_MEMORY_EXCEEDED);
> +		hci_dev_unlock(hdev);
> +		return;
> +	}
> +}
> +
> static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> 				    u8 dst_type, u8 sec_level, u8 auth_type)
> {
> 	struct hci_conn_params *params;
> 	struct hci_conn *conn;
> +	struct hci_request req;
> 	int err;
> 
> 	if (test_bit(HCI_ADVERTISING, &hdev->flags))
> @@ -643,9 +703,29 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
> 		conn->le_conn_max_interval = hdev->le_conn_max_interval;
> 	}
> 
> -	err = hci_create_le_conn(conn);
> -	if (err)
> +	hci_req_init(&req, hdev);
> +
> +	/* If controller is scanning, we stop it since some controllers are
> +	 * not able to scan and connect at the same time.
> +	 */
> +	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
> +		struct hci_cp_le_set_scan_enable cp;
> +
> +		memset(&cp, 0, sizeof(cp));
> +		cp.enable = LE_SCAN_DISABLE;
> +		hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
> +
> +		err = hci_req_run(&req, stop_scan_complete);
> +	} else {
> +		create_le_conn_req(&req, conn);
> +
> +		err = hci_req_run(&req, create_le_conn_complete);
> +	}

so I wonder why we not just add the disabling of the scan to the request right here. Our request queue will always fail on the first request and then do not execute the other ones. Especially when you have to send two requests this kind of request queue behavior is pretty nice.

I also think that this is currently a bit racy in error handling path. So you might want to check this in all cases. One other thing to consider is might be introducing helper for adding certain commands like disable LE scan if they are used more than once.

Regards

Marcel


^ permalink raw reply

* Re: [RFC v7 02/11] Bluetooth: Use connection parameters if any
From: Marcel Holtmann @ 2014-02-04  4:02 UTC (permalink / raw)
  To: Andre Guedes; +Cc: BlueZ development
In-Reply-To: <1391446588-18729-2-git-send-email-andre.guedes@openbossa.org>

Hi Andre,

> This patch changes hci_connect_le() so it uses the connection
> parameters specified for the certain device. If no parameters
> were configured, we use the default values.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_conn.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)

I know applied the first 2 patches in this series since they were nicely self-contained.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH v2 0/6] android/hal-audio: Fixes
From: Szymon Janc @ 2014-02-03 21:16 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1391446549-31455-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Monday 03 February 2014 17:55:43 Andrzej Kaczmarek wrote:
> v1 -> v2
> fixed build error introduced during rebase
> 
> 
> Andrzej Kaczmarek (6):
>   android/hal-audio: Remove unsupported mono channel mode
>   android/hal-audio: Be more verbose on SBC errors
>   android/hal-audio: Print calculated SBC parameters
>   android/hal-audio: Fix audio with large omtu value
>   android/hal-audio: Fix RTP sequence numbers
>   android/hal-audio: Add RTP timestamps
> 
>  android/hal-audio.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)

All patches in this set have been applied, thanks a lot.

-- 
Szymon K. Janc
szymon.janc@gmail.com

^ 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