[v2,2/2] Bluetooth A2DP aptX codec support

Submitted by Pali Rohár on July 28, 2018, 3:34 p.m.

Details

Message ID 20180728153453.32694-3-pali.rohar@gmail.com
State Superseded
Headers show
Series "WIP: Bluetooth A2DP aptX codec support" ( rev: 2 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Pali Rohár July 28, 2018, 3:34 p.m.
This patch provides support for aptX codec in bluetooth A2DP profile. In
pulseaudio it is implemented as a new profile a2dp_aptx_sink. For aptX
encoding it uses open source LGPLv2.1+ licensed libopenaptx library which
can be found at https://github.com/pali/libopenaptx.

Limitations:

Codec selection (either SBC or aptX) is done by bluez itself and it does
not provide API for switching codec. Therefore pulseaudio is not able to
change codec and it is up to bluez if it decide to use aptX or not.

Only standard aptX codec is supported for now. Support for other variants
like aptX HD, aptX Low Latency, FastStream may come up later.
---
 configure.ac                                 |  19 ++
 src/Makefile.am                              |   5 +
 src/modules/bluetooth/a2dp-codecs.h          | 118 ++++++++++-
 src/modules/bluetooth/bluez5-util.c          |  48 ++++-
 src/modules/bluetooth/bluez5-util.h          |   2 +
 src/modules/bluetooth/module-bluez5-device.c |  65 +++++-
 src/modules/bluetooth/pa-a2dp-codec-aptx.c   | 297 +++++++++++++++++++++++++++
 src/modules/bluetooth/pa-a2dp-codec.h        |   1 +
 8 files changed, 548 insertions(+), 7 deletions(-)
 create mode 100644 src/modules/bluetooth/pa-a2dp-codec-aptx.c

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index d2bfab23b..c2d13fa53 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1094,6 +1094,23 @@  AC_SUBST(HAVE_BLUEZ_5_NATIVE_HEADSET)
 AM_CONDITIONAL([HAVE_BLUEZ_5_NATIVE_HEADSET], [test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = x1])
 AS_IF([test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = "x1"], AC_DEFINE([HAVE_BLUEZ_5_NATIVE_HEADSET], 1, [Bluez 5 native headset backend enabled]))
 
+#### Bluetooth A2DP aptX codec (optional) ###
+
+AC_ARG_ENABLE([aptx],
+    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
+
+AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
+    [AC_CHECK_HEADER([openaptx.h],
+        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
+        [HAVE_OPENAPTX=0])])
+
+AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" = "xyes" && test "x$HAVE_OPENAPTX" = "x0"],
+    [AC_MSG_ERROR([*** libopenaptx from https://github.com/pali/libopenaptx not found])])
+
+AC_SUBST(HAVE_OPENAPTX)
+AM_CONDITIONAL([HAVE_OPENAPTX], [test "x$HAVE_OPENAPTX" = "x1"])
+AS_IF([test "x$HAVE_OPENAPTX" = "x1"], AC_DEFINE([HAVE_OPENAPTX], 1, [Have openaptx codec library]))
+
 #### UDEV support (optional) ####
 
 AC_ARG_ENABLE([udev],
@@ -1579,6 +1596,7 @@  AS_IF([test "x$HAVE_SYSTEMD_JOURNAL" = "x1"], ENABLE_SYSTEMD_JOURNAL=yes, ENABLE
 AS_IF([test "x$HAVE_BLUEZ_5" = "x1"], ENABLE_BLUEZ_5=yes, ENABLE_BLUEZ_5=no)
 AS_IF([test "x$HAVE_BLUEZ_5_OFONO_HEADSET" = "x1"], ENABLE_BLUEZ_5_OFONO_HEADSET=yes, ENABLE_BLUEZ_5_OFONO_HEADSET=no)
 AS_IF([test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = "x1"], ENABLE_BLUEZ_5_NATIVE_HEADSET=yes, ENABLE_BLUEZ_5_NATIVE_HEADSET=no)
+AS_IF([test "x$HAVE_OPENAPTX" = "x1"], ENABLE_APTX=yes, ENABLE_APTX=no)
 AS_IF([test "x$HAVE_HAL_COMPAT" = "x1"], ENABLE_HAL_COMPAT=yes, ENABLE_HAL_COMPAT=no)
 AS_IF([test "x$HAVE_TCPWRAP" = "x1"], ENABLE_TCPWRAP=yes, ENABLE_TCPWRAP=no)
 AS_IF([test "x$HAVE_LIBSAMPLERATE" = "x1"], ENABLE_LIBSAMPLERATE="yes (DEPRECATED)", ENABLE_LIBSAMPLERATE=no)
@@ -1637,6 +1655,7 @@  echo "
       Enable BlueZ 5:              ${ENABLE_BLUEZ_5}
         Enable ofono headsets:     ${ENABLE_BLUEZ_5_OFONO_HEADSET}
         Enable native headsets:    ${ENABLE_BLUEZ_5_NATIVE_HEADSET}
+        Enable aptX codec:         ${ENABLE_APTX}
     Enable udev:                   ${ENABLE_UDEV}
       Enable HAL->udev compat:     ${ENABLE_HAL_COMPAT}
     Enable systemd
diff --git a/src/Makefile.am b/src/Makefile.am
index 411b9e5e5..bbd797589 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2136,6 +2136,11 @@  libbluez5_util_la_SOURCES += modules/bluetooth/pa-a2dp-codec-sbc.c
 libbluez5_util_la_LIBADD += $(SBC_LIBS)
 libbluez5_util_la_CFLAGS += $(SBC_CFLAGS)
 
+if HAVE_OPENAPTX
+libbluez5_util_la_SOURCES += modules/bluetooth/pa-a2dp-codec-aptx.c
+libbluez5_util_la_LIBADD += -lopenaptx
+endif
+
 module_bluez5_discover_la_SOURCES = modules/bluetooth/module-bluez5-discover.c
 module_bluez5_discover_la_LDFLAGS = $(MODULE_LDFLAGS)
 module_bluez5_discover_la_LIBADD = $(MODULE_LIBADD) $(DBUS_LIBS) libbluez5-util.la
diff --git a/src/modules/bluetooth/a2dp-codecs.h b/src/modules/bluetooth/a2dp-codecs.h
index 004975586..0c3583434 100644
--- a/src/modules/bluetooth/a2dp-codecs.h
+++ b/src/modules/bluetooth/a2dp-codecs.h
@@ -4,6 +4,7 @@ 
  *
  *  Copyright (C) 2006-2010  Nokia Corporation
  *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *  Copyright (C) 2018       Pali Rohár <pali.rohar@gmail.com>
  *
  *
  *  This library is free software; you can redistribute it and/or
@@ -24,7 +25,18 @@ 
 #define A2DP_CODEC_SBC			0x00
 #define A2DP_CODEC_MPEG12		0x01
 #define A2DP_CODEC_MPEG24		0x02
-#define A2DP_CODEC_ATRAC		0x03
+#define A2DP_CODEC_ATRAC		0x04
+#define A2DP_CODEC_VENDOR		0xFF
+
+#define A2DP_VENDOR_APT_LIC_LTD		0x0000004f
+#define APT_LIC_LTD_CODEC_APTX		0x0001
+
+#define A2DP_VENDOR_QTIL		0x0000000a
+#define QTIL_CODEC_FASTSTREAM		0x0001
+#define QTIL_CODEC_APTX_LL		0x0002
+
+#define A2DP_VENDOR_QUALCOMM_TECH_INC	0x000000d7
+#define QUALCOMM_TECH_INC_CODEC_APTX_HD	0x0024
 
 #define SBC_SAMPLING_FREQ_16000		(1 << 3)
 #define SBC_SAMPLING_FREQ_32000		(1 << 2)
@@ -66,6 +78,26 @@ 
 #define MPEG_SAMPLING_FREQ_44100	(1 << 1)
 #define MPEG_SAMPLING_FREQ_48000	1
 
+#define APTX_CHANNEL_MODE_MONO		0x1
+#define APTX_CHANNEL_MODE_STEREO	0x2
+
+#define APTX_SAMPLING_FREQ_16000	0x8
+#define APTX_SAMPLING_FREQ_32000	0x4
+#define APTX_SAMPLING_FREQ_44100	0x2
+#define APTX_SAMPLING_FREQ_48000	0x1
+
+#define FASTSTREAM_DIRECTION_SINK	0x1
+#define FASTSTREAM_DIRECTION_SOURCE	0x2
+
+#define FASTSTREAM_SINK_SAMPLING_FREQ_44100	0x2
+#define FASTSTREAM_SINK_SAMPLING_FREQ_48000	0x1
+
+#define FASTSTREAM_SOURCE_SAMPLING_FREQ_16000	0x2
+
+typedef struct {
+	uint32_t vendor_id;
+	uint16_t codec_id;
+} __attribute__ ((packed)) a2dp_vendor_codec_t;
 
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 
@@ -89,6 +121,48 @@  typedef struct {
 	uint16_t bitrate;
 } __attribute__ ((packed)) a2dp_mpeg_t;
 
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t channel_mode:4;
+	uint8_t frequency:4;
+} __attribute__ ((packed)) a2dp_aptx_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t direction;
+	uint8_t sink_frequency:4;
+	uint8_t source_frequency:4;
+} __attribute__ ((packed)) a2dp_faststream_t;
+
+typedef struct {
+	uint8_t reserved;
+	uint16_t target_codec_level;
+	uint16_t initial_codec_level;
+	uint8_t sra_max_rate;
+	uint8_t sra_avg_time;
+	uint16_t good_working_level;
+} __attribute__ ((packed)) a2dp_aptx_ll_new_caps_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t channel_mode:4;
+	uint8_t frequency:4;
+	uint8_t bidirect_link:1;
+	uint8_t has_new_caps:1;
+	uint8_t reserved:6;
+	a2dp_aptx_ll_new_caps_t new_caps[0];
+} __attribute__ ((packed)) a2dp_aptx_ll_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t channel_mode:4;
+	uint8_t frequency:4;
+	uint8_t reserved0;
+	uint8_t reserved1;
+	uint8_t reserved2;
+	uint8_t reserved3;
+} __attribute__ ((packed)) a2dp_aptx_hd_t;
+
 #elif __BYTE_ORDER == __BIG_ENDIAN
 
 typedef struct {
@@ -111,6 +185,48 @@  typedef struct {
 	uint16_t bitrate;
 } __attribute__ ((packed)) a2dp_mpeg_t;
 
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t frequency:4;
+	uint8_t channel_mode:4;
+} __attribute__ ((packed)) a2dp_aptx_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t direction;
+	uint8_t source_frequency:4;
+	uint8_t sink_frequency:4;
+} __attribute__ ((packed)) a2dp_faststream_t;
+
+typedef struct {
+	uint8_t reserved;
+	uint16_t target_codec_level;
+	uint16_t initial_codec_level;
+	uint8_t sra_max_rate;
+	uint8_t sra_avg_time;
+	uint16_t good_working_level;
+} __attribute__ ((packed)) a2dp_aptx_ll_new_caps_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t frequency:4;
+	uint8_t channel_mode:4;
+	uint8_t reserved:6;
+	uint8_t has_new_caps:1;
+	uint8_t bidirect_link:1;
+	a2dp_aptx_ll_new_caps_t new_caps[0];
+} __attribute__ ((packed)) a2dp_aptx_ll_t;
+
+typedef struct {
+	a2dp_vendor_codec_t info;
+	uint8_t frequency:4;
+	uint8_t channel_mode:4;
+	uint8_t reserved0;
+	uint8_t reserved1;
+	uint8_t reserved2;
+	uint8_t reserved3;
+} __attribute__ ((packed)) a2dp_aptx_hd_t;
+
 #else
 #error "Unknown byte order"
 #endif
diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
index 9c4e3367b..c139f7fc3 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -50,7 +50,9 @@ 
 #define BLUEZ_ERROR_NOT_SUPPORTED "org.bluez.Error.NotSupported"
 
 #define A2DP_SOURCE_SBC_ENDPOINT "/MediaEndpoint/A2DPSourceSBC"
+#define A2DP_SOURCE_APTX_ENDPOINT "/MediaEndpoint/A2DPSourceAPTX"
 #define A2DP_SINK_SBC_ENDPOINT "/MediaEndpoint/A2DPSinkSBC"
+#define A2DP_SINK_APTX_ENDPOINT "/MediaEndpoint/A2DPSinkAPTX"
 
 #define ENDPOINT_INTROSPECT_XML                                         \
     DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE                           \
@@ -173,8 +175,22 @@  static bool device_supports_profile(pa_bluetooth_device *device, pa_bluetooth_pr
     switch (profile) {
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
+#ifdef HAVE_OPENAPTX
+            /* TODO: Implement once bluez provides API to check if codec is supported */
+            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
+#else
+            return false;
+#endif
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
+#ifdef HAVE_OPENAPTX
+            /* TODO: Implement once bluez provides API to check if codec is supported */
+            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
+#else
+            return false;
+#endif
         case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
             return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS)
                 || !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS_ALT)
@@ -961,7 +977,9 @@  static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
             if (!a->valid)
                 return;
 
+            register_endpoint(y, path, A2DP_SOURCE_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
             register_endpoint(y, path, A2DP_SOURCE_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
+            register_endpoint(y, path, A2DP_SINK_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
             register_endpoint(y, path, A2DP_SINK_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
 
         } else if (pa_streq(interface, BLUEZ_DEVICE_INTERFACE)) {
@@ -1258,8 +1276,12 @@  const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
     switch(profile) {
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
             return "a2dp_sink";
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
+            return "a2dp_aptx_sink";
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
             return "a2dp_source";
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
+            return "a2dp_aptx_source";
         case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
             return "headset_head_unit";
         case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
@@ -1275,8 +1297,12 @@  const char *pa_bluetooth_profile_to_a2dp_endpoint(pa_bluetooth_profile_t profile
     switch (profile) {
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
             return A2DP_SOURCE_SBC_ENDPOINT;
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
+            return A2DP_SOURCE_APTX_ENDPOINT;
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
             return A2DP_SINK_SBC_ENDPOINT;
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
+            return A2DP_SINK_APTX_ENDPOINT;
         default:
             return NULL;
     }
@@ -1289,6 +1315,11 @@  const pa_a2dp_codec_t *pa_bluetooth_profile_to_a2dp_codec(pa_bluetooth_profile_t
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
         case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
             return &pa_a2dp_codec_sbc;
+#ifdef HAVE_OPENAPTX
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
+        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
+            return &pa_a2dp_codec_aptx;
+#endif
         default:
             return NULL;
     }
@@ -1299,6 +1330,10 @@  const pa_a2dp_codec_t *pa_bluetooth_profile_to_a2dp_codec(pa_bluetooth_profile_t
 const pa_a2dp_codec_t *pa_a2dp_endpoint_to_a2dp_codec(const char *endpoint) {
     if (pa_streq(endpoint, A2DP_SOURCE_SBC_ENDPOINT) || pa_streq(endpoint, A2DP_SINK_SBC_ENDPOINT))
         return &pa_a2dp_codec_sbc;
+#ifdef HAVE_OPENAPTX
+    else if (pa_streq(endpoint, A2DP_SOURCE_APTX_ENDPOINT) || pa_streq(endpoint, A2DP_SINK_APTX_ENDPOINT))
+        return &pa_a2dp_codec_aptx;
+#endif
     else
         return NULL;
 }
@@ -1359,9 +1394,15 @@  static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
             if (pa_streq(endpoint_path, A2DP_SOURCE_SBC_ENDPOINT)) {
                 if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
                     p = PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK;
+            } else if (pa_streq(endpoint_path, A2DP_SOURCE_APTX_ENDPOINT)) {
+                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
+                    p = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
             } else if (pa_streq(endpoint_path, A2DP_SINK_SBC_ENDPOINT)) {
                 if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
                     p = PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE;
+            } else if (pa_streq(endpoint_path, A2DP_SINK_APTX_ENDPOINT)) {
+                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
+                    p = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
             }
 
             if (p == PA_BLUETOOTH_PROFILE_OFF) {
@@ -1546,7 +1587,8 @@  static DBusHandlerResult endpoint_handler(DBusConnection *c, DBusMessage *m, voi
 
     pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface, member);
 
-    if (!pa_streq(path, A2DP_SOURCE_SBC_ENDPOINT) && !pa_streq(path, A2DP_SINK_SBC_ENDPOINT))
+    if (!pa_streq(path, A2DP_SOURCE_SBC_ENDPOINT) && !pa_streq(path, A2DP_SINK_SBC_ENDPOINT) &&
+        !pa_streq(path, A2DP_SOURCE_APTX_ENDPOINT) && !pa_streq(path, A2DP_SINK_APTX_ENDPOINT))
         return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
     if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable", "Introspect")) {
@@ -1652,7 +1694,9 @@  pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe
     }
     y->matches_added = true;
 
+    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
     endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
+    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
     endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);
 
     get_managed_objects(y);
@@ -1721,7 +1765,9 @@  void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
         if (y->filter_added)
             dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
 
+        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
         endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
+        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
         endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);
 
         pa_dbus_connection_unref(y->connection);
diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
index 365b9ef6f..29d862fe1 100644
--- a/src/modules/bluetooth/bluez5-util.h
+++ b/src/modules/bluetooth/bluez5-util.h
@@ -55,7 +55,9 @@  typedef enum pa_bluetooth_hook {
 
 typedef enum profile {
     PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK,
+    PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK,
     PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE,
+    PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE,
     PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
     PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
     PA_BLUETOOTH_PROFILE_OFF
diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
index e626e80e9..e8a07d067 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -706,7 +706,8 @@  static void transport_config_mtu(struct userdata *u) {
     if (u->sink) {
         pa_sink_set_max_request_within_thread(u->sink, u->write_block_size);
         pa_sink_set_fixed_latency_within_thread(u->sink,
-                                                (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK ?
+                                                ((u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK ||
+                                                  u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK) ?
                                                  FIXED_LATENCY_PLAYBACK_A2DP : FIXED_LATENCY_PLAYBACK_SCO) +
                                                 pa_bytes_to_usec(u->write_block_size, &u->sample_spec));
 
@@ -752,7 +753,7 @@  static void setup_stream(struct userdata *u) {
     if (setsockopt(u->stream_fd, SOL_SOCKET, SO_TIMESTAMP, &one, sizeof(one)) < 0)
         pa_log_warn("Failed to enable SO_TIMESTAMP: %s", pa_cstrerror(errno));
 
-    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
+    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK)
         update_buffer_size(u);
 
     pa_log_debug("Stream properly set up, we're ready to roll!");
@@ -925,6 +926,7 @@  static int add_source(struct userdata *u) {
     if (!u->transport_acquired)
         switch (u->profile) {
             case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
+            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
             case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
                 data.suspend_cause = PA_SUSPEND_USER;
                 break;
@@ -937,6 +939,7 @@  static int add_source(struct userdata *u) {
                     pa_assert_not_reached();
                 break;
             case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
+            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
             case PA_BLUETOOTH_PROFILE_OFF:
             default:
                 pa_assert_not_reached();
@@ -1111,8 +1114,10 @@  static int add_sink(struct userdata *u) {
                     pa_assert_not_reached();
                 break;
             case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
+            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
                 /* Profile switch should have failed */
             case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
+            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
             case PA_BLUETOOTH_PROFILE_OFF:
             default:
                 pa_assert_not_reached();
@@ -1145,7 +1150,7 @@  static void transport_config(struct userdata *u) {
         u->sample_spec.rate = 8000;
     } else {
         const pa_a2dp_codec_t *codec = pa_bluetooth_profile_to_a2dp_codec(u->profile);
-        bool is_sink = (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
+        bool is_sink = (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
         pa_assert(codec);
         pa_assert(u->transport);
         codec->init(is_sink ? &u->encoder_info : &u->decoder_info, &u->sample_spec, is_sink, u->transport->config, u->transport->config_size);
@@ -1169,7 +1174,7 @@  static int setup_transport(struct userdata *u) {
 
     u->transport = t;
 
-    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
+    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
         transport_acquire(u, true); /* In case of error, the sink/sources will be created suspended */
     else {
         int transport_error;
@@ -1188,7 +1193,9 @@  static int setup_transport(struct userdata *u) {
 static pa_direction_t get_profile_direction(pa_bluetooth_profile_t p) {
     static const pa_direction_t profile_direction[] = {
         [PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK] = PA_DIRECTION_OUTPUT,
+        [PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK] = PA_DIRECTION_OUTPUT,
         [PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE] = PA_DIRECTION_INPUT,
+        [PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE] = PA_DIRECTION_INPUT,
         [PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
         [PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
         [PA_BLUETOOTH_PROFILE_OFF] = 0
@@ -1520,7 +1527,7 @@  static int start_thread(struct userdata *u) {
         /* If we are in the headset role or the device is an a2dp source,
          * the source should not become default unless there is no other
          * sound device available. */
-        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
+        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE)
             u->source->priority = 1500;
 
         pa_source_put(u->source);
@@ -1745,6 +1752,18 @@  static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
         p = PA_CARD_PROFILE_DATA(cp);
         break;
 
+    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
+        cp = pa_card_profile_new(name, _("High Fidelity aptX Playback (A2DP Sink)"), sizeof(pa_bluetooth_profile_t));
+        cp->priority = 50;
+        cp->n_sinks = 1;
+        cp->n_sources = 0;
+        cp->max_sink_channels = 2;
+        cp->max_source_channels = 0;
+        pa_hashmap_put(output_port->profiles, cp->name, cp);
+
+        p = PA_CARD_PROFILE_DATA(cp);
+        break;
+
     case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
         cp = pa_card_profile_new(name, _("High Fidelity SBC Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 20;
@@ -1757,6 +1776,18 @@  static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
         p = PA_CARD_PROFILE_DATA(cp);
         break;
 
+    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
+        cp = pa_card_profile_new(name, _("High Fidelity aptX Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
+        cp->priority = 25;
+        cp->n_sinks = 0;
+        cp->n_sources = 1;
+        cp->max_sink_channels = 0;
+        cp->max_source_channels = 2;
+        pa_hashmap_put(input_port->profiles, cp->name, cp);
+
+        p = PA_CARD_PROFILE_DATA(cp);
+        break;
+
     case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
         cp = pa_card_profile_new(name, _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
         cp->priority = 30;
@@ -1840,8 +1871,10 @@  off:
 }
 
 static int uuid_to_profile(const char *uuid, pa_bluetooth_profile_t *_r) {
+    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
     if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
         *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK;
+    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
     else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
         *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE;
     else if (pa_bluetooth_uuid_is_hsp_hs(uuid) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF))
@@ -1907,6 +1940,28 @@  static int add_card(struct userdata *u) {
         pa_hashmap_put(data.profiles, cp->name, cp);
     }
 
+    PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
+        pa_bluetooth_profile_t profile;
+
+        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
+        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
+        if (uuid_to_profile(uuid, &profile) < 0)
+            continue;
+
+        /* Handle APTX */
+        if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
+            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
+        else if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
+            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
+        else
+            continue;
+
+        if (!pa_hashmap_get(data.profiles, pa_bluetooth_profile_to_string(profile))) {
+            cp = create_card_profile(u, profile, data.ports);
+            pa_hashmap_put(data.profiles, cp->name, cp);
+        }
+    }
+
     pa_assert(!pa_hashmap_isempty(data.profiles));
 
     cp = pa_card_profile_new("off", _("Off"), sizeof(pa_bluetooth_profile_t));
diff --git a/src/modules/bluetooth/pa-a2dp-codec-aptx.c b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
new file mode 100644
index 000000000..8ce1fc67c
--- /dev/null
+++ b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
@@ -0,0 +1,297 @@ 
+/***
+  This file is part of PulseAudio.
+
+  Copyright 2018 Pali Rohár <pali.rohar@gmail.com>
+
+  PulseAudio is free software; you can redistribute it and/or modify
+  it under the terms of the GNU Lesser General Public License as
+  published by the Free Software Foundation; either version 2.1 of the
+  License, or (at your option) any later version.
+
+  PulseAudio 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 Lesser General Public
+  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
+***/
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <pulsecore/log.h>
+#include <pulsecore/macro.h>
+#include <pulsecore/once.h>
+#include <pulse/sample.h>
+
+#include <openaptx.h>
+
+#include "a2dp-codecs.h"
+#include "pa-a2dp-codec.h"
+
+static size_t pa_aptx_fill_capabilities(uint8_t *capabilities_buffer, size_t capabilities_size) {
+    a2dp_aptx_t *capabilities = (a2dp_aptx_t *) capabilities_buffer;
+
+    if (capabilities_size < sizeof(*capabilities)) {
+        pa_log_error("Invalid size of capabilities buffer");
+        return 0;
+    }
+
+    pa_zero(*capabilities);
+
+    capabilities->info.vendor_id = A2DP_VENDOR_APT_LIC_LTD;
+    capabilities->info.codec_id = APT_LIC_LTD_CODEC_APTX;
+    capabilities->channel_mode = APTX_CHANNEL_MODE_STEREO;
+    capabilities->frequency = APTX_SAMPLING_FREQ_16000 | APTX_SAMPLING_FREQ_32000 | APTX_SAMPLING_FREQ_44100 |
+                             APTX_SAMPLING_FREQ_48000;
+
+    return sizeof(*capabilities);
+}
+
+static bool pa_aptx_validate_configuration(const uint8_t *config_buffer, size_t config_size) {
+    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
+
+    if (config_size != sizeof(*config)) {
+        pa_log_error("Invalid size of config buffer");
+        return false;
+    }
+
+    if (config->info.vendor_id != A2DP_VENDOR_APT_LIC_LTD || config->info.codec_id != APT_LIC_LTD_CODEC_APTX) {
+        pa_log_error("Invalid vendor codec information in configuration");
+        return false;
+    }
+
+    if (config->frequency != APTX_SAMPLING_FREQ_16000 && config->frequency != APTX_SAMPLING_FREQ_32000 &&
+        config->frequency != APTX_SAMPLING_FREQ_44100 && config->frequency != APTX_SAMPLING_FREQ_48000) {
+        pa_log_error("Invalid sampling frequency in configuration");
+        return false;
+    }
+
+    if (config->channel_mode != APTX_CHANNEL_MODE_STEREO) {
+        pa_log_error("Invalid channel mode in configuration");
+        return false;
+    }
+
+    return true;
+}
+
+static size_t pa_aptx_select_configuration(const pa_sample_spec *sample_spec, const uint8_t *capabilities_buffer, size_t capabilities_size, uint8_t *config_buffer, size_t config_size) {
+    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
+    a2dp_aptx_t *capabilities = (a2dp_aptx_t *) capabilities_buffer;
+    int i;
+
+    static const struct {
+        uint32_t rate;
+        uint8_t cap;
+    } freq_table[] = {
+        { 16000U, APTX_SAMPLING_FREQ_16000 },
+        { 32000U, APTX_SAMPLING_FREQ_32000 },
+        { 44100U, APTX_SAMPLING_FREQ_44100 },
+        { 48000U, APTX_SAMPLING_FREQ_48000 }
+    };
+
+    if (capabilities_size != sizeof(*capabilities)) {
+        pa_log_error("Invalid size of capabilities buffer");
+        return 0;
+    }
+
+    if (config_size < sizeof(*config)) {
+        pa_log_error("Invalid size of config buffer");
+        return 0;
+    }
+
+    pa_zero(*config);
+
+    if (capabilities->info.vendor_id != A2DP_VENDOR_APT_LIC_LTD || capabilities->info.codec_id != APT_LIC_LTD_CODEC_APTX) {
+        pa_log_error("No supported vendor codec information");
+        return 0;
+    }
+
+    config->info.vendor_id = A2DP_VENDOR_APT_LIC_LTD;
+    config->info.codec_id = APT_LIC_LTD_CODEC_APTX;
+
+    if (sample_spec->channels != 2 || !(capabilities->channel_mode & APTX_CHANNEL_MODE_STEREO)) {
+        pa_log_error("No supported channel modes");
+        return 0;
+    }
+
+    config->channel_mode = APTX_CHANNEL_MODE_STEREO;
+
+    /* Find the lowest freq that is at least as high as the requested sampling rate */
+    for (i = 0; (unsigned) i < PA_ELEMENTSOF(freq_table); i++) {
+        if (freq_table[i].rate >= sample_spec->rate && (capabilities->frequency & freq_table[i].cap)) {
+            config->frequency = freq_table[i].cap;
+            break;
+        }
+    }
+
+    if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
+        for (--i; i >= 0; i--) {
+            if (capabilities->frequency & freq_table[i].cap) {
+                config->frequency = freq_table[i].cap;
+                break;
+            }
+        }
+
+        if (i < 0) {
+            pa_log_error("Not suitable sample rate");
+            return 0;
+        }
+    }
+
+    return sizeof(*config);
+}
+
+static void pa_aptx_init(void **info_ptr, pa_sample_spec *sample_spec, bool is_a2dp_sink, const uint8_t *config_buffer, size_t config_size) {
+    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
+
+    pa_assert(config_size == sizeof(*config));
+
+    sample_spec->format = PA_SAMPLE_S24LE;
+
+    if (*info_ptr)
+        aptx_reset((struct aptx_context *) *info_ptr);
+    else
+        *info_ptr = (void *)aptx_init(0);
+
+    switch (config->frequency) {
+        case APTX_SAMPLING_FREQ_16000:
+            sample_spec->rate = 16000U;
+            break;
+        case APTX_SAMPLING_FREQ_32000:
+            sample_spec->rate = 32000U;
+            break;
+        case APTX_SAMPLING_FREQ_44100:
+            sample_spec->rate = 44100U;
+            break;
+        case APTX_SAMPLING_FREQ_48000:
+            sample_spec->rate = 48000U;
+            break;
+        default:
+            pa_assert_not_reached();
+    }
+
+    switch (config->channel_mode) {
+        case APTX_CHANNEL_MODE_STEREO:
+            sample_spec->channels = 2;
+            break;
+        default:
+            pa_assert_not_reached();
+    }
+}
+
+static void pa_aptx_finish(void **info_ptr) {
+    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
+
+    if (aptx_c) {
+        aptx_finish(aptx_c);
+        *info_ptr = NULL;
+    }
+}
+
+static void pa_aptx_setup(void **info_ptr) {
+}
+
+static void pa_aptx_fill_blocksize(void **info_ptr, size_t read_link_mtu, size_t write_link_mtu, size_t *read_block_size, size_t *write_block_size) {
+    *write_block_size = (write_link_mtu/(8*4)) * 8*4*6;
+    *read_block_size = (read_link_mtu/(8*4)) * 8*4*6;
+}
+
+static bool pa_aptx_fix_latency(void **info_ptr) {
+    return false;
+}
+
+static size_t pa_aptx_encode(void **info_ptr, uint32_t timestamp, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size) {
+    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
+    uint8_t *d;
+    const uint8_t *p;
+    size_t to_write, to_encode;
+
+    p = input_buffer;
+    to_encode = input_size;
+
+    d = output_buffer;
+    to_write = output_size;
+
+    while (PA_LIKELY(to_encode > 0 && to_write > 0)) {
+        size_t written;
+        size_t encoded;
+        encoded = aptx_encode(aptx_c, p, to_encode, d, to_write, &written);
+
+        if (PA_UNLIKELY(encoded == 0)) {
+            pa_log_error("aptX encoding error");
+            return 0;
+        }
+
+        pa_assert_fp((size_t) encoded <= to_encode);
+        pa_assert_fp((size_t) written <= to_write);
+
+        p += encoded;
+        to_encode -= encoded;
+
+        d += written;
+        to_write -= written;
+    }
+
+    pa_assert(to_encode == 0);
+
+    PA_ONCE_BEGIN {
+        pa_log_debug("Using aptX encoder implementation: libopenaptx from https://github.com/pali/libopenaptx");
+    } PA_ONCE_END;
+
+    return d - output_buffer;
+}
+
+static size_t pa_aptx_decode(void **info_ptr, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
+    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
+
+    const uint8_t *p;
+    uint8_t *d;
+    size_t to_write, to_decode;
+
+    p = input_buffer;
+    to_decode = input_size;
+
+    d = output_buffer;
+    to_write = output_size;
+
+    while (PA_LIKELY(to_decode > 0)) {
+        size_t written;
+        size_t decoded;
+
+        decoded = aptx_decode(aptx_c, p, to_decode, d, to_write, &written);
+
+        if (PA_UNLIKELY(decoded == 0)) {
+            pa_log_error("aptX decoding error");
+            *processed = p - input_buffer;
+            return 0;
+        }
+
+        pa_assert_fp((size_t) decoded <= to_decode);
+
+        p += decoded;
+        to_decode -= decoded;
+
+        d += written;
+        to_write -= written;
+    }
+
+    *processed = p - input_buffer;
+    return d - output_buffer;
+}
+
+const pa_a2dp_codec_t pa_a2dp_codec_aptx = {
+    .codec_id = A2DP_CODEC_VENDOR,
+    .fill_capabilities = pa_aptx_fill_capabilities,
+    .validate_configuration = pa_aptx_validate_configuration,
+    .select_configuration = pa_aptx_select_configuration,
+    .init = pa_aptx_init,
+    .finish = pa_aptx_finish,
+    .setup = pa_aptx_setup,
+    .fill_blocksize = pa_aptx_fill_blocksize,
+    .fix_latency = pa_aptx_fix_latency,
+    .encode = pa_aptx_encode,
+    .decode = pa_aptx_decode,
+};
diff --git a/src/modules/bluetooth/pa-a2dp-codec.h b/src/modules/bluetooth/pa-a2dp-codec.h
index 68b1619c2..26e7653a6 100644
--- a/src/modules/bluetooth/pa-a2dp-codec.h
+++ b/src/modules/bluetooth/pa-a2dp-codec.h
@@ -36,5 +36,6 @@  typedef struct pa_a2dp_codec {
 } pa_a2dp_codec_t;
 
 extern const pa_a2dp_codec_t pa_a2dp_codec_sbc;
+extern const pa_a2dp_codec_t pa_a2dp_codec_aptx;
 
 #endif

Comments

On Sat, 2018-07-28 at 17:34 +0200, Pali Rohár wrote:
> This patch provides support for aptX codec in bluetooth A2DP profile. In
> pulseaudio it is implemented as a new profile a2dp_aptx_sink. For aptX
> encoding it uses open source LGPLv2.1+ licensed libopenaptx library which
> can be found at https://github.com/pali/libopenaptx.
> 
> Limitations:
> 
> Codec selection (either SBC or aptX) is done by bluez itself and it does
> not provide API for switching codec. Therefore pulseaudio is not able to
> change codec and it is up to bluez if it decide to use aptX or not.
> 
> Only standard aptX codec is supported for now. Support for other variants
> like aptX HD, aptX Low Latency, FastStream may come up later.
> ---
>  configure.ac                                 |  19 ++
>  src/Makefile.am                              |   5 +
>  src/modules/bluetooth/a2dp-codecs.h          | 118 ++++++++++-
>  src/modules/bluetooth/bluez5-util.c          |  48 ++++-
>  src/modules/bluetooth/bluez5-util.h          |   2 +
>  src/modules/bluetooth/module-bluez5-device.c |  65 +++++-
>  src/modules/bluetooth/pa-a2dp-codec-aptx.c   | 297 +++++++++++++++++++++++++++
>  src/modules/bluetooth/pa-a2dp-codec.h        |   1 +
>  8 files changed, 548 insertions(+), 7 deletions(-)
>  create mode 100644 src/modules/bluetooth/pa-a2dp-codec-aptx.c
> 
> diff --git a/configure.ac b/configure.ac
> index d2bfab23b..c2d13fa53 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1094,6 +1094,23 @@ AC_SUBST(HAVE_BLUEZ_5_NATIVE_HEADSET)
>  AM_CONDITIONAL([HAVE_BLUEZ_5_NATIVE_HEADSET], [test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = x1])
>  AS_IF([test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = "x1"], AC_DEFINE([HAVE_BLUEZ_5_NATIVE_HEADSET], 1, [Bluez 5 native headset backend enabled]))
>  
> +#### Bluetooth A2DP aptX codec (optional) ###
> +
> +AC_ARG_ENABLE([aptx],
> +    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
> +
> +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
> +    [AC_CHECK_HEADER([openaptx.h],
> +        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
> +        [HAVE_OPENAPTX=0])])

Have you considered providing a .pc file? Now we have to hardcode the
openaptx specific CFLAGS and LIBADD for libbluez5-util. If you ever
need to add new flags, all openaptx users need to update their build
systems. Also, if the library is installed to a non-standard location,
the .pc file can set the -L and -I flags to point to the right place.

> +
> +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" = "xyes" && test "x$HAVE_OPENAPTX" = "x0"],
> +    [AC_MSG_ERROR([*** libopenaptx from https://github.com/pali/libopenaptx not found])])
> +
> +AC_SUBST(HAVE_OPENAPTX)
> +AM_CONDITIONAL([HAVE_OPENAPTX], [test "x$HAVE_OPENAPTX" = "x1"])
> +AS_IF([test "x$HAVE_OPENAPTX" = "x1"], AC_DEFINE([HAVE_OPENAPTX], 1, [Have openaptx codec library]))
> +
>  #### UDEV support (optional) ####
>  
>  AC_ARG_ENABLE([udev],
> @@ -1579,6 +1596,7 @@ AS_IF([test "x$HAVE_SYSTEMD_JOURNAL" = "x1"], ENABLE_SYSTEMD_JOURNAL=yes, ENABLE
>  AS_IF([test "x$HAVE_BLUEZ_5" = "x1"], ENABLE_BLUEZ_5=yes, ENABLE_BLUEZ_5=no)
>  AS_IF([test "x$HAVE_BLUEZ_5_OFONO_HEADSET" = "x1"], ENABLE_BLUEZ_5_OFONO_HEADSET=yes, ENABLE_BLUEZ_5_OFONO_HEADSET=no)
>  AS_IF([test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = "x1"], ENABLE_BLUEZ_5_NATIVE_HEADSET=yes, ENABLE_BLUEZ_5_NATIVE_HEADSET=no)
> +AS_IF([test "x$HAVE_OPENAPTX" = "x1"], ENABLE_APTX=yes, ENABLE_APTX=no)
>  AS_IF([test "x$HAVE_HAL_COMPAT" = "x1"], ENABLE_HAL_COMPAT=yes, ENABLE_HAL_COMPAT=no)
>  AS_IF([test "x$HAVE_TCPWRAP" = "x1"], ENABLE_TCPWRAP=yes, ENABLE_TCPWRAP=no)
>  AS_IF([test "x$HAVE_LIBSAMPLERATE" = "x1"], ENABLE_LIBSAMPLERATE="yes (DEPRECATED)", ENABLE_LIBSAMPLERATE=no)
> @@ -1637,6 +1655,7 @@ echo "
>        Enable BlueZ 5:              ${ENABLE_BLUEZ_5}
>          Enable ofono headsets:     ${ENABLE_BLUEZ_5_OFONO_HEADSET}
>          Enable native headsets:    ${ENABLE_BLUEZ_5_NATIVE_HEADSET}
> +        Enable aptX codec:         ${ENABLE_APTX}
>      Enable udev:                   ${ENABLE_UDEV}
>        Enable HAL->udev compat:     ${ENABLE_HAL_COMPAT}
>      Enable systemd
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 411b9e5e5..bbd797589 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -2136,6 +2136,11 @@ libbluez5_util_la_SOURCES += modules/bluetooth/pa-a2dp-codec-sbc.c
>  libbluez5_util_la_LIBADD += $(SBC_LIBS)
>  libbluez5_util_la_CFLAGS += $(SBC_CFLAGS)
>  
> +if HAVE_OPENAPTX
> +libbluez5_util_la_SOURCES += modules/bluetooth/pa-a2dp-codec-aptx.c
> +libbluez5_util_la_LIBADD += -lopenaptx
> +endif
> +
>  module_bluez5_discover_la_SOURCES = modules/bluetooth/module-bluez5-discover.c
>  module_bluez5_discover_la_LDFLAGS = $(MODULE_LDFLAGS)
>  module_bluez5_discover_la_LIBADD = $(MODULE_LIBADD) $(DBUS_LIBS) libbluez5-util.la
> diff --git a/src/modules/bluetooth/a2dp-codecs.h b/src/modules/bluetooth/a2dp-codecs.h
> index 004975586..0c3583434 100644
> --- a/src/modules/bluetooth/a2dp-codecs.h
> +++ b/src/modules/bluetooth/a2dp-codecs.h
> @@ -4,6 +4,7 @@
>   *
>   *  Copyright (C) 2006-2010  Nokia Corporation
>   *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
> + *  Copyright (C) 2018       Pali Rohár <pali.rohar@gmail.com>
>   *
>   *
>   *  This library is free software; you can redistribute it and/or
> @@ -24,7 +25,18 @@
>  #define A2DP_CODEC_SBC			0x00
>  #define A2DP_CODEC_MPEG12		0x01
>  #define A2DP_CODEC_MPEG24		0x02
> -#define A2DP_CODEC_ATRAC		0x03
> +#define A2DP_CODEC_ATRAC		0x04
> +#define A2DP_CODEC_VENDOR		0xFF
> +
> +#define A2DP_VENDOR_APT_LIC_LTD		0x0000004f
> +#define APT_LIC_LTD_CODEC_APTX		0x0001
> +
> +#define A2DP_VENDOR_QTIL		0x0000000a
> +#define QTIL_CODEC_FASTSTREAM		0x0001
> +#define QTIL_CODEC_APTX_LL		0x0002
> +
> +#define A2DP_VENDOR_QUALCOMM_TECH_INC	0x000000d7
> +#define QUALCOMM_TECH_INC_CODEC_APTX_HD	0x0024
>  
>  #define SBC_SAMPLING_FREQ_16000		(1 << 3)
>  #define SBC_SAMPLING_FREQ_32000		(1 << 2)
> @@ -66,6 +78,26 @@
>  #define MPEG_SAMPLING_FREQ_44100	(1 << 1)
>  #define MPEG_SAMPLING_FREQ_48000	1
>  
> +#define APTX_CHANNEL_MODE_MONO		0x1
> +#define APTX_CHANNEL_MODE_STEREO	0x2
> +
> +#define APTX_SAMPLING_FREQ_16000	0x8
> +#define APTX_SAMPLING_FREQ_32000	0x4
> +#define APTX_SAMPLING_FREQ_44100	0x2
> +#define APTX_SAMPLING_FREQ_48000	0x1
> +
> +#define FASTSTREAM_DIRECTION_SINK	0x1
> +#define FASTSTREAM_DIRECTION_SOURCE	0x2
> +
> +#define FASTSTREAM_SINK_SAMPLING_FREQ_44100	0x2
> +#define FASTSTREAM_SINK_SAMPLING_FREQ_48000	0x1
> +
> +#define FASTSTREAM_SOURCE_SAMPLING_FREQ_16000	0x2
> +
> +typedef struct {
> +	uint32_t vendor_id;
> +	uint16_t codec_id;
> +} __attribute__ ((packed)) a2dp_vendor_codec_t;
>  
>  #if __BYTE_ORDER == __LITTLE_ENDIAN
>  
> @@ -89,6 +121,48 @@ typedef struct {
>  	uint16_t bitrate;
>  } __attribute__ ((packed)) a2dp_mpeg_t;
>  
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t channel_mode:4;
> +	uint8_t frequency:4;
> +} __attribute__ ((packed)) a2dp_aptx_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t direction;
> +	uint8_t sink_frequency:4;
> +	uint8_t source_frequency:4;
> +} __attribute__ ((packed)) a2dp_faststream_t;
> +
> +typedef struct {
> +	uint8_t reserved;
> +	uint16_t target_codec_level;
> +	uint16_t initial_codec_level;
> +	uint8_t sra_max_rate;
> +	uint8_t sra_avg_time;
> +	uint16_t good_working_level;
> +} __attribute__ ((packed)) a2dp_aptx_ll_new_caps_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t channel_mode:4;
> +	uint8_t frequency:4;
> +	uint8_t bidirect_link:1;
> +	uint8_t has_new_caps:1;
> +	uint8_t reserved:6;
> +	a2dp_aptx_ll_new_caps_t new_caps[0];
> +} __attribute__ ((packed)) a2dp_aptx_ll_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t channel_mode:4;
> +	uint8_t frequency:4;
> +	uint8_t reserved0;
> +	uint8_t reserved1;
> +	uint8_t reserved2;
> +	uint8_t reserved3;
> +} __attribute__ ((packed)) a2dp_aptx_hd_t;
> +
>  #elif __BYTE_ORDER == __BIG_ENDIAN
>  
>  typedef struct {
> @@ -111,6 +185,48 @@ typedef struct {
>  	uint16_t bitrate;
>  } __attribute__ ((packed)) a2dp_mpeg_t;
>  
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t frequency:4;
> +	uint8_t channel_mode:4;
> +} __attribute__ ((packed)) a2dp_aptx_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t direction;
> +	uint8_t source_frequency:4;
> +	uint8_t sink_frequency:4;
> +} __attribute__ ((packed)) a2dp_faststream_t;
> +
> +typedef struct {
> +	uint8_t reserved;
> +	uint16_t target_codec_level;
> +	uint16_t initial_codec_level;
> +	uint8_t sra_max_rate;
> +	uint8_t sra_avg_time;
> +	uint16_t good_working_level;
> +} __attribute__ ((packed)) a2dp_aptx_ll_new_caps_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t frequency:4;
> +	uint8_t channel_mode:4;
> +	uint8_t reserved:6;
> +	uint8_t has_new_caps:1;
> +	uint8_t bidirect_link:1;
> +	a2dp_aptx_ll_new_caps_t new_caps[0];
> +} __attribute__ ((packed)) a2dp_aptx_ll_t;
> +
> +typedef struct {
> +	a2dp_vendor_codec_t info;
> +	uint8_t frequency:4;
> +	uint8_t channel_mode:4;
> +	uint8_t reserved0;
> +	uint8_t reserved1;
> +	uint8_t reserved2;
> +	uint8_t reserved3;
> +} __attribute__ ((packed)) a2dp_aptx_hd_t;
> +
>  #else
>  #error "Unknown byte order"
>  #endif
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 9c4e3367b..c139f7fc3 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -50,7 +50,9 @@
>  #define BLUEZ_ERROR_NOT_SUPPORTED "org.bluez.Error.NotSupported"
>  
>  #define A2DP_SOURCE_SBC_ENDPOINT "/MediaEndpoint/A2DPSourceSBC"
> +#define A2DP_SOURCE_APTX_ENDPOINT "/MediaEndpoint/A2DPSourceAPTX"
>  #define A2DP_SINK_SBC_ENDPOINT "/MediaEndpoint/A2DPSinkSBC"
> +#define A2DP_SINK_APTX_ENDPOINT "/MediaEndpoint/A2DPSinkAPTX"
>  
>  #define ENDPOINT_INTROSPECT_XML                                         \
>      DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE                           \
> @@ -173,8 +175,22 @@ static bool device_supports_profile(pa_bluetooth_device *device, pa_bluetooth_pr
>      switch (profile) {
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
>              return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> +#ifdef HAVE_OPENAPTX
> +            /* TODO: Implement once bluez provides API to check if codec is supported */
> +            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);

Is someone working on that API?

> +#else
> +            return false;
> +#endif
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
>              return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> +#ifdef HAVE_OPENAPTX
> +            /* TODO: Implement once bluez provides API to check if codec is supported */
> +            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> +#else
> +            return false;
> +#endif
>          case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
>              return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS)
>                  || !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_HSP_HS_ALT)
> @@ -961,7 +977,9 @@ static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
>              if (!a->valid)
>                  return;
>  
> +            register_endpoint(y, path, A2DP_SOURCE_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
>              register_endpoint(y, path, A2DP_SOURCE_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> +            register_endpoint(y, path, A2DP_SINK_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
>              register_endpoint(y, path, A2DP_SINK_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);

We shouldn't register aptX endpoints if aptX support is not enabled.

Does the registration order matter? I have some vague recollection from
the earlier discussion that BlueZ chooses the codec based on the
endpoint registration order - is that true? If the order is important,
we should document that here.

>  
>          } else if (pa_streq(interface, BLUEZ_DEVICE_INTERFACE)) {
> @@ -1258,8 +1276,12 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
>      switch(profile) {
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
>              return "a2dp_sink";
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> +            return "a2dp_aptx_sink";
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
>              return "a2dp_source";
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> +            return "a2dp_aptx_source";
>          case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
>              return "headset_head_unit";
>          case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
> @@ -1275,8 +1297,12 @@ const char *pa_bluetooth_profile_to_a2dp_endpoint(pa_bluetooth_profile_t profile
>      switch (profile) {
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
>              return A2DP_SOURCE_SBC_ENDPOINT;
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> +            return A2DP_SOURCE_APTX_ENDPOINT;
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
>              return A2DP_SINK_SBC_ENDPOINT;
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> +            return A2DP_SINK_APTX_ENDPOINT;
>          default:
>              return NULL;
>      }
> @@ -1289,6 +1315,11 @@ const pa_a2dp_codec_t *pa_bluetooth_profile_to_a2dp_codec(pa_bluetooth_profile_t
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
>          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
>              return &pa_a2dp_codec_sbc;
> +#ifdef HAVE_OPENAPTX
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> +            return &pa_a2dp_codec_aptx;
> +#endif
>          default:
>              return NULL;
>      }
> @@ -1299,6 +1330,10 @@ const pa_a2dp_codec_t *pa_bluetooth_profile_to_a2dp_codec(pa_bluetooth_profile_t
>  const pa_a2dp_codec_t *pa_a2dp_endpoint_to_a2dp_codec(const char *endpoint) {
>      if (pa_streq(endpoint, A2DP_SOURCE_SBC_ENDPOINT) || pa_streq(endpoint, A2DP_SINK_SBC_ENDPOINT))
>          return &pa_a2dp_codec_sbc;
> +#ifdef HAVE_OPENAPTX
> +    else if (pa_streq(endpoint, A2DP_SOURCE_APTX_ENDPOINT) || pa_streq(endpoint, A2DP_SINK_APTX_ENDPOINT))
> +        return &pa_a2dp_codec_aptx;
> +#endif
>      else
>          return NULL;
>  }
> @@ -1359,9 +1394,15 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>              if (pa_streq(endpoint_path, A2DP_SOURCE_SBC_ENDPOINT)) {
>                  if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
>                      p = PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK;
> +            } else if (pa_streq(endpoint_path, A2DP_SOURCE_APTX_ENDPOINT)) {
> +                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
> +                    p = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
>              } else if (pa_streq(endpoint_path, A2DP_SINK_SBC_ENDPOINT)) {
>                  if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
>                      p = PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE;
> +            } else if (pa_streq(endpoint_path, A2DP_SINK_APTX_ENDPOINT)) {
> +                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
> +                    p = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
>              }
>  
>              if (p == PA_BLUETOOTH_PROFILE_OFF) {
> @@ -1546,7 +1587,8 @@ static DBusHandlerResult endpoint_handler(DBusConnection *c, DBusMessage *m, voi
>  
>      pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface, member);
>  
> -    if (!pa_streq(path, A2DP_SOURCE_SBC_ENDPOINT) && !pa_streq(path, A2DP_SINK_SBC_ENDPOINT))
> +    if (!pa_streq(path, A2DP_SOURCE_SBC_ENDPOINT) && !pa_streq(path, A2DP_SINK_SBC_ENDPOINT) &&
> +        !pa_streq(path, A2DP_SOURCE_APTX_ENDPOINT) && !pa_streq(path, A2DP_SINK_APTX_ENDPOINT))
>          return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>  
>      if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable", "Introspect")) {
> @@ -1652,7 +1694,9 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe
>      }
>      y->matches_added = true;
>  
> +    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
>      endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> +    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
>      endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);

We shouldn't set up endpoints for aptX if the aptX support is not
enabled.

>  
>      get_managed_objects(y);
> @@ -1721,7 +1765,9 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
>          if (y->filter_added)
>              dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
>  
> +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
>          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
>          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);

If endpoint_init() is made conditional, these need to be made
conditional too (according to the dbus docs, unregistering an object
path that hasn't been registered first is considered an application
bug).

>  
>          pa_dbus_connection_unref(y->connection);
> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> index 365b9ef6f..29d862fe1 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -55,7 +55,9 @@ typedef enum pa_bluetooth_hook {
>  
>  typedef enum profile {
>      PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK,
> +    PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK,
>      PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE,
> +    PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE,
>      PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
>      PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
>      PA_BLUETOOTH_PROFILE_OFF
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index e626e80e9..e8a07d067 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -706,7 +706,8 @@ static void transport_config_mtu(struct userdata *u) {
>      if (u->sink) {
>          pa_sink_set_max_request_within_thread(u->sink, u->write_block_size);
>          pa_sink_set_fixed_latency_within_thread(u->sink,
> -                                                (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK ?
> +                                                ((u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK ||
> +                                                  u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK) ?
>                                                   FIXED_LATENCY_PLAYBACK_A2DP : FIXED_LATENCY_PLAYBACK_SCO) +
>                                                  pa_bytes_to_usec(u->write_block_size, &u->sample_spec));
>  
> @@ -752,7 +753,7 @@ static void setup_stream(struct userdata *u) {
>      if (setsockopt(u->stream_fd, SOL_SOCKET, SO_TIMESTAMP, &one, sizeof(one)) < 0)
>          pa_log_warn("Failed to enable SO_TIMESTAMP: %s", pa_cstrerror(errno));
>  
> -    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
> +    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK)
>          update_buffer_size(u);
>  
>      pa_log_debug("Stream properly set up, we're ready to roll!");
> @@ -925,6 +926,7 @@ static int add_source(struct userdata *u) {
>      if (!u->transport_acquired)
>          switch (u->profile) {
>              case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
> +            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
>              case PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY:
>                  data.suspend_cause = PA_SUSPEND_USER;
>                  break;
> @@ -937,6 +939,7 @@ static int add_source(struct userdata *u) {
>                      pa_assert_not_reached();
>                  break;
>              case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
> +            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
>              case PA_BLUETOOTH_PROFILE_OFF:
>              default:
>                  pa_assert_not_reached();
> @@ -1111,8 +1114,10 @@ static int add_sink(struct userdata *u) {
>                      pa_assert_not_reached();
>                  break;
>              case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
> +            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
>                  /* Profile switch should have failed */
>              case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
> +            case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
>              case PA_BLUETOOTH_PROFILE_OFF:
>              default:
>                  pa_assert_not_reached();
> @@ -1145,7 +1150,7 @@ static void transport_config(struct userdata *u) {
>          u->sample_spec.rate = 8000;
>      } else {
>          const pa_a2dp_codec_t *codec = pa_bluetooth_profile_to_a2dp_codec(u->profile);
> -        bool is_sink = (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> +        bool is_sink = (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
>          pa_assert(codec);
>          pa_assert(u->transport);
>          codec->init(is_sink ? &u->encoder_info : &u->decoder_info, &u->sample_spec, is_sink, u->transport->config, u->transport->config_size);
> @@ -1169,7 +1174,7 @@ static int setup_transport(struct userdata *u) {
>  
>      u->transport = t;
>  
> -    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
> +    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
>          transport_acquire(u, true); /* In case of error, the sink/sources will be created suspended */
>      else {
>          int transport_error;
> @@ -1188,7 +1193,9 @@ static int setup_transport(struct userdata *u) {
>  static pa_direction_t get_profile_direction(pa_bluetooth_profile_t p) {
>      static const pa_direction_t profile_direction[] = {
>          [PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK] = PA_DIRECTION_OUTPUT,
> +        [PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK] = PA_DIRECTION_OUTPUT,
>          [PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE] = PA_DIRECTION_INPUT,
> +        [PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE] = PA_DIRECTION_INPUT,
>          [PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
>          [PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY] = PA_DIRECTION_INPUT | PA_DIRECTION_OUTPUT,
>          [PA_BLUETOOTH_PROFILE_OFF] = 0
> @@ -1520,7 +1527,7 @@ static int start_thread(struct userdata *u) {
>          /* If we are in the headset role or the device is an a2dp source,
>           * the source should not become default unless there is no other
>           * sound device available. */
> -        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
> +        if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE || u->profile == PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE)
>              u->source->priority = 1500;
>  
>          pa_source_put(u->source);
> @@ -1745,6 +1752,18 @@ static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
>          p = PA_CARD_PROFILE_DATA(cp);
>          break;
>  
> +    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> +        cp = pa_card_profile_new(name, _("High Fidelity aptX Playback (A2DP Sink)"), sizeof(pa_bluetooth_profile_t));
> +        cp->priority = 50;
> +        cp->n_sinks = 1;
> +        cp->n_sources = 0;
> +        cp->max_sink_channels = 2;
> +        cp->max_source_channels = 0;
> +        pa_hashmap_put(output_port->profiles, cp->name, cp);
> +
> +        p = PA_CARD_PROFILE_DATA(cp);
> +        break;
> +
>      case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
>          cp = pa_card_profile_new(name, _("High Fidelity SBC Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
>          cp->priority = 20;
> @@ -1757,6 +1776,18 @@ static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
>          p = PA_CARD_PROFILE_DATA(cp);
>          break;
>  
> +    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> +        cp = pa_card_profile_new(name, _("High Fidelity aptX Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
> +        cp->priority = 25;
> +        cp->n_sinks = 0;
> +        cp->n_sources = 1;
> +        cp->max_sink_channels = 0;
> +        cp->max_source_channels = 2;
> +        pa_hashmap_put(input_port->profiles, cp->name, cp);
> +
> +        p = PA_CARD_PROFILE_DATA(cp);
> +        break;
> +

My compiler started to be unsure whether p is always initialized:

  CC       modules/bluetooth/module_bluez5_device_la-module-bluez5-device.lo
modules/bluetooth/module-bluez5-device.c: In function ‘create_card_profile’:
modules/bluetooth/module-bluez5-device.c:1821:8: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     *p = profile;
     ~~~^~~~~~~~~

This is a false positive, but we should do something to make the
compiler shut up.

>      case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
>          cp = pa_card_profile_new(name, _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
>          cp->priority = 30;
> @@ -1840,8 +1871,10 @@ off:
>  }
>  
>  static int uuid_to_profile(const char *uuid, pa_bluetooth_profile_t *_r) {
> +    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
>      if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
>          *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK;
> +    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
>      else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
>          *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE;
>      else if (pa_bluetooth_uuid_is_hsp_hs(uuid) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF))
> @@ -1907,6 +1940,28 @@ static int add_card(struct userdata *u) {
>          pa_hashmap_put(data.profiles, cp->name, cp);
>      }
>  
> +    PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
> +        pa_bluetooth_profile_t profile;
> +
> +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
> +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
> +        if (uuid_to_profile(uuid, &profile) < 0)
> +            continue;
> +
> +        /* Handle APTX */
> +        if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
> +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
> +        else if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
> +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
> +        else
> +            continue;
> +
> +        if (!pa_hashmap_get(data.profiles, pa_bluetooth_profile_to_string(profile))) {
> +            cp = create_card_profile(u, profile, data.ports);
> +            pa_hashmap_put(data.profiles, cp->name, cp);
> +        }
> +    }
> +

We shouldn't create the card profile if aptX support is disabled.

>      pa_assert(!pa_hashmap_isempty(data.profiles));
>  
>      cp = pa_card_profile_new("off", _("Off"), sizeof(pa_bluetooth_profile_t));
> diff --git a/src/modules/bluetooth/pa-a2dp-codec-aptx.c b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
> new file mode 100644
> index 000000000..8ce1fc67c
> --- /dev/null
> +++ b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
> @@ -0,0 +1,297 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2018 Pali Rohár <pali.rohar@gmail.com>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio 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 Lesser General Public
> +  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulsecore/log.h>
> +#include <pulsecore/macro.h>
> +#include <pulsecore/once.h>
> +#include <pulse/sample.h>
> +
> +#include <openaptx.h>
> +
> +#include "a2dp-codecs.h"
> +#include "pa-a2dp-codec.h"
> +
> +static size_t pa_aptx_fill_capabilities(uint8_t *capabilities_buffer, size_t capabilities_size) {
> +    a2dp_aptx_t *capabilities = (a2dp_aptx_t *) capabilities_buffer;
> +
> +    if (capabilities_size < sizeof(*capabilities)) {
> +        pa_log_error("Invalid size of capabilities buffer");
> +        return 0;
> +    }
> +
> +    pa_zero(*capabilities);
> +
> +    capabilities->info.vendor_id = A2DP_VENDOR_APT_LIC_LTD;
> +    capabilities->info.codec_id = APT_LIC_LTD_CODEC_APTX;
> +    capabilities->channel_mode = APTX_CHANNEL_MODE_STEREO;
> +    capabilities->frequency = APTX_SAMPLING_FREQ_16000 | APTX_SAMPLING_FREQ_32000 | APTX_SAMPLING_FREQ_44100 |
> +                             APTX_SAMPLING_FREQ_48000;
> +
> +    return sizeof(*capabilities);
> +}
> +
> +static bool pa_aptx_validate_configuration(const uint8_t *config_buffer, size_t config_size) {
> +    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
> +
> +    if (config_size != sizeof(*config)) {
> +        pa_log_error("Invalid size of config buffer");
> +        return false;
> +    }
> +
> +    if (config->info.vendor_id != A2DP_VENDOR_APT_LIC_LTD || config->info.codec_id != APT_LIC_LTD_CODEC_APTX) {
> +        pa_log_error("Invalid vendor codec information in configuration");
> +        return false;
> +    }
> +
> +    if (config->frequency != APTX_SAMPLING_FREQ_16000 && config->frequency != APTX_SAMPLING_FREQ_32000 &&
> +        config->frequency != APTX_SAMPLING_FREQ_44100 && config->frequency != APTX_SAMPLING_FREQ_48000) {
> +        pa_log_error("Invalid sampling frequency in configuration");
> +        return false;
> +    }
> +
> +    if (config->channel_mode != APTX_CHANNEL_MODE_STEREO) {
> +        pa_log_error("Invalid channel mode in configuration");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static size_t pa_aptx_select_configuration(const pa_sample_spec *sample_spec, const uint8_t *capabilities_buffer, size_t capabilities_size, uint8_t *config_buffer, size_t config_size) {
> +    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
> +    a2dp_aptx_t *capabilities = (a2dp_aptx_t *) capabilities_buffer;
> +    int i;
> +
> +    static const struct {
> +        uint32_t rate;
> +        uint8_t cap;
> +    } freq_table[] = {
> +        { 16000U, APTX_SAMPLING_FREQ_16000 },
> +        { 32000U, APTX_SAMPLING_FREQ_32000 },
> +        { 44100U, APTX_SAMPLING_FREQ_44100 },
> +        { 48000U, APTX_SAMPLING_FREQ_48000 }
> +    };
> +
> +    if (capabilities_size != sizeof(*capabilities)) {
> +        pa_log_error("Invalid size of capabilities buffer");
> +        return 0;
> +    }
> +
> +    if (config_size < sizeof(*config)) {
> +        pa_log_error("Invalid size of config buffer");
> +        return 0;
> +    }
> +
> +    pa_zero(*config);
> +
> +    if (capabilities->info.vendor_id != A2DP_VENDOR_APT_LIC_LTD || capabilities->info.codec_id != APT_LIC_LTD_CODEC_APTX) {
> +        pa_log_error("No supported vendor codec information");
> +        return 0;
> +    }
> +
> +    config->info.vendor_id = A2DP_VENDOR_APT_LIC_LTD;
> +    config->info.codec_id = APT_LIC_LTD_CODEC_APTX;
> +
> +    if (sample_spec->channels != 2 || !(capabilities->channel_mode & APTX_CHANNEL_MODE_STEREO)) {
> +        pa_log_error("No supported channel modes");
> +        return 0;
> +    }

sample_spec->channels is the global default channel count, and it's
used only as a hint for choosing the preferable channel mode. Since we
support only one channel mode, we can ignore sample_spec->channels
altogether.

> +
> +    config->channel_mode = APTX_CHANNEL_MODE_STEREO;
> +
> +    /* Find the lowest freq that is at least as high as the requested sampling rate */
> +    for (i = 0; (unsigned) i < PA_ELEMENTSOF(freq_table); i++) {
> +        if (freq_table[i].rate >= sample_spec->rate && (capabilities->frequency & freq_table[i].cap)) {
> +            config->frequency = freq_table[i].cap;
> +            break;
> +        }
> +    }
> +
> +    if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
> +        for (--i; i >= 0; i--) {
> +            if (capabilities->frequency & freq_table[i].cap) {
> +                config->frequency = freq_table[i].cap;
> +                break;
> +            }
> +        }
> +
> +        if (i < 0) {
> +            pa_log_error("Not suitable sample rate");
> +            return 0;
> +        }
> +    }
> +
> +    return sizeof(*config);
> +}
> +
> +static void pa_aptx_init(void **info_ptr, pa_sample_spec *sample_spec, bool is_a2dp_sink, const uint8_t *config_buffer, size_t config_size) {
> +    a2dp_aptx_t *config = (a2dp_aptx_t *) config_buffer;
> +
> +    pa_assert(config_size == sizeof(*config));
> +
> +    sample_spec->format = PA_SAMPLE_S24LE;
> +
> +    if (*info_ptr)
> +        aptx_reset((struct aptx_context *) *info_ptr);
> +    else
> +        *info_ptr = (void *)aptx_init(0);
> +
> +    switch (config->frequency) {
> +        case APTX_SAMPLING_FREQ_16000:
> +            sample_spec->rate = 16000U;
> +            break;
> +        case APTX_SAMPLING_FREQ_32000:
> +            sample_spec->rate = 32000U;
> +            break;
> +        case APTX_SAMPLING_FREQ_44100:
> +            sample_spec->rate = 44100U;
> +            break;
> +        case APTX_SAMPLING_FREQ_48000:
> +            sample_spec->rate = 48000U;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    switch (config->channel_mode) {
> +        case APTX_CHANNEL_MODE_STEREO:
> +            sample_spec->channels = 2;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +}
> +
> +static void pa_aptx_finish(void **info_ptr) {
> +    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
> +
> +    if (aptx_c) {
> +        aptx_finish(aptx_c);
> +        *info_ptr = NULL;
> +    }
> +}
> +
> +static void pa_aptx_setup(void **info_ptr) {
> +}
> +
> +static void pa_aptx_fill_blocksize(void **info_ptr, size_t read_link_mtu, size_t write_link_mtu, size_t *read_block_size, size_t *write_block_size) {
> +    *write_block_size = (write_link_mtu/(8*4)) * 8*4*6;
> +    *read_block_size = (read_link_mtu/(8*4)) * 8*4*6;

Please add comments that explain the math here.

> +}
> +
> +static bool pa_aptx_fix_latency(void **info_ptr) {
> +    return false;
> +}
> +
> +static size_t pa_aptx_encode(void **info_ptr, uint32_t timestamp, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size) {
> +    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
> +    uint8_t *d;
> +    const uint8_t *p;
> +    size_t to_write, to_encode;
> +
> +    p = input_buffer;
> +    to_encode = input_size;
> +
> +    d = output_buffer;
> +    to_write = output_size;
> +
> +    while (PA_LIKELY(to_encode > 0 && to_write > 0)) {
> +        size_t written;
> +        size_t encoded;
> +        encoded = aptx_encode(aptx_c, p, to_encode, d, to_write, &written);
> +
> +        if (PA_UNLIKELY(encoded == 0)) {
> +            pa_log_error("aptX encoding error");
> +            return 0;
> +        }
> +
> +        pa_assert_fp((size_t) encoded <= to_encode);
> +        pa_assert_fp((size_t) written <= to_write);
> +
> +        p += encoded;
> +        to_encode -= encoded;
> +
> +        d += written;
> +        to_write -= written;
> +    }
> +
> +    pa_assert(to_encode == 0);
> +
> +    PA_ONCE_BEGIN {
> +        pa_log_debug("Using aptX encoder implementation: libopenaptx from https://github.com/pali/libopenaptx");
> +    } PA_ONCE_END;
> +
> +    return d - output_buffer;
> +}
> +
> +static size_t pa_aptx_decode(void **info_ptr, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
> +    struct aptx_context *aptx_c = (struct aptx_context *) *info_ptr;
> +
> +    const uint8_t *p;
> +    uint8_t *d;
> +    size_t to_write, to_decode;
> +
> +    p = input_buffer;
> +    to_decode = input_size;
> +
> +    d = output_buffer;
> +    to_write = output_size;
> +
> +    while (PA_LIKELY(to_decode > 0)) {
> +        size_t written;
> +        size_t decoded;
> +
> +        decoded = aptx_decode(aptx_c, p, to_decode, d, to_write, &written);
> +
> +        if (PA_UNLIKELY(decoded == 0)) {
> +            pa_log_error("aptX decoding error");
> +            *processed = p - input_buffer;
> +            return 0;
> +        }
> +
> +        pa_assert_fp((size_t) decoded <= to_decode);
> +
> +        p += decoded;
> +        to_decode -= decoded;
> +
> +        d += written;
> +        to_write -= written;
> +    }
> +
> +    *processed = p - input_buffer;
> +    return d - output_buffer;
> +}
> +
> +const pa_a2dp_codec_t pa_a2dp_codec_aptx = {
> +    .codec_id = A2DP_CODEC_VENDOR,
> +    .fill_capabilities = pa_aptx_fill_capabilities,
> +    .validate_configuration = pa_aptx_validate_configuration,
> +    .select_configuration = pa_aptx_select_configuration,
> +    .init = pa_aptx_init,
> +    .finish = pa_aptx_finish,
> +    .setup = pa_aptx_setup,
> +    .fill_blocksize = pa_aptx_fill_blocksize,
> +    .fix_latency = pa_aptx_fix_latency,
> +    .encode = pa_aptx_encode,
> +    .decode = pa_aptx_decode,
> +};
> diff --git a/src/modules/bluetooth/pa-a2dp-codec.h b/src/modules/bluetooth/pa-a2dp-codec.h
> index 68b1619c2..26e7653a6 100644
> --- a/src/modules/bluetooth/pa-a2dp-codec.h
> +++ b/src/modules/bluetooth/pa-a2dp-codec.h
> @@ -36,5 +36,6 @@ typedef struct pa_a2dp_codec {
>  } pa_a2dp_codec_t;
>  
>  extern const pa_a2dp_codec_t pa_a2dp_codec_sbc;
> +extern const pa_a2dp_codec_t pa_a2dp_codec_aptx;
>  
>  #endif
On Thu, 2018-09-13 at 11:12 +0200, Pali Rohár wrote:
> On Wednesday 05 September 2018 13:57:08 Tanu Kaskinen wrote:
> > > +#### Bluetooth A2DP aptX codec (optional) ###
> > > +
> > > +AC_ARG_ENABLE([aptx],
> > > +    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
> > > +
> > > +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
> > > +    [AC_CHECK_HEADER([openaptx.h],
> > > +        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
> > > +        [HAVE_OPENAPTX=0])])
> > 
> > Have you considered providing a .pc file? Now we have to hardcode the
> > openaptx specific CFLAGS and LIBADD for libbluez5-util. If you ever
> > need to add new flags, all openaptx users need to update their build
> > systems. Also, if the library is installed to a non-standard location,
> > the .pc file can set the -L and -I flags to point to the right place.
> 
> Intension is that library is small and does not need any special cflags
> or ldflags. So .pc file is not needed at all. And if library or include
> file is in non-standard location then user really need to specify where
> it is. But same argument can be used when .pc file is in non-standard
> location. User again need to do some magic.
> 
> > > +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> > > +#ifdef HAVE_OPENAPTX
> > > +            /* TODO: Implement once bluez provides API to check if codec is supported */
> > > +            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
> > 
> > Is someone working on that API?
> 
> I do not know.
> 
> > > +            register_endpoint(y, path, A2DP_SOURCE_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> > >              register_endpoint(y, path, A2DP_SOURCE_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> > > +            register_endpoint(y, path, A2DP_SINK_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
> > >              register_endpoint(y, path, A2DP_SINK_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
> > 
> > We shouldn't register aptX endpoints if aptX support is not enabled.
> 
> I thought that it would be better to not wrap above lines in another
> #ifdef, but rather function register_endpoint would do nothing when
> particular codec (e.g. aptX) was not enabled at compile time.

I thought that we'd end up telling bluetoothd that we support aptX when
we don't, but since register_endpoint() returns early, that's not an
issue. I still think #ifdefs are preferable here, since it makes it
obvious that the register_endpoint() calls don't do anything, but
that's pretty minor thing.

> > Does the registration order matter? I have some vague recollection from
> > the earlier discussion that BlueZ chooses the codec based on the
> > endpoint registration order - is that true? If the order is important,
> > we should document that here.
> 
> Yes, order is important. Bluez based on registration order choose codec.
> 
> > > @@ -1721,7 +1765,9 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
> > >          if (y->filter_added)
> > >              dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
> > >  
> > > +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
> > >          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> > > +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
> > >          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);
> > 
> > If endpoint_init() is made conditional, these need to be made
> > conditional too (according to the dbus docs, unregistering an object
> > path that hasn't been registered first is considered an application
> > bug).
> 
> ok.
> 
> > > +    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> > > +        cp = pa_card_profile_new(name, _("High Fidelity aptX Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
> > > +        cp->priority = 25;
> > > +        cp->n_sinks = 0;
> > > +        cp->n_sources = 1;
> > > +        cp->max_sink_channels = 0;
> > > +        cp->max_source_channels = 2;
> > > +        pa_hashmap_put(input_port->profiles, cp->name, cp);
> > > +
> > > +        p = PA_CARD_PROFILE_DATA(cp);
> > > +        break;
> > > +
> > 
> > My compiler started to be unsure whether p is always initialized:
> > 
> >   CC       modules/bluetooth/module_bluez5_device_la-module-bluez5-device.lo
> > modules/bluetooth/module-bluez5-device.c: In function ‘create_card_profile’:
> > modules/bluetooth/module-bluez5-device.c:1821:8: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> >      *p = profile;
> >      ~~~^~~~~~~~~
> > 
> > This is a false positive, but we should do something to make the
> > compiler shut up.
> 
> What is the preferred way?

I suggest intializing p to NULL at the beginning of the function and
having pa_assert(p) before the *p = profile line.

> 
> > >      case PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT:
> > >          cp = pa_card_profile_new(name, _("Headset Head Unit (HSP/HFP)"), sizeof(pa_bluetooth_profile_t));
> > >          cp->priority = 30;
> > > @@ -1840,8 +1871,10 @@ off:
> > >  }
> > >  
> > >  static int uuid_to_profile(const char *uuid, pa_bluetooth_profile_t *_r) {
> > > +    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
> > >      if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
> > >          *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK;
> > > +    /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
> > >      else if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
> > >          *_r = PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE;
> > >      else if (pa_bluetooth_uuid_is_hsp_hs(uuid) || pa_streq(uuid, PA_BLUETOOTH_UUID_HFP_HF))
> > > @@ -1907,6 +1940,28 @@ static int add_card(struct userdata *u) {
> > >          pa_hashmap_put(data.profiles, cp->name, cp);
> > >      }
> > >  
> > > +    PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
> > > +        pa_bluetooth_profile_t profile;
> > > +
> > > +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
> > > +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
> > > +        if (uuid_to_profile(uuid, &profile) < 0)
> > > +            continue;
> > > +
> > > +        /* Handle APTX */
> > > +        if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
> > > +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
> > > +        else if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
> > > +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
> > > +        else
> > > +            continue;
> > > +
> > > +        if (!pa_hashmap_get(data.profiles, pa_bluetooth_profile_to_string(profile))) {
> > > +            cp = create_card_profile(u, profile, data.ports);
> > > +            pa_hashmap_put(data.profiles, cp->name, cp);
> > > +        }
> > > +    }
> > > +
> > 
> > We shouldn't create the card profile if aptX support is disabled.
> 
> Ok.
> 
> > > +    if (sample_spec->channels != 2 || !(capabilities->channel_mode & APTX_CHANNEL_MODE_STEREO)) {
> > > +        pa_log_error("No supported channel modes");
> > > +        return 0;
> > > +    }
> > 
> > sample_spec->channels is the global default channel count, and it's
> > used only as a hint for choosing the preferable channel mode. Since we
> > support only one channel mode, we can ignore sample_spec->channels
> > altogether.
> 
> Ok.
> 
> > > +static void pa_aptx_fill_blocksize(void **info_ptr, size_t read_link_mtu, size_t write_link_mtu, size_t *read_block_size, size_t *write_block_size) {
> > > +    *write_block_size = (write_link_mtu/(8*4)) * 8*4*6;
> > > +    *read_block_size = (read_link_mtu/(8*4)) * 8*4*6;
> > 
> > Please add comments that explain the math here.
> 
> Ok.
> 
> Input is sequence of 4 raw 24bit signed stereo samples. And at one time
> we need to process multiple of 8 due to synchronization as aptX uses 3
> bits in every eight sequence for checksum. aptX compression ratio is
> fixed 6:1. Therefore 8*4*6 bytes are on input and 8*4 bytes on output.

Thanks! Makes sense.
Hi Pali,

On Mon, Sep 17, 2018 at 3:27 PM, Tanu Kaskinen <tanuk@iki.fi> wrote:
> On Thu, 2018-09-13 at 11:12 +0200, Pali Rohár wrote:
>> On Wednesday 05 September 2018 13:57:08 Tanu Kaskinen wrote:
>> > > +#### Bluetooth A2DP aptX codec (optional) ###
>> > > +
>> > > +AC_ARG_ENABLE([aptx],
>> > > +    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
>> > > +
>> > > +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
>> > > +    [AC_CHECK_HEADER([openaptx.h],
>> > > +        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
>> > > +        [HAVE_OPENAPTX=0])])
>> >
>> > Have you considered providing a .pc file? Now we have to hardcode the
>> > openaptx specific CFLAGS and LIBADD for libbluez5-util. If you ever
>> > need to add new flags, all openaptx users need to update their build
>> > systems. Also, if the library is installed to a non-standard location,
>> > the .pc file can set the -L and -I flags to point to the right place.
>>
>> Intension is that library is small and does not need any special cflags
>> or ldflags. So .pc file is not needed at all. And if library or include
>> file is in non-standard location then user really need to specify where
>> it is. But same argument can be used when .pc file is in non-standard
>> location. User again need to do some magic.

Long term I think it is best to use autotools to properly generate the
.pc file, etc, otherwise it might be difficult for distros to pick
this up. I might be able to help you with that if you are willing to
accept patches.
On Tuesday 18 September 2018 13:55:08 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Mon, Sep 17, 2018 at 3:27 PM, Tanu Kaskinen <tanuk@iki.fi> wrote:
> > On Thu, 2018-09-13 at 11:12 +0200, Pali Rohár wrote:
> >> On Wednesday 05 September 2018 13:57:08 Tanu Kaskinen wrote:
> >> > > +#### Bluetooth A2DP aptX codec (optional) ###
> >> > > +
> >> > > +AC_ARG_ENABLE([aptx],
> >> > > +    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
> >> > > +
> >> > > +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
> >> > > +    [AC_CHECK_HEADER([openaptx.h],
> >> > > +        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
> >> > > +        [HAVE_OPENAPTX=0])])
> >> >
> >> > Have you considered providing a .pc file? Now we have to hardcode the
> >> > openaptx specific CFLAGS and LIBADD for libbluez5-util. If you ever
> >> > need to add new flags, all openaptx users need to update their build
> >> > systems. Also, if the library is installed to a non-standard location,
> >> > the .pc file can set the -L and -I flags to point to the right place.
> >>
> >> Intension is that library is small and does not need any special cflags
> >> or ldflags. So .pc file is not needed at all. And if library or include
> >> file is in non-standard location then user really need to specify where
> >> it is. But same argument can be used when .pc file is in non-standard
> >> location. User again need to do some magic.
> 
> Long term I think it is best to use autotools to properly generate the
> .pc file, etc, otherwise it might be difficult for distros to pick
> this up. I might be able to help you with that if you are willing to
> accept patches.

Because I know autotools, how to use it and how it works, I'm saying No.
For small library I explicitly chose something which is easy and not big
hell moloch. I really do not need anything special nor any custom or
specific functionality. Also library has no dependences.
On Wednesday 05 September 2018 13:57:08 Tanu Kaskinen wrote:
> On Sat, 2018-07-28 at 17:34 +0200, Pali Rohár wrote:
> > This patch provides support for aptX codec in bluetooth A2DP profile. In
> > pulseaudio it is implemented as a new profile a2dp_aptx_sink. For aptX
> > encoding it uses open source LGPLv2.1+ licensed libopenaptx library which
> > can be found at https://github.com/pali/libopenaptx.
> > 
> > Limitations:
> > 
> > Codec selection (either SBC or aptX) is done by bluez itself and it does
> > not provide API for switching codec. Therefore pulseaudio is not able to
> > change codec and it is up to bluez if it decide to use aptX or not.
> > 
> > Only standard aptX codec is supported for now. Support for other variants
> > like aptX HD, aptX Low Latency, FastStream may come up later.
> > ---
> >  configure.ac                                 |  19 ++
> >  src/Makefile.am                              |   5 +
> >  src/modules/bluetooth/a2dp-codecs.h          | 118 ++++++++++-
> >  src/modules/bluetooth/bluez5-util.c          |  48 ++++-
> >  src/modules/bluetooth/bluez5-util.h          |   2 +
> >  src/modules/bluetooth/module-bluez5-device.c |  65 +++++-
> >  src/modules/bluetooth/pa-a2dp-codec-aptx.c   | 297 +++++++++++++++++++++++++++
> >  src/modules/bluetooth/pa-a2dp-codec.h        |   1 +
> >  8 files changed, 548 insertions(+), 7 deletions(-)
> >  create mode 100644 src/modules/bluetooth/pa-a2dp-codec-aptx.c
> > 
> > diff --git a/configure.ac b/configure.ac
> > index d2bfab23b..c2d13fa53 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1094,6 +1094,23 @@ AC_SUBST(HAVE_BLUEZ_5_NATIVE_HEADSET)
> >  AM_CONDITIONAL([HAVE_BLUEZ_5_NATIVE_HEADSET], [test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = x1])
> >  AS_IF([test "x$HAVE_BLUEZ_5_NATIVE_HEADSET" = "x1"], AC_DEFINE([HAVE_BLUEZ_5_NATIVE_HEADSET], 1, [Bluez 5 native headset backend enabled]))
> >  
> > +#### Bluetooth A2DP aptX codec (optional) ###
> > +
> > +AC_ARG_ENABLE([aptx],
> > +    AS_HELP_STRING([--disable-aptx],[Disable optional bluetooth A2DP aptX codec support (via libopenaptx)]))
> > +
> > +AS_IF([test "x$HAVE_BLUEZ_5" = "x1" && test "x$enable_aptx" != "xno"],
> > +    [AC_CHECK_HEADER([openaptx.h],
> > +        [AC_CHECK_LIB([openaptx], [aptx_init], [HAVE_OPENAPTX=1], [HAVE_OPENAPTX=0])],
> > +        [HAVE_OPENAPTX=0])])
> 
> Have you considered providing a .pc file?

I will think about it (again).

> Now we have to hardcode the
> openaptx specific CFLAGS and LIBADD for libbluez5-util.

As a first step, I will remove hardcoded CFLAGS and LIBADD from
libbluez5-util. In autoconf, everything is possible to discover, so
really no need to hardcode and let autoconf find them.

> If you ever need to add new flags,

This is something which is not going to happen.

> all openaptx users need to update their build
> systems. Also, if the library is installed to a non-standard location,
> the .pc file can set the -L and -I flags to point to the right place.

> > diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> > index 9c4e3367b..c139f7fc3 100644
> > --- a/src/modules/bluetooth/bluez5-util.c
> > +++ b/src/modules/bluetooth/bluez5-util.c
> > @@ -50,7 +50,9 @@
> >  #define BLUEZ_ERROR_NOT_SUPPORTED "org.bluez.Error.NotSupported"
> >  
> >  #define A2DP_SOURCE_SBC_ENDPOINT "/MediaEndpoint/A2DPSourceSBC"
> > +#define A2DP_SOURCE_APTX_ENDPOINT "/MediaEndpoint/A2DPSourceAPTX"
> >  #define A2DP_SINK_SBC_ENDPOINT "/MediaEndpoint/A2DPSinkSBC"
> > +#define A2DP_SINK_APTX_ENDPOINT "/MediaEndpoint/A2DPSinkAPTX"
> >  
> >  #define ENDPOINT_INTROSPECT_XML                                         \
> >      DBUS_INTROSPECT_1_0_XML_DOCTYPE_DECL_NODE                           \
> > @@ -173,8 +175,22 @@ static bool device_supports_profile(pa_bluetooth_device *device, pa_bluetooth_pr
> >      switch (profile) {
> >          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK:
> >              return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
> > +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK:
> > +#ifdef HAVE_OPENAPTX
> > +            /* TODO: Implement once bluez provides API to check if codec is supported */
> > +            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SINK);
> 
> Is someone working on that API?

Yes, Luiz posted patches to bluez mailing list, and I will use this API
in new patch version. Also I will add fallback code, so users of "old"
bluez would be still able to use pulseaudio, just with same limitation
as now (no codec switching).

> > +#else
> > +            return false;
> > +#endif
> >          case PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE:
> >              return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> > +        case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> > +#ifdef HAVE_OPENAPTX
> > +            /* TODO: Implement once bluez provides API to check if codec is supported */
> > +            return !!pa_hashmap_get(device->uuids, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> > +#else
> > +            return false;
> > +#endif

And all these #ifdefs are removed in new patch version. There would be
no codec specific code.

> > @@ -961,7 +977,9 @@ static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
> >              if (!a->valid)
> >                  return;
> >  
> > +            register_endpoint(y, path, A2DP_SOURCE_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> >              register_endpoint(y, path, A2DP_SOURCE_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> > +            register_endpoint(y, path, A2DP_SINK_APTX_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
> >              register_endpoint(y, path, A2DP_SINK_SBC_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
> 
> We shouldn't register aptX endpoints if aptX support is not enabled.

Yes, fixing this via new/update pulseaudio codec API.

> Does the registration order matter? I have some vague recollection from
> the earlier discussion that BlueZ chooses the codec based on the
> endpoint registration order - is that true? If the order is important,
> we should document that here.

Yes, you are right.

New/updated pulseaudio codec API contains priority list, so this code is
updated to respect codec API priority.

> > @@ -1652,7 +1694,9 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe
> >      }
> >      y->matches_added = true;
> >  
> > +    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
> >      endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> > +    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
> >      endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);
> 
> We shouldn't set up endpoints for aptX if the aptX support is not
> enabled.

Fixing it.

> >  
> >      get_managed_objects(y);
> > @@ -1721,7 +1765,9 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
> >          if (y->filter_added)
> >              dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
> >  
> > +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK);
> >          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK);
> > +        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE);
> >          endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE);
> 
> If endpoint_init() is made conditional, these need to be made
> conditional too (according to the dbus docs, unregistering an object
> path that hasn't been registered first is considered an application
> bug).

Yes, fixing it.

> > diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> > index 365b9ef6f..29d862fe1 100644
> > --- a/src/modules/bluetooth/bluez5-util.h
> > +++ b/src/modules/bluetooth/bluez5-util.h
> > @@ -55,7 +55,9 @@ typedef enum pa_bluetooth_hook {
> >  
> >  typedef enum profile {
> >      PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK,
> > +    PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK,
> >      PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE,
> > +    PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE,
> >      PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT,
> >      PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY,
> >      PA_BLUETOOTH_PROFILE_OFF

This profile enum would be dynamically constructed, so bluez5-util would
not contain codec specific data.

> > diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> > index e626e80e9..e8a07d067 100644
> > --- a/src/modules/bluetooth/module-bluez5-device.c
> > +++ b/src/modules/bluetooth/module-bluez5-device.c
> > @@ -1757,6 +1776,18 @@ static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro
> >          p = PA_CARD_PROFILE_DATA(cp);
> >          break;
> >  
> > +    case PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE:
> > +        cp = pa_card_profile_new(name, _("High Fidelity aptX Capture (A2DP Source)"), sizeof(pa_bluetooth_profile_t));
> > +        cp->priority = 25;
> > +        cp->n_sinks = 0;
> > +        cp->n_sources = 1;
> > +        cp->max_sink_channels = 0;
> > +        cp->max_source_channels = 2;
> > +        pa_hashmap_put(input_port->profiles, cp->name, cp);
> > +
> > +        p = PA_CARD_PROFILE_DATA(cp);
> > +        break;
> > +
> 
> My compiler started to be unsure whether p is always initialized:
> 
>   CC       modules/bluetooth/module_bluez5_device_la-module-bluez5-device.lo
> modules/bluetooth/module-bluez5-device.c: In function ‘create_card_profile’:
> modules/bluetooth/module-bluez5-device.c:1821:8: warning: ‘p’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>      *p = profile;
>      ~~~^~~~~~~~~
> 
> This is a false positive, but we should do something to make the
> compiler shut up.

Please look at compile warnings after I send new patch series. I'm
testing compilation on Debian 9 (with default gcc compiler) and
currently I do not have warnings.

So if if you find something in patch version, let me know.

> > @@ -1907,6 +1940,28 @@ static int add_card(struct userdata *u) {
> >          pa_hashmap_put(data.profiles, cp->name, cp);
> >      }
> >  
> > +    PA_HASHMAP_FOREACH(uuid, d->uuids, state) {
> > +        pa_bluetooth_profile_t profile;
> > +
> > +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SINK maps to both SBC and APTX */
> > +        /* FIXME: PA_BLUETOOTH_UUID_A2DP_SOURCE maps to both SBC and APTX */
> > +        if (uuid_to_profile(uuid, &profile) < 0)
> > +            continue;
> > +
> > +        /* Handle APTX */
> > +        if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SINK)
> > +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SINK;
> > +        else if (profile == PA_BLUETOOTH_PROFILE_A2DP_SBC_SOURCE)
> > +            profile = PA_BLUETOOTH_PROFILE_A2DP_APTX_SOURCE;
> > +        else
> > +            continue;
> > +
> > +        if (!pa_hashmap_get(data.profiles, pa_bluetooth_profile_to_string(profile))) {
> > +            cp = create_card_profile(u, profile, data.ports);
> > +            pa_hashmap_put(data.profiles, cp->name, cp);
> > +        }
> > +    }
> > +
> 
> We shouldn't create the card profile if aptX support is disabled.

Yes, this will be handled in new codec API in new patch version.

> >      pa_assert(!pa_hashmap_isempty(data.profiles));
> >  
> >      cp = pa_card_profile_new("off", _("Off"), sizeof(pa_bluetooth_profile_t));
> > diff --git a/src/modules/bluetooth/pa-a2dp-codec-aptx.c b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
> > new file mode 100644
> > index 000000000..8ce1fc67c
> > --- /dev/null
> > +++ b/src/modules/bluetooth/pa-a2dp-codec-aptx.c
> > +static void pa_aptx_fill_blocksize(void **info_ptr, size_t read_link_mtu, size_t write_link_mtu, size_t *read_block_size, size_t *write_block_size) {
> > +    *write_block_size = (write_link_mtu/(8*4)) * 8*4*6;
> > +    *read_block_size = (read_link_mtu/(8*4)) * 8*4*6;
> 
> Please add comments that explain the math here.

Adding comment into code.