[pulseaudio-discuss] [PATCH v2 1/4] bluetooth: Add basic support for HEADSET profiles

[Luiz Augusto von Dentz]

Hi Tanu,

On Thu, Aug 21, 2014 at 2:29 PM, Tanu Kaskinen
<tanu.kaskinen at linux.intel.com> wrote:
> On Thu, 2014-08-21 at 14:12 +0300, Luiz Augusto von Dentz wrote:
>> 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.
>
> An assertion would probably make sense. Could you put that in a separate
> patch, though?

Sure


-- 
Luiz Augusto von Dentz