[PATCH 0/9] android: Add SBC encoding

[PATCH 0/9] android: Add SBC encoding

[Andrzej Kaczmarek]

Hi,

This series of patches adds SBC encoding to Audio HAL.

There are 2 issues which I'm aware of, but at the moment I can't tell
whether it's something in code or it's because of incomplete A2DP HAL
implementation:
- when streaming and trying to change ringtone (on Nexus 4), ringtone
  sound playback using built-in speaker is choppy
- after disconnecting some A2DP devices AudioFliger crashes (seems like
  problem I worked around before with buffer size, yet I still have no
  idea why this happens)

It's possible that after proper A2DP audio state notification is done,
both issues will be "solved", but this still needs to be tested.



Andrzej Kaczmarek (9):
  android: Add MTU data to Open Stream Audio IPC
  android: Build Audio HAL with SBC
  android/hal-audio: Rename sbc_init to avoid collision with libsbc
  android/hal-audio: Initialize SBC encoder
  android/hal-audio: Calculate SBC stream parameters
  android/hal-audio: Add resume to codec callbacks
  android/hal-audio: Return proper buffer size to AudioFlinger
  android/hal-audio: Read fd from Output Stream response
  android/hal-audio: Add proper SBC encoding

 android/Android.mk  |  14 ++-
 android/Makefile.am |   2 +
 android/a2dp.c      |   8 +-
 android/audio-msg.h |   1 +
 android/hal-audio.c | 263 +++++++++++++++++++++++++++++++++++++++++++++++++---
 android/rtp.h       |  76 +++++++++++++++
 configure.ac        |   7 ++
 7 files changed, 353 insertions(+), 18 deletions(-)
 create mode 100644 android/rtp.h

-- 
1.8.5.2

[PATCH 1/9] android: Add MTU data to Open Stream Audio IPC

[Andrzej Kaczmarek]

MTU value for transport channel is sent in Open Stream response, which
is required to calculate number of frames which can be packed into
single media packet.

This is to avoid including GPLv2 licensed headers in Audio HAL
implementation.
---
 android/a2dp.c      | 8 ++++++--
 android/audio-msg.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/android/a2dp.c b/android/a2dp.c
index 2288912..e7ca8b8 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -1252,6 +1252,7 @@ static void bt_stream_open(const void *buf, uint16_t len)
 	struct audio_rsp_open_stream *rsp;
 	struct a2dp_setup *setup;
 	int fd;
+	uint16_t omtu;
 
 	DBG("");
 
@@ -1262,14 +1263,17 @@ static void bt_stream_open(const void *buf, uint16_t len)
 		return;
 	}
 
-	if (!avdtp_stream_get_transport(setup->stream, &fd, NULL, NULL, NULL)) {
+	if (!avdtp_stream_get_transport(setup->stream, &fd, NULL, &omtu,
+						NULL)) {
 		error("avdtp_stream_get_transport: failed");
 		audio_ipc_send_rsp(AUDIO_OP_OPEN_STREAM, AUDIO_STATUS_FAILED);
 		return;
 	}
 
-	len = sizeof(struct audio_preset) + setup->preset->len;
+	len = sizeof(struct audio_rsp_open_stream) +
+			sizeof(struct audio_preset) + setup->preset->len;
 	rsp = g_malloc0(len);
+	rsp->mtu = omtu;
 	rsp->preset->len = setup->preset->len;
 	memcpy(rsp->preset->data, setup->preset->data, setup->preset->len);
 
diff --git a/android/audio-msg.h b/android/audio-msg.h
index 8f03274..17cde09 100644
--- a/android/audio-msg.h
+++ b/android/audio-msg.h
@@ -63,6 +63,7 @@ struct audio_cmd_open_stream {
 } __attribute__((packed));
 
 struct audio_rsp_open_stream {
+	uint16_t mtu;
 	struct audio_preset preset[0];
 } __attribute__((packed));
 
-- 
1.8.5.2

[PATCH 2/9] android: Build Audio HAL with SBC

[Andrzej Kaczmarek]

Build for Android requires libsbc sources to be available in
external/bluetooth/sbc. Build for host requires libsbc package to be
installed.
---
 android/Android.mk  | 14 +++++++++++---
 android/Makefile.am |  2 ++
 configure.ac        |  7 +++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/android/Android.mk b/android/Android.mk
index 7e97ec8..63a4a24 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -3,8 +3,9 @@ LOCAL_PATH := $(call my-dir)
 # Retrieve BlueZ version from configure.ac file
 BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
 
-# Specify pathmap for glib
-pathmap_INCL += glib:external/bluetooth/glib
+# Specify pathmap for glib and sbc
+pathmap_INCL += glib:external/bluetooth/glib \
+	sbc:external/bluetooth/sbc
 
 # Specify common compiler flags
 BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\" \
@@ -220,11 +221,18 @@ include $(BUILD_EXECUTABLE)
 
 include $(CLEAR_VARS)
 
-LOCAL_SRC_FILES := hal-audio.c
+LOCAL_SRC_FILES := hal-audio.c \
+	../../sbc/sbc/sbc.c \
+	../../sbc/sbc/sbc_primitives.c \
+	../../sbc/sbc/sbc_primitives_armv6.c \
+	../../sbc/sbc/sbc_primitives_iwmmxt.c \
+	../../sbc/sbc/sbc_primitives_mmx.c \
+	../../sbc/sbc/sbc_primitives_neon.c \
 
 LOCAL_C_INCLUDES = \
 	$(call include-path-for, system-core) \
 	$(call include-path-for, libhardware) \
+	$(call include-path-for, sbc) \
 
 LOCAL_SHARED_LIBRARIES := \
 	libcutils \
diff --git a/android/Makefile.am b/android/Makefile.am
index 8d2714d..01d8996 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -130,6 +130,8 @@ android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
 
 android_audio_a2dp_default_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/android
 
+android_audio_a2dp_default_la_LIBADD = @SBC_LIBS@
+
 android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
 					-no-undefined -pthread
 
diff --git a/configure.ac b/configure.ac
index c85f208..79c3705 100644
--- a/configure.ac
+++ b/configure.ac
@@ -252,6 +252,13 @@ AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
 					[enable_android=${enableval}])
 AM_CONDITIONAL(ANDROID, test "${enable_android}" = "yes")
 
+if (test "${enable_android}" = "yes"); then
+	PKG_CHECK_MODULES(SBC, sbc >= 1.0, dummy=yes,
+					AC_MSG_ERROR(libsbc1 >= 1.0 is required))
+	AC_SUBST(SBC_CFLAGS)
+	AC_SUBST(SBC_LIBS)
+fi
+
 AC_DEFINE_UNQUOTED(ANDROID_STORAGEDIR, "${storagedir}/android",
 			[Directory for the Android daemon storage files])
 
-- 
1.8.5.2

Re: [PATCH 2/9] android: Build Audio HAL with SBC

[Marcel Holtmann]

Hi Andrzej,

> Build for Android requires libsbc sources to be available in
> external/bluetooth/sbc. Build for host requires libsbc package to be
> installed.
> ---
> android/Android.mk  | 14 +++++++++++---
> android/Makefile.am |  2 ++
> configure.ac        |  7 +++++++
> 3 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/android/Android.mk b/android/Android.mk
> index 7e97ec8..63a4a24 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -3,8 +3,9 @@ LOCAL_PATH := $(call my-dir)
> # Retrieve BlueZ version from configure.ac file
> BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
> 
> -# Specify pathmap for glib
> -pathmap_INCL += glib:external/bluetooth/glib
> +# Specify pathmap for glib and sbc
> +pathmap_INCL += glib:external/bluetooth/glib \
> +	sbc:external/bluetooth/sbc
> 
> # Specify common compiler flags
> BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\" \
> @@ -220,11 +221,18 @@ include $(BUILD_EXECUTABLE)
> 
> include $(CLEAR_VARS)
> 
> -LOCAL_SRC_FILES := hal-audio.c
> +LOCAL_SRC_FILES := hal-audio.c \
> +	../../sbc/sbc/sbc.c \
> +	../../sbc/sbc/sbc_primitives.c \
> +	../../sbc/sbc/sbc_primitives_armv6.c \
> +	../../sbc/sbc/sbc_primitives_iwmmxt.c \
> +	../../sbc/sbc/sbc_primitives_mmx.c \
> +	../../sbc/sbc/sbc_primitives_neon.c \

why? Can we not just build libsbc for Android?

I rather install an extra library and not have to make this builtin.

> 
> LOCAL_C_INCLUDES = \
> 	$(call include-path-for, system-core) \
> 	$(call include-path-for, libhardware) \
> +	$(call include-path-for, sbc) \
> 
> LOCAL_SHARED_LIBRARIES := \
> 	libcutils \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 8d2714d..01d8996 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -130,6 +130,8 @@ android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
> 
> android_audio_a2dp_default_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/android
> 
> +android_audio_a2dp_default_la_LIBADD = @SBC_LIBS@
> +
> android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> 					-no-undefined -pthread
> 
> diff --git a/configure.ac b/configure.ac
> index c85f208..79c3705 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -252,6 +252,13 @@ AC_ARG_ENABLE(android, AC_HELP_STRING([--enable-android],
> 					[enable_android=${enableval}])
> AM_CONDITIONAL(ANDROID, test "${enable_android}" = "yes")
> 
> +if (test "${enable_android}" = "yes"); then
> +	PKG_CHECK_MODULES(SBC, sbc >= 1.0, dummy=yes,
> +					AC_MSG_ERROR(libsbc1 >= 1.0 is required))

This should not read libsbc1. There is no such thing as libsbc1. It should read SBC library.

> +	AC_SUBST(SBC_CFLAGS)
> +	AC_SUBST(SBC_LIBS)
> +fi
> +
> AC_DEFINE_UNQUOTED(ANDROID_STORAGEDIR, "${storagedir}/android",
> 			[Directory for the Android daemon storage files])

Regards

Marcel

Re: [PATCH 2/9] android: Build Audio HAL with SBC

[Andrzej Kaczmarek]

Hi Marcel,

On 17 January 2014 19:27, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrzej,
>
>> Build for Android requires libsbc sources to be available in
>> external/bluetooth/sbc. Build for host requires libsbc package to be
>> installed.
>> ---
>> android/Android.mk  | 14 +++++++++++---
>> android/Makefile.am |  2 ++
>> configure.ac        |  7 +++++++
>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/android/Android.mk b/android/Android.mk
>> index 7e97ec8..63a4a24 100644
>> --- a/android/Android.mk
>> +++ b/android/Android.mk
>> @@ -3,8 +3,9 @@ LOCAL_PATH := $(call my-dir)
>> # Retrieve BlueZ version from configure.ac file
>> BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
>>
>> -# Specify pathmap for glib
>> -pathmap_INCL += glib:external/bluetooth/glib
>> +# Specify pathmap for glib and sbc
>> +pathmap_INCL += glib:external/bluetooth/glib \
>> +     sbc:external/bluetooth/sbc
>>
>> # Specify common compiler flags
>> BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\" \
>> @@ -220,11 +221,18 @@ include $(BUILD_EXECUTABLE)
>>
>> include $(CLEAR_VARS)
>>
>> -LOCAL_SRC_FILES := hal-audio.c
>> +LOCAL_SRC_FILES := hal-audio.c \
>> +     ../../sbc/sbc/sbc.c \
>> +     ../../sbc/sbc/sbc_primitives.c \
>> +     ../../sbc/sbc/sbc_primitives_armv6.c \
>> +     ../../sbc/sbc/sbc_primitives_iwmmxt.c \
>> +     ../../sbc/sbc/sbc_primitives_mmx.c \
>> +     ../../sbc/sbc/sbc_primitives_neon.c \
>
> why? Can we not just build libsbc for Android?

This way we can use upstream sbc git without forking it to add Android.mk.

> I rather install an extra library and not have to make this builtin.

Sure, this can be changed. Static or dynamic library?


BR,
Andrzej

Re: [PATCH 2/9] android: Build Audio HAL with SBC

[Marcel Holtmann]

Hi Andrzej,

>>> Build for Android requires libsbc sources to be available in
>>> external/bluetooth/sbc. Build for host requires libsbc package to be
>>> installed.
>>> ---
>>> android/Android.mk  | 14 +++++++++++---
>>> android/Makefile.am |  2 ++
>>> configure.ac        |  7 +++++++
>>> 3 files changed, 20 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/android/Android.mk b/android/Android.mk
>>> index 7e97ec8..63a4a24 100644
>>> --- a/android/Android.mk
>>> +++ b/android/Android.mk
>>> @@ -3,8 +3,9 @@ LOCAL_PATH := $(call my-dir)
>>> # Retrieve BlueZ version from configure.ac file
>>> BLUEZ_VERSION := $(shell grep ^AC_INIT $(LOCAL_PATH)/../configure.ac | cpp -P -D'AC_INIT(_,v)=v')
>>> 
>>> -# Specify pathmap for glib
>>> -pathmap_INCL += glib:external/bluetooth/glib
>>> +# Specify pathmap for glib and sbc
>>> +pathmap_INCL += glib:external/bluetooth/glib \
>>> +     sbc:external/bluetooth/sbc
>>> 
>>> # Specify common compiler flags
>>> BLUEZ_COMMON_CFLAGS := -DVERSION=\"$(BLUEZ_VERSION)\" \
>>> @@ -220,11 +221,18 @@ include $(BUILD_EXECUTABLE)
>>> 
>>> include $(CLEAR_VARS)
>>> 
>>> -LOCAL_SRC_FILES := hal-audio.c
>>> +LOCAL_SRC_FILES := hal-audio.c \
>>> +     ../../sbc/sbc/sbc.c \
>>> +     ../../sbc/sbc/sbc_primitives.c \
>>> +     ../../sbc/sbc/sbc_primitives_armv6.c \
>>> +     ../../sbc/sbc/sbc_primitives_iwmmxt.c \
>>> +     ../../sbc/sbc/sbc_primitives_mmx.c \
>>> +     ../../sbc/sbc/sbc_primitives_neon.c \
>> 
>> why? Can we not just build libsbc for Android?
> 
> This way we can use upstream sbc git without forking it to add Android.mk.

okay. Seems fine for now to get this started, but eventually we need to sort this out.

>> I rather install an extra library and not have to make this builtin.
> 
> Sure, this can be changed. Static or dynamic library?

I would build it at least as dynamic library. Even you build that from android/Android.mk file from bluez tree. If you make it static or include it then you have to worry about LGPL obligations. They are different for a shared library compared to a static library/include.

Regards

Marcel

[PATCH 3/9] android/hal-audio: Rename sbc_init to avoid collision with libsbc

[Andrzej Kaczmarek]

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

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 2f6f8c2..f53dba0 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -56,7 +56,7 @@ struct sbc_data {
 };
 
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
-static int sbc_init(struct audio_preset *preset, void **codec_data);
+static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
 static int sbc_cleanup(void *codec_data);
 static int sbc_get_config(void *codec_data,
 					struct audio_input_config *config);
@@ -80,7 +80,7 @@ static const struct audio_codec audio_codecs[] = {
 
 		.get_presets = sbc_get_presets,
 
-		.init = sbc_init,
+		.init = sbc_codec_init,
 		.cleanup = sbc_cleanup,
 		.get_config = sbc_get_config,
 	}
@@ -184,7 +184,7 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
 	return i;
 }
 
-static int sbc_init(struct audio_preset *preset, void **codec_data)
+static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
 {
 	struct sbc_data *sbc_data;
 
-- 
1.8.5.2

[PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Andrzej Kaczmarek]

---
 android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index f53dba0..e5c646c 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -32,6 +32,7 @@
 #include "hal-log.h"
 #include "hal-msg.h"
 #include "../profiles/audio/a2dp-codecs.h"
+#include <sbc/sbc.h>
 
 static const uint8_t a2dp_src_uuid[] = {
 		0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
@@ -53,6 +54,8 @@ struct audio_input_config {
 
 struct sbc_data {
 	a2dp_sbc_t sbc;
+
+	sbc_t enc;
 };
 
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
@@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
 	return i;
 }
 
+static void sbc_init_encoder(struct sbc_data *sbc_data)
+{
+	a2dp_sbc_t *in = &sbc_data->sbc;
+	sbc_t *out = &sbc_data->enc;
+
+	DBG("");
+
+	sbc_init(out, 0L);
+
+	switch (in->frequency) {
+	case SBC_SAMPLING_FREQ_16000:
+		out->frequency = SBC_FREQ_16000;
+		break;
+	case SBC_SAMPLING_FREQ_32000:
+		out->frequency = SBC_FREQ_32000;
+		break;
+	case SBC_SAMPLING_FREQ_44100:
+		out->frequency = SBC_FREQ_44100;
+		break;
+	case SBC_SAMPLING_FREQ_48000:
+		out->frequency = SBC_FREQ_48000;
+		break;
+	}
+
+	out->subbands = in->subbands == SBC_SUBBANDS_4 ? SBC_SB_4 : SBC_SB_8;
+
+	switch (in->channel_mode) {
+	case SBC_CHANNEL_MODE_MONO:
+		out->mode = SBC_MODE_MONO;
+		break;
+	case SBC_CHANNEL_MODE_DUAL_CHANNEL:
+		out->mode = SBC_MODE_DUAL_CHANNEL;
+		break;
+	case SBC_CHANNEL_MODE_JOINT_STEREO:
+		out->mode = SBC_MODE_JOINT_STEREO;
+		break;
+	case SBC_CHANNEL_MODE_STEREO:
+		out->mode = SBC_MODE_STEREO;
+		break;
+	}
+
+	out->endian = SBC_LE;
+
+	out->bitpool = in->max_bitpool;
+
+	out->allocation = in->allocation_method == SBC_ALLOCATION_SNR ?
+				SBC_AM_SNR : SBC_AM_LOUDNESS;
+
+	switch (in->block_length) {
+	case SBC_BLOCK_LENGTH_4:
+		out->blocks = SBC_BLK_4;
+		break;
+	case SBC_BLOCK_LENGTH_8:
+		out->blocks = SBC_BLK_8;
+		break;
+	case SBC_BLOCK_LENGTH_12:
+		out->blocks = SBC_BLK_12;
+		break;
+	case SBC_BLOCK_LENGTH_16:
+		out->blocks = SBC_BLK_16;
+		break;
+	}
+}
+
 static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
 {
 	struct sbc_data *sbc_data;
@@ -199,6 +266,8 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
 
 	memcpy(&sbc_data->sbc, preset->data, preset->len);
 
+	sbc_init_encoder(sbc_data);
+
 	*codec_data = sbc_data;
 
 	return AUDIO_STATUS_SUCCESS;
@@ -206,8 +275,11 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
 
 static int sbc_cleanup(void *codec_data)
 {
+	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+
 	DBG("");
 
+	sbc_finish(&sbc_data->enc);
 	free(codec_data);
 
 	return AUDIO_STATUS_SUCCESS;
-- 
1.8.5.2

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Marcel Holtmann]

Hi Andrzej,

> ---
> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index f53dba0..e5c646c 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -32,6 +32,7 @@
> #include "hal-log.h"
> #include "hal-msg.h"
> #include "../profiles/audio/a2dp-codecs.h"
> +#include <sbc/sbc.h>
> 
> static const uint8_t a2dp_src_uuid[] = {
> 		0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
> @@ -53,6 +54,8 @@ struct audio_input_config {
> 
> struct sbc_data {
> 	a2dp_sbc_t sbc;
> +
> +	sbc_t enc;
> };
> 
> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
> 	return i;
> }
> 
> +static void sbc_init_encoder(struct sbc_data *sbc_data)
> +{
> +	a2dp_sbc_t *in = &sbc_data->sbc;
> +	sbc_t *out = &sbc_data->enc;
> +
> +	DBG("");
> +
> +	sbc_init(out, 0L);
> +
> +	switch (in->frequency) {
> +	case SBC_SAMPLING_FREQ_16000:
> +		out->frequency = SBC_FREQ_16000;
> +		break;
> +	case SBC_SAMPLING_FREQ_32000:
> +		out->frequency = SBC_FREQ_32000;
> +		break;
> +	case SBC_SAMPLING_FREQ_44100:
> +		out->frequency = SBC_FREQ_44100;
> +		break;
> +	case SBC_SAMPLING_FREQ_48000:
> +		out->frequency = SBC_FREQ_48000;
> +		break;
> +	}
> +
> +	out->subbands = in->subbands == SBC_SUBBANDS_4 ? SBC_SB_4 : SBC_SB_8;
> +
> +	switch (in->channel_mode) {
> +	case SBC_CHANNEL_MODE_MONO:
> +		out->mode = SBC_MODE_MONO;
> +		break;
> +	case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> +		out->mode = SBC_MODE_DUAL_CHANNEL;
> +		break;
> +	case SBC_CHANNEL_MODE_JOINT_STEREO:
> +		out->mode = SBC_MODE_JOINT_STEREO;
> +		break;
> +	case SBC_CHANNEL_MODE_STEREO:
> +		out->mode = SBC_MODE_STEREO;
> +		break;
> +	}
> +
> +	out->endian = SBC_LE;
> +
> +	out->bitpool = in->max_bitpool;
> +
> +	out->allocation = in->allocation_method == SBC_ALLOCATION_SNR ?
> +				SBC_AM_SNR : SBC_AM_LOUDNESS;
> +
> +	switch (in->block_length) {
> +	case SBC_BLOCK_LENGTH_4:
> +		out->blocks = SBC_BLK_4;
> +		break;
> +	case SBC_BLOCK_LENGTH_8:
> +		out->blocks = SBC_BLK_8;
> +		break;
> +	case SBC_BLOCK_LENGTH_12:
> +		out->blocks = SBC_BLK_12;
> +		break;
> +	case SBC_BLOCK_LENGTH_16:
> +		out->blocks = SBC_BLK_16;
> +		break;
> +	}

aren’t the values all the same? This looks pretty complicated for something that should be dead simple. Does Android really had to duplicate every single definition with the same prefix?

> +}
> +
> static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
> {
> 	struct sbc_data *sbc_data;
> @@ -199,6 +266,8 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
> 
> 	memcpy(&sbc_data->sbc, preset->data, preset->len);
> 
> +	sbc_init_encoder(sbc_data);
> +
> 	*codec_data = sbc_data;
> 
> 	return AUDIO_STATUS_SUCCESS;
> @@ -206,8 +275,11 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
> 
> static int sbc_cleanup(void *codec_data)
> {
> +	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
> +
> 	DBG("");
> 
> +	sbc_finish(&sbc_data->enc);
> 	free(codec_data);
> 
> 	return AUDIO_STATUS_SUCCESS;

Regards

Marcel

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Andrzej Kaczmarek]

Hi Marcel,

On 17 January 2014 19:24, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrzej,
>
>> ---
>> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++=
++++++
>> 1 file changed, 72 insertions(+)
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index f53dba0..e5c646c 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -32,6 +32,7 @@
>> #include "hal-log.h"
>> #include "hal-msg.h"
>> #include "../profiles/audio/a2dp-codecs.h"
>> +#include <sbc/sbc.h>
>>
>> static const uint8_t a2dp_src_uuid[] =3D {
>>               0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>
>> struct sbc_data {
>>       a2dp_sbc_t sbc;
>> +
>> +     sbc_t enc;
>> };
>>
>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *pre=
set, size_t *len)
>>       return i;
>> }
>>
>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>> +{
>> +     a2dp_sbc_t *in =3D &sbc_data->sbc;
>> +     sbc_t *out =3D &sbc_data->enc;
>> +
>> +     DBG("");
>> +
>> +     sbc_init(out, 0L);
>> +
>> +     switch (in->frequency) {
>> +     case SBC_SAMPLING_FREQ_16000:
>> +             out->frequency =3D SBC_FREQ_16000;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_32000:
>> +             out->frequency =3D SBC_FREQ_32000;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_44100:
>> +             out->frequency =3D SBC_FREQ_44100;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_48000:
>> +             out->frequency =3D SBC_FREQ_48000;
>> +             break;
>> +     }
>> +
>> +     out->subbands =3D in->subbands =3D=3D SBC_SUBBANDS_4 ? SBC_SB_4 : =
SBC_SB_8;
>> +
>> +     switch (in->channel_mode) {
>> +     case SBC_CHANNEL_MODE_MONO:
>> +             out->mode =3D SBC_MODE_MONO;
>> +             break;
>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>> +             out->mode =3D SBC_MODE_DUAL_CHANNEL;
>> +             break;
>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>> +             out->mode =3D SBC_MODE_JOINT_STEREO;
>> +             break;
>> +     case SBC_CHANNEL_MODE_STEREO:
>> +             out->mode =3D SBC_MODE_STEREO;
>> +             break;
>> +     }
>> +
>> +     out->endian =3D SBC_LE;
>> +
>> +     out->bitpool =3D in->max_bitpool;
>> +
>> +     out->allocation =3D in->allocation_method =3D=3D SBC_ALLOCATION_SN=
R ?
>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>> +
>> +     switch (in->block_length) {
>> +     case SBC_BLOCK_LENGTH_4:
>> +             out->blocks =3D SBC_BLK_4;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_8:
>> +             out->blocks =3D SBC_BLK_8;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_12:
>> +             out->blocks =3D SBC_BLK_12;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_16:
>> +             out->blocks =3D SBC_BLK_16;
>> +             break;
>> +     }
>
> aren=92t the values all the same? This looks pretty complicated for somet=
hing that should be dead simple. Does Android really had to duplicate every=
 single definition with the same prefix?

Actually symbols for 'in' come from a2dp-codecs.h and 'out' from sbc.h
;-) And they have different values since A2DP uses shifted-bit values
while sbc.h are just ordinal values so cannot assign them directly.


BR,
Andrzej

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Marcel Holtmann]

Hi Andrezj,

>>> ---
>>> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 72 insertions(+)
>>> 
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index f53dba0..e5c646c 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -32,6 +32,7 @@
>>> #include "hal-log.h"
>>> #include "hal-msg.h"
>>> #include "../profiles/audio/a2dp-codecs.h"
>>> +#include <sbc/sbc.h>
>>> 
>>> static const uint8_t a2dp_src_uuid[] = {
>>>              0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>> 
>>> struct sbc_data {
>>>      a2dp_sbc_t sbc;
>>> +
>>> +     sbc_t enc;
>>> };
>>> 
>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
>>>      return i;
>>> }
>>> 
>>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>>> +{
>>> +     a2dp_sbc_t *in = &sbc_data->sbc;
>>> +     sbc_t *out = &sbc_data->enc;
>>> +
>>> +     DBG("");
>>> +
>>> +     sbc_init(out, 0L);
>>> +
>>> +     switch (in->frequency) {
>>> +     case SBC_SAMPLING_FREQ_16000:
>>> +             out->frequency = SBC_FREQ_16000;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_32000:
>>> +             out->frequency = SBC_FREQ_32000;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_44100:
>>> +             out->frequency = SBC_FREQ_44100;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_48000:
>>> +             out->frequency = SBC_FREQ_48000;
>>> +             break;
>>> +     }
>>> +
>>> +     out->subbands = in->subbands == SBC_SUBBANDS_4 ? SBC_SB_4 : SBC_SB_8;
>>> +
>>> +     switch (in->channel_mode) {
>>> +     case SBC_CHANNEL_MODE_MONO:
>>> +             out->mode = SBC_MODE_MONO;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>>> +             out->mode = SBC_MODE_DUAL_CHANNEL;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>>> +             out->mode = SBC_MODE_JOINT_STEREO;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_STEREO:
>>> +             out->mode = SBC_MODE_STEREO;
>>> +             break;
>>> +     }
>>> +
>>> +     out->endian = SBC_LE;
>>> +
>>> +     out->bitpool = in->max_bitpool;
>>> +
>>> +     out->allocation = in->allocation_method == SBC_ALLOCATION_SNR ?
>>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>>> +
>>> +     switch (in->block_length) {
>>> +     case SBC_BLOCK_LENGTH_4:
>>> +             out->blocks = SBC_BLK_4;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_8:
>>> +             out->blocks = SBC_BLK_8;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_12:
>>> +             out->blocks = SBC_BLK_12;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_16:
>>> +             out->blocks = SBC_BLK_16;
>>> +             break;
>>> +     }
>> 
>> aren’t the values all the same? This looks pretty complicated for something that should be dead simple. Does Android really had to duplicate every single definition with the same prefix?
> 
> Actually symbols for 'in' come from a2dp-codecs.h and 'out' from sbc.h
> ;-) And they have different values since A2DP uses shifted-bit values
> while sbc.h are just ordinal values so cannot assign them directly.

so this a problem we created by ourselves. Yeah. Seems no cookie for me tonight ;)

We need to start fixing a2dp-codecs.h then and prefix it with A2DP. This current situation is bad. Luiz?

Regards

Marcel

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Jan 17, 2014 at 9:28 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andrezj,
>
>>>> ---
>>>> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++=
++++++++
>>>> 1 file changed, 72 insertions(+)
>>>>
>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>> index f53dba0..e5c646c 100644
>>>> --- a/android/hal-audio.c
>>>> +++ b/android/hal-audio.c
>>>> @@ -32,6 +32,7 @@
>>>> #include "hal-log.h"
>>>> #include "hal-msg.h"
>>>> #include "../profiles/audio/a2dp-codecs.h"
>>>> +#include <sbc/sbc.h>
>>>>
>>>> static const uint8_t a2dp_src_uuid[] =3D {
>>>>              0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>>>
>>>> struct sbc_data {
>>>>      a2dp_sbc_t sbc;
>>>> +
>>>> +     sbc_t enc;
>>>> };
>>>>
>>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *p=
reset, size_t *len)
>>>>      return i;
>>>> }
>>>>
>>>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>>>> +{
>>>> +     a2dp_sbc_t *in =3D &sbc_data->sbc;
>>>> +     sbc_t *out =3D &sbc_data->enc;
>>>> +
>>>> +     DBG("");
>>>> +
>>>> +     sbc_init(out, 0L);
>>>> +
>>>> +     switch (in->frequency) {
>>>> +     case SBC_SAMPLING_FREQ_16000:
>>>> +             out->frequency =3D SBC_FREQ_16000;
>>>> +             break;
>>>> +     case SBC_SAMPLING_FREQ_32000:
>>>> +             out->frequency =3D SBC_FREQ_32000;
>>>> +             break;
>>>> +     case SBC_SAMPLING_FREQ_44100:
>>>> +             out->frequency =3D SBC_FREQ_44100;
>>>> +             break;
>>>> +     case SBC_SAMPLING_FREQ_48000:
>>>> +             out->frequency =3D SBC_FREQ_48000;
>>>> +             break;
>>>> +     }
>>>> +
>>>> +     out->subbands =3D in->subbands =3D=3D SBC_SUBBANDS_4 ? SBC_SB_4 =
: SBC_SB_8;
>>>> +
>>>> +     switch (in->channel_mode) {
>>>> +     case SBC_CHANNEL_MODE_MONO:
>>>> +             out->mode =3D SBC_MODE_MONO;
>>>> +             break;
>>>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>>>> +             out->mode =3D SBC_MODE_DUAL_CHANNEL;
>>>> +             break;
>>>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>>>> +             out->mode =3D SBC_MODE_JOINT_STEREO;
>>>> +             break;
>>>> +     case SBC_CHANNEL_MODE_STEREO:
>>>> +             out->mode =3D SBC_MODE_STEREO;
>>>> +             break;
>>>> +     }
>>>> +
>>>> +     out->endian =3D SBC_LE;
>>>> +
>>>> +     out->bitpool =3D in->max_bitpool;
>>>> +
>>>> +     out->allocation =3D in->allocation_method =3D=3D SBC_ALLOCATION_=
SNR ?
>>>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>>>> +
>>>> +     switch (in->block_length) {
>>>> +     case SBC_BLOCK_LENGTH_4:
>>>> +             out->blocks =3D SBC_BLK_4;
>>>> +             break;
>>>> +     case SBC_BLOCK_LENGTH_8:
>>>> +             out->blocks =3D SBC_BLK_8;
>>>> +             break;
>>>> +     case SBC_BLOCK_LENGTH_12:
>>>> +             out->blocks =3D SBC_BLK_12;
>>>> +             break;
>>>> +     case SBC_BLOCK_LENGTH_16:
>>>> +             out->blocks =3D SBC_BLK_16;
>>>> +             break;
>>>> +     }
>>>
>>> aren=92t the values all the same? This looks pretty complicated for som=
ething that should be dead simple. Does Android really had to duplicate eve=
ry single definition with the same prefix?
>>
>> Actually symbols for 'in' come from a2dp-codecs.h and 'out' from sbc.h
>> ;-) And they have different values since A2DP uses shifted-bit values
>> while sbc.h are just ordinal values so cannot assign them directly.
>
> so this a problem we created by ourselves. Yeah. Seems no cookie for me t=
onight ;)
>
> We need to start fixing a2dp-codecs.h then and prefix it with A2DP. This =
current situation is bad. Luiz?

Looks like it, what about a helper function inside sbc that takes care
of this conversion?


--=20
Luiz Augusto von Dentz

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Marcel Holtmann]

Hi Luiz,

>>>>> ---
>>>>> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 72 insertions(+)
>>>>> 
>>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>>> index f53dba0..e5c646c 100644
>>>>> --- a/android/hal-audio.c
>>>>> +++ b/android/hal-audio.c
>>>>> @@ -32,6 +32,7 @@
>>>>> #include "hal-log.h"
>>>>> #include "hal-msg.h"
>>>>> #include "../profiles/audio/a2dp-codecs.h"
>>>>> +#include <sbc/sbc.h>
>>>>> 
>>>>> static const uint8_t a2dp_src_uuid[] = {
>>>>>             0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>>>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>>>> 
>>>>> struct sbc_data {
>>>>>     a2dp_sbc_t sbc;
>>>>> +
>>>>> +     sbc_t enc;
>>>>> };
>>>>> 
>>>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>>>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *preset, size_t *len)
>>>>>     return i;
>>>>> }
>>>>> 
>>>>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>>>>> +{
>>>>> +     a2dp_sbc_t *in = &sbc_data->sbc;
>>>>> +     sbc_t *out = &sbc_data->enc;
>>>>> +
>>>>> +     DBG("");
>>>>> +
>>>>> +     sbc_init(out, 0L);
>>>>> +
>>>>> +     switch (in->frequency) {
>>>>> +     case SBC_SAMPLING_FREQ_16000:
>>>>> +             out->frequency = SBC_FREQ_16000;
>>>>> +             break;
>>>>> +     case SBC_SAMPLING_FREQ_32000:
>>>>> +             out->frequency = SBC_FREQ_32000;
>>>>> +             break;
>>>>> +     case SBC_SAMPLING_FREQ_44100:
>>>>> +             out->frequency = SBC_FREQ_44100;
>>>>> +             break;
>>>>> +     case SBC_SAMPLING_FREQ_48000:
>>>>> +             out->frequency = SBC_FREQ_48000;
>>>>> +             break;
>>>>> +     }
>>>>> +
>>>>> +     out->subbands = in->subbands == SBC_SUBBANDS_4 ? SBC_SB_4 : SBC_SB_8;
>>>>> +
>>>>> +     switch (in->channel_mode) {
>>>>> +     case SBC_CHANNEL_MODE_MONO:
>>>>> +             out->mode = SBC_MODE_MONO;
>>>>> +             break;
>>>>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>>>>> +             out->mode = SBC_MODE_DUAL_CHANNEL;
>>>>> +             break;
>>>>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>>>>> +             out->mode = SBC_MODE_JOINT_STEREO;
>>>>> +             break;
>>>>> +     case SBC_CHANNEL_MODE_STEREO:
>>>>> +             out->mode = SBC_MODE_STEREO;
>>>>> +             break;
>>>>> +     }
>>>>> +
>>>>> +     out->endian = SBC_LE;
>>>>> +
>>>>> +     out->bitpool = in->max_bitpool;
>>>>> +
>>>>> +     out->allocation = in->allocation_method == SBC_ALLOCATION_SNR ?
>>>>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>>>>> +
>>>>> +     switch (in->block_length) {
>>>>> +     case SBC_BLOCK_LENGTH_4:
>>>>> +             out->blocks = SBC_BLK_4;
>>>>> +             break;
>>>>> +     case SBC_BLOCK_LENGTH_8:
>>>>> +             out->blocks = SBC_BLK_8;
>>>>> +             break;
>>>>> +     case SBC_BLOCK_LENGTH_12:
>>>>> +             out->blocks = SBC_BLK_12;
>>>>> +             break;
>>>>> +     case SBC_BLOCK_LENGTH_16:
>>>>> +             out->blocks = SBC_BLK_16;
>>>>> +             break;
>>>>> +     }
>>>> 
>>>> aren’t the values all the same? This looks pretty complicated for something that should be dead simple. Does Android really had to duplicate every single definition with the same prefix?
>>> 
>>> Actually symbols for 'in' come from a2dp-codecs.h and 'out' from sbc.h
>>> ;-) And they have different values since A2DP uses shifted-bit values
>>> while sbc.h are just ordinal values so cannot assign them directly.
>> 
>> so this a problem we created by ourselves. Yeah. Seems no cookie for me tonight ;)
>> 
>> We need to start fixing a2dp-codecs.h then and prefix it with A2DP. This current situation is bad. Luiz?
> 
> Looks like it, what about a helper function inside sbc that takes care
> of this conversion?

please send me libsbc patches that I can look at it. I am open for adding this.

Regards

Marcel

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Luiz Augusto von Dentz]

Hi Andrzej,

On Fri, Jan 17, 2014 at 9:26 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Hi Marcel,
>
> On 17 January 2014 19:24, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Andrzej,
>>
>>> ---
>>> android/hal-audio.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++=
+++++++
>>> 1 file changed, 72 insertions(+)
>>>
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index f53dba0..e5c646c 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -32,6 +32,7 @@
>>> #include "hal-log.h"
>>> #include "hal-msg.h"
>>> #include "../profiles/audio/a2dp-codecs.h"
>>> +#include <sbc/sbc.h>
>>>
>>> static const uint8_t a2dp_src_uuid[] =3D {
>>>               0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>>
>>> struct sbc_data {
>>>       a2dp_sbc_t sbc;
>>> +
>>> +     sbc_t enc;
>>> };
>>>
>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *pr=
eset, size_t *len)
>>>       return i;
>>> }
>>>
>>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>>> +{
>>> +     a2dp_sbc_t *in =3D &sbc_data->sbc;
>>> +     sbc_t *out =3D &sbc_data->enc;
>>> +
>>> +     DBG("");
>>> +
>>> +     sbc_init(out, 0L);
>>> +
>>> +     switch (in->frequency) {
>>> +     case SBC_SAMPLING_FREQ_16000:
>>> +             out->frequency =3D SBC_FREQ_16000;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_32000:
>>> +             out->frequency =3D SBC_FREQ_32000;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_44100:
>>> +             out->frequency =3D SBC_FREQ_44100;
>>> +             break;
>>> +     case SBC_SAMPLING_FREQ_48000:
>>> +             out->frequency =3D SBC_FREQ_48000;
>>> +             break;
>>> +     }
>>> +
>>> +     out->subbands =3D in->subbands =3D=3D SBC_SUBBANDS_4 ? SBC_SB_4 :=
 SBC_SB_8;
>>> +
>>> +     switch (in->channel_mode) {
>>> +     case SBC_CHANNEL_MODE_MONO:
>>> +             out->mode =3D SBC_MODE_MONO;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>>> +             out->mode =3D SBC_MODE_DUAL_CHANNEL;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>>> +             out->mode =3D SBC_MODE_JOINT_STEREO;
>>> +             break;
>>> +     case SBC_CHANNEL_MODE_STEREO:
>>> +             out->mode =3D SBC_MODE_STEREO;
>>> +             break;
>>> +     }
>>> +
>>> +     out->endian =3D SBC_LE;
>>> +
>>> +     out->bitpool =3D in->max_bitpool;
>>> +
>>> +     out->allocation =3D in->allocation_method =3D=3D SBC_ALLOCATION_S=
NR ?
>>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>>> +
>>> +     switch (in->block_length) {
>>> +     case SBC_BLOCK_LENGTH_4:
>>> +             out->blocks =3D SBC_BLK_4;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_8:
>>> +             out->blocks =3D SBC_BLK_8;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_12:
>>> +             out->blocks =3D SBC_BLK_12;
>>> +             break;
>>> +     case SBC_BLOCK_LENGTH_16:
>>> +             out->blocks =3D SBC_BLK_16;
>>> +             break;
>>> +     }
>>
>> aren=92t the values all the same? This looks pretty complicated for some=
thing that should be dead simple. Does Android really had to duplicate ever=
y single definition with the same prefix?
>
> Actually symbols for 'in' come from a2dp-codecs.h and 'out' from sbc.h
> ;-) And they have different values since A2DP uses shifted-bit values
> while sbc.h are just ordinal values so cannot assign them directly.

Perhaps we should create some helper function in sbc code to do that
translation for us, I was even considering stuff the rtp packing there
as well.


--=20
Luiz Augusto von Dentz

Re: [PATCH 4/9] android/hal-audio: Initialize SBC encoder

[Luiz Augusto von Dentz]

Hi Marcel,

On Fri, Jan 17, 2014 at 8:24 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andrzej,
>
>> ---
>> android/hal-audio.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++=
++++++
>> 1 file changed, 72 insertions(+)
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index f53dba0..e5c646c 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -32,6 +32,7 @@
>> #include "hal-log.h"
>> #include "hal-msg.h"
>> #include "../profiles/audio/a2dp-codecs.h"
>> +#include <sbc/sbc.h>
>>
>> static const uint8_t a2dp_src_uuid[] =3D {
>>               0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>> @@ -53,6 +54,8 @@ struct audio_input_config {
>>
>> struct sbc_data {
>>       a2dp_sbc_t sbc;
>> +
>> +     sbc_t enc;
>> };
>>
>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>> @@ -184,6 +187,70 @@ static int sbc_get_presets(struct audio_preset *pre=
set, size_t *len)
>>       return i;
>> }
>>
>> +static void sbc_init_encoder(struct sbc_data *sbc_data)
>> +{
>> +     a2dp_sbc_t *in =3D &sbc_data->sbc;
>> +     sbc_t *out =3D &sbc_data->enc;
>> +
>> +     DBG("");
>> +
>> +     sbc_init(out, 0L);
>> +
>> +     switch (in->frequency) {
>> +     case SBC_SAMPLING_FREQ_16000:
>> +             out->frequency =3D SBC_FREQ_16000;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_32000:
>> +             out->frequency =3D SBC_FREQ_32000;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_44100:
>> +             out->frequency =3D SBC_FREQ_44100;
>> +             break;
>> +     case SBC_SAMPLING_FREQ_48000:
>> +             out->frequency =3D SBC_FREQ_48000;
>> +             break;
>> +     }
>> +
>> +     out->subbands =3D in->subbands =3D=3D SBC_SUBBANDS_4 ? SBC_SB_4 : =
SBC_SB_8;
>> +
>> +     switch (in->channel_mode) {
>> +     case SBC_CHANNEL_MODE_MONO:
>> +             out->mode =3D SBC_MODE_MONO;
>> +             break;
>> +     case SBC_CHANNEL_MODE_DUAL_CHANNEL:
>> +             out->mode =3D SBC_MODE_DUAL_CHANNEL;
>> +             break;
>> +     case SBC_CHANNEL_MODE_JOINT_STEREO:
>> +             out->mode =3D SBC_MODE_JOINT_STEREO;
>> +             break;
>> +     case SBC_CHANNEL_MODE_STEREO:
>> +             out->mode =3D SBC_MODE_STEREO;
>> +             break;
>> +     }
>> +
>> +     out->endian =3D SBC_LE;
>> +
>> +     out->bitpool =3D in->max_bitpool;
>> +
>> +     out->allocation =3D in->allocation_method =3D=3D SBC_ALLOCATION_SN=
R ?
>> +                             SBC_AM_SNR : SBC_AM_LOUDNESS;
>> +
>> +     switch (in->block_length) {
>> +     case SBC_BLOCK_LENGTH_4:
>> +             out->blocks =3D SBC_BLK_4;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_8:
>> +             out->blocks =3D SBC_BLK_8;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_12:
>> +             out->blocks =3D SBC_BLK_12;
>> +             break;
>> +     case SBC_BLOCK_LENGTH_16:
>> +             out->blocks =3D SBC_BLK_16;
>> +             break;
>> +     }
>
> aren=92t the values all the same? This looks pretty complicated for somet=
hing that should be dead simple. Does Android really had to duplicate every=
 single definition with the same prefix?

This is the conversion from A2DP configuration to libsbc
representation but I believe we don't really need to do the switch
statement, which anyway are done with defines from sbc.h instead of
a2dp-codecs.h.

[PATCH 5/9] android/hal-audio: Calculate SBC stream parameters

[Andrzej Kaczmarek]

This patch adds necessary calculations for SBC stream parameters.

Both input and output buffers are expected to have exact amount of
data to fill single media packet (based on transport channel MTU).

Frame duration will be used to synchronize input and output streams.
---
 android/hal-audio.c | 50 ++++++++++++++++++++++++++++++-----
 android/rtp.h       | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 6 deletions(-)
 create mode 100644 android/rtp.h

diff --git a/android/hal-audio.c b/android/hal-audio.c
index e5c646c..3f53295 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -33,6 +33,7 @@
 #include "hal-msg.h"
 #include "../profiles/audio/a2dp-codecs.h"
 #include <sbc/sbc.h>
+#include "rtp.h"
 
 static const uint8_t a2dp_src_uuid[] = {
 		0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
@@ -46,6 +47,12 @@ static pthread_t ipc_th = 0;
 static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+struct media_packet {
+	struct rtp_header hdr;
+	struct rtp_payload payload;
+	uint8_t data[0];
+};
+
 struct audio_input_config {
 	uint32_t rate;
 	uint32_t channels;
@@ -56,10 +63,19 @@ struct sbc_data {
 	a2dp_sbc_t sbc;
 
 	sbc_t enc;
+
+	size_t in_frame_len;
+	size_t in_buf_size;
+
+	size_t out_buf_size;
+	uint8_t *out_buf;
+
+	unsigned frame_duration;
 };
 
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
-static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
+static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
+				void **codec_data);
 static int sbc_cleanup(void *codec_data);
 static int sbc_get_config(void *codec_data,
 					struct audio_input_config *config);
@@ -69,7 +85,8 @@ struct audio_codec {
 
 	int (*get_presets) (struct audio_preset *preset, size_t *len);
 
-	int (*init) (struct audio_preset *preset, void **codec_data);
+	int (*init) (struct audio_preset *preset, uint16_t mtu,
+				void **codec_data);
 	int (*cleanup) (void *codec_data);
 	int (*get_config) (void *codec_data,
 					struct audio_input_config *config);
@@ -251,9 +268,14 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
 	}
 }
 
-static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
+static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
+				void **codec_data)
 {
 	struct sbc_data *sbc_data;
+	size_t hdr_len = sizeof(struct media_packet);
+	size_t in_frame_len;
+	size_t out_frame_len;
+	size_t num_frames;
 
 	DBG("");
 
@@ -268,6 +290,18 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
 
 	sbc_init_encoder(sbc_data);
 
+	in_frame_len = sbc_get_codesize(&sbc_data->enc);
+	out_frame_len = sbc_get_frame_length(&sbc_data->enc);
+	num_frames = (mtu - hdr_len) / out_frame_len;
+
+	sbc_data->in_frame_len = in_frame_len;
+	sbc_data->in_buf_size = num_frames * in_frame_len;
+
+	sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
+	sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
+
+	sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
+
 	*codec_data = sbc_data;
 
 	return AUDIO_STATUS_SUCCESS;
@@ -280,6 +314,7 @@ static int sbc_cleanup(void *codec_data)
 	DBG("");
 
 	sbc_finish(&sbc_data->enc);
+	free(sbc_data->out_buf);
 	free(codec_data);
 
 	return AUDIO_STATUS_SUCCESS;
@@ -511,7 +546,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
 	return result;
 }
 
-static int ipc_open_stream_cmd(uint8_t endpoint_id,
+static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
 					struct audio_preset **caps)
 {
 	char buf[BLUEZ_AUDIO_MTU];
@@ -534,6 +569,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
 	if (result == AUDIO_STATUS_SUCCESS) {
 		size_t buf_len = sizeof(struct audio_preset) +
 					rsp->preset[0].len;
+		*mtu = rsp->mtu;
 		*caps = malloc(buf_len);
 		memcpy(*caps, &rsp->preset, buf_len);
 	} else {
@@ -919,6 +955,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	struct a2dp_stream_out *out;
 	struct audio_preset *preset;
 	const struct audio_codec *codec;
+	uint16_t mtu;
 
 	out = calloc(1, sizeof(struct a2dp_stream_out));
 	if (!out)
@@ -946,7 +983,8 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	/* TODO: for now we always use endpoint 0 */
 	out->ep = &audio_endpoints[0];
 
-	if (ipc_open_stream_cmd(out->ep->id, &preset) != AUDIO_STATUS_SUCCESS)
+	if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
+			AUDIO_STATUS_SUCCESS)
 		goto fail;
 
 	if (!preset)
@@ -954,7 +992,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 
 	codec = out->ep->codec;
 
-	codec->init(preset, &out->ep->codec_data);
+	codec->init(preset, mtu, &out->ep->codec_data);
 	codec->get_config(out->ep->codec_data, &out->cfg);
 
 	DBG("rate=%d channels=%d format=%d", out->cfg.rate,
diff --git a/android/rtp.h b/android/rtp.h
new file mode 100644
index 0000000..45fddcf
--- /dev/null
+++ b/android/rtp.h
@@ -0,0 +1,76 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ *  This library 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.
+ *
+ *  This library 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
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+
+struct rtp_header {
+	unsigned cc:4;
+	unsigned x:1;
+	unsigned p:1;
+	unsigned v:2;
+
+	unsigned pt:7;
+	unsigned m:1;
+
+	uint16_t sequence_number;
+	uint32_t timestamp;
+	uint32_t ssrc;
+	uint32_t csrc[0];
+} __attribute__ ((packed));
+
+struct rtp_payload {
+	unsigned frame_count:4;
+	unsigned rfa0:1;
+	unsigned is_last_fragment:1;
+	unsigned is_first_fragment:1;
+	unsigned is_fragmented:1;
+} __attribute__ ((packed));
+
+#elif __BYTE_ORDER == __BIG_ENDIAN
+
+struct rtp_header {
+	unsigned v:2;
+	unsigned p:1;
+	unsigned x:1;
+	unsigned cc:4;
+
+	unsigned m:1;
+	unsigned pt:7;
+
+	uint16_t sequence_number;
+	uint32_t timestamp;
+	uint32_t ssrc;
+	uint32_t csrc[0];
+} __attribute__ ((packed));
+
+struct rtp_payload {
+	unsigned is_fragmented:1;
+	unsigned is_first_fragment:1;
+	unsigned is_last_fragment:1;
+	unsigned rfa0:1;
+	unsigned frame_count:4;
+} __attribute__ ((packed));
+
+#else
+#error "Unknown byte order"
+#endif
-- 
1.8.5.2

Re: [PATCH 5/9] android/hal-audio: Calculate SBC stream parameters

[Marcel Holtmann]

Hi Andrzej,

> This patch adds necessary calculations for SBC stream parameters.
> 
> Both input and output buffers are expected to have exact amount of
> data to fill single media packet (based on transport channel MTU).
> 
> Frame duration will be used to synchronize input and output streams.
> ---
> android/hal-audio.c | 50 ++++++++++++++++++++++++++++++-----
> android/rtp.h       | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 120 insertions(+), 6 deletions(-)
> create mode 100644 android/rtp.h
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index e5c646c..3f53295 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -33,6 +33,7 @@
> #include "hal-msg.h"
> #include "../profiles/audio/a2dp-codecs.h"
> #include <sbc/sbc.h>
> +#include "rtp.h"
> 
> static const uint8_t a2dp_src_uuid[] = {
> 		0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
> @@ -46,6 +47,12 @@ static pthread_t ipc_th = 0;
> static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
> static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
> 
> +struct media_packet {
> +	struct rtp_header hdr;
> +	struct rtp_payload payload;
> +	uint8_t data[0];
> +};
> +
> struct audio_input_config {
> 	uint32_t rate;
> 	uint32_t channels;
> @@ -56,10 +63,19 @@ struct sbc_data {
> 	a2dp_sbc_t sbc;
> 
> 	sbc_t enc;
> +
> +	size_t in_frame_len;
> +	size_t in_buf_size;
> +
> +	size_t out_buf_size;
> +	uint8_t *out_buf;
> +
> +	unsigned frame_duration;
> };
> 
> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
> +				void **codec_data);
> static int sbc_cleanup(void *codec_data);
> static int sbc_get_config(void *codec_data,
> 					struct audio_input_config *config);
> @@ -69,7 +85,8 @@ struct audio_codec {
> 
> 	int (*get_presets) (struct audio_preset *preset, size_t *len);
> 
> -	int (*init) (struct audio_preset *preset, void **codec_data);
> +	int (*init) (struct audio_preset *preset, uint16_t mtu,
> +				void **codec_data);
> 	int (*cleanup) (void *codec_data);
> 	int (*get_config) (void *codec_data,
> 					struct audio_input_config *config);
> @@ -251,9 +268,14 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
> 	}
> }
> 
> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
> +				void **codec_data)
> {
> 	struct sbc_data *sbc_data;
> +	size_t hdr_len = sizeof(struct media_packet);
> +	size_t in_frame_len;
> +	size_t out_frame_len;
> +	size_t num_frames;
> 
> 	DBG("");
> 
> @@ -268,6 +290,18 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
> 
> 	sbc_init_encoder(sbc_data);
> 
> +	in_frame_len = sbc_get_codesize(&sbc_data->enc);
> +	out_frame_len = sbc_get_frame_length(&sbc_data->enc);
> +	num_frames = (mtu - hdr_len) / out_frame_len;
> +
> +	sbc_data->in_frame_len = in_frame_len;
> +	sbc_data->in_buf_size = num_frames * in_frame_len;
> +
> +	sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
> +	sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
> +
> +	sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
> +
> 	*codec_data = sbc_data;
> 
> 	return AUDIO_STATUS_SUCCESS;
> @@ -280,6 +314,7 @@ static int sbc_cleanup(void *codec_data)
> 	DBG("");
> 
> 	sbc_finish(&sbc_data->enc);
> +	free(sbc_data->out_buf);
> 	free(codec_data);
> 
> 	return AUDIO_STATUS_SUCCESS;
> @@ -511,7 +546,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
> 	return result;
> }
> 
> -static int ipc_open_stream_cmd(uint8_t endpoint_id,
> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
> 					struct audio_preset **caps)
> {
> 	char buf[BLUEZ_AUDIO_MTU];
> @@ -534,6 +569,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
> 	if (result == AUDIO_STATUS_SUCCESS) {
> 		size_t buf_len = sizeof(struct audio_preset) +
> 					rsp->preset[0].len;
> +		*mtu = rsp->mtu;
> 		*caps = malloc(buf_len);
> 		memcpy(*caps, &rsp->preset, buf_len);
> 	} else {
> @@ -919,6 +955,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
> 	struct a2dp_stream_out *out;
> 	struct audio_preset *preset;
> 	const struct audio_codec *codec;
> +	uint16_t mtu;
> 
> 	out = calloc(1, sizeof(struct a2dp_stream_out));
> 	if (!out)
> @@ -946,7 +983,8 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
> 	/* TODO: for now we always use endpoint 0 */
> 	out->ep = &audio_endpoints[0];
> 
> -	if (ipc_open_stream_cmd(out->ep->id, &preset) != AUDIO_STATUS_SUCCESS)
> +	if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
> +			AUDIO_STATUS_SUCCESS)
> 		goto fail;
> 
> 	if (!preset)
> @@ -954,7 +992,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
> 
> 	codec = out->ep->codec;
> 
> -	codec->init(preset, &out->ep->codec_data);
> +	codec->init(preset, mtu, &out->ep->codec_data);
> 	codec->get_config(out->ep->codec_data, &out->cfg);
> 
> 	DBG("rate=%d channels=%d format=%d", out->cfg.rate,
> diff --git a/android/rtp.h b/android/rtp.h
> new file mode 100644
> index 0000000..45fddcf
> --- /dev/null
> +++ b/android/rtp.h
> @@ -0,0 +1,76 @@
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
> + *
> + *
> + *  This library 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.
> + *
> + *  This library 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
> + *  Lesser General Public License for more details.
> + *
> + *  You should have received a copy of the GNU Lesser General Public
> + *  License along with this library; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */

where is this file coming from? Why do we need a copy of it?

Regards

Marcel

Re: [PATCH 5/9] android/hal-audio: Calculate SBC stream parameters

[Andrzej Kaczmarek]

Hi Marcel,

On 17 January 2014 19:25, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andrzej,
>
>> This patch adds necessary calculations for SBC stream parameters.
>>
>> Both input and output buffers are expected to have exact amount of
>> data to fill single media packet (based on transport channel MTU).
>>
>> Frame duration will be used to synchronize input and output streams.
>> ---
>> android/hal-audio.c | 50 ++++++++++++++++++++++++++++++-----
>> android/rtp.h       | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 120 insertions(+), 6 deletions(-)
>> create mode 100644 android/rtp.h
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index e5c646c..3f53295 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -33,6 +33,7 @@
>> #include "hal-msg.h"
>> #include "../profiles/audio/a2dp-codecs.h"
>> #include <sbc/sbc.h>
>> +#include "rtp.h"
>>
>> static const uint8_t a2dp_src_uuid[] = {
>>               0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>> @@ -46,6 +47,12 @@ static pthread_t ipc_th = 0;
>> static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
>> static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
>>
>> +struct media_packet {
>> +     struct rtp_header hdr;
>> +     struct rtp_payload payload;
>> +     uint8_t data[0];
>> +};
>> +
>> struct audio_input_config {
>>       uint32_t rate;
>>       uint32_t channels;
>> @@ -56,10 +63,19 @@ struct sbc_data {
>>       a2dp_sbc_t sbc;
>>
>>       sbc_t enc;
>> +
>> +     size_t in_frame_len;
>> +     size_t in_buf_size;
>> +
>> +     size_t out_buf_size;
>> +     uint8_t *out_buf;
>> +
>> +     unsigned frame_duration;
>> };
>>
>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>> +                             void **codec_data);
>> static int sbc_cleanup(void *codec_data);
>> static int sbc_get_config(void *codec_data,
>>                                       struct audio_input_config *config);
>> @@ -69,7 +85,8 @@ struct audio_codec {
>>
>>       int (*get_presets) (struct audio_preset *preset, size_t *len);
>>
>> -     int (*init) (struct audio_preset *preset, void **codec_data);
>> +     int (*init) (struct audio_preset *preset, uint16_t mtu,
>> +                             void **codec_data);
>>       int (*cleanup) (void *codec_data);
>>       int (*get_config) (void *codec_data,
>>                                       struct audio_input_config *config);
>> @@ -251,9 +268,14 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>>       }
>> }
>>
>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>> +                             void **codec_data)
>> {
>>       struct sbc_data *sbc_data;
>> +     size_t hdr_len = sizeof(struct media_packet);
>> +     size_t in_frame_len;
>> +     size_t out_frame_len;
>> +     size_t num_frames;
>>
>>       DBG("");
>>
>> @@ -268,6 +290,18 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>>
>>       sbc_init_encoder(sbc_data);
>>
>> +     in_frame_len = sbc_get_codesize(&sbc_data->enc);
>> +     out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>> +     num_frames = (mtu - hdr_len) / out_frame_len;
>> +
>> +     sbc_data->in_frame_len = in_frame_len;
>> +     sbc_data->in_buf_size = num_frames * in_frame_len;
>> +
>> +     sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
>> +     sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
>> +
>> +     sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>> +
>>       *codec_data = sbc_data;
>>
>>       return AUDIO_STATUS_SUCCESS;
>> @@ -280,6 +314,7 @@ static int sbc_cleanup(void *codec_data)
>>       DBG("");
>>
>>       sbc_finish(&sbc_data->enc);
>> +     free(sbc_data->out_buf);
>>       free(codec_data);
>>
>>       return AUDIO_STATUS_SUCCESS;
>> @@ -511,7 +546,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>>       return result;
>> }
>>
>> -static int ipc_open_stream_cmd(uint8_t endpoint_id,
>> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
>>                                       struct audio_preset **caps)
>> {
>>       char buf[BLUEZ_AUDIO_MTU];
>> @@ -534,6 +569,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>       if (result == AUDIO_STATUS_SUCCESS) {
>>               size_t buf_len = sizeof(struct audio_preset) +
>>                                       rsp->preset[0].len;
>> +             *mtu = rsp->mtu;
>>               *caps = malloc(buf_len);
>>               memcpy(*caps, &rsp->preset, buf_len);
>>       } else {
>> @@ -919,6 +955,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>       struct a2dp_stream_out *out;
>>       struct audio_preset *preset;
>>       const struct audio_codec *codec;
>> +     uint16_t mtu;
>>
>>       out = calloc(1, sizeof(struct a2dp_stream_out));
>>       if (!out)
>> @@ -946,7 +983,8 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>       /* TODO: for now we always use endpoint 0 */
>>       out->ep = &audio_endpoints[0];
>>
>> -     if (ipc_open_stream_cmd(out->ep->id, &preset) != AUDIO_STATUS_SUCCESS)
>> +     if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
>> +                     AUDIO_STATUS_SUCCESS)
>>               goto fail;
>>
>>       if (!preset)
>> @@ -954,7 +992,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>
>>       codec = out->ep->codec;
>>
>> -     codec->init(preset, &out->ep->codec_data);
>> +     codec->init(preset, mtu, &out->ep->codec_data);
>>       codec->get_config(out->ep->codec_data, &out->cfg);
>>
>>       DBG("rate=%d channels=%d format=%d", out->cfg.rate,
>> diff --git a/android/rtp.h b/android/rtp.h
>> new file mode 100644
>> index 0000000..45fddcf
>> --- /dev/null
>> +++ b/android/rtp.h
>> @@ -0,0 +1,76 @@
>> +/*
>> + *
>> + *  BlueZ - Bluetooth protocol stack for Linux
>> + *
>> + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
>> + *
>> + *
>> + *  This library 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.
>> + *
>> + *  This library 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
>> + *  Lesser General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU Lesser General Public
>> + *  License along with this library; if not, write to the Free Software
>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> + *
>> + */
>
> where is this file coming from? Why do we need a copy of it?

>From BlueZ but it was removed (was used in pcm_bluetooth.c) so I just
added it again since we need RTP header structures. Or should these
just be added inline to hal-audio.c?


BR,
Andrzej

Re: [PATCH 5/9] android/hal-audio: Calculate SBC stream parameters

[Luiz Augusto von Dentz]

Hi Andrzej,

On Fri, Jan 17, 2014 at 9:17 PM, Andrzej Kaczmarek
<andrzej.kaczmarek@tieto.com> wrote:
> Hi Marcel,
>
> On 17 January 2014 19:25, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Andrzej,
>>
>>> This patch adds necessary calculations for SBC stream parameters.
>>>
>>> Both input and output buffers are expected to have exact amount of
>>> data to fill single media packet (based on transport channel MTU).
>>>
>>> Frame duration will be used to synchronize input and output streams.
>>> ---
>>> android/hal-audio.c | 50 ++++++++++++++++++++++++++++++-----
>>> android/rtp.h       | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 120 insertions(+), 6 deletions(-)
>>> create mode 100644 android/rtp.h
>>>
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index e5c646c..3f53295 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -33,6 +33,7 @@
>>> #include "hal-msg.h"
>>> #include "../profiles/audio/a2dp-codecs.h"
>>> #include <sbc/sbc.h>
>>> +#include "rtp.h"
>>>
>>> static const uint8_t a2dp_src_uuid[] = {
>>>               0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>> @@ -46,6 +47,12 @@ static pthread_t ipc_th = 0;
>>> static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
>>> static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
>>>
>>> +struct media_packet {
>>> +     struct rtp_header hdr;
>>> +     struct rtp_payload payload;
>>> +     uint8_t data[0];
>>> +};
>>> +
>>> struct audio_input_config {
>>>       uint32_t rate;
>>>       uint32_t channels;
>>> @@ -56,10 +63,19 @@ struct sbc_data {
>>>       a2dp_sbc_t sbc;
>>>
>>>       sbc_t enc;
>>> +
>>> +     size_t in_frame_len;
>>> +     size_t in_buf_size;
>>> +
>>> +     size_t out_buf_size;
>>> +     uint8_t *out_buf;
>>> +
>>> +     unsigned frame_duration;
>>> };
>>>
>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
>>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>>> +                             void **codec_data);
>>> static int sbc_cleanup(void *codec_data);
>>> static int sbc_get_config(void *codec_data,
>>>                                       struct audio_input_config *config);
>>> @@ -69,7 +85,8 @@ struct audio_codec {
>>>
>>>       int (*get_presets) (struct audio_preset *preset, size_t *len);
>>>
>>> -     int (*init) (struct audio_preset *preset, void **codec_data);
>>> +     int (*init) (struct audio_preset *preset, uint16_t mtu,
>>> +                             void **codec_data);
>>>       int (*cleanup) (void *codec_data);
>>>       int (*get_config) (void *codec_data,
>>>                                       struct audio_input_config *config);
>>> @@ -251,9 +268,14 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>>>       }
>>> }
>>>
>>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>>> +                             void **codec_data)
>>> {
>>>       struct sbc_data *sbc_data;
>>> +     size_t hdr_len = sizeof(struct media_packet);
>>> +     size_t in_frame_len;
>>> +     size_t out_frame_len;
>>> +     size_t num_frames;
>>>
>>>       DBG("");
>>>
>>> @@ -268,6 +290,18 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>>>
>>>       sbc_init_encoder(sbc_data);
>>>
>>> +     in_frame_len = sbc_get_codesize(&sbc_data->enc);
>>> +     out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>>> +     num_frames = (mtu - hdr_len) / out_frame_len;
>>> +
>>> +     sbc_data->in_frame_len = in_frame_len;
>>> +     sbc_data->in_buf_size = num_frames * in_frame_len;
>>> +
>>> +     sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
>>> +     sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
>>> +
>>> +     sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>>> +
>>>       *codec_data = sbc_data;
>>>
>>>       return AUDIO_STATUS_SUCCESS;
>>> @@ -280,6 +314,7 @@ static int sbc_cleanup(void *codec_data)
>>>       DBG("");
>>>
>>>       sbc_finish(&sbc_data->enc);
>>> +     free(sbc_data->out_buf);
>>>       free(codec_data);
>>>
>>>       return AUDIO_STATUS_SUCCESS;
>>> @@ -511,7 +546,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>>>       return result;
>>> }
>>>
>>> -static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
>>>                                       struct audio_preset **caps)
>>> {
>>>       char buf[BLUEZ_AUDIO_MTU];
>>> @@ -534,6 +569,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>>       if (result == AUDIO_STATUS_SUCCESS) {
>>>               size_t buf_len = sizeof(struct audio_preset) +
>>>                                       rsp->preset[0].len;
>>> +             *mtu = rsp->mtu;
>>>               *caps = malloc(buf_len);
>>>               memcpy(*caps, &rsp->preset, buf_len);
>>>       } else {
>>> @@ -919,6 +955,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>       struct a2dp_stream_out *out;
>>>       struct audio_preset *preset;
>>>       const struct audio_codec *codec;
>>> +     uint16_t mtu;
>>>
>>>       out = calloc(1, sizeof(struct a2dp_stream_out));
>>>       if (!out)
>>> @@ -946,7 +983,8 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>       /* TODO: for now we always use endpoint 0 */
>>>       out->ep = &audio_endpoints[0];
>>>
>>> -     if (ipc_open_stream_cmd(out->ep->id, &preset) != AUDIO_STATUS_SUCCESS)
>>> +     if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
>>> +                     AUDIO_STATUS_SUCCESS)
>>>               goto fail;
>>>
>>>       if (!preset)
>>> @@ -954,7 +992,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>
>>>       codec = out->ep->codec;
>>>
>>> -     codec->init(preset, &out->ep->codec_data);
>>> +     codec->init(preset, mtu, &out->ep->codec_data);
>>>       codec->get_config(out->ep->codec_data, &out->cfg);
>>>
>>>       DBG("rate=%d channels=%d format=%d", out->cfg.rate,
>>> diff --git a/android/rtp.h b/android/rtp.h
>>> new file mode 100644
>>> index 0000000..45fddcf
>>> --- /dev/null
>>> +++ b/android/rtp.h
>>> @@ -0,0 +1,76 @@
>>> +/*
>>> + *
>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>> + *
>>> + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
>>> + *
>>> + *
>>> + *  This library 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.
>>> + *
>>> + *  This library 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
>>> + *  Lesser General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU Lesser General Public
>>> + *  License along with this library; if not, write to the Free Software
>>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>>> + *
>>> + */
>>
>> where is this file coming from? Why do we need a copy of it?
>
> From BlueZ but it was removed (was used in pcm_bluetooth.c) so I just
> added it again since we need RTP header structures. Or should these
> just be added inline to hal-audio.c?

I would just add these to hal-audio.c, nothing else should need it.


-- 
Luiz Augusto von Dentz

Re: [PATCH 5/9] android/hal-audio: Calculate SBC stream parameters

[Marcel Holtmann]

Hi Luiz,

>>>> This patch adds necessary calculations for SBC stream parameters.
>>>> 
>>>> Both input and output buffers are expected to have exact amount of
>>>> data to fill single media packet (based on transport channel MTU).
>>>> 
>>>> Frame duration will be used to synchronize input and output streams.
>>>> ---
>>>> android/hal-audio.c | 50 ++++++++++++++++++++++++++++++-----
>>>> android/rtp.h       | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 120 insertions(+), 6 deletions(-)
>>>> create mode 100644 android/rtp.h
>>>> 
>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>> index e5c646c..3f53295 100644
>>>> --- a/android/hal-audio.c
>>>> +++ b/android/hal-audio.c
>>>> @@ -33,6 +33,7 @@
>>>> #include "hal-msg.h"
>>>> #include "../profiles/audio/a2dp-codecs.h"
>>>> #include <sbc/sbc.h>
>>>> +#include "rtp.h"
>>>> 
>>>> static const uint8_t a2dp_src_uuid[] = {
>>>>              0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>>> @@ -46,6 +47,12 @@ static pthread_t ipc_th = 0;
>>>> static pthread_mutex_t close_mutex = PTHREAD_MUTEX_INITIALIZER;
>>>> static pthread_mutex_t sk_mutex = PTHREAD_MUTEX_INITIALIZER;
>>>> 
>>>> +struct media_packet {
>>>> +     struct rtp_header hdr;
>>>> +     struct rtp_payload payload;
>>>> +     uint8_t data[0];
>>>> +};
>>>> +
>>>> struct audio_input_config {
>>>>      uint32_t rate;
>>>>      uint32_t channels;
>>>> @@ -56,10 +63,19 @@ struct sbc_data {
>>>>      a2dp_sbc_t sbc;
>>>> 
>>>>      sbc_t enc;
>>>> +
>>>> +     size_t in_frame_len;
>>>> +     size_t in_buf_size;
>>>> +
>>>> +     size_t out_buf_size;
>>>> +     uint8_t *out_buf;
>>>> +
>>>> +     unsigned frame_duration;
>>>> };
>>>> 
>>>> static int sbc_get_presets(struct audio_preset *preset, size_t *len);
>>>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data);
>>>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>>>> +                             void **codec_data);
>>>> static int sbc_cleanup(void *codec_data);
>>>> static int sbc_get_config(void *codec_data,
>>>>                                      struct audio_input_config *config);
>>>> @@ -69,7 +85,8 @@ struct audio_codec {
>>>> 
>>>>      int (*get_presets) (struct audio_preset *preset, size_t *len);
>>>> 
>>>> -     int (*init) (struct audio_preset *preset, void **codec_data);
>>>> +     int (*init) (struct audio_preset *preset, uint16_t mtu,
>>>> +                             void **codec_data);
>>>>      int (*cleanup) (void *codec_data);
>>>>      int (*get_config) (void *codec_data,
>>>>                                      struct audio_input_config *config);
>>>> @@ -251,9 +268,14 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>>>>      }
>>>> }
>>>> 
>>>> -static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>>>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
>>>> +                             void **codec_data)
>>>> {
>>>>      struct sbc_data *sbc_data;
>>>> +     size_t hdr_len = sizeof(struct media_packet);
>>>> +     size_t in_frame_len;
>>>> +     size_t out_frame_len;
>>>> +     size_t num_frames;
>>>> 
>>>>      DBG("");
>>>> 
>>>> @@ -268,6 +290,18 @@ static int sbc_codec_init(struct audio_preset *preset, void **codec_data)
>>>> 
>>>>      sbc_init_encoder(sbc_data);
>>>> 
>>>> +     in_frame_len = sbc_get_codesize(&sbc_data->enc);
>>>> +     out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>>>> +     num_frames = (mtu - hdr_len) / out_frame_len;
>>>> +
>>>> +     sbc_data->in_frame_len = in_frame_len;
>>>> +     sbc_data->in_buf_size = num_frames * in_frame_len;
>>>> +
>>>> +     sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
>>>> +     sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
>>>> +
>>>> +     sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>>>> +
>>>>      *codec_data = sbc_data;
>>>> 
>>>>      return AUDIO_STATUS_SUCCESS;
>>>> @@ -280,6 +314,7 @@ static int sbc_cleanup(void *codec_data)
>>>>      DBG("");
>>>> 
>>>>      sbc_finish(&sbc_data->enc);
>>>> +     free(sbc_data->out_buf);
>>>>      free(codec_data);
>>>> 
>>>>      return AUDIO_STATUS_SUCCESS;
>>>> @@ -511,7 +546,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>>>>      return result;
>>>> }
>>>> 
>>>> -static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>>> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
>>>>                                      struct audio_preset **caps)
>>>> {
>>>>      char buf[BLUEZ_AUDIO_MTU];
>>>> @@ -534,6 +569,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>>>      if (result == AUDIO_STATUS_SUCCESS) {
>>>>              size_t buf_len = sizeof(struct audio_preset) +
>>>>                                      rsp->preset[0].len;
>>>> +             *mtu = rsp->mtu;
>>>>              *caps = malloc(buf_len);
>>>>              memcpy(*caps, &rsp->preset, buf_len);
>>>>      } else {
>>>> @@ -919,6 +955,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>>      struct a2dp_stream_out *out;
>>>>      struct audio_preset *preset;
>>>>      const struct audio_codec *codec;
>>>> +     uint16_t mtu;
>>>> 
>>>>      out = calloc(1, sizeof(struct a2dp_stream_out));
>>>>      if (!out)
>>>> @@ -946,7 +983,8 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>>      /* TODO: for now we always use endpoint 0 */
>>>>      out->ep = &audio_endpoints[0];
>>>> 
>>>> -     if (ipc_open_stream_cmd(out->ep->id, &preset) != AUDIO_STATUS_SUCCESS)
>>>> +     if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
>>>> +                     AUDIO_STATUS_SUCCESS)
>>>>              goto fail;
>>>> 
>>>>      if (!preset)
>>>> @@ -954,7 +992,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>>> 
>>>>      codec = out->ep->codec;
>>>> 
>>>> -     codec->init(preset, &out->ep->codec_data);
>>>> +     codec->init(preset, mtu, &out->ep->codec_data);
>>>>      codec->get_config(out->ep->codec_data, &out->cfg);
>>>> 
>>>>      DBG("rate=%d channels=%d format=%d", out->cfg.rate,
>>>> diff --git a/android/rtp.h b/android/rtp.h
>>>> new file mode 100644
>>>> index 0000000..45fddcf
>>>> --- /dev/null
>>>> +++ b/android/rtp.h
>>>> @@ -0,0 +1,76 @@
>>>> +/*
>>>> + *
>>>> + *  BlueZ - Bluetooth protocol stack for Linux
>>>> + *
>>>> + *  Copyright (C) 2004-2010  Marcel Holtmann <marcel@holtmann.org>
>>>> + *
>>>> + *
>>>> + *  This library 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.
>>>> + *
>>>> + *  This library 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
>>>> + *  Lesser General Public License for more details.
>>>> + *
>>>> + *  You should have received a copy of the GNU Lesser General Public
>>>> + *  License along with this library; if not, write to the Free Software
>>>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>>>> + *
>>>> + */
>>> 
>>> where is this file coming from? Why do we need a copy of it?
>> 
>> From BlueZ but it was removed (was used in pcm_bluetooth.c) so I just
>> added it again since we need RTP header structures. Or should these
>> just be added inline to hal-audio.c?
> 
> I would just add these to hal-audio.c, nothing else should need it.

can we try to add it to libsbc and see how it works out. I like to see options.

Regards

Marcel

[PATCH 6/9] android/hal-audio: Add resume to codec callbacks

[Andrzej Kaczmarek]

Once stream is resumed it may be required to reset some state of codec,
i.e. in case of SBC we need to reset monotonic clock and frames count
which are used for synchronization.
---
 android/hal-audio.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 3f53295..aeb6ea4 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -71,6 +71,9 @@ struct sbc_data {
 	uint8_t *out_buf;
 
 	unsigned frame_duration;
+
+	struct timespec start;
+	unsigned frames_sent;
 };
 
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
@@ -79,6 +82,7 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 static int sbc_cleanup(void *codec_data);
 static int sbc_get_config(void *codec_data,
 					struct audio_input_config *config);
+static void sbc_resume(void *codec_data);
 
 struct audio_codec {
 	uint8_t type;
@@ -90,6 +94,7 @@ struct audio_codec {
 	int (*cleanup) (void *codec_data);
 	int (*get_config) (void *codec_data,
 					struct audio_input_config *config);
+	void (*resume) (void *codec_data);
 	ssize_t (*write_data) (void *codec_data, const void *buffer,
 				size_t bytes);
 };
@@ -103,6 +108,7 @@ static const struct audio_codec audio_codecs[] = {
 		.init = sbc_codec_init,
 		.cleanup = sbc_cleanup,
 		.get_config = sbc_get_config,
+		.resume = sbc_resume,
 	}
 };
 
@@ -349,6 +355,17 @@ static int sbc_get_config(void *codec_data,
 	return AUDIO_STATUS_SUCCESS;
 }
 
+static void sbc_resume(void *codec_data)
+{
+	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+
+	DBG("");
+
+	clock_gettime(CLOCK_MONOTONIC, &sbc_data->start);
+
+	sbc_data->frames_sent = 0;
+}
+
 static void audio_ipc_cleanup(void)
 {
 	if (audio_sk >= 0) {
@@ -671,6 +688,8 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		if (ipc_resume_stream_cmd(out->ep->id) != AUDIO_STATUS_SUCCESS)
 			return -1;
 
+		out->ep->codec->resume(out->ep->codec_data);
+
 		out->audio_state = AUDIO_A2DP_STATE_STARTED;
 	}
 
-- 
1.8.5.2

[PATCH 7/9] android/hal-audio: Return proper buffer size to AudioFlinger

[Andrzej Kaczmarek]

---
 android/hal-audio.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index aeb6ea4..f2cb12a 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -82,6 +82,7 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 static int sbc_cleanup(void *codec_data);
 static int sbc_get_config(void *codec_data,
 					struct audio_input_config *config);
+static size_t sbc_get_buffer_size(void *codec_data);
 static void sbc_resume(void *codec_data);
 
 struct audio_codec {
@@ -94,6 +95,7 @@ struct audio_codec {
 	int (*cleanup) (void *codec_data);
 	int (*get_config) (void *codec_data,
 					struct audio_input_config *config);
+	size_t (*get_buffer_size) (void *codec_data);
 	void (*resume) (void *codec_data);
 	ssize_t (*write_data) (void *codec_data, const void *buffer,
 				size_t bytes);
@@ -108,6 +110,7 @@ static const struct audio_codec audio_codecs[] = {
 		.init = sbc_codec_init,
 		.cleanup = sbc_cleanup,
 		.get_config = sbc_get_config,
+		.get_buffer_size = sbc_get_buffer_size,
 		.resume = sbc_resume,
 	}
 };
@@ -355,6 +358,15 @@ static int sbc_get_config(void *codec_data,
 	return AUDIO_STATUS_SUCCESS;
 }
 
+static size_t sbc_get_buffer_size(void *codec_data)
+{
+	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+
+	DBG("");
+
+	return sbc_data->in_buf_size;
+}
+
 static void sbc_resume(void *codec_data)
 {
 	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
@@ -728,8 +740,11 @@ static int out_set_sample_rate(struct audio_stream *stream, uint32_t rate)
 
 static size_t out_get_buffer_size(const struct audio_stream *stream)
 {
+	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
+
 	DBG("");
-	return 20 * 512;
+
+	return out->ep->codec->get_buffer_size(out->ep->codec_data);
 }
 
 static uint32_t out_get_channels(const struct audio_stream *stream)
-- 
1.8.5.2

[PATCH 8/9] android/hal-audio: Read fd from Output Stream response

[Andrzej Kaczmarek]

---
 android/hal-audio.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index f2cb12a..d8438f7 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -575,7 +575,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
 	return result;
 }
 
-static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
+static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu, int *fd,
 					struct audio_preset **caps)
 {
 	char buf[BLUEZ_AUDIO_MTU];
@@ -593,7 +593,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
 	cmd.id = endpoint_id;
 
 	result = audio_ipc_cmd(AUDIO_SERVICE_ID, AUDIO_OP_OPEN_STREAM,
-				sizeof(cmd), &cmd, &rsp_len, rsp, NULL);
+				sizeof(cmd), &cmd, &rsp_len, rsp, fd);
 
 	if (result == AUDIO_STATUS_SUCCESS) {
 		size_t buf_len = sizeof(struct audio_preset) +
@@ -990,6 +990,7 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	struct audio_preset *preset;
 	const struct audio_codec *codec;
 	uint16_t mtu;
+	int fd;
 
 	out = calloc(1, sizeof(struct a2dp_stream_out));
 	if (!out)
@@ -1017,13 +1018,15 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
 	/* TODO: for now we always use endpoint 0 */
 	out->ep = &audio_endpoints[0];
 
-	if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
+	if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
 			AUDIO_STATUS_SUCCESS)
 		goto fail;
 
-	if (!preset)
+	if (!preset || fd < 0)
 		goto fail;
 
+	out->ep->fd = fd;
+
 	codec = out->ep->codec;
 
 	codec->init(preset, mtu, &out->ep->codec_data);
-- 
1.8.5.2

Re: [PATCH 8/9] android/hal-audio: Read fd from Output Stream response

[Szymon Janc]

Hi Andrzej,

On Friday 17 January 2014 16:40:12 Andrzej Kaczmarek wrote:
> ---
>  android/hal-audio.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index f2cb12a..d8438f7 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -575,7 +575,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>  	return result;
>  }
> 
> -static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu, int *fd,
> struct audio_preset **caps)
>  {
>  	char buf[BLUEZ_AUDIO_MTU];
> @@ -593,7 +593,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
> uint16_t *mtu, cmd.id = endpoint_id;
> 
>  	result = audio_ipc_cmd(AUDIO_SERVICE_ID, AUDIO_OP_OPEN_STREAM,
> -				sizeof(cmd), &cmd, &rsp_len, rsp, NULL);
> +				sizeof(cmd), &cmd, &rsp_len, rsp, fd);
> 
>  	if (result == AUDIO_STATUS_SUCCESS) {
>  		size_t buf_len = sizeof(struct audio_preset) +
> @@ -990,6 +990,7 @@ static int audio_open_output_stream(struct
> audio_hw_device *dev, struct audio_preset *preset;
>  	const struct audio_codec *codec;
>  	uint16_t mtu;
> +	int fd;
> 
>  	out = calloc(1, sizeof(struct a2dp_stream_out));
>  	if (!out)
> @@ -1017,13 +1018,15 @@ static int audio_open_output_stream(struct
> audio_hw_device *dev, /* TODO: for now we always use endpoint 0 */
>  	out->ep = &audio_endpoints[0];
> 
> -	if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
> +	if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
>  			AUDIO_STATUS_SUCCESS)
>  		goto fail;
> 
> -	if (!preset)
> +	if (!preset || fd < 0)
>  		goto fail;

For sanity, code under fail label should be updated to handle that either 
preset or fd might be valid here.

> 
> +	out->ep->fd = fd;
> +

I might be missing something but fd is never closed. Should this be done in 
audio_close_output_stream() ?

>  	codec = out->ep->codec;
> 
>  	codec->init(preset, mtu, &out->ep->codec_data);

-- 
Szymon K. Janc
szymon.janc@gmail.com

Re: [PATCH 8/9] android/hal-audio: Read fd from Output Stream response

[Luiz Augusto von Dentz]

Hi Szymon,

On Fri, Jan 17, 2014 at 8:13 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
> Hi Andrzej,
>
> On Friday 17 January 2014 16:40:12 Andrzej Kaczmarek wrote:
>> ---
>>  android/hal-audio.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index f2cb12a..d8438f7 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -575,7 +575,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>>       return result;
>>  }
>>
>> -static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
>> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu, int *fd,
>> struct audio_preset **caps)
>>  {
>>       char buf[BLUEZ_AUDIO_MTU];
>> @@ -593,7 +593,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
>> uint16_t *mtu, cmd.id = endpoint_id;
>>
>>       result = audio_ipc_cmd(AUDIO_SERVICE_ID, AUDIO_OP_OPEN_STREAM,
>> -                             sizeof(cmd), &cmd, &rsp_len, rsp, NULL);
>> +                             sizeof(cmd), &cmd, &rsp_len, rsp, fd);
>>
>>       if (result == AUDIO_STATUS_SUCCESS) {
>>               size_t buf_len = sizeof(struct audio_preset) +
>> @@ -990,6 +990,7 @@ static int audio_open_output_stream(struct
>> audio_hw_device *dev, struct audio_preset *preset;
>>       const struct audio_codec *codec;
>>       uint16_t mtu;
>> +     int fd;
>>
>>       out = calloc(1, sizeof(struct a2dp_stream_out));
>>       if (!out)
>> @@ -1017,13 +1018,15 @@ static int audio_open_output_stream(struct
>> audio_hw_device *dev, /* TODO: for now we always use endpoint 0 */
>>       out->ep = &audio_endpoints[0];
>>
>> -     if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
>> +     if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
>>                       AUDIO_STATUS_SUCCESS)
>>               goto fail;
>>
>> -     if (!preset)
>> +     if (!preset || fd < 0)
>>               goto fail;
>
> For sanity, code under fail label should be updated to handle that either
> preset or fd might be valid here.
>
>>
>> +     out->ep->fd = fd;
>> +
>
> I might be missing something but fd is never closed. Should this be done in
> audio_close_output_stream() ?

Yep, the fd should be closed every time we suspend as we will get
another fd on open so we will end up with duplicated fds.


-- 
Luiz Augusto von Dentz

Re: [PATCH 8/9] android/hal-audio: Read fd from Output Stream response

[Lukasz Rymanowski]

Hi Luiz,

On Fri, Jan 17, 2014 at 8:57 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Szymon,
>
> On Fri, Jan 17, 2014 at 8:13 PM, Szymon Janc <szymon.janc@gmail.com> wrote:
>> Hi Andrzej,
>>
>> On Friday 17 January 2014 16:40:12 Andrzej Kaczmarek wrote:
>>> ---
>>>  android/hal-audio.c | 11 +++++++----
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index f2cb12a..d8438f7 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -575,7 +575,7 @@ static int ipc_close_cmd(uint8_t endpoint_id)
>>>       return result;
>>>  }
>>>
>>> -static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu,
>>> +static int ipc_open_stream_cmd(uint8_t endpoint_id, uint16_t *mtu, int *fd,
>>> struct audio_preset **caps)
>>>  {
>>>       char buf[BLUEZ_AUDIO_MTU];
>>> @@ -593,7 +593,7 @@ static int ipc_open_stream_cmd(uint8_t endpoint_id,
>>> uint16_t *mtu, cmd.id = endpoint_id;
>>>
>>>       result = audio_ipc_cmd(AUDIO_SERVICE_ID, AUDIO_OP_OPEN_STREAM,
>>> -                             sizeof(cmd), &cmd, &rsp_len, rsp, NULL);
>>> +                             sizeof(cmd), &cmd, &rsp_len, rsp, fd);
>>>
>>>       if (result == AUDIO_STATUS_SUCCESS) {
>>>               size_t buf_len = sizeof(struct audio_preset) +
>>> @@ -990,6 +990,7 @@ static int audio_open_output_stream(struct
>>> audio_hw_device *dev, struct audio_preset *preset;
>>>       const struct audio_codec *codec;
>>>       uint16_t mtu;
>>> +     int fd;
>>>
>>>       out = calloc(1, sizeof(struct a2dp_stream_out));
>>>       if (!out)
>>> @@ -1017,13 +1018,15 @@ static int audio_open_output_stream(struct
>>> audio_hw_device *dev, /* TODO: for now we always use endpoint 0 */
>>>       out->ep = &audio_endpoints[0];
>>>
>>> -     if (ipc_open_stream_cmd(out->ep->id, &mtu, &preset) !=
>>> +     if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
>>>                       AUDIO_STATUS_SUCCESS)
>>>               goto fail;
>>>
>>> -     if (!preset)
>>> +     if (!preset || fd < 0)
>>>               goto fail;
>>
>> For sanity, code under fail label should be updated to handle that either
>> preset or fd might be valid here.
>>
>>>
>>> +     out->ep->fd = fd;
>>> +
>>
>> I might be missing something but fd is never closed. Should this be done in
>> audio_close_output_stream() ?
>
> Yep, the fd should be closed every time we suspend as we will get
> another fd on open so we will end up with duplicated fds.
>
Why?
Stream can be suspended in two ways.
1) AudioFlinger can do it with out_standby() and to resume it it just
use just out_write().
2) Some other part of Android eg Phone app with out_set_parameters()
and to resume it it will use same function. Open is called if stream
is closed. Probably Szymon idea is good here.

\Lukasz
>
>
> Luiz Augusto von Dentz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 9/9] android/hal-audio: Add proper SBC encoding

[Andrzej Kaczmarek]

Input and output stream is configured in a way that each input buffer
can be encoded to exactly one output buffer.

Reading from AudioFlinger is synchronized based on amounts of frames
which were expected to be sent since stream was resumed, i.e. as long
as we sent enough data we can wait for period of single media packet
before we need another buffer from input. Without synchronization
we'd receive next input buffer as soon as we process current one.
---
 android/hal-audio.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 3 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index d8438f7..86ef97b 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -24,6 +24,7 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <unistd.h>
+#include <arpa/inet.h>
 
 #include <hardware/audio.h>
 #include <hardware/hardware.h>
@@ -74,8 +75,22 @@ struct sbc_data {
 
 	struct timespec start;
 	unsigned frames_sent;
+
+	uint16_t seq;
 };
 
+static inline void timespec_diff(struct timespec *a, struct timespec *b,
+					struct timespec *res)
+{
+	res->tv_sec = a->tv_sec - b->tv_sec;
+	res->tv_nsec = a->tv_nsec - b->tv_nsec;
+
+	if (res->tv_nsec < 0) {
+		res->tv_sec--;
+		res->tv_nsec += 1000000000; /* 1sec */
+	}
+}
+
 static int sbc_get_presets(struct audio_preset *preset, size_t *len);
 static int sbc_codec_init(struct audio_preset *preset, uint16_t mtu,
 				void **codec_data);
@@ -84,6 +99,8 @@ static int sbc_get_config(void *codec_data,
 					struct audio_input_config *config);
 static size_t sbc_get_buffer_size(void *codec_data);
 static void sbc_resume(void *codec_data);
+static ssize_t sbc_write_data(void *codec_data, const void *buffer,
+					size_t bytes, int fd);
 
 struct audio_codec {
 	uint8_t type;
@@ -98,7 +115,7 @@ struct audio_codec {
 	size_t (*get_buffer_size) (void *codec_data);
 	void (*resume) (void *codec_data);
 	ssize_t (*write_data) (void *codec_data, const void *buffer,
-				size_t bytes);
+				size_t bytes, int fd);
 };
 
 static const struct audio_codec audio_codecs[] = {
@@ -112,6 +129,7 @@ static const struct audio_codec audio_codecs[] = {
 		.get_config = sbc_get_config,
 		.get_buffer_size = sbc_get_buffer_size,
 		.resume = sbc_resume,
+		.write_data = sbc_write_data,
 	}
 };
 
@@ -378,6 +396,74 @@ static void sbc_resume(void *codec_data)
 	sbc_data->frames_sent = 0;
 }
 
+static ssize_t sbc_write_data(void *codec_data, const void *buffer,
+				size_t bytes, int fd)
+{
+	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+	size_t consumed = 0;
+	size_t encoded = 0;
+	struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
+	size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
+	struct timespec cur;
+	struct timespec diff;
+	unsigned expected_frames;
+	int ret;
+
+	mp->hdr.v = 2;
+	mp->hdr.pt = 1;
+	mp->hdr.sequence_number = htons(sbc_data->seq++);
+	mp->hdr.ssrc = htonl(1);
+	mp->payload.frame_count = 0;
+
+	while (bytes - consumed >= sbc_data->in_frame_len) {
+		ssize_t written = 0;
+
+		ret = sbc_encode(&sbc_data->enc, buffer + consumed,
+					sbc_data->in_frame_len,
+					mp->data + encoded, free_space,
+					&written);
+
+		if (ret < 0) {
+			DBG("failed to encode block");
+			break;
+		}
+
+		mp->payload.frame_count++;
+
+		consumed += ret;
+		encoded += written;
+		free_space -= written;
+	}
+
+	ret = write(fd, mp, sizeof(*mp) + encoded);
+	if (ret < 0) {
+		int err = errno;
+		DBG("error writing data: %d (%s)", err, strerror(err));
+	}
+
+	if (consumed != bytes || free_space != 0) {
+		/*
+		 * we should encode all input data and fill output buffer
+		 * if we did not, something went wrong but we can't really
+		 * handle this so this is just sanity check
+		 */
+		DBG("some data were not encoded");
+	}
+
+	sbc_data->frames_sent += mp->payload.frame_count;
+
+	clock_gettime(CLOCK_MONOTONIC, &cur);
+	timespec_diff(&cur, &sbc_data->start, &diff);
+	expected_frames = (diff.tv_sec * 1000000 + diff.tv_nsec / 1000) /
+				sbc_data->frame_duration;
+
+	if (sbc_data->frames_sent >= expected_frames)
+		usleep(sbc_data->frame_duration * mp->payload.frame_count);
+
+	/* we always assume that all data was processed and sent */
+	return bytes;
+}
+
 static void audio_ipc_cleanup(void)
 {
 	if (audio_sk >= 0) {
@@ -710,9 +796,13 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
 		return -1;
 	}
 
-	/* TODO: encode data using codec */
+	if (out->ep->fd < 0) {
+		DBG("no transport");
+		return -1;
+	}
 
-	return bytes;
+	return out->ep->codec->write_data(out->ep->codec_data, buffer,
+						bytes, out->ep->fd);
 }
 
 static uint32_t out_get_sample_rate(const struct audio_stream *stream)
-- 
1.8.5.2