"a2dp-codecs.h" copied from bluez, then added a2dp_aptxhd_t, a2dp_ldac_t.
I don't want to debate too much about naming. On 12/13/18 11:35 PM, Pali Rohár wrote: > On Thursday 13 December 2018 19:43:36 EHfive wrote: >> +#define A2DP_CODEC_SBC 0x00 >> +#define A2DP_CODEC_MPEG12 0x01 >> +#define A2DP_CODEC_MPEG24 0x02 >> +#define A2DP_CODEC_ATRAC 0x03 > This is incorrect, Atrac codec has A2DP ID 0x04, see: > https://www.bluetooth.com/specifications/assigned-numbers/audio-video > >> +#define MAX_BITPOOL 64 >> +#define MIN_BITPOOL 2 > These two constants are SBC specific, so make them with SBC_ prefix. > >> +#define APTX_VENDOR_ID 0x0000004f > This ID is allocated for APT Ltd. company. Not just for aptX codec. So > maybe better name is A2DP_VENDOR_APT_LIC_LTD? > >> +#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 > This ID is allocated for Sony Corporation company. Not just for LDAC > codec. > > See: > https://www.bluetooth.com/specifications/assigned-numbers/company-identifiers > >> +#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; > 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. > >> +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; >> +} > 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". > >> +#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" > This would not work correctly. You need for each codec different > endpoint. 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 ) > > Also currently bluez does not allows you to choose which endpoint will > be used... As bluez does not provide API for it yet. >
signature.asc
Description: OpenPGP digital signature
_______________________________________________ pulseaudio-discuss mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
