Hi Tanu, On Wed, Aug 20, 2014 at 2:40 PM, Tanu Kaskinen <tanu.kaskinen at linux.intel.com> wrote: > On Tue, 2014-08-19 at 15:54 +0300, Luiz Augusto von Dentz wrote: >> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org> >> >> This commit adds basic support for devices implementing HSP Headset >> Unit, HSP Audio Gateway, HFP Handsfree Unit, HFP Audio Gateway to the >> BlueZ 5 bluetooth audio devices driver module (module-bluez5-device). >> --- >> src/modules/bluetooth/bluez5-util.c | 4 + >> src/modules/bluetooth/bluez5-util.h | 6 + >> src/modules/bluetooth/module-bluez5-device.c | 436 ++++++++++++++++++++------- >> 3 files changed, 337 insertions(+), 109 deletions(-) >> >> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c >> index 5b6b372..adb8351 100644 >> --- a/src/modules/bluetooth/bluez5-util.c >> +++ b/src/modules/bluetooth/bluez5-util.c >> @@ -1109,6 +1109,10 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) { >> return "a2dp_sink"; >> case PA_BLUETOOTH_PROFILE_A2DP_SOURCE: >> return "a2dp_source"; >> + case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT: >> + return "headset_head_unit"; >> + case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY: >> + return "headset_audio_gateway"; >> case PA_BLUETOOTH_PROFILE_OFF: >> return "off"; >> } >> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h >> index 63bae35..0121733 100644 >> --- a/src/modules/bluetooth/bluez5-util.h >> +++ b/src/modules/bluetooth/bluez5-util.h >> @@ -26,6 +26,10 @@ >> >> #define PA_BLUETOOTH_UUID_A2DP_SOURCE "0000110a-0000-1000-8000-00805f9b34fb" >> #define PA_BLUETOOTH_UUID_A2DP_SINK "0000110b-0000-1000-8000-00805f9b34fb" >> +#define PA_BLUETOOTH_UUID_HSP_HS "00001108-0000-1000-8000-00805f9b34fb" >> +#define PA_BLUETOOTH_UUID_HSP_AG "00001112-0000-1000-8000-00805f9b34fb" >> +#define PA_BLUETOOTH_UUID_HFP_HF "0000111e-0000-1000-8000-00805f9b34fb" >> +#define PA_BLUETOOTH_UUID_HFP_AG "0000111f-0000-1000-8000-00805f9b34fb" >> >> typedef struct pa_bluetooth_transport pa_bluetooth_transport; >> typedef struct pa_bluetooth_device pa_bluetooth_device; >> @@ -41,6 +45,8 @@ typedef enum pa_bluetooth_hook { >> typedef enum profile { >> PA_BLUETOOTH_PROFILE_A2DP_SINK, >> PA_BLUETOOTH_PROFILE_A2DP_SOURCE, >> + PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT, >> + PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY, >> PA_BLUETOOTH_PROFILE_OFF >> } pa_bluetooth_profile_t; >> #define PA_BLUETOOTH_PROFILE_COUNT PA_BLUETOOTH_PROFILE_OFF >> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c >> index 57b2791..7d591f9 100644 >> --- a/src/modules/bluetooth/module-bluez5-device.c >> +++ b/src/modules/bluetooth/module-bluez5-device.c >> @@ -33,6 +33,7 @@ >> #include <pulse/timeval.h> >> >> #include <pulsecore/core-error.h> >> +#include <pulsecore/core-rtclock.h> >> #include <pulsecore/core-util.h> >> #include <pulsecore/i18n.h> >> #include <pulsecore/module.h> >> @@ -59,7 +60,9 @@ PA_MODULE_USAGE("path=<device object path>"); >> >> #define MAX_PLAYBACK_CATCH_UP_USEC (100 * PA_USEC_PER_MSEC) >> #define FIXED_LATENCY_PLAYBACK_A2DP (25 * PA_USEC_PER_MSEC) >> +#define FIXED_LATENCY_PLAYBACK_SCO (125 * PA_USEC_PER_MSEC) >> #define FIXED_LATENCY_RECORD_A2DP (25 * PA_USEC_PER_MSEC) >> +#define FIXED_LATENCY_RECORD_SCO (25 * PA_USEC_PER_MSEC) >> >> #define BITPOOL_DEC_LIMIT 32 >> #define BITPOOL_DEC_STEP 5 >> @@ -236,6 +239,159 @@ static void connect_ports(struct userdata *u, void *new_data, pa_direction_t dir >> } >> >> /* Run from IO thread */ >> +static int sco_process_render(struct userdata *u) { >> + ssize_t l; >> + pa_memchunk memchunk; >> + >> + pa_assert(u); >> + pa_assert(u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || >> + u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY); >> + pa_assert(u->sink); >> + >> + /* First, render some data */ >> + if (!u->write_memchunk.memblock) > > This check is unnecessary. I forget this one, I will fix it. >> @@ -1679,8 +1894,11 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot >> pa_assert(t); >> >> /* Update profile availability */ >> - if (!(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)))) >> + if (!(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)))) { >> + pa_log_warn("Transport %s has no profile mapping", pa_bluetooth_profile_to_string(t->profile)); >> return; >> + } >> + > > What's the motivation behind this (pretty unrelated) change? Presumably > you encountered a situation where the warning is printed, what caused > that situation? This was a bug in the previous version where headset_head_unit and headset_audio_gateway had been changed in pa_bluetooth_profile_to_string so they mismatched and silently fail, usually this should never happen so perhaps we should assert instead. > I could fix the tiny issue of a redundant check and push this patch, but > is that safe, or should I wait until the implementation is more > complete? > > -- > Tanu > -- Luiz Augusto von Dentz