[pulseaudio-discuss] [PATCH v13 05/10] bluetooth: Add A2DP aptX and aptX HD codecs support

[Andrey Semashev]

On 2019-10-06 20:58, Pali Rohár wrote:
> This patch provides support for aptX and aptX HD codecs in bluetooth A2DP
> profile. It uses open source LGPLv2.1+ licensed libopenaptx library which
> can be found at https://github.com/pali/libopenaptx.
> 
> aptX for s24 stereo samples provides fixed 6:1 compression ratio and
> bitrate 352.8 kbit/s, aptX HD provides fixed 4:1 compression ratio and
> bitrate 529.2 kbit/s.
> 
> According to soundexpert research, aptX codec used in bluetooth A2DP is no
> better than SBC High Quality settings. And you cannot hear difference
> between aptX and SBC High Quality, aptX is just a copper-less overpriced
> audio cable.
> 
> aptX HD is high-bitrate version of aptX. It has clearly noticeable increase
> in sound quality (not dramatic though taking into account the increase in
> bitrate).
> 
> http://soundexpert.org/news/-/blogs/audio-quality-of-bluetooth-aptx
> ---
>   configure.ac                            |  36 +++
>   src/Makefile.am                         |   6 +
>   src/modules/bluetooth/a2dp-codec-aptx.c | 479 ++++++++++++++++++++++++++++++++
>   src/modules/bluetooth/a2dp-codec-util.c |   8 +
>   4 files changed, 529 insertions(+)
>   create mode 100644 src/modules/bluetooth/a2dp-codec-aptx.c

[snip]

> +
> +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 aptx_context *aptx_c = (struct aptx_context *) codec_info;
> +    size_t written;
> +
> +    *processed = aptx_encode(aptx_c, input_buffer, input_size, output_buffer, output_size, &written);
> +    if (PA_UNLIKELY(*processed == 0 || *processed != input_size))
> +        pa_log_error("aptX encoding error");
> +
> +    return written;
> +}
> +
> +static size_t encode_buffer_hd(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 aptx_hd_info *aptx_hd_info = (struct aptx_hd_info *) codec_info;
> +    struct rtp_header *header;
> +    size_t written;
> +
> +    if (PA_UNLIKELY(output_size < sizeof(*header))) {
> +        *processed = 0;
> +        return 0;
> +    }
> +
> +    written = encode_buffer(aptx_hd_info->aptx_c, timestamp, input_buffer, input_size, output_buffer + sizeof(*header), output_size - sizeof(*header), processed);
> +
> +    if (PA_LIKELY(written > 0)) {
> +        header = (struct rtp_header *) output_buffer;

I'm not sure I understand it correctly, but encode_buffer seems to be 
producing encoded content at the beginning of the output buffer, and 
encode_buffer_hd produces an RTP header followed by encoded content. Is 
this difference intentional? What is expected to be placed in the output 
buffer - a complete RTP packet or RTP payload only?

The same comment applies to decode_buffer and decode_buffer_hd below.

> +        pa_zero(*header);
> +        header->v = 2;
> +        header->pt = 96;
> +        header->sequence_number = htons(aptx_hd_info->seq_num++);
> +        header->timestamp = htonl(timestamp);
> +        header->ssrc = htonl(1);
> +        written += sizeof(*header);
> +    }
> +
> +    return written;
> +}
> +
> +static size_t decode_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 aptx_context *aptx_c = (struct aptx_context *) codec_info;
> +    size_t written;
> +
> +    *processed = aptx_decode(aptx_c, input_buffer, input_size, output_buffer, output_size, &written);
> +
> +    /* Due to aptX latency, aptx_decode starts filling output buffer after 90 input samples.
> +     * If input buffer contains less than 90 samples, aptx_decode returns zero (=no output)
> +     * but set *processed to non zero as input samples were processed. So do not check for
> +     * return value of aptx_decode, zero is valid. Decoding error is indicating by fact that
> +     * not all input samples were processed. */
> +    if (PA_UNLIKELY(*processed != input_size))
> +        pa_log_error("aptX decoding error");
> +
> +    return written;
> +}
> +
> +static size_t decode_buffer_hd(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 aptx_hd_info *aptx_hd_info = (struct aptx_hd_info *) codec_info;
> +    struct rtp_header *header;
> +    size_t written;
> +
> +    if (PA_UNLIKELY(input_size < sizeof(*header))) {
> +        *processed = 0;
> +        return 0;
> +    }
> +
> +    header = (struct rtp_header *) input_buffer;
> +    written = decode_buffer(aptx_hd_info->aptx_c, timestamp, input_buffer + sizeof(*header), input_size - sizeof(*header), output_buffer, output_size, processed);
> +    *timestamp = ntohl(header->timestamp);
> +    *processed += sizeof(*header);
> +    return written;
> +}