Linux bluetooth development
 help / color / mirror / Atom feed
* a2dp codec selection
From: tropp @ 2014-02-28 16:45 UTC (permalink / raw)
  To: linux-bluetooth

hi, 
i am using a2dp to connect my bluetooth headphones to my pc. the headphones are supporting the mandatory sbc codec and additionally the aac and aptX codecs. the following defines in profiles/audio/a2dp-codecs.h let me assume that at least sbc, mp3, aac and atrac are supported audio codecs by bluez: 
 
#define A2DP_CODEC_SBC         0x00 
#define A2DP_CODEC_MPEG12    0x01 
#define A2DP_CODEC_MPEG24    0x02 
#define A2DP_CODEC_ATRAC      0x03 
#define A2DP_CODEC_VENDOR    0xFF 
 
i want to use the aac codec for the best audio quality. therefore i added the following lines to my /etc/bluetooth/audio.conf 
 
[A2DP] 
SBCSources=0 
MPEG24Sources=1
 
is there any possibility to check which codec is currently used if a a2dp link is established? i want to confirm that aac is really used, or if i am using sbc, which bitrate is selected.

i am using ubuntu 12.10.

regards, tropp

^ permalink raw reply

* Re: [bluez-5.14] connect fails with 'org.bluez.Error.NotAvailable'
From: Tobias Jakobi @ 2014-02-28 16:57 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_P9FD+NwSAfG=1bWg0Ep2Ku7eTqwpLN2aX6p5-+rc=3sA@mail.gmail.com>

Anderson Lizardo wrote:
> Looks like you are missing a "power on" on the server side. Notice
> that the client side shows "Powered: yes" on the info command.
> 
> Best Regards,
> 
Hello Anderson!

Sadly this doesn't solve the issue. The BT device on the server side is
already powered on. Otherwise I wouldn't be able to establish the
connection with the Android smartphone in the first place.

With best wishes,
Tobias


^ permalink raw reply

* [PATCH BlueZ v0 0/9] GATT API: Add Register Services Async
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi

This patchset is an extension of "[PATCH BlueZ v7 00/11] GATT API:
External Services". It adds the following features:
  * new gdbus helper to allow notifying the clients when
    GetManagedObjects() reply is received and all proxies reported.
  * reply for RegisterService() after validating the fetched objects
  * adds the service declaration to the local GATT database

Claudio Takahasi (9):
  gdbus: Add g_dbus_client_set_proxies_ready_watch()
  gatt: Add proxy added handler
  gatt: Add proxy removed handler
  gatt: Add GATT service to the local database
  gatt: Make RegisterService() async
  test: Add external service GATT skeleton
  test: Add signal handling for gatt-service
  test: Add registering external service
  bluetooth.conf: Add ObjectManager interface

 .gitignore          |   1 +
 Makefile.tools      |   5 +
 gdbus/client.c      |  18 ++++
 gdbus/gdbus.h       |   4 +
 src/bluetooth.conf  |   1 +
 src/gatt-dbus.c     | 135 +++++++++++++++++++++++++-
 test/gatt-service.c | 267 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 427 insertions(+), 4 deletions(-)
 create mode 100644 test/gatt-service.c

-- 
1.8.3.1


^ permalink raw reply

* [PATCH BlueZ v0 1/9] gdbus: Add g_dbus_client_set_proxies_ready_watch()
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch adds a new gdbus helper to notify the clients that
GetManagedObjects reply was received and the last proxy has been
informed previously by the proxy_added callback.
---
 gdbus/client.c | 18 ++++++++++++++++++
 gdbus/gdbus.h  |  4 ++++
 2 files changed, 22 insertions(+)

diff --git a/gdbus/client.c b/gdbus/client.c
index be8cc29..f479742 100644
--- a/gdbus/client.c
+++ b/gdbus/client.c
@@ -56,6 +56,8 @@ struct GDBusClient {
 	void *signal_data;
 	GDBusProxyFunction proxy_added;
 	GDBusProxyFunction proxy_removed;
+	GDBusClientReadyFunction ready;
+	void *ready_data;
 	GDBusPropertyFunction property_changed;
 	void *user_data;
 	GList *proxy_list;
@@ -982,6 +984,9 @@ static void parse_managed_objects(GDBusClient *client, DBusMessage *msg)
 
 		dbus_message_iter_next(&dict);
 	}
+
+	if (client->ready)
+		client->ready(client->ready_data);
 }
 
 static void get_managed_objects_reply(DBusPendingCall *call, void *user_data)
@@ -1261,3 +1266,16 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
 
 	return TRUE;
 }
+
+gboolean g_dbus_client_set_proxies_ready_watch(GDBusClient *client,
+						GDBusClientReadyFunction ready,
+						void *user_data)
+{
+	if (client == NULL)
+		return FALSE;
+
+	client->ready = ready;
+	client->ready_data = user_data;
+
+	return TRUE;
+}
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 9542109..aa407aa 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -337,6 +337,7 @@ gboolean g_dbus_proxy_method_call(GDBusProxy *proxy, const char *method,
 				GDBusReturnFunction function, void *user_data,
 				GDBusDestroyFunction destroy);
 
+typedef void (* GDBusClientReadyFunction) (void *user_data);
 typedef void (* GDBusProxyFunction) (GDBusProxy *proxy, void *user_data);
 typedef void (* GDBusPropertyFunction) (GDBusProxy *proxy, const char *name,
 					DBusMessageIter *iter, void *user_data);
@@ -365,6 +366,9 @@ gboolean g_dbus_client_set_proxy_handlers(GDBusClient *client,
 					GDBusProxyFunction proxy_removed,
 					GDBusPropertyFunction property_changed,
 					void *user_data);
+gboolean g_dbus_client_set_proxies_ready_watch(GDBusClient *client,
+						GDBusClientReadyFunction ready,
+						void *user_data);
 
 #ifdef __cplusplus
 }
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 2/9] gatt: Add proxy added handler
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch creates a list of GATT objects sorting the entries based on
the object path to allow inserting the attributes following hierarchical
association.
---
 src/gatt-dbus.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index fd614f9..f8486f3 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -42,11 +42,15 @@
 #include "gatt-dbus.h"
 
 #define GATT_MGR_IFACE			"org.bluez.GattManager1"
+#define GATT_SERVICE_IFACE		"org.bluez.GattService1"
+#define GATT_CHR_IFACE			"org.bluez.GattCharacteristic1"
+#define GATT_DESCRIPTOR_IFACE		"org.bluez.GattDescriptor1"
 
 struct external_app {
 	char *owner;
 	char *path;
 	GDBusClient *client;
+	GSList *proxies;
 	unsigned int watch;
 };
 
@@ -75,6 +79,43 @@ static void external_app_watch_destroy(gpointer user_data)
 	g_free(eapp);
 }
 
+static int proxy_path_cmp(gconstpointer a, gconstpointer b)
+{
+	GDBusProxy *proxy1 = (GDBusProxy *) a;
+	GDBusProxy *proxy2 = (GDBusProxy *) b;
+	const char *path1 = g_dbus_proxy_get_path(proxy1);
+	const char *path2 = g_dbus_proxy_get_path(proxy2);
+
+	return g_strcmp0(path1, path2);
+}
+
+static void proxy_added(GDBusProxy *proxy, void *user_data)
+{
+	struct external_app *eapp = user_data;
+	const char *interface, *path;
+
+	interface = g_dbus_proxy_get_interface(proxy);
+	path = g_dbus_proxy_get_path(proxy);
+
+	if (!g_str_has_prefix(path, eapp->path))
+		return;
+
+	if (g_strcmp0(interface, GATT_CHR_IFACE) != 0 &&
+			g_strcmp0(interface, GATT_SERVICE_IFACE) != 0 &&
+			g_strcmp0(interface, GATT_DESCRIPTOR_IFACE) != 0)
+		return;
+
+	DBG("path %s iface %s", path, interface);
+
+	/*
+	 * Object path follows a hierarchical organization. Add the
+	 * proxies sorted by path helps the logic to register the
+	 * object path later.
+	 */
+	eapp->proxies = g_slist_insert_sorted(eapp->proxies, proxy,
+							proxy_path_cmp);
+}
+
 static struct external_app *new_external_app(DBusConnection *conn,
 					const char *sender, const char *path)
 {
@@ -99,6 +140,9 @@ static struct external_app *new_external_app(DBusConnection *conn,
 	eapp->client = client;
 	eapp->path = g_strdup(path);
 
+	g_dbus_client_set_proxy_handlers(client, proxy_added, NULL, NULL,
+								eapp);
+
 	return eapp;
 }
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 3/9] gatt: Add proxy removed handler
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

---
 src/gatt-dbus.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index f8486f3..28d7f78 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -116,6 +116,19 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
 							proxy_path_cmp);
 }
 
+static void proxy_removed(GDBusProxy *proxy, void *user_data)
+{
+	struct external_app *eapp = user_data;
+	const char *interface, *path;
+
+	interface = g_dbus_proxy_get_interface(proxy);
+	path = g_dbus_proxy_get_path(proxy);
+
+	DBG("path %s iface %s", path, interface);
+
+	eapp->proxies = g_slist_remove(eapp->proxies, proxy);
+}
+
 static struct external_app *new_external_app(DBusConnection *conn,
 					const char *sender, const char *path)
 {
@@ -140,8 +153,8 @@ static struct external_app *new_external_app(DBusConnection *conn,
 	eapp->client = client;
 	eapp->path = g_strdup(path);
 
-	g_dbus_client_set_proxy_handlers(client, proxy_added, NULL, NULL,
-								eapp);
+	g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
+								NULL, eapp);
 
 	return eapp;
 }
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 4/9] gatt: Add GATT service to the local database
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch creates GATT service attribute based on its GDBusProxy
object, and inserts the declaration to the local database.
---
 src/gatt-dbus.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 28d7f78..93d5b7c 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -39,6 +39,7 @@
 #include "log.h"
 
 #include "error.h"
+#include "gatt.h"
 #include "gatt-dbus.h"
 
 #define GATT_MGR_IFACE			"org.bluez.GattManager1"
@@ -129,6 +130,56 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
 	eapp->proxies = g_slist_remove(eapp->proxies, proxy);
 }
 
+static int register_external_service(const struct external_app *eapp,
+							GDBusProxy *proxy)
+{
+	DBusMessageIter iter;
+	const char *str, *path, *iface;
+	bt_uuid_t uuid;
+
+	path = g_dbus_proxy_get_path(proxy);
+	iface = g_dbus_proxy_get_interface(proxy);
+	if (g_strcmp0(eapp->path, path) != 0 ||
+			g_strcmp0(iface, GATT_SERVICE_IFACE) != 0)
+		return -EINVAL;
+
+	if (!g_dbus_proxy_get_property(proxy, "UUID", &iter))
+		return -EINVAL;
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return -EINVAL;
+
+	dbus_message_iter_get_basic(&iter, &str);
+
+	if (bt_string_to_uuid(&uuid, str) < 0)
+		return -EINVAL;
+
+	if (btd_gatt_add_service(&uuid) == NULL)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void client_ready(void *user_data)
+{
+	struct external_app *eapp = user_data;
+	GDBusProxy *proxy;
+
+	if (eapp->proxies == NULL)
+		goto fail;
+
+	proxy = eapp->proxies->data;
+	if (register_external_service(eapp, proxy) < 0)
+		goto fail;
+
+	DBG("Added GATT service %s", eapp->path);
+
+	return;
+
+fail:
+	DBG("No proxies: inconsistent service (%s)!", eapp->path);
+}
+
 static struct external_app *new_external_app(DBusConnection *conn,
 					const char *sender, const char *path)
 {
@@ -156,6 +207,8 @@ static struct external_app *new_external_app(DBusConnection *conn,
 	g_dbus_client_set_proxy_handlers(client, proxy_added, proxy_removed,
 								NULL, eapp);
 
+	g_dbus_client_set_proxies_ready_watch(client, client_ready, eapp);
+
 	return eapp;
 }
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 5/9] gatt: Make RegisterService() async
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

GDBus proxy objects consistency should be checked, and attributes
declaration inserted in the local GATT database before replying
the caller.
---
 src/gatt-dbus.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c
index 93d5b7c..b8c7b17 100644
--- a/src/gatt-dbus.c
+++ b/src/gatt-dbus.c
@@ -50,6 +50,7 @@
 struct external_app {
 	char *owner;
 	char *path;
+	DBusMessage *reg;
 	GDBusClient *client;
 	GSList *proxies;
 	unsigned int watch;
@@ -74,6 +75,8 @@ static void external_app_watch_destroy(gpointer user_data)
 	external_apps = g_slist_remove(external_apps, eapp);
 
 	g_dbus_client_unref(eapp->client);
+	if (eapp->reg)
+		dbus_message_unref(eapp->reg);
 
 	g_free(eapp->owner);
 	g_free(eapp->path);
@@ -164,6 +167,8 @@ static void client_ready(void *user_data)
 {
 	struct external_app *eapp = user_data;
 	GDBusProxy *proxy;
+	DBusConnection *conn = btd_get_dbus_connection();
+	DBusMessage *reply;
 
 	if (eapp->proxies == NULL)
 		goto fail;
@@ -174,17 +179,28 @@ static void client_ready(void *user_data)
 
 	DBG("Added GATT service %s", eapp->path);
 
-	return;
+	reply = dbus_message_new_method_return(eapp->reg);
+	goto reply;
 
 fail:
-	DBG("No proxies: inconsistent service (%s)!", eapp->path);
+	error("Could not register external service: %s", eapp->path);
+
+	reply = btd_error_invalid_args(eapp->reg);
+	/* TODO: missing eapp cleanup */
+
+reply:
+	dbus_message_unref(eapp->reg);
+	eapp->reg = NULL;
+
+	g_dbus_send_message(conn, reply);
 }
 
 static struct external_app *new_external_app(DBusConnection *conn,
-					const char *sender, const char *path)
+					DBusMessage *msg, const char *path)
 {
 	struct external_app *eapp;
 	GDBusClient *client;
+	const char *sender = dbus_message_get_sender(msg);
 
 	client = g_dbus_client_new(conn, sender, "/");
 	if (client == NULL)
@@ -201,6 +217,7 @@ static struct external_app *new_external_app(DBusConnection *conn,
 	}
 
 	eapp->owner = g_strdup(sender);
+	eapp->reg = dbus_message_ref(msg);
 	eapp->client = client;
 	eapp->path = g_strdup(path);
 
@@ -230,7 +247,7 @@ static DBusMessage *register_service(DBusConnection *conn,
 	if (g_slist_find_custom(external_apps, path, external_app_path_cmp))
 		return btd_error_already_exists(msg);
 
-	eapp = new_external_app(conn, dbus_message_get_sender(msg), path);
+	eapp = new_external_app(conn, msg, path);
 	if (eapp == NULL)
 		return btd_error_failed(msg, "Not enough resources");
 
@@ -238,7 +255,7 @@ static DBusMessage *register_service(DBusConnection *conn,
 
 	DBG("New app %p: %s", eapp, path);
 
-	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+	return NULL;
 }
 
 static DBusMessage *unregister_service(DBusConnection *conn,
@@ -248,7 +265,7 @@ static DBusMessage *unregister_service(DBusConnection *conn,
 }
 
 static const GDBusMethodTable methods[] = {
-	{ GDBUS_EXPERIMENTAL_METHOD("RegisterService",
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("RegisterService",
 				GDBUS_ARGS({ "service", "o"},
 						{ "options", "a{sv}"}),
 				NULL, register_service) },
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 6/9] test: Add external service GATT skeleton
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch adds the initial code for an external GATT service example.
It implements the API defined at doc/gatt-api.txt
---
 .gitignore          |   1 +
 Makefile.tools      |   5 +++
 test/gatt-service.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 test/gatt-service.c

diff --git a/.gitignore b/.gitignore
index 94c0c78..d8e06ba 100644
--- a/.gitignore
+++ b/.gitignore
@@ -76,6 +76,7 @@ test/sap_client.pyc
 test/bluezutils.pyc
 unit/test-ringbuf
 unit/test-queue
+test/gatt-service
 unit/test-eir
 unit/test-uuid
 unit/test-crc
diff --git a/Makefile.tools b/Makefile.tools
index 9f7ba9f..cf800da 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -392,3 +392,8 @@ test_scripts += test/sap_client.py test/bluezutils.py \
 		test/test-heartrate test/test-alert test/test-hfp \
 		test/test-cyclingspeed test/opp-client test/ftp-client \
 		test/pbap-client test/map-client
+
+noinst_PROGRAMS += test/gatt-service
+
+test_gatt_service_SOURCES = test/gatt-service.c
+test_gatt_service_LDADD = @GLIB_LIBS@ @DBUS_LIBS@ gdbus/libgdbus-internal.la
diff --git a/test/gatt-service.c b/test/gatt-service.c
new file mode 100644
index 0000000..18ab7da
--- /dev/null
+++ b/test/gatt-service.c
@@ -0,0 +1,121 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2014  Instituto Nokia de Tecnologia - INdT
+ *
+ *
+ *  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 <errno.h>
+#include <stdio.h>
+
+#include <glib.h>
+#include <dbus/dbus.h>
+#include <gdbus/gdbus.h>
+
+#define GATT_SERVICE_IFACE		"org.bluez.GattService1"
+
+/* Immediate Alert Service UUID */
+#define IAS_UUID			"00001802-0000-1000-8000-00805f9b34fb"
+
+static GMainLoop *main_loop;
+static GSList *services;
+
+static gboolean service_get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	const char *uuid = user_data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+
+	return TRUE;
+}
+
+static gboolean service_get_includes(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	return TRUE;
+}
+
+static gboolean service_exist_includes(const GDBusPropertyTable *property,
+							void *user_data)
+{
+	return FALSE;
+}
+
+static const GDBusPropertyTable service_properties[] = {
+	{ "UUID", "s", service_get_uuid },
+	{ "Includes", "ao", service_get_includes, NULL,
+					service_exist_includes },
+	{ }
+};
+
+static char *register_service(DBusConnection *conn, const char *uuid)
+{
+	static int id = 1;
+	char *path;
+
+	path = g_strdup_printf("/service%d", id++);
+	if (g_dbus_register_interface(conn, path, GATT_SERVICE_IFACE,
+				NULL, NULL, service_properties,
+				g_strdup(uuid), g_free) == FALSE) {
+		printf("Couldn't register service interface\n");
+		g_free(path);
+		return NULL;
+	}
+
+	return path;
+}
+
+static void create_services(DBusConnection *conn)
+{
+	char *service_path;
+
+	service_path = register_service(conn, IAS_UUID);
+
+	services = g_slist_prepend(services, service_path);
+
+	printf("Registered service: %s\n", service_path);
+}
+
+int main(int argc, char *argv[])
+{
+	DBusConnection *dbus_conn;
+
+	dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
+
+	main_loop = g_main_loop_new(NULL, FALSE);
+
+	g_dbus_attach_object_manager(dbus_conn);
+
+	printf("gatt-service unique name: %s\n",
+				dbus_bus_get_unique_name(dbus_conn));
+
+	create_services(dbus_conn);
+
+	g_main_loop_run(main_loop);
+
+	g_slist_free_full(services, g_free);
+	dbus_connection_unref(dbus_conn);
+
+	return 0;
+}
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 7/9] test: Add signal handling for gatt-service
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch implements signal handling to run cleanup tasks before
exiting.
---
 test/gatt-service.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/test/gatt-service.c b/test/gatt-service.c
index 18ab7da..ecb5cc3 100644
--- a/test/gatt-service.c
+++ b/test/gatt-service.c
@@ -27,6 +27,9 @@
 
 #include <errno.h>
 #include <stdio.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <sys/signalfd.h>
 
 #include <glib.h>
 #include <dbus/dbus.h>
@@ -97,9 +100,83 @@ static void create_services(DBusConnection *conn)
 	printf("Registered service: %s\n", service_path);
 }
 
+static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
+							gpointer user_data)
+{
+	static bool __terminated = false;
+	struct signalfd_siginfo si;
+	ssize_t result;
+	int fd;
+
+	if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+		return FALSE;
+
+	fd = g_io_channel_unix_get_fd(channel);
+
+	result = read(fd, &si, sizeof(si));
+	if (result != sizeof(si))
+		return FALSE;
+
+	switch (si.ssi_signo) {
+	case SIGINT:
+	case SIGTERM:
+		if (!__terminated) {
+			printf("Terminating\n");
+			g_main_loop_quit(main_loop);
+		}
+
+		__terminated = true;
+		break;
+	}
+
+	return TRUE;
+}
+
+static guint setup_signalfd(void)
+{
+	GIOChannel *channel;
+	guint source;
+	sigset_t mask;
+	int fd;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, SIGINT);
+	sigaddset(&mask, SIGTERM);
+
+	if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) {
+		perror("Failed to set signal mask");
+		return 0;
+	}
+
+	fd = signalfd(-1, &mask, 0);
+	if (fd < 0) {
+		perror("Failed to create signal descriptor");
+		return 0;
+	}
+
+	channel = g_io_channel_unix_new(fd);
+
+	g_io_channel_set_close_on_unref(channel, TRUE);
+	g_io_channel_set_encoding(channel, NULL, NULL);
+	g_io_channel_set_buffered(channel, FALSE);
+
+	source = g_io_add_watch(channel,
+				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				signal_handler, NULL);
+
+	g_io_channel_unref(channel);
+
+	return source;
+}
+
 int main(int argc, char *argv[])
 {
 	DBusConnection *dbus_conn;
+	guint signal;
+
+	signal = setup_signalfd();
+	if (signal == 0)
+		return -errno;
 
 	dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
 
@@ -114,6 +191,8 @@ int main(int argc, char *argv[])
 
 	g_main_loop_run(main_loop);
 
+	g_source_remove(signal);
+
 	g_slist_free_full(services, g_free);
 	dbus_connection_unref(dbus_conn);
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 8/9] test: Add registering external service
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

This patch extends gatt-service to call RegisterService() when org.bluez
service gets connected to the system bus.
---
 test/gatt-service.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/test/gatt-service.c b/test/gatt-service.c
index ecb5cc3..f8ae7cc 100644
--- a/test/gatt-service.c
+++ b/test/gatt-service.c
@@ -35,6 +35,7 @@
 #include <dbus/dbus.h>
 #include <gdbus/gdbus.h>
 
+#define GATT_MGR_IFACE			"org.bluez.GattManager1"
 #define GATT_SERVICE_IFACE		"org.bluez.GattService1"
 
 /* Immediate Alert Service UUID */
@@ -100,6 +101,65 @@ static void create_services(DBusConnection *conn)
 	printf("Registered service: %s\n", service_path);
 }
 
+static void register_external_service_reply(DBusPendingCall *call,
+							void *user_data)
+{
+	DBusMessage *reply = dbus_pending_call_steal_reply(call);
+	DBusError derr;
+
+	dbus_error_init(&derr);
+	dbus_set_error_from_message(&derr, reply);
+
+	if (dbus_error_is_set(&derr))
+		printf("RegisterService: %s\n", derr.message);
+	else
+		printf("RegisterService: OK\n");
+
+	dbus_message_unref(reply);
+	dbus_error_free(&derr);
+}
+
+static void register_external_service(gpointer a, gpointer b)
+{
+	DBusConnection *conn = b;
+	const char *path = a;
+	DBusMessage *msg;
+	DBusPendingCall *call;
+	DBusMessageIter iter, dict;
+
+	msg = dbus_message_new_method_call("org.bluez", "/org/bluez",
+					GATT_MGR_IFACE, "RegisterService");
+	if (msg == NULL) {
+		printf("Couldn't allocate D-Bus message\n");
+		return;
+	}
+
+	dbus_message_iter_init_append(msg, &iter);
+
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &path);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, "{sv}", &dict);
+
+	/* TODO: Add options dictionary */
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	if (g_dbus_send_message_with_reply(conn, msg, &call, -1) == FALSE) {
+		dbus_message_unref(msg);
+		return;
+	}
+
+	dbus_pending_call_set_notify(call, register_external_service_reply,
+								NULL, NULL);
+
+	dbus_pending_call_unref(call);
+}
+
+static void connect_handler(DBusConnection *conn, void *user_data)
+{
+	g_slist_foreach(services, register_external_service, conn);
+}
+
 static gboolean signal_handler(GIOChannel *channel, GIOCondition cond,
 							gpointer user_data)
 {
@@ -171,6 +231,7 @@ static guint setup_signalfd(void)
 
 int main(int argc, char *argv[])
 {
+	GDBusClient *client;
 	DBusConnection *dbus_conn;
 	guint signal;
 
@@ -189,8 +250,14 @@ int main(int argc, char *argv[])
 
 	create_services(dbus_conn);
 
+	client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
+
+	g_dbus_client_set_connect_watch(client, connect_handler, NULL);
+
 	g_main_loop_run(main_loop);
 
+	g_dbus_client_unref(client);
+
 	g_source_remove(signal);
 
 	g_slist_free_full(services, g_free);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ v0 9/9] bluetooth.conf: Add ObjectManager interface
From: Claudio Takahasi @ 2014-02-28 17:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Claudio Takahasi
In-Reply-To: <1393607136-16908-1-git-send-email-claudio.takahasi@openbossa.org>

---
 src/bluetooth.conf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/bluetooth.conf b/src/bluetooth.conf
index 0495200..ad8891a 100644
--- a/src/bluetooth.conf
+++ b/src/bluetooth.conf
@@ -18,6 +18,7 @@
     <allow send_interface="org.bluez.Profile1"/>
     <allow send_interface="org.bluez.HeartRateWatcher1"/>
     <allow send_interface="org.bluez.CyclingSpeedWatcher1"/>
+    <allow send_interface="org.freedesktop.DBus.ObjectManager"/>
   </policy>
 
   <policy at_console="true">
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH 4/6] android/socket: Reserve channel for HSP AG
From: Szymon Janc @ 2014-02-28 17:06 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20140228164459.GA23792@localhost.P-661HNU-F1>

Hi Johan,

On Friday 28 of February 2014 18:44:59 Johan Hedberg wrote:
> Hi Szymon,
> 
> On Fri, Feb 28, 2014, Szymon Janc wrote:
> > ---
> >  android/socket.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/android/socket.c b/android/socket.c
> > index ee98b54..f6863c8 100644
> > --- a/android/socket.c
> > +++ b/android/socket.c
> > @@ -47,8 +47,9 @@
> >  
> >  #define RFCOMM_CHANNEL_MAX 30
> >  
> > +#define HSP_AG_DEFAULT_CHANNEL	11
> >  #define OPP_DEFAULT_CHANNEL	12
> > -#define HFAG_DEFAULT_CHANNEL	13
> > +#define HFP_AG_DEFAULT_CHANNEL	13
> >  #define PBAP_DEFAULT_CHANNEL	19
> 
> Wasn't our plan to mirror the BlueZ doc/assigned-numbers.txt here? At
> least the OPP and HSP channels don't match up with that.

We followed Bluedroid channels (although for HSP I did check which one is
used:), but we could go with BlueZ assigned-numbers.txt. That would require
fixed channels numbers for MAP, but that shouldn't be a problem.

Will send a patch for that.

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [bluez-5.14] connect fails with 'org.bluez.Error.NotAvailable'
From: Anderson Lizardo @ 2014-02-28 17:11 UTC (permalink / raw)
  To: Tobias Jakobi; +Cc: BlueZ development
In-Reply-To: <5310BFEF.8010405@gmx.net>

Hi Tobias,

On Fri, Feb 28, 2014 at 12:57 PM, Tobias Jakobi <liquid.acid@gmx.net> wrote:
> Anderson Lizardo wrote:
>> Looks like you are missing a "power on" on the server side. Notice
>> that the client side shows "Powered: yes" on the info command.
>>
>> Best Regards,
>>
> Hello Anderson!
>
> Sadly this doesn't solve the issue. The BT device on the server side is
> already powered on. Otherwise I wouldn't be able to establish the
> connection with the Android smartphone in the first place.

I misunderstood the first output, it was actually the "info" command
for the server from the client side. I thought it was the "show"
command on the server side. That explains the missing "Powered"
property :)

-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

^ permalink raw reply

* Re: [PATCH 4/6] android/socket: Reserve channel for HSP AG
From: Marcel Holtmann @ 2014-02-28 17:19 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <1848977.f8HFz4UXRu@uw000953>

Hi Szymon,

>>> ---
>>> android/socket.c | 14 ++++++++++++--
>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/android/socket.c b/android/socket.c
>>> index ee98b54..f6863c8 100644
>>> --- a/android/socket.c
>>> +++ b/android/socket.c
>>> @@ -47,8 +47,9 @@
>>> 
>>> #define RFCOMM_CHANNEL_MAX 30
>>> 
>>> +#define HSP_AG_DEFAULT_CHANNEL	11
>>> #define OPP_DEFAULT_CHANNEL	12
>>> -#define HFAG_DEFAULT_CHANNEL	13
>>> +#define HFP_AG_DEFAULT_CHANNEL	13
>>> #define PBAP_DEFAULT_CHANNEL	19
>> 
>> Wasn't our plan to mirror the BlueZ doc/assigned-numbers.txt here? At
>> least the OPP and HSP channels don't match up with that.
> 
> We followed Bluedroid channels (although for HSP I did check which one is
> used:), but we could go with BlueZ assigned-numbers.txt. That would require
> fixed channels numbers for MAP, but that shouldn't be a problem.

lets go with the Bluedroid ones, but make sure we document it somewhere which ones are used for what. I do not want to have to dig that out of the code.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH 4/6] android/socket: Reserve channel for HSP AG
From: Szymon Janc @ 2014-02-28 17:45 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <F7302E45-4F3B-4DC7-9C56-ACF7FBF33DD9@holtmann.org>

Hi Marcel,

On Friday 28 of February 2014 09:19:53 Marcel Holtmann wrote:
> Hi Szymon,
> 
> >>> ---
> >>> android/socket.c | 14 ++++++++++++--
> >>> 1 file changed, 12 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/android/socket.c b/android/socket.c
> >>> index ee98b54..f6863c8 100644
> >>> --- a/android/socket.c
> >>> +++ b/android/socket.c
> >>> @@ -47,8 +47,9 @@
> >>> 
> >>> #define RFCOMM_CHANNEL_MAX 30
> >>> 
> >>> +#define HSP_AG_DEFAULT_CHANNEL	11
> >>> #define OPP_DEFAULT_CHANNEL	12
> >>> -#define HFAG_DEFAULT_CHANNEL	13
> >>> +#define HFP_AG_DEFAULT_CHANNEL	13
> >>> #define PBAP_DEFAULT_CHANNEL	19
> >> 
> >> Wasn't our plan to mirror the BlueZ doc/assigned-numbers.txt here? At
> >> least the OPP and HSP channels don't match up with that.
> > 
> > We followed Bluedroid channels (although for HSP I did check which one is
> > used:), but we could go with BlueZ assigned-numbers.txt. That would require
> > fixed channels numbers for MAP, but that shouldn't be a problem.
> 
> lets go with the Bluedroid ones, but make sure we document it somewhere which ones are used for what. I do not want to have to dig that out of the code.

With Bluedroid there is bit awkward situation. Only PBAP and OPP channels are
explicitly reserved.

There is also comment that channel 1 is reserved for HFP AG but... when HFP AG
record is registered it skips channel 1 (due to it being already 'reserved' :)).

This results in HSP AG using channel 2, and HFP using channel 3 when both are
enabled. If one would disable HSP, then HFP would have channel 2. So those are
kind of dynamic :)

Opinions?:)

-- 
Best regards, 
Szymon Janc

^ permalink raw reply

* Re: [PATCH 4/6] android/socket: Reserve channel for HSP AG
From: Marcel Holtmann @ 2014-02-28 17:54 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Johan Hedberg, linux-bluetooth
In-Reply-To: <2637562.cfYiBUqBE8@uw000953>

Hi Szymon,

>>>>> ---
>>>>> android/socket.c | 14 ++++++++++++--
>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>> 
>>>>> diff --git a/android/socket.c b/android/socket.c
>>>>> index ee98b54..f6863c8 100644
>>>>> --- a/android/socket.c
>>>>> +++ b/android/socket.c
>>>>> @@ -47,8 +47,9 @@
>>>>> 
>>>>> #define RFCOMM_CHANNEL_MAX 30
>>>>> 
>>>>> +#define HSP_AG_DEFAULT_CHANNEL	11
>>>>> #define OPP_DEFAULT_CHANNEL	12
>>>>> -#define HFAG_DEFAULT_CHANNEL	13
>>>>> +#define HFP_AG_DEFAULT_CHANNEL	13
>>>>> #define PBAP_DEFAULT_CHANNEL	19
>>>> 
>>>> Wasn't our plan to mirror the BlueZ doc/assigned-numbers.txt here? At
>>>> least the OPP and HSP channels don't match up with that.
>>> 
>>> We followed Bluedroid channels (although for HSP I did check which one is
>>> used:), but we could go with BlueZ assigned-numbers.txt. That would require
>>> fixed channels numbers for MAP, but that shouldn't be a problem.
>> 
>> lets go with the Bluedroid ones, but make sure we document it somewhere which ones are used for what. I do not want to have to dig that out of the code.
> 
> With Bluedroid there is bit awkward situation. Only PBAP and OPP channels are
> explicitly reserved.
> 
> There is also comment that channel 1 is reserved for HFP AG but... when HFP AG
> record is registered it skips channel 1 (due to it being already 'reserved' :)).
> 
> This results in HSP AG using channel 2, and HFP using channel 3 when both are
> enabled. If one would disable HSP, then HFP would have channel 2. So those are
> kind of dynamic :)

that is funny indeed. Then lets just go with our assigned numbers where we want to use fixed ones. And do them dynamic for the ones where we need to be dynamic if at all.

Regards

Marcel


^ permalink raw reply

* [PATCH 0/5] Refactor codec handling
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek

Hi,

Here are few patches to refactor a bit and cleanup hal-audio code. Primary
goal here is to decouple codec abstraction from any write/sync operations so
in case new codec is added, it can just do encoding and does not need to care
about writing anything to socket or dealing with synchronization.

In addition, "sleeping code" is changed to use clock_nanosleep with absolute
time which results in almost perfect distribution of media packets over time,
where previously we had to "catch up" from time to time. This also seemed to
be quite unreliable over long periods of time since after some time playback
became distorted. Now it seems this issue is gone.


Andrzej Kaczmarek (5):
  android/hal-audio: Add open/close_endpoint helpers
  android/hal-audio: Add encode_mediapacket function
  android/hal-audio: Write and sync in common code
  android/hal-audio: Use payload length for codec init
  android/hal-audio: Provide better audio synchronization

 android/hal-audio.c | 409 ++++++++++++++++++++++++++++------------------------
 1 file changed, 220 insertions(+), 189 deletions(-)

-- 
1.8.5.4


^ permalink raw reply

* [PATCH 1/5] android/hal-audio: Add open/close_endpoint helpers
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1393610480-5775-1-git-send-email-andrzej.kaczmarek@tieto.com>

---
 android/hal-audio.c | 112 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 63 insertions(+), 49 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index e1f3f0d..36e8d2e 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -914,6 +914,67 @@ static void unregister_endpoints(void)
 	}
 }
 
+static int set_blocking(int fd)
+{
+	int flags;
+
+	flags = fcntl(fd, F_GETFL, 0);
+	if (flags < 0) {
+		error("fcntl(F_GETFL): %s (%d)", strerror(errno), errno);
+		return -errno;
+	}
+
+	if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
+		error("fcntl(F_SETFL): %s (%d)", strerror(errno), errno);
+		return -errno;
+	}
+
+	return 0;
+}
+
+static bool open_endpoint(struct audio_endpoint *ep,
+						struct audio_input_config *cfg)
+{
+	struct audio_preset *preset;
+	const struct audio_codec *codec;
+	uint16_t mtu;
+	int fd;
+
+	if (ipc_open_stream_cmd(ep->id, &mtu, &fd, &preset) !=
+							AUDIO_STATUS_SUCCESS)
+		return false;
+
+	if (!preset || fd < 0)
+		return false;
+
+	if (set_blocking(fd) < 0) {
+		free(preset);
+		return false;
+	}
+
+	ep->fd = fd;
+
+	codec = ep->codec;
+	codec->init(preset, mtu, &ep->codec_data);
+	codec->get_config(ep->codec_data, cfg);
+
+	free(preset);
+
+	return true;
+}
+
+static void close_endpoint(struct audio_endpoint *ep)
+{
+	ipc_close_stream_cmd(ep->id);
+	if (ep->fd >= 0) {
+		close(ep->fd);
+		ep->fd = -1;
+	}
+
+	ep->codec->cleanup(ep->codec_data);
+	ep->codec_data = NULL;
+}
+
 static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
 								size_t bytes)
 {
@@ -1260,24 +1321,6 @@ static int in_remove_audio_effect(const struct audio_stream *stream,
 	return -ENOSYS;
 }
 
-static int set_blocking(int fd)
-{
-	int flags;
-
-	flags = fcntl(fd, F_GETFL, 0);
-	if (flags < 0) {
-		error("fcntl(F_GETFL): %s (%d)", strerror(errno), errno);
-		return -errno;
-	}
-
-	if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
-		error("fcntl(F_SETFL): %s (%d)", strerror(errno), errno);
-		return -errno;
-	}
-
-	return 0;
-}
-
 static int audio_open_output_stream(struct audio_hw_device *dev,
 					audio_io_handle_t handle,
 					audio_devices_t devices,
@@ -1288,10 +1331,6 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 {
 	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
 	struct a2dp_stream_out *out;
-	struct audio_preset *preset;
-	const struct audio_codec *codec;
-	uint16_t mtu;
-	int fd;
 
 	out = calloc(1, sizeof(struct a2dp_stream_out));
 	if (!out)
@@ -1319,29 +1358,12 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	/* TODO: for now we always use endpoint 0 */
 	out->ep = &audio_endpoints[0];
 
-	if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
-							AUDIO_STATUS_SUCCESS)
-		goto fail;
-
-	if (!preset || fd < 0)
-		goto fail;
-
-	if (set_blocking(fd) < 0) {
-		free(preset);
+	if (!open_endpoint(out->ep, &out->cfg))
 		goto fail;
-	}
-
-	out->ep->fd = fd;
-	codec = out->ep->codec;
-
-	codec->init(preset, mtu, &out->ep->codec_data);
-	codec->get_config(out->ep->codec_data, &out->cfg);
 
 	DBG("rate=%d channels=%d format=%d", out->cfg.rate,
 					out->cfg.channels, out->cfg.format);
 
-	free(preset);
-
 	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
 		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
 		if (!out->downmix_buf)
@@ -1367,18 +1389,10 @@ static void audio_close_output_stream(struct audio_hw_device *dev,
 {
 	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("");
 
-	ipc_close_stream_cmd(ep->id);
-	if (ep->fd >= 0) {
-		close(ep->fd);
-		ep->fd = -1;
-	}
-
-	ep->codec->cleanup(ep->codec_data);
-	ep->codec_data = NULL;
+	close_endpoint(a2dp_dev->out->ep);
 
 	free(out->downmix_buf);
 
-- 
1.8.5.4


^ permalink raw reply related

* [PATCH 2/5] android/hal-audio: Add encode_mediapacket function
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1393610480-5775-1-git-send-email-andrzej.kaczmarek@tieto.com>

This patch moves code which encodes data for media packet into single
function. It will simply try to put as much encoded frames as will fit
in provided media packet buffer. This is first step to make common code
responsible for media stream write and synchronizarion instead of codec
abstraction code as it is handled now.
---
 android/hal-audio.c | 102 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 57 insertions(+), 45 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 36e8d2e..78889e5 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -129,6 +129,7 @@ struct sbc_data {
 	size_t in_frame_len;
 	size_t in_buf_size;
 
+	size_t out_frame_len;
 	size_t out_buf_size;
 	uint8_t *out_buf;
 
@@ -417,6 +418,7 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 	sbc_data->in_frame_len = in_frame_len;
 	sbc_data->in_buf_size = num_frames * in_frame_len;
 
+	sbc_data->out_frame_len = out_frame_len;
 	sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
 	sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
 
@@ -536,75 +538,85 @@ static int write_media_packet(int fd, struct sbc_data *sbc_data,
 	return ret;
 }
 
+static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
+					size_t len, struct media_packet *mp,
+					size_t mp_data_len, size_t *written)
+{
+	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+	size_t consumed = 0;
+	size_t encoded = 0;
+	uint8_t frame_count = 0;
+
+	while (len - consumed >= sbc_data->in_frame_len &&
+			mp_data_len - encoded >= sbc_data->out_frame_len &&
+			frame_count < MAX_FRAMES_IN_PAYLOAD) {
+		ssize_t read;
+		ssize_t written = 0;
+
+		read = sbc_encode(&sbc_data->enc, buffer + consumed,
+				sbc_data->in_frame_len, mp->data + encoded,
+				mp_data_len - encoded, &written);
+
+		if (read < 0) {
+			error("SBC: failed to encode block at frame %d (%zd)",
+							frame_count, read);
+			break;
+		}
+
+		frame_count++;
+		consumed += read;
+		encoded += written;
+	}
+
+	*written = encoded;
+	mp->payload.frame_count = frame_count;
+
+	return consumed;
+}
+
 static ssize_t sbc_write_data(void *codec_data, const void *buffer,
 							size_t bytes, int fd)
 {
 	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
 	size_t consumed = 0;
-	size_t encoded = 0;
 	struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
 	size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
-	int ret;
-	ssize_t bytes_read;
 
 	mp->hdr.v = 2;
 	mp->hdr.pt = 1;
 	mp->hdr.ssrc = htonl(1);
-	mp->hdr.timestamp = htonl(sbc_data->timestamp);
-	mp->payload.frame_count = 0;
 
-	while (bytes - consumed >= sbc_data->in_frame_len) {
-		ssize_t written = 0;
+	while (consumed < bytes) {
+		size_t written = 0;
+		ssize_t read;
+		int ret;
 
-		bytes_read = sbc_encode(&sbc_data->enc, buffer + consumed,
-					sbc_data->in_frame_len,
-					mp->data + encoded, free_space,
-					&written);
+		read = sbc_encode_mediapacket(codec_data, buffer + consumed,
+						bytes - consumed, mp,
+						free_space,
+						&written);
 
-		if (bytes_read < 0) {
-			error("SBC: failed to encode block (%zd)", bytes_read);
-			break;
+		if (read <= 0) {
+			error("sbc_encode_mediapacket failed (%zd)", read);
+			goto done;
 		}
 
-		mp->payload.frame_count++;
+		consumed += read;
 
-		consumed += bytes_read;
-		encoded += written;
-		free_space -= written;
+		mp->hdr.sequence_number = htons(sbc_data->seq++);
+		mp->hdr.timestamp = htonl(sbc_data->timestamp);
 
 		/* AudioFlinger provides PCM 16bit stereo only, thus sample size
 		 * is always 4 bytes
 		 */
-		sbc_data->timestamp += (bytes_read / 4);
+		sbc_data->timestamp += (read / 4);
 
-		/* write data if we either filled media packed or encoded all
-		 * input data
-		 */
-		if (mp->payload.frame_count == sbc_data->frames_per_packet ||
-				bytes == consumed ||
-				mp->payload.frame_count ==
-							MAX_FRAMES_IN_PAYLOAD) {
-			mp->hdr.sequence_number = htons(sbc_data->seq++);
-
-			ret = write_media_packet(fd, sbc_data, mp, encoded);
-			if (ret < 0)
-				return ret;
-
-			encoded = 0;
-			free_space = sbc_data->out_buf_size - sizeof(*mp);
-			mp->hdr.timestamp = htonl(sbc_data->timestamp);
-			mp->payload.frame_count = 0;
-		}
-	}
-
-	if (consumed != bytes) {
-		/* we should encode all input data
-		 * if we did not, something went wrong but we can't really
-		 * handle this so this is just sanity check
-		 */
-		error("SBC: failed to encode complete input buffer");
+		ret = write_media_packet(fd, sbc_data, mp, written);
+		if (ret < 0)
+			return ret;
 	}
 
+done:
 	/* we always assume that all data was processed and sent */
 	return bytes;
 }
-- 
1.8.5.4


^ permalink raw reply related

* [PATCH 3/5] android/hal-audio: Write and sync in common code
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1393610480-5775-1-git-send-email-andrzej.kaczmarek@tieto.com>

There's no need for codec abstraction to take care of writing and
synchronizing actual media stream since this is something that common
code can do regardless of codec used.

Data are synchronized based on number of samples consumed from input
stream instead of frames encoded to output stream which is also more
reliable since it's not affected by encoder errors.
---
 android/hal-audio.c | 188 +++++++++++++++++++++++++---------------------------
 1 file changed, 91 insertions(+), 97 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 78889e5..b4d78ca 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -130,17 +130,9 @@ struct sbc_data {
 	size_t in_buf_size;
 
 	size_t out_frame_len;
-	size_t out_buf_size;
-	uint8_t *out_buf;
 
 	unsigned frame_duration;
 	unsigned frames_per_packet;
-
-	struct timespec start;
-	unsigned frames_sent;
-	uint32_t timestamp;
-
-	uint16_t seq;
 };
 
 static inline void timespec_diff(struct timespec *a, struct timespec *b,
@@ -162,9 +154,9 @@ static int sbc_cleanup(void *codec_data);
 static int sbc_get_config(void *codec_data, struct audio_input_config *config);
 static size_t sbc_get_buffer_size(void *codec_data);
 static size_t sbc_get_mediapacket_duration(void *codec_data);
-static void sbc_resume(void *codec_data);
-static ssize_t sbc_write_data(void *codec_data, const void *buffer,
-							size_t bytes, int fd);
+static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
+					size_t len, struct media_packet *mp,
+					size_t mp_data_len, size_t *written);
 
 struct audio_codec {
 	uint8_t type;
@@ -178,9 +170,9 @@ struct audio_codec {
 					struct audio_input_config *config);
 	size_t (*get_buffer_size) (void *codec_data);
 	size_t (*get_mediapacket_duration) (void *codec_data);
-	void (*resume) (void *codec_data);
-	ssize_t (*write_data) (void *codec_data, const void *buffer,
-							size_t bytes, int fd);
+	ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
+					size_t len, struct media_packet *mp,
+					size_t mp_data_len, size_t *written);
 };
 
 static const struct audio_codec audio_codecs[] = {
@@ -194,8 +186,7 @@ static const struct audio_codec audio_codecs[] = {
 		.get_config = sbc_get_config,
 		.get_buffer_size = sbc_get_buffer_size,
 		.get_mediapacket_duration = sbc_get_mediapacket_duration,
-		.resume = sbc_resume,
-		.write_data = sbc_write_data,
+		.encode_mediapacket = sbc_encode_mediapacket,
 	}
 };
 
@@ -208,6 +199,13 @@ struct audio_endpoint {
 	const struct audio_codec *codec;
 	void *codec_data;
 	int fd;
+
+	struct media_packet *mp;
+	size_t mp_data_len;
+
+	uint16_t seq;
+	uint32_t samples;
+	struct timespec start;
 };
 
 static struct audio_endpoint audio_endpoints[MAX_AUDIO_ENDPOINTS];
@@ -419,8 +417,6 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 	sbc_data->in_buf_size = num_frames * in_frame_len;
 
 	sbc_data->out_frame_len = out_frame_len;
-	sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
-	sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
 
 	sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
 	sbc_data->frames_per_packet = num_frames;
@@ -438,7 +434,6 @@ static int sbc_cleanup(void *codec_data)
 	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
 
 	sbc_finish(&sbc_data->enc);
-	free(sbc_data->out_buf);
 	free(codec_data);
 
 	return AUDIO_STATUS_SUCCESS;
@@ -486,28 +481,18 @@ static size_t sbc_get_mediapacket_duration(void *codec_data)
 	return sbc_data->frame_duration * sbc_data->frames_per_packet;
 }
 
-static void sbc_resume(void *codec_data)
-{
-	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
-
-	DBG("");
-
-	clock_gettime(CLOCK_MONOTONIC, &sbc_data->start);
-
-	sbc_data->frames_sent = 0;
-	sbc_data->timestamp = 0;
-}
-
-static int write_media_packet(int fd, struct sbc_data *sbc_data,
-				struct media_packet *mp, size_t data_len)
+static int write_media_packet(struct a2dp_stream_out *out, size_t mp_data_len,
+							uint32_t input_samples)
 {
+	struct audio_endpoint *ep = out->ep;
+	struct media_packet *mp = ep->mp;
 	struct timespec cur;
 	struct timespec diff;
-	unsigned expected_frames;
+	uint32_t expected_samples;
 	int ret;
 
 	while (true) {
-		ret = write(fd, mp, sizeof(*mp) + data_len);
+		ret = write(ep->fd, mp, sizeof(*mp) + mp_data_len);
 		if (ret >= 0)
 			break;
 
@@ -515,12 +500,10 @@ static int write_media_packet(int fd, struct sbc_data *sbc_data,
 			return -errno;
 	}
 
-	sbc_data->frames_sent += mp->payload.frame_count;
-
 	clock_gettime(CLOCK_MONOTONIC, &cur);
-	timespec_diff(&cur, &sbc_data->start, &diff);
-	expected_frames = (diff.tv_sec * 1000000 + diff.tv_nsec / 1000) /
-						sbc_data->frame_duration;
+	timespec_diff(&cur, &ep->start, &diff);
+	expected_samples = (diff.tv_sec * 1000000ll + diff.tv_nsec / 1000ll) *
+						out->cfg.rate / 1000000ll;
 
 	/* AudioFlinger does not seem to provide any *working*
 	 * API to provide data in some interval and will just
@@ -531,9 +514,8 @@ static int write_media_packet(int fd, struct sbc_data *sbc_data,
 	 * lagging behind audio stream, we can sleep for
 	 * duration of single media packet.
 	 */
-	if (sbc_data->frames_sent >= expected_frames)
-		usleep(sbc_data->frame_duration *
-				mp->payload.frame_count);
+	if (ep->samples >= expected_samples)
+		usleep(input_samples * 1000000 / out->cfg.rate);
 
 	return ret;
 }
@@ -574,53 +556,6 @@ static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
 	return consumed;
 }
 
-static ssize_t sbc_write_data(void *codec_data, const void *buffer,
-							size_t bytes, int fd)
-{
-	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
-	size_t consumed = 0;
-	struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
-	size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
-
-	mp->hdr.v = 2;
-	mp->hdr.pt = 1;
-	mp->hdr.ssrc = htonl(1);
-
-	while (consumed < bytes) {
-		size_t written = 0;
-		ssize_t read;
-		int ret;
-
-		read = sbc_encode_mediapacket(codec_data, buffer + consumed,
-						bytes - consumed, mp,
-						free_space,
-						&written);
-
-		if (read <= 0) {
-			error("sbc_encode_mediapacket failed (%zd)", read);
-			goto done;
-		}
-
-		consumed += read;
-
-		mp->hdr.sequence_number = htons(sbc_data->seq++);
-		mp->hdr.timestamp = htonl(sbc_data->timestamp);
-
-		/* AudioFlinger provides PCM 16bit stereo only, thus sample size
-		 * is always 4 bytes
-		 */
-		sbc_data->timestamp += (read / 4);
-
-		ret = write_media_packet(fd, sbc_data, mp, written);
-		if (ret < 0)
-			return ret;
-	}
-
-done:
-	/* we always assume that all data was processed and sent */
-	return bytes;
-}
-
 static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
 			void *param, size_t *rsp_len, void *rsp, int *fd)
 {
@@ -970,6 +905,13 @@ static bool open_endpoint(struct audio_endpoint *ep,
 	codec->init(preset, mtu, &ep->codec_data);
 	codec->get_config(ep->codec_data, cfg);
 
+	ep->mp = calloc(mtu, 1);
+	ep->mp->hdr.v = 2;
+	ep->mp->hdr.pt = 1;
+	ep->mp->hdr.ssrc = htonl(1);
+
+	ep->mp_data_len = mtu - sizeof(*ep->mp);
+
 	free(preset);
 
 	return true;
@@ -983,6 +925,8 @@ static void close_endpoint(struct audio_endpoint *ep)
 		ep->fd = -1;
 	}
 
+	free(ep->mp);
+
 	ep->codec->cleanup(ep->codec_data);
 	ep->codec_data = NULL;
 }
@@ -1002,10 +946,59 @@ static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
 	}
 }
 
+static bool write_data(struct a2dp_stream_out *out, const void *buffer,
+								size_t bytes)
+{
+	struct audio_endpoint *ep = out->ep;
+	struct media_packet *mp = (struct media_packet *) ep->mp;
+	size_t free_space = ep->mp_data_len;
+	size_t consumed = 0;
+
+	while (consumed < bytes) {
+		size_t written = 0;
+		ssize_t read;
+		uint32_t samples;
+		int ret;
+
+		read = ep->codec->encode_mediapacket(ep->codec_data,
+							buffer + consumed,
+							bytes - consumed, mp,
+							free_space, &written);
+
+		/* This is non-fatal and we can just assume buffer was processed
+		 * properly and wait for next one.
+		 */
+		if (read <= 0)
+			goto done;
+
+		consumed += read;
+
+		mp->hdr.sequence_number = htons(ep->seq++);
+		mp->hdr.timestamp = htonl(ep->samples);
+
+		/* AudioFlinger provides 16bit PCM, so sample size is 2 bytes
+		 * multipled by number of channels. Number of channels is simply
+		 * number of bits set in channels mask.
+		 */
+		samples = read / (2 * popcount(out->cfg.channels));
+		ep->samples += samples;
+
+		ret = write_media_packet(out, written, samples);
+		if (ret < 0)
+			return false;
+	}
+
+done:
+	return true;
+}
+
+
 static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 								size_t bytes)
 {
 	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
+	const void *in_buf = buffer;
+	size_t in_len = bytes;
 
 	/* just return in case we're closing */
 	if (out->audio_state == AUDIO_A2DP_STATE_NONE)
@@ -1018,7 +1011,8 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		if (ipc_resume_stream_cmd(out->ep->id) != AUDIO_STATUS_SUCCESS)
 			return -1;
 
-		out->ep->codec->resume(out->ep->codec_data);
+		clock_gettime(CLOCK_MONOTONIC, &out->ep->start);
+		out->ep->samples = 0;
 
 		out->audio_state = AUDIO_A2DP_STATE_STARTED;
 	}
@@ -1048,14 +1042,14 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 
 		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;
+		in_buf = out->downmix_buf;
+		in_len = bytes / 2;
 	}
 
-	return out->ep->codec->write_data(out->ep->codec_data, buffer,
-							bytes, out->ep->fd);
+	if (!write_data(out, in_buf, in_len))
+		return -1;
+
+	return bytes;
 }
 
 static uint32_t out_get_sample_rate(const struct audio_stream *stream)
-- 
1.8.5.4


^ permalink raw reply related

* [PATCH 4/5] android/hal-audio: Use payload length for codec init
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1393610480-5775-1-git-send-email-andrzej.kaczmarek@tieto.com>

It makes more sense to pass maximum payload length to codec since this
is actually used for calculations. MTU value is used only to allocate
proper buffer.
---
 android/hal-audio.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index b4d78ca..3a7b026 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -387,11 +387,10 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
 			in->min_bitpool, in->max_bitpool);
 }
 
-static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
+static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
 							void **codec_data)
 {
 	struct sbc_data *sbc_data;
-	size_t hdr_len = sizeof(struct media_packet);
 	size_t in_frame_len;
 	size_t out_frame_len;
 	size_t num_frames;
@@ -411,7 +410,7 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 
 	in_frame_len = sbc_get_codesize(&sbc_data->enc);
 	out_frame_len = sbc_get_frame_length(&sbc_data->enc);
-	num_frames = (mtu - hdr_len) / out_frame_len;
+	num_frames = payload_len / out_frame_len;
 
 	sbc_data->in_frame_len = in_frame_len;
 	sbc_data->in_buf_size = num_frames * in_frame_len;
@@ -421,8 +420,8 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 	sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
 	sbc_data->frames_per_packet = num_frames;
 
-	DBG("mtu=%u in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
-			mtu, in_frame_len, out_frame_len, num_frames);
+	DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
+				in_frame_len, out_frame_len, num_frames);
 
 	*codec_data = sbc_data;
 
@@ -885,6 +884,7 @@ static bool open_endpoint(struct audio_endpoint *ep,
 	struct audio_preset *preset;
 	const struct audio_codec *codec;
 	uint16_t mtu;
+	uint16_t payload_len;
 	int fd;
 
 	if (ipc_open_stream_cmd(ep->id, &mtu, &fd, &preset) !=
@@ -899,10 +899,14 @@ static bool open_endpoint(struct audio_endpoint *ep,
 		return false;
 	}
 
+	DBG("mtu=%u", mtu);
+
+	payload_len = mtu - sizeof(*ep->mp);
+
 	ep->fd = fd;
 
 	codec = ep->codec;
-	codec->init(preset, mtu, &ep->codec_data);
+	codec->init(preset, payload_len, &ep->codec_data);
 	codec->get_config(ep->codec_data, cfg);
 
 	ep->mp = calloc(mtu, 1);
@@ -910,7 +914,7 @@ static bool open_endpoint(struct audio_endpoint *ep,
 	ep->mp->hdr.pt = 1;
 	ep->mp->hdr.ssrc = htonl(1);
 
-	ep->mp_data_len = mtu - sizeof(*ep->mp);
+	ep->mp_data_len = payload_len;
 
 	free(preset);
 
-- 
1.8.5.4


^ permalink raw reply related

* [PATCH 5/5] android/hal-audio: Provide better audio synchronization
From: Andrzej Kaczmarek @ 2014-02-28 18:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrzej Kaczmarek
In-Reply-To: <1393610480-5775-1-git-send-email-andrzej.kaczmarek@tieto.com>

Instead of waiting some fixed amount of useconds before next media
packet is encoded we use absolute time to wait precisely for a moment
when next packet is expected to be produced. This greatly improves flow
as is can compensate for time spent on encoding and writing data.
---
 android/hal-audio.c | 91 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 42 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 3a7b026..3ef058e 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -147,6 +147,27 @@ static inline void timespec_diff(struct timespec *a, struct timespec *b,
 	}
 }
 
+static void timespec_add(struct timespec *base, uint64_t time_us,
+							struct timespec *res)
+{
+	res->tv_sec = base->tv_sec + time_us / 1000000;
+	res->tv_nsec = base->tv_nsec + (time_us % 1000000) * 1000;
+
+	if (res->tv_nsec >= 1000000000) {
+		res->tv_sec++;
+		res->tv_nsec -= 1000000000;
+	}
+}
+
+#if defined(ANDROID)
+/* Bionic does not have clock_nanosleep() prototype in time.h even though
+ * it provides its implementation.
+ */
+extern int clock_nanosleep(clockid_t clock_id, int flags,
+					const struct timespec *request,
+					struct timespec *remain);
+#endif
+
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
 static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 							void **codec_data);
@@ -480,45 +501,6 @@ static size_t sbc_get_mediapacket_duration(void *codec_data)
 	return sbc_data->frame_duration * sbc_data->frames_per_packet;
 }
 
-static int write_media_packet(struct a2dp_stream_out *out, size_t mp_data_len,
-							uint32_t input_samples)
-{
-	struct audio_endpoint *ep = out->ep;
-	struct media_packet *mp = ep->mp;
-	struct timespec cur;
-	struct timespec diff;
-	uint32_t expected_samples;
-	int ret;
-
-	while (true) {
-		ret = write(ep->fd, mp, sizeof(*mp) + mp_data_len);
-		if (ret >= 0)
-			break;
-
-		if (errno != EINTR)
-			return -errno;
-	}
-
-	clock_gettime(CLOCK_MONOTONIC, &cur);
-	timespec_diff(&cur, &ep->start, &diff);
-	expected_samples = (diff.tv_sec * 1000000ll + diff.tv_nsec / 1000ll) *
-						out->cfg.rate / 1000000ll;
-
-	/* AudioFlinger does not seem to provide any *working*
-	 * API to provide data in some interval and will just
-	 * send another buffer as soon as we process current
-	 * one. To prevent overflowing L2CAP socket, we need to
-	 * introduce some artificial delay here base on how many
-	 * audio frames were sent so far, i.e. if we're not
-	 * lagging behind audio stream, we can sleep for
-	 * duration of single media packet.
-	 */
-	if (ep->samples >= expected_samples)
-		usleep(input_samples * 1000000 / out->cfg.rate);
-
-	return ret;
-}
-
 static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
 					size_t len, struct media_packet *mp,
 					size_t mp_data_len, size_t *written)
@@ -963,6 +945,25 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
 		ssize_t read;
 		uint32_t samples;
 		int ret;
+		uint64_t time_us;
+		struct timespec anchor;
+
+		time_us = ep->samples * 1000000ll / out->cfg.rate;
+
+		timespec_add(&ep->start, time_us, &anchor);
+
+		while (true) {
+			ret = clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME,
+								&anchor, NULL);
+
+			if (!ret)
+				break;
+
+			if (ret != EINTR) {
+				error("clock_nanosleep failed (%d)", ret);
+				return false;
+			}
+		}
 
 		read = ep->codec->encode_mediapacket(ep->codec_data,
 							buffer + consumed,
@@ -987,9 +988,15 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
 		samples = read / (2 * popcount(out->cfg.channels));
 		ep->samples += samples;
 
-		ret = write_media_packet(out, written, samples);
-		if (ret < 0)
-			return false;
+		while (true) {
+			ret = write(ep->fd, mp, sizeof(*mp) + written);
+
+			if (ret >= 0)
+				break;
+
+			if (errno != EINTR)
+				return false;
+		}
 	}
 
 done:
-- 
1.8.5.4


^ permalink raw reply related

* [PATCH 1/2] Bluetooth: Fix trying to disable scanning twice
From: johan.hedberg @ 2014-02-28 18:26 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

The discovery process has a timer for disabling scanning, however
scanning might be disabled through other means too like the auto-connect
process.  We should therefore ensure that the timer is never active
after sending a HCI command to disable scanning.

There was some existing code in stop_scan_complete trying to avoid the
timer when a connect request interrupts a discovery procedure, but the
other way around was not covered. This patch covers both scenarios by
canceling the timer as soon as we get a successful command complete for
the disabling HCI command.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c  | 1 -
 net/bluetooth/hci_event.c | 5 +++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 7e47e4240c95..5330fcfde93d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -628,7 +628,6 @@ static void stop_scan_complete(struct hci_dev *hdev, u8 status)
 	/* 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);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a1075c713a9d..e3335b03c992 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1018,6 +1018,11 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		break;
 
 	case LE_SCAN_DISABLE:
+		/* Cancel this timer so that we don't try to disable scanning
+		 * when it's already disabled.
+		 */
+		cancel_delayed_work(&hdev->le_scan_disable);
+
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 		break;
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 2/2] Bluetooth: Remove unnecessary stop_scan_complete function
From: johan.hedberg @ 2014-02-28 18:26 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393611973-3183-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

The stop_scan_complete function was used as an intermediate step before
doing the actual connection creation. Since we're using hci_request
there's no reason to have this extra function around, i.e. we can simply
put both HCI commands into the same request.

The single task that the intermediate function had, i.e. indicating
discovery as stopped is now taken care of by a new
HCI_LE_SCAN_INTERRUPTED flag which allows us to do the discovery state
update when the stop scan command completes.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_conn.c    | 51 +++++++--------------------------------------
 net/bluetooth/hci_event.c   |  7 +++++++
 3 files changed, 16 insertions(+), 43 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0409f0119d2b..be150cf8cd43 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -140,6 +140,7 @@ enum {
 	HCI_FAST_CONNECTABLE,
 	HCI_BREDR_ENABLED,
 	HCI_6LOWPAN_ENABLED,
+	HCI_LE_SCAN_INTERRUPTED,
 };
 
 /* A mask for the flags that are supposed to remain when a reset happens
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 5330fcfde93d..7c713c4675ba 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -605,44 +605,6 @@ static void hci_req_add_le_create_conn(struct hci_request *req,
 	conn->state = BT_CONNECT;
 }
 
-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);
-		hci_le_conn_failed(conn, status);
-		hci_dev_unlock(hdev);
-		return;
-	}
-
-	/* Since we may have prematurely stopped discovery procedure, we should
-	 * update discovery state.
-	 */
-	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
-
-	hci_req_init(&req, hdev);
-
-	hci_req_add_le_create_conn(&req, conn);
-
-	err = hci_req_run(&req, create_le_conn_complete);
-	if (err) {
-		hci_dev_lock(hdev);
-		hci_le_conn_failed(conn, HCI_ERROR_MEMORY_EXCEEDED);
-		hci_dev_unlock(hdev);
-		return;
-	}
-}
-
 struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 				u8 dst_type, u8 sec_level, u8 auth_type)
 {
@@ -721,16 +683,19 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	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.
+	 * not able to scan and connect at the same time. Also set the
+	 * HCI_LE_SCAN_INTERRUPTED flag so that the command complete
+	 * handler for scan disabling knows to set the correct discovery
+	 * state.
 	 */
 	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
 		hci_req_add_le_scan_disable(&req);
-		err = hci_req_run(&req, stop_scan_complete);
-	} else {
-		hci_req_add_le_create_conn(&req, conn);
-		err = hci_req_run(&req, create_le_conn_complete);
+		set_bit(HCI_LE_SCAN_INTERRUPTED, &hdev->dev_flags);
 	}
 
+	hci_req_add_le_create_conn(&req, conn);
+
+	err = hci_req_run(&req, create_le_conn_complete);
 	if (err) {
 		hci_conn_del(conn);
 		return ERR_PTR(err);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index e3335b03c992..c3b0a08f5ab4 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1024,6 +1024,13 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		cancel_delayed_work(&hdev->le_scan_disable);
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
+		/* The HCI_LE_SCAN_INTERRUPTED flag indicates that we
+		 * interrupted scanning due to a connect request. Mark
+		 * therefore discovery as stopped.
+		 */
+		if (test_and_clear_bit(HCI_LE_SCAN_INTERRUPTED,
+				       &hdev->dev_flags))
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
 		break;
 
 	default:
-- 
1.8.5.3


^ permalink raw reply related


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