<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> "a2dp-codecs.h" copied from bluez, then added a2dp_aptxhd_t, a2dp_ldac_t. I don't want to debate too much about naming. <div class="moz-cite-prefix">On 12/13/18 11:35 PM, Pali Rohár wrote: </div> <blockquote type="cite" cite="mid:20181213153522.cbpdn5xilfmjdz2p@pali"> <pre class="moz-quote-pre" wrap="">On Thursday 13 December 2018 19:43:36 EHfive wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#define A2DP_CODEC_SBC 0x00 +#define A2DP_CODEC_MPEG12 0x01 +#define A2DP_CODEC_MPEG24 0x02 +#define A2DP_CODEC_ATRAC 0x03 </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This is incorrect, Atrac codec has A2DP ID 0x04, see: <a class="moz-txt-link-freetext" href="https://www.bluetooth.com/specifications/assigned-numbers/audio-video">https://www.bluetooth.com/specifications/assigned-numbers/audio-video</a> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#define MAX_BITPOOL 64 +#define MIN_BITPOOL 2 </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> These two constants are SBC specific, so make them with SBC_ prefix. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#define APTX_VENDOR_ID 0x0000004f </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This ID is allocated for APT Ltd. company. Not just for aptX codec. So maybe better name is A2DP_VENDOR_APT_LIC_LTD? </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#define APTX_CODEC_ID 0x0001 + +#define APTX_CHANNEL_MODE_MONO 0x01 +#define APTX_CHANNEL_MODE_STEREO 0x02 + +#define APTX_SAMPLING_FREQ_16000 0x08 +#define APTX_SAMPLING_FREQ_32000 0x04 +#define APTX_SAMPLING_FREQ_44100 0x02 +#define APTX_SAMPLING_FREQ_48000 0x01 + +#define LDAC_VENDOR_ID 0x0000012d </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This ID is allocated for Sony Corporation company. Not just for LDAC codec. See: <a class="moz-txt-link-freetext" href="https://www.bluetooth.com/specifications/assigned-numbers/company-identifiers">https://www.bluetooth.com/specifications/assigned-numbers/company-identifiers</a> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#elif __BYTE_ORDER == __BIG_ENDIAN + +typedef struct { + uint8_t frequency:4; + uint8_t channel_mode:4; + uint8_t block_length:4; + uint8_t subbands:2; + uint8_t allocation_method:2; + uint8_t min_bitpool; + uint8_t max_bitpool; +} __attribute__ ((packed)) a2dp_sbc_t; + +typedef struct { + uint8_t layer:3; + uint8_t crc:1; + uint8_t channel_mode:4; + uint8_t rfa:1; + uint8_t mpf:1; + uint8_t frequency:6; + uint16_t bitrate; </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This value for big endian systems seems to be broken. As you need to store value in little endian. So maybe define as? uint8_t bitrate_low; uint8_t bitrate_high; Or as uint8_t bitrate[2]; Or better drop big endian support? Broken big endian support is not useful at all. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+static size_t +pa_sbc_decode(const void *read_buf, size_t read_buf_size, void *write_buf, size_t write_buf_size, size_t *_decoded, + uint32_t *timestamp, void **codec_data) { + const struct rtp_header *header; + const struct rtp_payload *payload; + const void *p; + void *d; + size_t to_write, to_decode; + size_t total_written = 0; + sbc_info_t *sbc_info = *codec_data; + pa_assert(sbc_info); + + header = read_buf; + payload = (struct rtp_payload *) ((uint8_t *) read_buf + sizeof(*header)); + + *timestamp = ntohl(header->timestamp); + + p = (uint8_t *) read_buf + sizeof(*header) + sizeof(*payload); + to_decode = read_buf_size - sizeof(*header) - sizeof(*payload); + + d = write_buf; + to_write = write_buf_size; + + *_decoded = 0; + while (PA_LIKELY(to_decode > 0)) { + size_t written; + ssize_t decoded; + + decoded = sbc_decode(&sbc_info->sbc, + p, to_decode, + d, to_write, + &written); + + if (PA_UNLIKELY(decoded <= 0)) { + pa_log_error("SBC decoding error (%li)", (long) decoded); + *_decoded = 0; + return 0; + } + + total_written += written; + + /* Reset frame length, it can be changed due to bitpool change */ + sbc_info->frame_length = sbc_get_frame_length(&sbc_info->sbc); + + pa_assert_fp((size_t) decoded <= to_decode); + pa_assert_fp((size_t) decoded == sbc_info->frame_length); + + pa_assert_fp((size_t) written == sbc_info->codesize); + + *_decoded += decoded; + p = (const uint8_t *) p + decoded; + to_decode -= decoded; + + d = (uint8_t *) d + written; + to_write -= written; + } + + return total_written; +} </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> Seems that this decode procedure with while loop is similar/same for all codecs. Months ago I sent to this mailing list different proposal for codec API which tries to "fix" also this problem. Look into mailing list archive for "[PATCH v2 1/2] Modular API for Bluetooth A2DP codec". </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+#define A2DP_SOURCE_ENDPOINT "/MediaEndpoint/A2DPSource" +#define A2DP_SINK_ENDPOINT "/MediaEndpoint/A2DPSink" + +#define A2DP_SBC_SRC_ENDPOINT A2DP_SOURCE_ENDPOINT "/SBC" +#define A2DP_SBC_SNK_ENDPOINT A2DP_SINK_ENDPOINT "/SBC" + +#define A2DP_VENDOR_SRC_ENDPOINT A2DP_SOURCE_ENDPOINT "/VENDOR" +#define A2DP_VENDOR_SNK_ENDPOINT A2DP_SINK_ENDPOINT "/VENDOR" </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> This would not work correctly. You need for each codec different endpoint.</pre> </blockquote> Actually, there do have multiple endpoints . The result is, it support multi-codec, but can't switch to other codec. Endpoint registered first has higher priority. (see enum pa_a2dp_codec_index ) <blockquote type="cite" cite="mid:20181213153522.cbpdn5xilfmjdz2p@pali"> <pre class="moz-quote-pre" wrap=""> Also currently bluez does not allows you to choose which endpoint will be used... As bluez does not provide API for it yet. </pre> </blockquote> </body> </html>