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
Hi Luiz,
On Fri, Jan 17, 2014 at 8:57 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Szymon,
>
> On Fri, Jan 17, 2014 at 8:13 PM, Szymon Janc <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <[email protected]>
>>>> + *
>>>> + *
>>>> + * 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
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
Hi Szymon,
On Fri, Jan 17, 2014 at 8:13 PM, Szymon Janc <[email protected]> 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
Hi Andrzej,
On Fri, Jan 17, 2014 at 9:17 PM, Andrzej Kaczmarek
<[email protected]> wrote:
> Hi Marcel,
>
> On 17 January 2014 19:25, Marcel Holtmann <[email protected]> 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 <[email protected]>
>>> + *
>>> + *
>>> + * 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
Hi Marcel,
On Fri, Jan 17, 2014 at 9:28 PM, Marcel Holtmann <[email protected]> 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
Hi Andrzej,
On Fri, Jan 17, 2014 at 9:26 PM, Andrzej Kaczmarek
<[email protected]> wrote:
> Hi Marcel,
>
> On 17 January 2014 19:24, Marcel Holtmann <[email protected]> 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
Hi Marcel,
On Fri, Jan 17, 2014 at 8:24 PM, Marcel Holtmann <[email protected]> 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.
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
Hi Marcel,
On 17 January 2014 19:24, Marcel Holtmann <[email protected]> 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
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
Hi Marcel,
On 17 January 2014 19:25, Marcel Holtmann <[email protected]> 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 <[email protected]>
>> + *
>> + *
>> + * 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
Hi Marcel,
On 17 January 2014 19:27, Marcel Holtmann <[email protected]> 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
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
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 <[email protected]>
> + *
> + *
> + * 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
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
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
[email protected]
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
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 <[email protected]>
+ *
+ *
+ * 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
---
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
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
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
---
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
---
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
---
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
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