[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 3 commits: bluetooth: Fix usage of MTU, buffer sizes and return values of encode/decode methods
Tanu Kaskinen
gitlab at gitlab.freedesktop.org
Wed Jul 24 14:37:12 UTC 2019
Tanu Kaskinen pushed to branch master at PulseAudio / pulseaudio
Commits:
018b38ec by Pali Rohár at 2019-07-24T14:29:45Z
bluetooth: Fix usage of MTU, buffer sizes and return values of encode/decode methods
Add explanation why minimal bitpool value is used in SBC codec as initial
bitpool value for A2DP source.
Set buffer size for reading/writing from/to A2DP socket to exact link MTU
value. This would ensure that A2DP codec does not produce larger packet as
maximal possible size which can be sent.
Because A2DP socket is of SOCK_SEQPACKET type, it is guaranteed that
we do not read two packets via one read/recvmsg call.
Properly check for all return values of encode/encode methods of A2DP codec
functions. They may fail at different levels. Also encode or decode API
method may return zero length buffer (e.g. because of algorithmic delay of
codec), so do not fail in this case.
- - - - -
064277b4 by Pali Rohár at 2019-07-24T14:29:45Z
bluetooth: Change A2DP codec API of reset() method to indicate failure
SBC codec reset() method may fail, so propagate this failure to caller.
- - - - -
9e70d052 by Pali Rohár at 2019-07-24T14:29:45Z
bluetooth: Fix usage of RTP structures in SBC codec
Rename struct rtp_payload to rtp_sbc_payload as it is specific for SBC
codec payload.
Add proper checks for endianity in rtp.h header and use uint8_t type
where appropriated.
Field frame_count is only 4 bit number, so add checks to prevent overflow.
And because is_fragmented field is not parsed by decoder there is no
support for decoding fragmented SBC frames. So throw an error in this case.
- - - - -
4 changed files:
- src/modules/bluetooth/a2dp-codec-api.h
- src/modules/bluetooth/a2dp-codec-sbc.c
- src/modules/bluetooth/module-bluez5-device.c
- src/modules/bluetooth/rtp.h
Changes:
=====================================
src/modules/bluetooth/a2dp-codec-api.h
=====================================
@@ -69,12 +69,15 @@ typedef struct pa_a2dp_codec {
void *(*init)(bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec);
/* Deinitialize and release codec info data in codec_info */
void (*deinit)(void *codec_info);
- /* Reset internal state of codec info data in codec_info */
- void (*reset)(void *codec_info);
+ /* Reset internal state of codec info data in codec_info, returns
+ * a negative value on failure */
+ int (*reset)(void *codec_info);
- /* Get read block size for codec */
+ /* Get read block size for codec, it is minimal size of buffer
+ * needed to decode read_link_mtu bytes of encoded data */
size_t (*get_read_block_size)(void *codec_info, size_t read_link_mtu);
- /* Get write block size for codec */
+ /* Get write block size for codec, it is maximal size of buffer
+ * which can produce at most write_link_mtu bytes of encoded data */
size_t (*get_write_block_size)(void *codec_info, size_t write_link_mtu);
/* Reduce encoder bitrate for codec, returns new write block size or zero
=====================================
src/modules/bluetooth/a2dp-codec-sbc.c
=====================================
@@ -426,7 +426,11 @@ static void *init(bool for_encoding, bool for_backchannel, const uint8_t *config
sbc_info->min_bitpool = config->min_bitpool;
sbc_info->max_bitpool = config->max_bitpool;
- /* Set minimum bitpool for source to get the maximum possible block_size */
+ /* Set minimum bitpool for source to get the maximum possible block_size
+ * in get_block_size() function. This block_size is length of buffer used
+ * for decoded audio data and so is inversely proportional to frame length
+ * which depends on bitpool value. Bitpool is controlled by other side from
+ * range [min_bitpool, max_bitpool]. */
sbc_info->initial_bitpool = for_encoding ? sbc_info->max_bitpool : sbc_info->min_bitpool;
set_params(sbc_info);
@@ -462,27 +466,33 @@ static void set_bitpool(struct sbc_info *sbc_info, uint8_t bitpool) {
pa_log_debug("Bitpool has changed to %u", sbc_info->sbc.bitpool);
}
-static void reset(void *codec_info) {
+static int reset(void *codec_info) {
struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
int ret;
ret = sbc_reinit(&sbc_info->sbc, 0);
if (ret != 0) {
pa_log_error("SBC reinitialization failed: %d", ret);
- return;
+ return -1;
}
/* sbc_reinit() sets also default parameters, so reset them back */
set_params(sbc_info);
sbc_info->seq_num = 0;
+ return 0;
}
static size_t get_block_size(void *codec_info, size_t link_mtu) {
struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
+ size_t rtp_size = sizeof(struct rtp_header) + sizeof(struct rtp_sbc_payload);
+ size_t frame_count = (link_mtu - rtp_size) / sbc_info->frame_length;
- return (link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
- / sbc_info->frame_length * sbc_info->codesize;
+ /* frame_count is only 4 bit number */
+ if (frame_count > 15)
+ frame_count = 15;
+
+ return frame_count * sbc_info->codesize;
}
static size_t reduce_encoder_bitrate(void *codec_info, size_t write_link_mtu) {
@@ -508,14 +518,14 @@ static size_t reduce_encoder_bitrate(void *codec_info, size_t write_link_mtu) {
static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
struct rtp_header *header;
- struct rtp_payload *payload;
+ struct rtp_sbc_payload *payload;
uint8_t *d;
const uint8_t *p;
size_t to_write, to_encode;
- unsigned frame_count;
+ uint8_t frame_count;
header = (struct rtp_header*) output_buffer;
- payload = (struct rtp_payload*) (output_buffer + sizeof(*header));
+ payload = (struct rtp_sbc_payload*) (output_buffer + sizeof(*header));
frame_count = 0;
@@ -525,7 +535,8 @@ static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t
d = output_buffer + sizeof(*header) + sizeof(*payload);
to_write = output_size - sizeof(*header) - sizeof(*payload);
- while (PA_LIKELY(to_encode > 0 && to_write > 0)) {
+ /* frame_count is only 4 bit number */
+ while (PA_LIKELY(to_encode > 0 && to_write > 0 && frame_count < 15)) {
ssize_t written;
ssize_t encoded;
@@ -536,8 +547,12 @@ static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t
if (PA_UNLIKELY(encoded <= 0)) {
pa_log_error("SBC encoding error (%li)", (long) encoded);
- *processed = p - input_buffer;
- return 0;
+ break;
+ }
+
+ if (PA_UNLIKELY(written < 0)) {
+ pa_log_error("SBC encoding error (%li)", (long) written);
+ break;
}
pa_assert_fp((size_t) encoded <= to_encode);
@@ -559,8 +574,13 @@ static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t
pa_log_debug("Using SBC codec implementation: %s", pa_strnull(sbc_get_implementation_info(&sbc_info->sbc)));
} PA_ONCE_END;
+ if (PA_UNLIKELY(frame_count == 0)) {
+ *processed = 0;
+ return 0;
+ }
+
/* write it to the fifo */
- memset(output_buffer, 0, sizeof(*header) + sizeof(*payload));
+ pa_memzero(output_buffer, sizeof(*header) + sizeof(*payload));
header->v = 2;
/* A2DP spec: "A payload type in the RTP dynamic range shall be chosen".
@@ -581,13 +601,23 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
struct rtp_header *header;
- struct rtp_payload *payload;
+ struct rtp_sbc_payload *payload;
const uint8_t *p;
uint8_t *d;
size_t to_write, to_decode;
+ uint8_t frame_count;
header = (struct rtp_header *) input_buffer;
- payload = (struct rtp_payload*) (input_buffer + sizeof(*header));
+ payload = (struct rtp_sbc_payload*) (input_buffer + sizeof(*header));
+
+ frame_count = payload->frame_count;
+
+ /* TODO: Add support for decoding fragmented SBC frames */
+ if (payload->is_fragmented) {
+ pa_log_error("Unsupported fragmented SBC frame");
+ *processed = 0;
+ return 0;
+ }
p = input_buffer + sizeof(*header) + sizeof(*payload);
to_decode = input_size - sizeof(*header) - sizeof(*payload);
@@ -595,7 +625,7 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
d = output_buffer;
to_write = output_size;
- while (PA_LIKELY(to_decode > 0)) {
+ while (PA_LIKELY(to_decode > 0 && to_write > 0 && frame_count > 0)) {
size_t written;
ssize_t decoded;
@@ -606,8 +636,7 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
if (PA_UNLIKELY(decoded <= 0)) {
pa_log_error("SBC decoding error (%li)", (long) decoded);
- *processed = p - input_buffer;
- return 0;
+ break;
}
/* Reset frame length, it can be changed due to bitpool change */
@@ -616,6 +645,7 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
pa_assert_fp((size_t) decoded <= to_decode);
pa_assert_fp((size_t) decoded == sbc_info->frame_length);
+ pa_assert_fp((size_t) written <= to_write);
pa_assert_fp((size_t) written == sbc_info->codesize);
p += decoded;
@@ -623,6 +653,8 @@ static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_
d += written;
to_write -= written;
+
+ frame_count--;
}
*processed = p - input_buffer;
=====================================
src/modules/bluetooth/module-bluez5-device.c
=====================================
@@ -412,30 +412,45 @@ static int sco_process_push(struct userdata *u) {
static void a2dp_prepare_encoder_buffer(struct userdata *u) {
pa_assert(u);
- if (u->encoder_buffer_size >= u->write_link_mtu)
- return;
+ if (u->encoder_buffer_size < u->write_link_mtu) {
+ pa_xfree(u->encoder_buffer);
+ u->encoder_buffer = pa_xmalloc(u->write_link_mtu);
+ }
- u->encoder_buffer_size = 2 * u->write_link_mtu;
- pa_xfree(u->encoder_buffer);
- u->encoder_buffer = pa_xmalloc(u->encoder_buffer_size);
+ /* Encoder buffer cannot be larger then link MTU, otherwise
+ * encode method would produce larger packets then link MTU */
+ u->encoder_buffer_size = u->write_link_mtu;
}
/* Run from IO thread */
static void a2dp_prepare_decoder_buffer(struct userdata *u) {
pa_assert(u);
- if (u->decoder_buffer_size >= u->read_link_mtu)
- return;
+ if (u->decoder_buffer_size < u->read_link_mtu) {
+ pa_xfree(u->decoder_buffer);
+ u->decoder_buffer = pa_xmalloc(u->read_link_mtu);
+ }
- u->decoder_buffer_size = 2 * u->read_link_mtu;
- pa_xfree(u->decoder_buffer);
- u->decoder_buffer = pa_xmalloc(u->decoder_buffer_size);
+ /* Decoder buffer cannot be larger then link MTU, otherwise
+ * decode method would produce larger output then read_block_size */
+ u->decoder_buffer_size = u->read_link_mtu;
}
/* Run from IO thread */
static int a2dp_write_buffer(struct userdata *u, size_t nbytes) {
int ret = 0;
+ /* Encoder function of A2DP codec may provide empty buffer, in this case do
+ * not post any empty buffer via A2DP socket. It may be because of codec
+ * internal state, e.g. encoder is waiting for more samples so it can
+ * provide encoded data. */
+ if (PA_UNLIKELY(!nbytes)) {
+ u->write_index += (uint64_t) u->write_memchunk.length;
+ pa_memblock_unref(u->write_memchunk.memblock);
+ pa_memchunk_reset(&u->write_memchunk);
+ return 0;
+ }
+
for (;;) {
ssize_t l;
@@ -508,10 +523,10 @@ static int a2dp_process_render(struct userdata *u) {
pa_memblock_release(u->write_memchunk.memblock);
- if (length == 0)
+ if (processed != u->write_memchunk.length) {
+ pa_log_error("Encoding error");
return -1;
-
- pa_assert(processed == u->write_memchunk.length);
+ }
return a2dp_write_buffer(u, length);
}
@@ -530,6 +545,8 @@ static int a2dp_process_push(struct userdata *u) {
memchunk.memblock = pa_memblock_new(u->core->mempool, u->read_block_size);
memchunk.index = memchunk.length = 0;
+ a2dp_prepare_decoder_buffer(u);
+
for (;;) {
bool found_tstamp = false;
pa_usec_t tstamp;
@@ -537,8 +554,6 @@ static int a2dp_process_push(struct userdata *u) {
ssize_t l;
size_t processed;
- a2dp_prepare_decoder_buffer(u);
-
l = pa_read(u->stream_fd, u->decoder_buffer, u->decoder_buffer_size, &u->stream_write_type);
if (l <= 0) {
@@ -568,9 +583,12 @@ static int a2dp_process_push(struct userdata *u) {
memchunk.length = pa_memblock_get_length(memchunk.memblock);
memchunk.length = u->a2dp_codec->decode_buffer(u->decoder_info, u->decoder_buffer, l, ptr, memchunk.length, &processed);
- if (memchunk.length == 0) {
- pa_memblock_release(memchunk.memblock);
- ret = 0;
+
+ pa_memblock_release(memchunk.memblock);
+
+ if (processed != (size_t) l) {
+ pa_log_error("Decoding error");
+ ret = -1;
break;
}
@@ -578,9 +596,11 @@ static int a2dp_process_push(struct userdata *u) {
pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->decoder_sample_spec));
pa_smoother_resume(u->read_smoother, tstamp, true);
- pa_memblock_release(memchunk.memblock);
-
- pa_source_post(u->source, &memchunk);
+ /* Decoding of A2DP codec data may result in empty buffer, in this case
+ * do not post empty audio samples. It may happen due to algorithmic
+ * delay of audio codec. */
+ if (PA_LIKELY(memchunk.length))
+ pa_source_post(u->source, &memchunk);
ret = l;
break;
@@ -749,7 +769,7 @@ static void transport_config_mtu(struct userdata *u) {
}
/* Run from I/O thread */
-static void setup_stream(struct userdata *u) {
+static int setup_stream(struct userdata *u) {
struct pollfd *pollfd;
int one;
@@ -757,16 +777,18 @@ static void setup_stream(struct userdata *u) {
/* return if stream is already set up */
if (u->stream_setup_done)
- return;
+ return 0;
pa_log_info("Transport %s resuming", u->transport->path);
if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
pa_assert(u->a2dp_codec);
- u->a2dp_codec->reset(u->encoder_info);
+ if (u->a2dp_codec->reset(u->encoder_info) < 0)
+ return -1;
} else if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) {
pa_assert(u->a2dp_codec);
- u->a2dp_codec->reset(u->decoder_info);
+ if (u->a2dp_codec->reset(u->decoder_info) < 0)
+ return -1;
}
transport_config_mtu(u);
@@ -791,6 +813,8 @@ static void setup_stream(struct userdata *u) {
if (u->source)
u->read_smoother = pa_smoother_new(PA_USEC_PER_SEC, 2*PA_USEC_PER_SEC, true, true, 10, pa_rtclock_now(), true);
+
+ return 0;
}
/* Called from I/O thread, returns true if the transport was acquired or
@@ -802,8 +826,10 @@ static bool setup_transport_and_stream(struct userdata *u) {
if (transport_error < 0) {
if (transport_error != -EAGAIN)
return false;
- } else
- setup_stream(u);
+ } else {
+ if (setup_stream(u) < 0)
+ return false;
+ }
return true;
}
=====================================
src/modules/bluetooth/rtp.h
=====================================
@@ -3,6 +3,7 @@
* BlueZ - Bluetooth protocol stack for Linux
*
* Copyright (C) 2004-2010 Marcel Holtmann <marcel at holtmann.org>
+ * Copyright (C) 2019 Pali Rohár <pali.rohar at gmail.com>
*
*
* This library is free software; you can redistribute it and/or
@@ -19,16 +20,20 @@
* License along with this library; if not, see <http://www.gnu.org/licenses/>.
*/
-#if __BYTE_ORDER == __LITTLE_ENDIAN
+#include <endian.h>
+#include <stdint.h>
+
+#if defined(__BYTE_ORDER) && defined(__LITTLE_ENDIAN) && \
+ __BYTE_ORDER == __LITTLE_ENDIAN
struct rtp_header {
- unsigned cc:4;
- unsigned x:1;
- unsigned p:1;
- unsigned v:2;
+ uint8_t cc:4;
+ uint8_t x:1;
+ uint8_t p:1;
+ uint8_t v:2;
- unsigned pt:7;
- unsigned m:1;
+ uint8_t pt:7;
+ uint8_t m:1;
uint16_t sequence_number;
uint32_t timestamp;
@@ -36,24 +41,25 @@ struct rtp_header {
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;
+struct rtp_sbc_payload {
+ uint8_t frame_count:4;
+ uint8_t rfa0:1;
+ uint8_t is_last_fragment:1;
+ uint8_t is_first_fragment:1;
+ uint8_t is_fragmented:1;
} __attribute__ ((packed));
-#elif __BYTE_ORDER == __BIG_ENDIAN
+#elif defined(__BYTE_ORDER) && defined(__BIG_ENDIAN) && \
+ __BYTE_ORDER == __BIG_ENDIAN
struct rtp_header {
- unsigned v:2;
- unsigned p:1;
- unsigned x:1;
- unsigned cc:4;
+ uint8_t v:2;
+ uint8_t p:1;
+ uint8_t x:1;
+ uint8_t cc:4;
- unsigned m:1;
- unsigned pt:7;
+ uint8_t m:1;
+ uint8_t pt:7;
uint16_t sequence_number;
uint32_t timestamp;
@@ -61,12 +67,12 @@ struct rtp_header {
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;
+struct rtp_sbc_payload {
+ uint8_t is_fragmented:1;
+ uint8_t is_first_fragment:1;
+ uint8_t is_last_fragment:1;
+ uint8_t rfa0:1;
+ uint8_t frame_count:4;
} __attribute__ ((packed));
#else
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/compare/3929798a53c6fbc83b3d54d801c1c07cee4c78f5...9e70d0520182700b66438bebacb4570b7c56aa59
--
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/compare/3929798a53c6fbc83b3d54d801c1c07cee4c78f5...9e70d0520182700b66438bebacb4570b7c56aa59
You're receiving this email because of your account on gitlab.freedesktop.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20190724/623a1147/attachment-0001.html>
More information about the pulseaudio-commits
mailing list