[pulseaudio-discuss] [PATCH v10 02/11] bluetooth: Fix usage of RTP structures in SBC codec

[Pali Rohár]

Rename struct rtp_payload to rtp_sbc_payload as it is specific for SBC
codec. Add proper checks for endianity in rtp.h header and use uint8_t type
where appropriated. And because rtp_sbc_payload structure is not parsed by
decoder there is no support for fragmented SBC frames. Add warning for it.
---
 src/modules/bluetooth/a2dp-codec-sbc.c | 16 ++++++----
 src/modules/bluetooth/rtp.h            | 58 +++++++++++++++++++---------------
 2 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/src/modules/bluetooth/a2dp-codec-sbc.c b/src/modules/bluetooth/a2dp-codec-sbc.c
index f339b570d..6ab0b46cd 100644
--- a/src/modules/bluetooth/a2dp-codec-sbc.c
+++ b/src/modules/bluetooth/a2dp-codec-sbc.c
@@ -480,7 +480,7 @@ static void reset(void *codec_info) {
 
 static void get_buffer_size(void *codec_info, size_t link_mtu, size_t *decoded_buffer_size, size_t *encoded_buffer_size) {
     struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
-    size_t rtp_size = sizeof(struct rtp_header) + sizeof(struct rtp_payload);
+    size_t rtp_size = sizeof(struct rtp_header) + sizeof(struct rtp_sbc_payload);
     size_t num_of_frames = (link_mtu - rtp_size) / sbc_info->frame_length;
 
     *decoded_buffer_size = num_of_frames * sbc_info->codesize;
@@ -510,14 +510,14 @@ static int reduce_encoder_bitrate(void *codec_info) {
 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;
 
     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;
 
@@ -562,7 +562,7 @@ static size_t encode_buffer(void *codec_info, uint32_t timestamp, const uint8_t
     } PA_ONCE_END;
 
     /* 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".
@@ -583,13 +583,17 @@ 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;
 
     header = (struct rtp_header *) input_buffer;
-    payload = (struct rtp_payload*) (input_buffer + sizeof(*header));
+    payload = (struct rtp_sbc_payload*) (input_buffer + sizeof(*header));
+
+    /* TODO: Add support for decoding fragmented SBC frames */
+    if (payload->is_fragmented)
+        pa_log_warn("SBC frame is fragmented, decoding may fail");
 
     p = input_buffer + sizeof(*header) + sizeof(*payload);
     to_decode = input_size - sizeof(*header) - sizeof(*payload);
diff --git a/src/modules/bluetooth/rtp.h b/src/modules/bluetooth/rtp.h
index 20694c1e1..813d9e390 100644
--- a/src/modules/bluetooth/rtp.h
+++ b/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
-- 
2.11.0