2014-06-02 11:20:06

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 0/5] android: aptX support

Hi,

Here's v2 for remaining aptX patches. Changes:
- hardcoded list of codecs (aptX won't load if there's no .so)
- .so for aptX is hardcoded to libbt-aptx.so (can be symlinked)


Andrzej Kaczmarek (5):
android/hal-audio: Always call qos_update
android/hal-audio: Allow codec to init on startup
android/hal-audio-aptx: Add initial support for aptX codec
android/hal-audio-aptx: Load aptX encoder library
android/hal-audio-aptx: Add encoding

android/Android.mk | 2 +
android/Makefile.am | 1 +
android/hal-audio-aptx.c | 282 +++++++++++++++++++++++++++++++++++++++++++++++
android/hal-audio.c | 35 +++++-
android/hal-audio.h | 5 +
5 files changed, 320 insertions(+), 5 deletions(-)
create mode 100644 android/hal-audio-aptx.c

--
1.9.3



2014-06-02 11:51:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] android/hal-audio-aptx: Load aptX encoder library

Hi Andrzej,

> This patch adds loading of aptX encoder library which should be provided
> by user. hal-audio-aptx will try to load 'libbt-aptx.so' so it should be
> available in search patch, preferably in /system/lib.
> ---
> android/hal-audio-aptx.c | 51 ++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/android/hal-audio-aptx.c b/android/hal-audio-aptx.c
> index b7b5dfc..28db0f6 100644
> --- a/android/hal-audio-aptx.c
> +++ b/android/hal-audio-aptx.c
> @@ -27,6 +27,8 @@
> #include "../src/shared/util.h"
> #include "../profiles/audio/a2dp-codecs.h"
>
> +#define APTX_SO_NAME "libbt-aptx.so"
> +
> struct aptx_data {
> a2dp_aptx_t aptx;
>
> @@ -61,20 +63,61 @@ static const a2dp_aptx_t aptx_presets[] = {
> },
> };
>
> +static void *aptx_dl;

this is a horrible name for a variable. I had to wrap my brain around to where the _dl comes from ;)

The dlopen manpage calls it handle so lets use aptx_handle here as variable name

> +static int aptx_sizeof = 0;

This name also give me a headache. aptx_btencsize seems to be more natural and clearer on what we store here.

I am also not sure you need to initialize this to 0. It is protected by the aptx_handle variable.

> +static int (*aptx_init)(void *, short);
> +static int (*aptx_encode)(void *, void *, void *, void *);
> +

> static bool aptx_load(void)
> {
> - /* TODO: load aptX codec library */
> - return false;
> + const char * (*aptx_version)(void);
> + const char * (*aptx_build)(void);
> + int (*aptx_sizeofenc)(void);

> +
> + aptx_dl = dlopen(APTX_SO_NAME, RTLD_LAZY);
> + if (!aptx_dl) {
> + error("APTX: failed to open library %s (%s)", APTX_SO_NAME,
> + dlerror());
> + return false;
> + }
> +
> + aptx_version = dlsym(aptx_dl, "aptxbtenc_version");
> + aptx_build = dlsym(aptx_dl, "aptxbtenc_build");
> +
> + if (aptx_version && aptx_build)
> + info("APTX: using library version %s build %s", aptx_version(),
> + aptx_build());
> + else
> + warn("APTX: cannot retrieve library version");
> +
> + aptx_sizeofenc = dlsym(aptx_dl, "SizeofAptxbtenc");
> + aptx_init = dlsym(aptx_dl, "aptxbtenc_init");
> + aptx_encode = dlsym(aptx_dl, "aptxbtenc_encodestereo");
> + if (!aptx_sizeofenc || !aptx_init || !aptx_encode) {
> + error("APTX: failed initialize library");
> + dlclose(aptx_dl);
> + aptx_dl = NULL;
> + return false;
> + }
> +
> + aptx_sizeof = aptx_sizeofenc();
> +
> + info("APTX: codec library initialized (size=%d)", aptx_sizeof);
> +
> + return true;
> }
>
> static void aptx_unload(void)
> {
> - /* TODO: unload aptX codec library */
> + if (aptx_dl) {
> + dlclose(aptx_dl);
> + aptx_dl = NULL;
> + }
> }
>
> static bool aptx_available(void)
> {
> - return false;
> + return aptx_dl != NULL;

At some point we need to start using !!aptx_handle syntax here instead of the != NULL. But this is more a general comment on how I like the coding style to change. Nothing that needs to be done right away. Just keep it in mind.

Regards

Marcel


2014-06-02 11:20:08

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 2/5] android/hal-audio: Allow codec to init on startup

This patch adds optional load/unload methods for codec which can be
used to initialize some static data for codec, e.g. load shared library
which provides encoder. Unlike init/cleanup which are called on stream
open/close these methods are called when audio device is opened/closed
thus most likely only once.

Additional optional 'available' method is provided which is used by HAL
core to check whether given codec is initialized properly and can have
endpoint registered, i.e. codec may only be available if there's .so
with encoder available on filesystem.
---
android/hal-audio.c | 28 +++++++++++++++++++++++++++-
android/hal-audio.h | 4 ++++
2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index f15e767..3c62686 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -421,11 +421,19 @@ static int register_endpoints(void)
size_t i;

for (i = 0; i < NUM_CODECS; i++, ep++) {
- const struct audio_codec *codec = audio_codecs[i]();
+ audio_codec_get_t get_codec = audio_codecs[i];
+ const struct audio_codec *codec = get_codec();

if (!codec)
return AUDIO_STATUS_FAILED;

+ /*
+ * Skip codecs which are unavailable, i.e. were not loaded due
+ * to missing .so
+ */
+ if (codec->available && !codec->available())
+ continue;
+
ep->id = ipc_open_cmd(codec);

if (!ep->id)
@@ -1275,11 +1283,20 @@ static int audio_dump(const audio_hw_device_t *device, int fd)
static int audio_close(hw_device_t *device)
{
struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)device;
+ size_t i;

DBG("");

unregister_endpoints();

+ for (i = 0; i < NUM_CODECS; i++) {
+ audio_codec_get_t get_codec = audio_codecs[i];
+ const struct audio_codec *codec = get_codec();
+
+ if (codec->unload)
+ codec->unload();
+ }
+
shutdown(listen_sk, SHUT_RDWR);
shutdown(audio_sk, SHUT_RDWR);

@@ -1419,6 +1436,7 @@ static int audio_open(const hw_module_t *module, const char *name,
hw_device_t **device)
{
struct a2dp_audio_dev *a2dp_dev;
+ size_t i;
int err;

DBG("");
@@ -1457,6 +1475,14 @@ static int audio_open(const hw_module_t *module, const char *name,
a2dp_dev->dev.close_input_stream = audio_close_input_stream;
a2dp_dev->dev.dump = audio_dump;

+ for (i = 0; i < NUM_CODECS; i++) {
+ audio_codec_get_t get_codec = audio_codecs[i];
+ const struct audio_codec *codec = get_codec();
+
+ if (codec->load)
+ codec->load();
+ }
+
/*
* Note that &a2dp_dev->dev.common is the same pointer as a2dp_dev.
* This results from the structure of following structs:a2dp_audio_dev,
diff --git a/android/hal-audio.h b/android/hal-audio.h
index be71473..a47a7ea 100644
--- a/android/hal-audio.h
+++ b/android/hal-audio.h
@@ -75,6 +75,10 @@ struct audio_codec {
uint8_t type;
bool use_rtp;

+ bool (*load) (void);
+ void (*unload) (void);
+ bool (*available) (void);
+
int (*get_presets) (struct audio_preset *preset, size_t *len);

bool (*init) (struct audio_preset *preset, uint16_t mtu,
--
1.9.3


2014-06-02 11:20:09

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 3/5] android/hal-audio-aptx: Add initial support for aptX codec

This patch adds support for aptX codec. Since this is proprietary codec
it requires to obtain license form vendor (CSR) in order to use it.
Also shared library which provices encoder implementation is required
since this implementation only wraps it into audio HAL.
---
android/Android.mk | 2 +
android/Makefile.am | 1 +
android/hal-audio-aptx.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++
android/hal-audio.c | 1 +
android/hal-audio.h | 1 +
5 files changed, 215 insertions(+)
create mode 100644 android/hal-audio-aptx.c

diff --git a/android/Android.mk b/android/Android.mk
index de909d6..f28b8f3 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -257,6 +257,7 @@ include $(CLEAR_VARS)
LOCAL_SRC_FILES := \
bluez/android/hal-audio.c \
bluez/android/hal-audio-sbc.c \
+ bluez/android/hal-audio-aptx.c \

LOCAL_C_INCLUDES = \
$(call include-path-for, system-core) \
@@ -268,6 +269,7 @@ LOCAL_SHARED_LIBRARIES := \
libsbc \

LOCAL_CFLAGS := $(BLUEZ_COMMON_CFLAGS)
+LOCAL_LDFLAGS := -ldl

LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
LOCAL_MODULE_TAGS := optional
diff --git a/android/Makefile.am b/android/Makefile.am
index bcaec4c..30d3c9a 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -170,6 +170,7 @@ android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
android/hal-audio.h \
android/hal-audio.c \
android/hal-audio-sbc.c \
+ android/hal-audio-aptx.c \
android/hardware/audio.h \
android/hardware/audio_effect.h \
android/hardware/hardware.h \
diff --git a/android/hal-audio-aptx.c b/android/hal-audio-aptx.c
new file mode 100644
index 0000000..b7b5dfc
--- /dev/null
+++ b/android/hal-audio-aptx.c
@@ -0,0 +1,210 @@
+/*
+ * Copyright (C) 2014 Tieto Poland
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include <stdint.h>
+#include <stdbool.h>
+#include <string.h>
+#include <malloc.h>
+#include <dlfcn.h>
+
+#include "audio-msg.h"
+#include "hal-audio.h"
+#include "hal-log.h"
+#include "../src/shared/util.h"
+#include "../profiles/audio/a2dp-codecs.h"
+
+struct aptx_data {
+ a2dp_aptx_t aptx;
+
+ void *enc;
+};
+
+static const a2dp_aptx_t aptx_presets[] = {
+ {
+ .info = {
+ .vendor_id = APTX_VENDOR_ID,
+ .codec_id = APTX_CODEC_ID,
+ },
+ .frequency = APTX_SAMPLING_FREQ_44100 |
+ APTX_SAMPLING_FREQ_48000,
+ .channel_mode = APTX_CHANNEL_MODE_STEREO,
+ },
+ {
+ .info = {
+ .vendor_id = APTX_VENDOR_ID,
+ .codec_id = APTX_CODEC_ID,
+ },
+ .frequency = APTX_SAMPLING_FREQ_48000,
+ .channel_mode = APTX_CHANNEL_MODE_STEREO,
+ },
+ {
+ .info = {
+ .vendor_id = APTX_VENDOR_ID,
+ .codec_id = APTX_CODEC_ID,
+ },
+ .frequency = APTX_SAMPLING_FREQ_44100,
+ .channel_mode = APTX_CHANNEL_MODE_STEREO,
+ },
+};
+
+static bool aptx_load(void)
+{
+ /* TODO: load aptX codec library */
+ return false;
+}
+
+static void aptx_unload(void)
+{
+ /* TODO: unload aptX codec library */
+}
+
+static bool aptx_available(void)
+{
+ return false;
+}
+
+static int aptx_get_presets(struct audio_preset *preset, size_t *len)
+{
+ int i;
+ int count;
+ size_t new_len = 0;
+ uint8_t *ptr = (uint8_t *) preset;
+ size_t preset_size = sizeof(*preset) + sizeof(a2dp_aptx_t);
+
+ DBG("");
+
+ count = sizeof(aptx_presets) / sizeof(aptx_presets[0]);
+
+ for (i = 0; i < count; i++) {
+ preset = (struct audio_preset *) ptr;
+
+ if (new_len + preset_size > *len)
+ break;
+
+ preset->len = sizeof(a2dp_aptx_t);
+ memcpy(preset->data, &aptx_presets[i], preset->len);
+
+ new_len += preset_size;
+ ptr += preset_size;
+ }
+
+ *len = new_len;
+
+ return i;
+}
+
+static bool aptx_codec_init(struct audio_preset *preset, uint16_t payload_len,
+ void **codec_data)
+{
+ struct aptx_data *aptx_data;
+
+ DBG("");
+
+ if (preset->len != sizeof(a2dp_aptx_t)) {
+ error("APTX: preset size mismatch");
+ return false;
+ }
+
+ aptx_data = new0(struct aptx_data, 1);
+ if (!aptx_data)
+ return false;
+
+ memcpy(&aptx_data->aptx, preset->data, preset->len);
+
+ /* TODO: initialize encoder */
+
+ *codec_data = aptx_data;
+
+ return true;
+}
+
+static bool aptx_cleanup(void *codec_data)
+{
+ struct aptx_data *aptx_data = (struct aptx_data *) codec_data;
+
+ free(aptx_data->enc);
+ free(codec_data);
+
+ return true;
+}
+
+static bool aptx_get_config(void *codec_data, struct audio_input_config *config)
+{
+ struct aptx_data *aptx_data = (struct aptx_data *) codec_data;
+
+ config->rate = aptx_data->aptx.frequency & APTX_SAMPLING_FREQ_48000 ?
+ 48000 : 44100;
+ config->channels = AUDIO_CHANNEL_OUT_STEREO;
+ config->format = AUDIO_FORMAT_PCM_16_BIT;
+
+ return true;
+}
+
+static size_t aptx_get_buffer_size(void *codec_data)
+{
+ /* TODO: return actual value */
+ return 0;
+}
+
+static size_t aptx_get_mediapacket_duration(void *codec_data)
+{
+ /* TODO: return actual value */
+ return 0;
+}
+
+static ssize_t aptx_encode_mediapacket(void *codec_data, const uint8_t *buffer,
+ size_t len, struct media_packet *mp,
+ size_t mp_data_len, size_t *written)
+{
+ /* TODO: add encoding */
+
+ return len;
+}
+
+static bool aptx_update_qos(void *codec_data, uint8_t op)
+{
+ /*
+ * aptX has constant bitrate of 352kbps (with constant 4:1 compression
+ * ratio) thus QoS is not possible here.
+ */
+
+ return false;
+}
+
+static const struct audio_codec codec = {
+ .type = A2DP_CODEC_VENDOR,
+ .use_rtp = false,
+
+ .load = aptx_load,
+ .unload = aptx_unload,
+ .available = aptx_available,
+
+ .get_presets = aptx_get_presets,
+
+ .init = aptx_codec_init,
+ .cleanup = aptx_cleanup,
+ .get_config = aptx_get_config,
+ .get_buffer_size = aptx_get_buffer_size,
+ .get_mediapacket_duration = aptx_get_mediapacket_duration,
+ .encode_mediapacket = aptx_encode_mediapacket,
+ .update_qos = aptx_update_qos,
+};
+
+const struct audio_codec *codec_aptx(void)
+{
+ return &codec;
+}
diff --git a/android/hal-audio.c b/android/hal-audio.c
index 3c62686..4c982b0 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -97,6 +97,7 @@ extern int clock_nanosleep(clockid_t clock_id, int flags,
#endif

static const audio_codec_get_t audio_codecs[] = {
+ codec_aptx,
codec_sbc,
};

diff --git a/android/hal-audio.h b/android/hal-audio.h
index a47a7ea..12635bf 100644
--- a/android/hal-audio.h
+++ b/android/hal-audio.h
@@ -100,3 +100,4 @@ struct audio_codec {
typedef const struct audio_codec * (*audio_codec_get_t) (void);

const struct audio_codec *codec_sbc(void);
+const struct audio_codec *codec_aptx(void);
--
1.9.3


2014-06-02 11:20:11

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 5/5] android/hal-audio-aptx: Add encoding

---
android/hal-audio-aptx.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/android/hal-audio-aptx.c b/android/hal-audio-aptx.c
index 28db0f6..04e29f9 100644
--- a/android/hal-audio-aptx.c
+++ b/android/hal-audio-aptx.c
@@ -168,7 +168,15 @@ static bool aptx_codec_init(struct audio_preset *preset, uint16_t payload_len,

memcpy(&aptx_data->aptx, preset->data, preset->len);

- /* TODO: initialize encoder */
+ aptx_data->enc = calloc(1, aptx_sizeof);
+ if (!aptx_data->enc) {
+ error("APTX: failed to create encoder");
+ free(aptx_data);
+ return false;
+ }
+
+ /* 1 = big-endian, this is what devices are using */
+ aptx_init(aptx_data->enc, 1);

*codec_data = aptx_data;

@@ -213,9 +221,30 @@ static ssize_t aptx_encode_mediapacket(void *codec_data, const uint8_t *buffer,
size_t len, struct media_packet *mp,
size_t mp_data_len, size_t *written)
{
- /* TODO: add encoding */
+ struct aptx_data *aptx_data = (struct aptx_data *) codec_data;
+ const int16_t *ptr = (const void *) buffer;
+ size_t bytes_in = 0;
+ size_t bytes_out = 0;
+
+ while ((len - bytes_in) >= 16 && (mp_data_len - bytes_out) >= 4) {
+ int pcm_l[4], pcm_r[4];
+ int i;
+
+ for (i = 0; i < 4; i++) {
+ pcm_l[i] = ptr[0];
+ pcm_r[i] = ptr[1];
+ ptr += 2;
+ }
+
+ aptx_encode(aptx_data->enc, pcm_l, pcm_r, &mp->data[bytes_out]);
+
+ bytes_in += 16;
+ bytes_out += 4;
+ }
+
+ *written = bytes_out;

- return len;
+ return bytes_in;
}

static bool aptx_update_qos(void *codec_data, uint8_t op)
--
1.9.3


2014-06-02 11:20:10

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 4/5] android/hal-audio-aptx: Load aptX encoder library

This patch adds loading of aptX encoder library which should be provided
by user. hal-audio-aptx will try to load 'libbt-aptx.so' so it should be
available in search patch, preferably in /system/lib.
---
android/hal-audio-aptx.c | 51 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/android/hal-audio-aptx.c b/android/hal-audio-aptx.c
index b7b5dfc..28db0f6 100644
--- a/android/hal-audio-aptx.c
+++ b/android/hal-audio-aptx.c
@@ -27,6 +27,8 @@
#include "../src/shared/util.h"
#include "../profiles/audio/a2dp-codecs.h"

+#define APTX_SO_NAME "libbt-aptx.so"
+
struct aptx_data {
a2dp_aptx_t aptx;

@@ -61,20 +63,61 @@ static const a2dp_aptx_t aptx_presets[] = {
},
};

+static void *aptx_dl;
+static int aptx_sizeof = 0;
+static int (*aptx_init)(void *, short);
+static int (*aptx_encode)(void *, void *, void *, void *);
+
static bool aptx_load(void)
{
- /* TODO: load aptX codec library */
- return false;
+ const char * (*aptx_version)(void);
+ const char * (*aptx_build)(void);
+ int (*aptx_sizeofenc)(void);
+
+ aptx_dl = dlopen(APTX_SO_NAME, RTLD_LAZY);
+ if (!aptx_dl) {
+ error("APTX: failed to open library %s (%s)", APTX_SO_NAME,
+ dlerror());
+ return false;
+ }
+
+ aptx_version = dlsym(aptx_dl, "aptxbtenc_version");
+ aptx_build = dlsym(aptx_dl, "aptxbtenc_build");
+
+ if (aptx_version && aptx_build)
+ info("APTX: using library version %s build %s", aptx_version(),
+ aptx_build());
+ else
+ warn("APTX: cannot retrieve library version");
+
+ aptx_sizeofenc = dlsym(aptx_dl, "SizeofAptxbtenc");
+ aptx_init = dlsym(aptx_dl, "aptxbtenc_init");
+ aptx_encode = dlsym(aptx_dl, "aptxbtenc_encodestereo");
+ if (!aptx_sizeofenc || !aptx_init || !aptx_encode) {
+ error("APTX: failed initialize library");
+ dlclose(aptx_dl);
+ aptx_dl = NULL;
+ return false;
+ }
+
+ aptx_sizeof = aptx_sizeofenc();
+
+ info("APTX: codec library initialized (size=%d)", aptx_sizeof);
+
+ return true;
}

static void aptx_unload(void)
{
- /* TODO: unload aptX codec library */
+ if (aptx_dl) {
+ dlclose(aptx_dl);
+ aptx_dl = NULL;
+ }
}

static bool aptx_available(void)
{
- return false;
+ return aptx_dl != NULL;
}

static int aptx_get_presets(struct audio_preset *preset, size_t *len)
--
1.9.3


2014-06-02 11:20:07

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH v2 1/5] android/hal-audio: Always call qos_update

As it turned out, it's better to always call update_qos and just
provide dummy callback from codecs which do not support it.
---
android/hal-audio.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 8b82498..f15e767 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -544,8 +544,7 @@ static bool resume_endpoint(struct audio_endpoint *ep)
ep->samples = 0;
ep->resync = false;

- if (ep->codec->update_qos)
- ep->codec->update_qos(ep->codec_data, QOS_POLICY_DEFAULT);
+ ep->codec->update_qos(ep->codec_data, QOS_POLICY_DEFAULT);

return true;
}
@@ -704,8 +703,7 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
if (diff > MAX_DELAY) {
warn("lag is %jums, resyncing", diff / 1000);

- if (ep->codec->update_qos)
- ep->codec->update_qos(ep->codec_data,
+ ep->codec->update_qos(ep->codec_data,
QOS_POLICY_DECREASE);
ep->resync = true;
}
--
1.9.3