2014-04-09 14:16:51

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 0/7] android: Audio improvements

Hi,

Here are few patches which should further improve our audio HAL. I've had
them in my tree for couple of weeks and didn't notice any problems with
streaming (tested on Nexus 4 and 5).

Patches 1-3 are simple fixes/cleanup.

Patch 4 changes L2CAP socket used in audio HAL to non-blocking (with
proper polling on socket state) which enables more sophisticated
audio stream handling introduced in subsequent patches.

Patch 5 changes way in which socket congestion (due to any reason) is
handled by simply skipping some audio data instead of trying to send
data indefinitely. This helps to maintain synchronization between audio
clock and data sent which is crucial for sleeping code to work properly.

Patches 6 and 7 enables dynamic audio quality control (for SBC it's
just bitpool adjustment) which enchances patch 5, i.e we not only skip
audio data but also reduce audio quality and effectively reduce stream
bandwidth which makes more possible that streaming will be smooth.

Patches 4-7 are inspired by current PulseAudio implementation.



Andrzej Kaczmarek (7):
android/a2dp: Prefer master role on incoming connections
android/hal-audio: Remove preset with 48000 frequency
android/hal-audio: Add resume_endpoint helper
android/hal-audio: Make audio socket non-blocking
android/hal-audio: Resync audio when lagging too much
android/hal-audio: Add support to control audio quality
android/hal-audio: Adjust audio quality automatically

android/a2dp.c | 1 +
android/hal-audio.c | 320 +++++++++++++++++++++++++++++++++++++---------------
2 files changed, 232 insertions(+), 89 deletions(-)

--
1.9.1



2014-04-16 09:58:14

by Andrzej Kaczmarek

[permalink] [raw]
Subject: Re: [PATCH 6/7] android/hal-audio: Add support to control audio quality

Hi Luiz,

On 11 April 2014 09:59, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Andrzej,
>
> On Thu, Apr 10, 2014 at 11:25 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Andrzej,
>>
>> On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
>> <[email protected]> wrote:
>>> This patch adds new codec abstraction call which can be used to adjust
>>> audio quality while playing. As for now we can either decrease quality
>>> or restore default one.
>>>
>>> It's up to actual codec capabilities and implementation how this can be
>>> handled. In case of SBC bitpool is decreased by fixed amount (5) until
>>> min allowable value is reached (33) - the same values are used in
>>> PulseAudio.
>>> ---
>>> android/hal-audio.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 65 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index e58be2b..2db927c 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -47,6 +47,9 @@
>>>
>>> #define MAX_DELAY 100000 /* 100ms */
>>>
>>> +#define SBC_QUALITY_MIN_BITPOOL 33
>>> +#define SBC_QUALITY_STEP 5
>>> +
>>> static const uint8_t a2dp_src_uuid[] = {
>>> 0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>>> 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
>>> @@ -128,6 +131,8 @@ struct sbc_data {
>>>
>>> sbc_t enc;
>>>
>>> + uint16_t payload_len;
>>> +
>>> size_t in_frame_len;
>>> size_t in_buf_size;
>>>
>>> @@ -189,6 +194,10 @@ static size_t sbc_get_mediapacket_duration(void *codec_data);
>>> static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
>>> size_t len, struct media_packet *mp,
>>> size_t mp_data_len, size_t *written);
>>> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op);
>>> +
>>> +#define QUALITY_CTRL_DEFAULT 0x00
>>> +#define QUALITY_CTRL_DECREASE 0x01
>>
>> Lets call it QOS_POLICY_*
>>
>>> struct audio_codec {
>>> uint8_t type;
>>> @@ -205,6 +214,7 @@ struct audio_codec {
>>> ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
>>> size_t len, struct media_packet *mp,
>>> size_t mp_data_len, size_t *written);
>>> + bool (*quality_ctrl) (void *codec_data, uint8_t op);
>>
>> I prefer to use update_qos
>>
>>> };
>>>
>>> static const struct audio_codec audio_codecs[] = {
>>> @@ -219,6 +229,7 @@ static const struct audio_codec audio_codecs[] = {
>>> .get_buffer_size = sbc_get_buffer_size,
>>> .get_mediapacket_duration = sbc_get_mediapacket_duration,
>>> .encode_mediapacket = sbc_encode_mediapacket,
>>> + .quality_ctrl = sbc_quality_ctrl,
>>
>> sbc_update_qos
>>
>>> }
>>> };
>>>
>>> @@ -412,14 +423,33 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>>> in->min_bitpool, in->max_bitpool);
>>> }
>>>
>>> -static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>>> - void **codec_data)
>>> +static void sbc_codec_calculate(struct sbc_data *sbc_data)
>>> {
>>> - struct sbc_data *sbc_data;
>>> size_t in_frame_len;
>>> size_t out_frame_len;
>>> size_t num_frames;
>>>
>>> + in_frame_len = sbc_get_codesize(&sbc_data->enc);
>>> + out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>>> + num_frames = sbc_data->payload_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_frame_len = out_frame_len;
>>> +
>>> + sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>>> + sbc_data->frames_per_packet = num_frames;
>>> +
>>> + DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
>>> + in_frame_len, out_frame_len, num_frames);
>>> +}
>>
>> Looks like you remembered to update the frame size, however how about
>> the latency? Does it gets updated automagically?
>>
>>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>>> + void **codec_data)
>>> +{
>>> + struct sbc_data *sbc_data;
>>> +
>>> if (preset->len != sizeof(a2dp_sbc_t)) {
>>> error("SBC: preset size mismatch");
>>> return AUDIO_STATUS_FAILED;
>>> @@ -433,20 +463,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>>>
>>> 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 = payload_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_frame_len = out_frame_len;
>>> -
>>> - sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>>> - sbc_data->frames_per_packet = num_frames;
>>> + sbc_data->payload_len = payload_len;
>>>
>>> - DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
>>> - in_frame_len, out_frame_len, num_frames);
>>> + sbc_codec_calculate(sbc_data);
>>>
>>> *codec_data = sbc_data;
>>>
>>> @@ -541,6 +560,36 @@ static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
>>> return consumed;
>>> }
>>>
>>> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op)
>>> +{
>>> + struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
>>> + uint8_t curr_bitpool = sbc_data->enc.bitpool;
>>> + uint8_t new_bitpool = curr_bitpool;
>>> +
>>> + switch (op) {
>>> + case QUALITY_CTRL_DEFAULT:
>>> + new_bitpool = sbc_data->sbc.max_bitpool;
>>> + break;
>>> +
>>> + case QUALITY_CTRL_DECREASE:
>>> + new_bitpool = curr_bitpool - SBC_QUALITY_STEP;
>>> + if (new_bitpool < SBC_QUALITY_MIN_BITPOOL)
>>> + new_bitpool = SBC_QUALITY_MIN_BITPOOL;
>>> + break;
>>> + }
>
> One detail is missing here, if max_bitpool is bellow the
> SBC_QUALITY_MIN_BITPOOL you should do nothing otherwise we might send
> frames using a bitpool outside of the negotiated range.

I changed this to try to decrease bitpool only if we're above
SBC_QUALITY_MIN_BITPOOL so if we are already below, we won't touch
bitpool.

BR,
Andrzej

2014-04-16 09:56:19

by Andrzej Kaczmarek

[permalink] [raw]
Subject: Re: [PATCH 6/7] android/hal-audio: Add support to control audio quality

Hi Luiz,

On 10 April 2014 10:25, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Andrzej,
>
> On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
> <[email protected]> wrote:
>> This patch adds new codec abstraction call which can be used to adjust
>> audio quality while playing. As for now we can either decrease quality
>> or restore default one.
>>
>> It's up to actual codec capabilities and implementation how this can be
>> handled. In case of SBC bitpool is decreased by fixed amount (5) until
>> min allowable value is reached (33) - the same values are used in
>> PulseAudio.
>> ---
>> android/hal-audio.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 65 insertions(+), 16 deletions(-)
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index e58be2b..2db927c 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -47,6 +47,9 @@
>>
>> #define MAX_DELAY 100000 /* 100ms */
>>
>> +#define SBC_QUALITY_MIN_BITPOOL 33
>> +#define SBC_QUALITY_STEP 5
>> +
>> static const uint8_t a2dp_src_uuid[] = {
>> 0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>> 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
>> @@ -128,6 +131,8 @@ struct sbc_data {
>>
>> sbc_t enc;
>>
>> + uint16_t payload_len;
>> +
>> size_t in_frame_len;
>> size_t in_buf_size;
>>
>> @@ -189,6 +194,10 @@ static size_t sbc_get_mediapacket_duration(void *codec_data);
>> static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
>> size_t len, struct media_packet *mp,
>> size_t mp_data_len, size_t *written);
>> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op);
>> +
>> +#define QUALITY_CTRL_DEFAULT 0x00
>> +#define QUALITY_CTRL_DECREASE 0x01
>
> Lets call it QOS_POLICY_*
>
>> struct audio_codec {
>> uint8_t type;
>> @@ -205,6 +214,7 @@ struct audio_codec {
>> ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
>> size_t len, struct media_packet *mp,
>> size_t mp_data_len, size_t *written);
>> + bool (*quality_ctrl) (void *codec_data, uint8_t op);
>
> I prefer to use update_qos
>
>> };
>>
>> static const struct audio_codec audio_codecs[] = {
>> @@ -219,6 +229,7 @@ static const struct audio_codec audio_codecs[] = {
>> .get_buffer_size = sbc_get_buffer_size,
>> .get_mediapacket_duration = sbc_get_mediapacket_duration,
>> .encode_mediapacket = sbc_encode_mediapacket,
>> + .quality_ctrl = sbc_quality_ctrl,
>
> sbc_update_qos
>
>> }
>> };
>>
>> @@ -412,14 +423,33 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>> in->min_bitpool, in->max_bitpool);
>> }
>>
>> -static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>> - void **codec_data)
>> +static void sbc_codec_calculate(struct sbc_data *sbc_data)
>> {
>> - struct sbc_data *sbc_data;
>> size_t in_frame_len;
>> size_t out_frame_len;
>> size_t num_frames;
>>
>> + in_frame_len = sbc_get_codesize(&sbc_data->enc);
>> + out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>> + num_frames = sbc_data->payload_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_frame_len = out_frame_len;
>> +
>> + sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>> + sbc_data->frames_per_packet = num_frames;
>> +
>> + DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
>> + in_frame_len, out_frame_len, num_frames);
>> +}
>
> Looks like you remembered to update the frame size, however how about
> the latency? Does it gets updated automagically?

Latency is calculated using frame size on request, so as soon as
AudioFlinger calls out_get_latency it will get new value. I don't see
any callback to inform AF about the change but it does query for
latency from time to time so should be enough.

BR,
Andrzej

2014-04-11 11:07:42

by Andrzej Kaczmarek

[permalink] [raw]
Subject: Re: [PATCH 2/7] android/hal-audio: Remove preset with 48000 frequency

Hi Luiz,

On 10 April 2014 10:11, Luiz Augusto von Dentz <[email protected]> wrote:
>
> Hi Andrzej,
>
> On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
> <[email protected]> wrote:
> > Having two presets with all parameters identical except of frequency is
> > redundant since SNK has mandatory support for both 44100 and 48000 so
> > in case we're INT we'll always set 44100 as this is first one we try.
> > ---
> > android/hal-audio.c | 9 ---------
> > 1 file changed, 9 deletions(-)
> >
> > diff --git a/android/hal-audio.c b/android/hal-audio.c
> > index 23eb6b8..2f28b13 100644
> > --- a/android/hal-audio.c
> > +++ b/android/hal-audio.c
> > @@ -265,15 +265,6 @@ static const a2dp_sbc_t sbc_presets[] = {
> > .min_bitpool = MIN_BITPOOL,
> > .max_bitpool = MAX_BITPOOL
> > },
> > - {
> > - .frequency = SBC_SAMPLING_FREQ_48000,
> > - .channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO,
> > - .subbands = SBC_SUBBANDS_8,
> > - .allocation_method = SBC_ALLOCATION_LOUDNESS,
> > - .block_length = SBC_BLOCK_LENGTH_16,
> > - .min_bitpool = MIN_BITPOOL,
> > - .max_bitpool = MAX_BITPOOL
> > - },
> > };
> >
> > static int sbc_get_presets(struct audio_preset *preset, size_t *len)
> > --
> > 1.9.1
>
> Well the idea was that we could extend AUDIO_OP_OPEN_STREAM to
> include the preset index but I guess it does not make sense for
> Android since it does not support 48Khz, well anything different than
> 44.1Khz since it cannot mix otherwise, so perhaps it is safe to remove
> it. On the other hand we do have support for 48Khz which the remote
> can configure but we don't do any resampling so I wonder how this work
> out.

So if this was original idea I'll leave 48kHz since Android does
support it (AudioFlinger can resample to both 44.1kHz and 48kHz
without problems) and we already report proper sampling rate from
hal-audio.

BR,
Andrzej

2014-04-11 07:59:24

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 6/7] android/hal-audio: Add support to control audio quality

Hi Andrzej,

On Thu, Apr 10, 2014 at 11:25 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Andrzej,
>
> On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
> <[email protected]> wrote:
>> This patch adds new codec abstraction call which can be used to adjust
>> audio quality while playing. As for now we can either decrease quality
>> or restore default one.
>>
>> It's up to actual codec capabilities and implementation how this can be
>> handled. In case of SBC bitpool is decreased by fixed amount (5) until
>> min allowable value is reached (33) - the same values are used in
>> PulseAudio.
>> ---
>> android/hal-audio.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----------
>> 1 file changed, 65 insertions(+), 16 deletions(-)
>>
>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>> index e58be2b..2db927c 100644
>> --- a/android/hal-audio.c
>> +++ b/android/hal-audio.c
>> @@ -47,6 +47,9 @@
>>
>> #define MAX_DELAY 100000 /* 100ms */
>>
>> +#define SBC_QUALITY_MIN_BITPOOL 33
>> +#define SBC_QUALITY_STEP 5
>> +
>> static const uint8_t a2dp_src_uuid[] = {
>> 0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
>> 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
>> @@ -128,6 +131,8 @@ struct sbc_data {
>>
>> sbc_t enc;
>>
>> + uint16_t payload_len;
>> +
>> size_t in_frame_len;
>> size_t in_buf_size;
>>
>> @@ -189,6 +194,10 @@ static size_t sbc_get_mediapacket_duration(void *codec_data);
>> static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
>> size_t len, struct media_packet *mp,
>> size_t mp_data_len, size_t *written);
>> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op);
>> +
>> +#define QUALITY_CTRL_DEFAULT 0x00
>> +#define QUALITY_CTRL_DECREASE 0x01
>
> Lets call it QOS_POLICY_*
>
>> struct audio_codec {
>> uint8_t type;
>> @@ -205,6 +214,7 @@ struct audio_codec {
>> ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
>> size_t len, struct media_packet *mp,
>> size_t mp_data_len, size_t *written);
>> + bool (*quality_ctrl) (void *codec_data, uint8_t op);
>
> I prefer to use update_qos
>
>> };
>>
>> static const struct audio_codec audio_codecs[] = {
>> @@ -219,6 +229,7 @@ static const struct audio_codec audio_codecs[] = {
>> .get_buffer_size = sbc_get_buffer_size,
>> .get_mediapacket_duration = sbc_get_mediapacket_duration,
>> .encode_mediapacket = sbc_encode_mediapacket,
>> + .quality_ctrl = sbc_quality_ctrl,
>
> sbc_update_qos
>
>> }
>> };
>>
>> @@ -412,14 +423,33 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
>> in->min_bitpool, in->max_bitpool);
>> }
>>
>> -static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>> - void **codec_data)
>> +static void sbc_codec_calculate(struct sbc_data *sbc_data)
>> {
>> - struct sbc_data *sbc_data;
>> size_t in_frame_len;
>> size_t out_frame_len;
>> size_t num_frames;
>>
>> + in_frame_len = sbc_get_codesize(&sbc_data->enc);
>> + out_frame_len = sbc_get_frame_length(&sbc_data->enc);
>> + num_frames = sbc_data->payload_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_frame_len = out_frame_len;
>> +
>> + sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>> + sbc_data->frames_per_packet = num_frames;
>> +
>> + DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
>> + in_frame_len, out_frame_len, num_frames);
>> +}
>
> Looks like you remembered to update the frame size, however how about
> the latency? Does it gets updated automagically?
>
>> +static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>> + void **codec_data)
>> +{
>> + struct sbc_data *sbc_data;
>> +
>> if (preset->len != sizeof(a2dp_sbc_t)) {
>> error("SBC: preset size mismatch");
>> return AUDIO_STATUS_FAILED;
>> @@ -433,20 +463,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>>
>> 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 = payload_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_frame_len = out_frame_len;
>> -
>> - sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>> - sbc_data->frames_per_packet = num_frames;
>> + sbc_data->payload_len = payload_len;
>>
>> - DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
>> - in_frame_len, out_frame_len, num_frames);
>> + sbc_codec_calculate(sbc_data);
>>
>> *codec_data = sbc_data;
>>
>> @@ -541,6 +560,36 @@ static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
>> return consumed;
>> }
>>
>> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op)
>> +{
>> + struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
>> + uint8_t curr_bitpool = sbc_data->enc.bitpool;
>> + uint8_t new_bitpool = curr_bitpool;
>> +
>> + switch (op) {
>> + case QUALITY_CTRL_DEFAULT:
>> + new_bitpool = sbc_data->sbc.max_bitpool;
>> + break;
>> +
>> + case QUALITY_CTRL_DECREASE:
>> + new_bitpool = curr_bitpool - SBC_QUALITY_STEP;
>> + if (new_bitpool < SBC_QUALITY_MIN_BITPOOL)
>> + new_bitpool = SBC_QUALITY_MIN_BITPOOL;
>> + break;
>> + }

One detail is missing here, if max_bitpool is bellow the
SBC_QUALITY_MIN_BITPOOL you should do nothing otherwise we might send
frames using a bitpool outside of the negotiated range.

>> + if (new_bitpool == curr_bitpool)
>> + return false;
>> +
>> + sbc_data->enc.bitpool = new_bitpool;
>> +
>> + sbc_codec_calculate(sbc_data);
>> +
>> + info("SBC: bitpool chaged: %d -> %d", curr_bitpool, new_bitpool);
>> +
>> + return true;
>> +}
>> +
>> static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
>> void *param, size_t *rsp_len, void *rsp, int *fd)
>> {
>> --
>> 1.9.1
>>
>> --
>> 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
>
>
>
> --
> Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2014-04-10 08:25:52

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 6/7] android/hal-audio: Add support to control audio quality

Hi Andrzej,

On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
<[email protected]> wrote:
> This patch adds new codec abstraction call which can be used to adjust
> audio quality while playing. As for now we can either decrease quality
> or restore default one.
>
> It's up to actual codec capabilities and implementation how this can be
> handled. In case of SBC bitpool is decreased by fixed amount (5) until
> min allowable value is reached (33) - the same values are used in
> PulseAudio.
> ---
> android/hal-audio.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 65 insertions(+), 16 deletions(-)
>
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index e58be2b..2db927c 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -47,6 +47,9 @@
>
> #define MAX_DELAY 100000 /* 100ms */
>
> +#define SBC_QUALITY_MIN_BITPOOL 33
> +#define SBC_QUALITY_STEP 5
> +
> static const uint8_t a2dp_src_uuid[] = {
> 0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
> 0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
> @@ -128,6 +131,8 @@ struct sbc_data {
>
> sbc_t enc;
>
> + uint16_t payload_len;
> +
> size_t in_frame_len;
> size_t in_buf_size;
>
> @@ -189,6 +194,10 @@ static size_t sbc_get_mediapacket_duration(void *codec_data);
> static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
> size_t len, struct media_packet *mp,
> size_t mp_data_len, size_t *written);
> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op);
> +
> +#define QUALITY_CTRL_DEFAULT 0x00
> +#define QUALITY_CTRL_DECREASE 0x01

Lets call it QOS_POLICY_*

> struct audio_codec {
> uint8_t type;
> @@ -205,6 +214,7 @@ struct audio_codec {
> ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
> size_t len, struct media_packet *mp,
> size_t mp_data_len, size_t *written);
> + bool (*quality_ctrl) (void *codec_data, uint8_t op);

I prefer to use update_qos

> };
>
> static const struct audio_codec audio_codecs[] = {
> @@ -219,6 +229,7 @@ static const struct audio_codec audio_codecs[] = {
> .get_buffer_size = sbc_get_buffer_size,
> .get_mediapacket_duration = sbc_get_mediapacket_duration,
> .encode_mediapacket = sbc_encode_mediapacket,
> + .quality_ctrl = sbc_quality_ctrl,

sbc_update_qos

> }
> };
>
> @@ -412,14 +423,33 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
> in->min_bitpool, in->max_bitpool);
> }
>
> -static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
> - void **codec_data)
> +static void sbc_codec_calculate(struct sbc_data *sbc_data)
> {
> - struct sbc_data *sbc_data;
> size_t in_frame_len;
> size_t out_frame_len;
> size_t num_frames;
>
> + in_frame_len = sbc_get_codesize(&sbc_data->enc);
> + out_frame_len = sbc_get_frame_length(&sbc_data->enc);
> + num_frames = sbc_data->payload_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_frame_len = out_frame_len;
> +
> + sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
> + sbc_data->frames_per_packet = num_frames;
> +
> + DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
> + in_frame_len, out_frame_len, num_frames);
> +}

Looks like you remembered to update the frame size, however how about
the latency? Does it gets updated automagically?

> +static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
> + void **codec_data)
> +{
> + struct sbc_data *sbc_data;
> +
> if (preset->len != sizeof(a2dp_sbc_t)) {
> error("SBC: preset size mismatch");
> return AUDIO_STATUS_FAILED;
> @@ -433,20 +463,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
>
> 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 = payload_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_frame_len = out_frame_len;
> -
> - sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
> - sbc_data->frames_per_packet = num_frames;
> + sbc_data->payload_len = payload_len;
>
> - DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
> - in_frame_len, out_frame_len, num_frames);
> + sbc_codec_calculate(sbc_data);
>
> *codec_data = sbc_data;
>
> @@ -541,6 +560,36 @@ static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
> return consumed;
> }
>
> +static bool sbc_quality_ctrl(void *codec_data, uint8_t op)
> +{
> + struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
> + uint8_t curr_bitpool = sbc_data->enc.bitpool;
> + uint8_t new_bitpool = curr_bitpool;
> +
> + switch (op) {
> + case QUALITY_CTRL_DEFAULT:
> + new_bitpool = sbc_data->sbc.max_bitpool;
> + break;
> +
> + case QUALITY_CTRL_DECREASE:
> + new_bitpool = curr_bitpool - SBC_QUALITY_STEP;
> + if (new_bitpool < SBC_QUALITY_MIN_BITPOOL)
> + new_bitpool = SBC_QUALITY_MIN_BITPOOL;
> + break;
> + }
> +
> + if (new_bitpool == curr_bitpool)
> + return false;
> +
> + sbc_data->enc.bitpool = new_bitpool;
> +
> + sbc_codec_calculate(sbc_data);
> +
> + info("SBC: bitpool chaged: %d -> %d", curr_bitpool, new_bitpool);
> +
> + return true;
> +}
> +
> static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
> void *param, size_t *rsp_len, void *rsp, int *fd)
> {
> --
> 1.9.1
>
> --
> 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



--
Luiz Augusto von Dentz

2014-04-10 08:11:47

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 2/7] android/hal-audio: Remove preset with 48000 frequency

Hi Andrzej,

On Wed, Apr 9, 2014 at 5:16 PM, Andrzej Kaczmarek
<[email protected]> wrote:
> Having two presets with all parameters identical except of frequency is
> redundant since SNK has mandatory support for both 44100 and 48000 so
> in case we're INT we'll always set 44100 as this is first one we try.
> ---
> android/hal-audio.c | 9 ---------
> 1 file changed, 9 deletions(-)
>
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 23eb6b8..2f28b13 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -265,15 +265,6 @@ static const a2dp_sbc_t sbc_presets[] = {
> .min_bitpool = MIN_BITPOOL,
> .max_bitpool = MAX_BITPOOL
> },
> - {
> - .frequency = SBC_SAMPLING_FREQ_48000,
> - .channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO,
> - .subbands = SBC_SUBBANDS_8,
> - .allocation_method = SBC_ALLOCATION_LOUDNESS,
> - .block_length = SBC_BLOCK_LENGTH_16,
> - .min_bitpool = MIN_BITPOOL,
> - .max_bitpool = MAX_BITPOOL
> - },
> };
>
> static int sbc_get_presets(struct audio_preset *preset, size_t *len)
> --
> 1.9.1

Well the idea was that we could extend AUDIO_OP_OPEN_STREAM to
include the preset index but I guess it does not make sense for
Android since it does not support 48Khz, well anything different than
44.1Khz since it cannot mix otherwise, so perhaps it is safe to remove
it. On the other hand we do have support for 48Khz which the remote
can configure but we don't do any resampling so I wonder how this work
out.


--
Luiz Augusto von Dentz

2014-04-09 14:16:57

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 6/7] android/hal-audio: Add support to control audio quality

This patch adds new codec abstraction call which can be used to adjust
audio quality while playing. As for now we can either decrease quality
or restore default one.

It's up to actual codec capabilities and implementation how this can be
handled. In case of SBC bitpool is decreased by fixed amount (5) until
min allowable value is reached (33) - the same values are used in
PulseAudio.
---
android/hal-audio.c | 81 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 65 insertions(+), 16 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index e58be2b..2db927c 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -47,6 +47,9 @@

#define MAX_DELAY 100000 /* 100ms */

+#define SBC_QUALITY_MIN_BITPOOL 33
+#define SBC_QUALITY_STEP 5
+
static const uint8_t a2dp_src_uuid[] = {
0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
@@ -128,6 +131,8 @@ struct sbc_data {

sbc_t enc;

+ uint16_t payload_len;
+
size_t in_frame_len;
size_t in_buf_size;

@@ -189,6 +194,10 @@ static size_t sbc_get_mediapacket_duration(void *codec_data);
static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
size_t len, struct media_packet *mp,
size_t mp_data_len, size_t *written);
+static bool sbc_quality_ctrl(void *codec_data, uint8_t op);
+
+#define QUALITY_CTRL_DEFAULT 0x00
+#define QUALITY_CTRL_DECREASE 0x01

struct audio_codec {
uint8_t type;
@@ -205,6 +214,7 @@ struct audio_codec {
ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
size_t len, struct media_packet *mp,
size_t mp_data_len, size_t *written);
+ bool (*quality_ctrl) (void *codec_data, uint8_t op);
};

static const struct audio_codec audio_codecs[] = {
@@ -219,6 +229,7 @@ static const struct audio_codec audio_codecs[] = {
.get_buffer_size = sbc_get_buffer_size,
.get_mediapacket_duration = sbc_get_mediapacket_duration,
.encode_mediapacket = sbc_encode_mediapacket,
+ .quality_ctrl = sbc_quality_ctrl,
}
};

@@ -412,14 +423,33 @@ static void sbc_init_encoder(struct sbc_data *sbc_data)
in->min_bitpool, in->max_bitpool);
}

-static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
- void **codec_data)
+static void sbc_codec_calculate(struct sbc_data *sbc_data)
{
- struct sbc_data *sbc_data;
size_t in_frame_len;
size_t out_frame_len;
size_t num_frames;

+ in_frame_len = sbc_get_codesize(&sbc_data->enc);
+ out_frame_len = sbc_get_frame_length(&sbc_data->enc);
+ num_frames = sbc_data->payload_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_frame_len = out_frame_len;
+
+ sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
+ sbc_data->frames_per_packet = num_frames;
+
+ DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
+ in_frame_len, out_frame_len, num_frames);
+}
+
+static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,
+ void **codec_data)
+{
+ struct sbc_data *sbc_data;
+
if (preset->len != sizeof(a2dp_sbc_t)) {
error("SBC: preset size mismatch");
return AUDIO_STATUS_FAILED;
@@ -433,20 +463,9 @@ static int sbc_codec_init(struct audio_preset *preset, uint16_t payload_len,

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 = payload_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_frame_len = out_frame_len;
-
- sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
- sbc_data->frames_per_packet = num_frames;
+ sbc_data->payload_len = payload_len;

- DBG("in_frame_len=%zu out_frame_len=%zu frames_per_packet=%zu",
- in_frame_len, out_frame_len, num_frames);
+ sbc_codec_calculate(sbc_data);

*codec_data = sbc_data;

@@ -541,6 +560,36 @@ static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t *buffer,
return consumed;
}

+static bool sbc_quality_ctrl(void *codec_data, uint8_t op)
+{
+ struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
+ uint8_t curr_bitpool = sbc_data->enc.bitpool;
+ uint8_t new_bitpool = curr_bitpool;
+
+ switch (op) {
+ case QUALITY_CTRL_DEFAULT:
+ new_bitpool = sbc_data->sbc.max_bitpool;
+ break;
+
+ case QUALITY_CTRL_DECREASE:
+ new_bitpool = curr_bitpool - SBC_QUALITY_STEP;
+ if (new_bitpool < SBC_QUALITY_MIN_BITPOOL)
+ new_bitpool = SBC_QUALITY_MIN_BITPOOL;
+ break;
+ }
+
+ if (new_bitpool == curr_bitpool)
+ return false;
+
+ sbc_data->enc.bitpool = new_bitpool;
+
+ sbc_codec_calculate(sbc_data);
+
+ info("SBC: bitpool chaged: %d -> %d", curr_bitpool, new_bitpool);
+
+ return true;
+}
+
static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
void *param, size_t *rsp_len, void *rsp, int *fd)
{
--
1.9.1


2014-04-09 14:16:52

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 1/7] android/a2dp: Prefer master role on incoming connections

---
android/a2dp.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/android/a2dp.c b/android/a2dp.c
index 4ea16e2..8fb84a3 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -1577,6 +1577,7 @@ bool bt_a2dp_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
BT_IO_OPT_PSM, L2CAP_PSM_AVDTP,
BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+ BT_IO_OPT_MASTER, true,
BT_IO_OPT_INVALID);
if (!server) {
error("Failed to listen on AVDTP channel: %s", err->message);
--
1.9.1


2014-04-09 14:16:56

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 5/7] android/hal-audio: Resync audio when lagging too much

In case we're more than 100ms behind actual audio clock, we start to
skip writing data until we're back in sync. Delay value of 100ms is
what PulseAudio use for the same purpose.
---
android/hal-audio.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 3c4197e..e58be2b 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -45,6 +45,8 @@

#define MAX_FRAMES_IN_PAYLOAD 15

+#define MAX_DELAY 100000 /* 100ms */
+
static const uint8_t a2dp_src_uuid[] = {
0x00, 0x00, 0x11, 0x0a, 0x00, 0x00, 0x10, 0x00,
0x80, 0x00, 0x00, 0x80, 0x5f, 0x9b, 0x34, 0xfb };
@@ -236,6 +238,8 @@ struct audio_endpoint {
uint16_t seq;
uint32_t samples;
struct timespec start;
+
+ bool resync;
};

static struct audio_endpoint audio_endpoints[MAX_AUDIO_ENDPOINTS];
@@ -905,6 +909,7 @@ static bool resume_endpoint(struct audio_endpoint *ep)
return false;

ep->samples = 0;
+ ep->resync = false;

return true;
}
@@ -1022,10 +1027,15 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
audio_sent = ep->samples * 1000000ll / out->cfg.rate;
audio_passed = timespec_diff_us(&current, &ep->start);

- /* if we're ahead of stream then wait for next write point */
+ /* if we're ahead of stream then wait for next write point
+ * if we're lagging more than 100ms then stop writing and just
+ * skip data until we're back in sync
+ */
if (audio_sent > audio_passed) {
struct timespec anchor;

+ ep->resync = false;
+
timespec_add(&ep->start, audio_sent, &anchor);

while (true) {
@@ -1042,18 +1052,28 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
return false;
}
}
- }
+ } else if (!ep->resync) {
+ uint64_t diff = audio_passed - audio_sent;

- /* wait some time for socket to be ready for write,
- * but we'll just skip writing data if timeout occurs
- */
- if (!wait_for_endpoint(ep, &do_write))
- return false;
+ if (diff > MAX_DELAY) {
+ warn("lag is %jums, resyncing", diff / 1000);
+ ep->resync = true;
+ }
+ }

- if (do_write)
- if (!write_to_endpoint(ep, written))
+ /* in resync mode we'll just drop mediapackets */
+ if (!ep->resync) {
+ /* wait some time for socket to be ready for write,
+ * but we'll just skip writing data if timeout occurs
+ */
+ if (!wait_for_endpoint(ep, &do_write))
return false;

+ if (do_write)
+ if (!write_to_endpoint(ep, written))
+ return false;
+ }
+
/* AudioFlinger provides 16bit PCM, so sample size is 2 bytes
* multiplied by number of channels. Number of channels is
* simply number of bits set in channels mask.
--
1.9.1


2014-04-09 14:16:58

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 7/7] android/hal-audio: Adjust audio quality automatically

In case we go out of sync with audio clock and start skipping data to
catch up, we also decrease audio quality to have better change to avoid
going out of sync in future, i.e. due to poor radio link quality.

Quality is reset to default value on stream resume.
---
android/hal-audio.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 2db927c..aad9925 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -960,6 +960,8 @@ static bool resume_endpoint(struct audio_endpoint *ep)
ep->samples = 0;
ep->resync = false;

+ ep->codec->quality_ctrl(ep->codec_data, QUALITY_CTRL_DEFAULT);
+
return true;
}

@@ -1106,6 +1108,9 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,

if (diff > MAX_DELAY) {
warn("lag is %jums, resyncing", diff / 1000);
+
+ ep->codec->quality_ctrl(ep->codec_data,
+ QUALITY_CTRL_DECREASE);
ep->resync = true;
}
}
--
1.9.1


2014-04-09 14:16:53

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 2/7] android/hal-audio: Remove preset with 48000 frequency

Having two presets with all parameters identical except of frequency is
redundant since SNK has mandatory support for both 44100 and 48000 so
in case we're INT we'll always set 44100 as this is first one we try.
---
android/hal-audio.c | 9 ---------
1 file changed, 9 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 23eb6b8..2f28b13 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -265,15 +265,6 @@ static const a2dp_sbc_t sbc_presets[] = {
.min_bitpool = MIN_BITPOOL,
.max_bitpool = MAX_BITPOOL
},
- {
- .frequency = SBC_SAMPLING_FREQ_48000,
- .channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO,
- .subbands = SBC_SUBBANDS_8,
- .allocation_method = SBC_ALLOCATION_LOUDNESS,
- .block_length = SBC_BLOCK_LENGTH_16,
- .min_bitpool = MIN_BITPOOL,
- .max_bitpool = MAX_BITPOOL
- },
};

static int sbc_get_presets(struct audio_preset *preset, size_t *len)
--
1.9.1


2014-04-09 14:16:55

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 4/7] android/hal-audio: Make audio socket non-blocking

This patch makes writing to audio socket non-blocking since there's no
point in waiting indefinitely to write some data. Instead, we wait no
more than 500ms for socket to be ready and just skip packet otherwise.
---
android/hal-audio.c | 191 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 130 insertions(+), 61 deletions(-)

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 558331a..3c4197e 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -147,6 +147,27 @@ static void timespec_add(struct timespec *base, uint64_t time_us,
}
}

+static 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 uint64_t timespec_diff_us(struct timespec *a, struct timespec *b)
+{
+ struct timespec res;
+
+ timespec_diff(a, b, &res);
+
+ return res.tv_sec * 1000000ll + res.tv_nsec / 1000ll;
+}
+
#if defined(ANDROID)
/* Bionic does not have clock_nanosleep() prototype in time.h even though
* it provides its implementation.
@@ -821,26 +842,6 @@ static void unregister_endpoints(void)
}
}

-static int set_blocking(int fd)
-{
- int flags;
-
- flags = fcntl(fd, F_GETFL, 0);
- if (flags < 0) {
- int err = -errno;
- error("fcntl(F_GETFL): %s (%d)", strerror(-err), -err);
- return err;
- }
-
- if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
- int err = -errno;
- error("fcntl(F_SETFL): %s (%d)", strerror(-err), -err);
- return err;
- }
-
- return 0;
-}
-
static bool open_endpoint(struct audio_endpoint *ep,
struct audio_input_config *cfg)
{
@@ -854,9 +855,6 @@ static bool open_endpoint(struct audio_endpoint *ep,
AUDIO_STATUS_SUCCESS)
return false;

- if (set_blocking(fd) < 0)
- goto failed;
-
DBG("mtu=%u", mtu);

payload_len = mtu - sizeof(*ep->mp);
@@ -906,7 +904,6 @@ static bool resume_endpoint(struct audio_endpoint *ep)
if (ipc_resume_stream_cmd(ep->id) != AUDIO_STATUS_SUCCESS)
return false;

- clock_gettime(CLOCK_MONOTONIC, &ep->start);
ep->samples = 0;

return true;
@@ -927,6 +924,64 @@ static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
}
}

+static bool wait_for_endpoint(struct audio_endpoint *ep, bool *writable)
+{
+ int ret;
+
+ while (true) {
+ struct pollfd pollfd;
+
+ pollfd.fd = ep->fd;
+ pollfd.events = POLLOUT;
+ pollfd.revents = 0;
+
+ ret = poll(&pollfd, 1, 500);
+
+ if (ret >= 0) {
+ *writable = !!(pollfd.revents & POLLOUT);
+ break;
+ }
+
+ if (errno != EINTR) {
+ ret = errno;
+ error("poll failed (%d)", ret);
+ return false;
+ }
+ }
+
+ return true;
+}
+
+static bool write_to_endpoint(struct audio_endpoint *ep, size_t bytes)
+{
+ struct media_packet *mp = (struct media_packet *) ep->mp;
+ int ret;
+
+ while (true) {
+ ret = write(ep->fd, mp, sizeof(*mp) + bytes);
+
+ if (ret >= 0)
+ break;
+
+ /* this should not happen so let's issue warning, but do not
+ * fail, we can try to write next packet
+ */
+ if (errno == EAGAIN) {
+ ret = errno;
+ warn("write failed (%d)", ret);
+ break;
+ }
+
+ if (errno != EINTR) {
+ ret = errno;
+ error("write failed (%d)", ret);
+ return false;
+ }
+ }
+
+ return true;
+}
+
static bool write_data(struct a2dp_stream_out *out, const void *buffer,
size_t bytes)
{
@@ -940,58 +995,72 @@ static bool write_data(struct a2dp_stream_out *out, const void *buffer,
ssize_t read;
uint32_t samples;
int ret;
- uint64_t time_us;
- struct timespec anchor;
+ struct timespec current;
+ uint64_t audio_sent, audio_passed;
+ bool do_write = false;

- time_us = ep->samples * 1000000ll / out->cfg.rate;
+ /* prepare media packet in advance so we don't waste time after
+ * wakeup
+ */
+ mp->hdr.sequence_number = htons(ep->seq++);
+ mp->hdr.timestamp = htonl(ep->samples);
+ read = ep->codec->encode_mediapacket(ep->codec_data,
+ buffer + consumed,
+ bytes - consumed, mp,
+ free_space, &written);

- timespec_add(&ep->start, time_us, &anchor);
+ /* not much we can do here, let's just ignore remaining
+ * data and continue
+ */
+ if (read <= 0)
+ return true;

- while (true) {
- ret = clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME,
- &anchor, NULL);
+ /* calculate where are we and where we should be */
+ clock_gettime(CLOCK_MONOTONIC, &current);
+ if (!ep->samples)
+ memcpy(&ep->start, &current, sizeof(ep->start));
+ audio_sent = ep->samples * 1000000ll / out->cfg.rate;
+ audio_passed = timespec_diff_us(&current, &ep->start);

- if (!ret)
- break;
+ /* if we're ahead of stream then wait for next write point */
+ if (audio_sent > audio_passed) {
+ struct timespec anchor;

- if (ret != EINTR) {
- error("clock_nanosleep failed (%d)", ret);
- return false;
+ timespec_add(&ep->start, audio_sent, &anchor);
+
+ while (true) {
+ ret = clock_nanosleep(CLOCK_MONOTONIC,
+ TIMER_ABSTIME, &anchor,
+ NULL);
+
+ if (!ret)
+ break;
+
+ if (ret != EINTR) {
+ error("clock_nanosleep failed (%d)",
+ ret);
+ return false;
+ }
}
}

- read = ep->codec->encode_mediapacket(ep->codec_data,
- buffer + consumed,
- bytes - consumed, mp,
- free_space, &written);
-
- /* This is non-fatal and we can just assume buffer was processed
- * properly and wait for next one.
+ /* wait some time for socket to be ready for write,
+ * but we'll just skip writing data if timeout occurs
*/
- if (read <= 0)
- return true;
-
- consumed += read;
+ if (!wait_for_endpoint(ep, &do_write))
+ return false;

- mp->hdr.sequence_number = htons(ep->seq++);
- mp->hdr.timestamp = htonl(ep->samples);
+ if (do_write)
+ if (!write_to_endpoint(ep, written))
+ return false;

/* AudioFlinger provides 16bit PCM, so sample size is 2 bytes
- * multipled by number of channels. Number of channels is simply
- * number of bits set in channels mask.
+ * multiplied by number of channels. Number of channels is
+ * simply number of bits set in channels mask.
*/
samples = read / (2 * popcount(out->cfg.channels));
ep->samples += samples;
-
- while (true) {
- ret = write(ep->fd, mp, sizeof(*mp) + written);
-
- if (ret >= 0)
- break;
-
- if (errno != EINTR)
- return false;
- }
+ consumed += read;
}

return true;
--
1.9.1


2014-04-09 14:16:54

by Andrzej Kaczmarek

[permalink] [raw]
Subject: [PATCH 3/7] android/hal-audio: Add resume_endpoint helper

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

diff --git a/android/hal-audio.c b/android/hal-audio.c
index 2f28b13..558331a 100644
--- a/android/hal-audio.c
+++ b/android/hal-audio.c
@@ -901,6 +901,17 @@ static void close_endpoint(struct audio_endpoint *ep)
ep->codec_data = NULL;
}

+static bool resume_endpoint(struct audio_endpoint *ep)
+{
+ if (ipc_resume_stream_cmd(ep->id) != AUDIO_STATUS_SUCCESS)
+ return false;
+
+ clock_gettime(CLOCK_MONOTONIC, &ep->start);
+ ep->samples = 0;
+
+ return true;
+}
+
static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
size_t bytes)
{
@@ -1001,12 +1012,9 @@ static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
if (out->audio_state == AUDIO_A2DP_STATE_STANDBY) {
DBG("stream in standby, auto-start");

- if (ipc_resume_stream_cmd(out->ep->id) != AUDIO_STATUS_SUCCESS)
+ if (!resume_endpoint(out->ep))
return -1;

- clock_gettime(CLOCK_MONOTONIC, &out->ep->start);
- out->ep->samples = 0;
-
out->audio_state = AUDIO_A2DP_STATE_STARTED;
}

--
1.9.1