[pulseaudio-discuss] [PATCH v7 04/13] bluetooth: Modular API for A2DP codecs

Tanu Kaskinen tanuk at iki.fi
Thu Apr 4 15:19:32 UTC 2019


On Sat, 2019-02-23 at 10:45 +0100, Pali Rohár wrote:
> This patch introduce new modular API for bluetooth A2DP codecs. Its
> benefits are:
> 
> * bluez5-util and module-bluez5-device does not contain any codec specific
>   code, they are codec independent.
> 
> * For adding new A2DP codec it is needed just to adjust one table in
>   a2dp-codec-util.c file. All codec specific functions are in separate
>   codec file.
> 
> * Support for backchannel (microphone voice). Some A2DP codecs (like
>   FastStream or aptX Low Latency) are bi-directional and can be used for
>   both music playback and audio call.
> 
> * Support for more configurations per codec. This allows to implement low
>   quality mode of some codec together with high quality.
> 
> Current SBC codec implementation was moved from bluez5-util and
> module-bluez5-device to its own file and converted to this new A2DP API.
> ---
>  src/Makefile.am                              |  12 +-
>  src/modules/bluetooth/a2dp-codec-api.h       |  80 ++++
>  src/modules/bluetooth/a2dp-codec-sbc.c       | 638 +++++++++++++++++++++++++++
>  src/modules/bluetooth/a2dp-codec-util.c      |  56 +++
>  src/modules/bluetooth/a2dp-codec-util.h      |  34 ++
>  src/modules/bluetooth/bluez5-util.c          | 348 +++++----------
>  src/modules/bluetooth/bluez5-util.h          |   5 +
>  src/modules/bluetooth/module-bluez5-device.c | 563 ++++++++---------------
>  8 files changed, 1119 insertions(+), 617 deletions(-)
>  create mode 100644 src/modules/bluetooth/a2dp-codec-api.h
>  create mode 100644 src/modules/bluetooth/a2dp-codec-sbc.c
>  create mode 100644 src/modules/bluetooth/a2dp-codec-util.c
>  create mode 100644 src/modules/bluetooth/a2dp-codec-util.h
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 89f532278..5ef870e32 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -2131,6 +2131,9 @@ module_bluetooth_discover_la_CFLAGS = $(AM_CFLAGS) -DPA_MODULE_NAME=module_bluet
>  libbluez5_util_la_SOURCES = \
>  		modules/bluetooth/bluez5-util.c \
>  		modules/bluetooth/bluez5-util.h \
> +		modules/bluetooth/a2dp-codec-api.h \
> +		modules/bluetooth/a2dp-codec-util.c \
> +		modules/bluetooth/a2dp-codec-util.h \
>  		modules/bluetooth/a2dp-codecs.h \
>  		modules/bluetooth/rtp.h
>  if HAVE_BLUEZ_5_OFONO_HEADSET
> @@ -2145,6 +2148,11 @@ endif
>  libbluez5_util_la_LDFLAGS = -avoid-version
>  libbluez5_util_la_LIBADD = $(MODULE_LIBADD) $(DBUS_LIBS)
>  libbluez5_util_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS)
> +libbluez5_util_la_CPPFLAGS = $(AM_CPPFLAGS)
> +
> +libbluez5_util_la_SOURCES += modules/bluetooth/a2dp-codec-sbc.c
> +libbluez5_util_la_LIBADD += $(SBC_LIBS)
> +libbluez5_util_la_CFLAGS += $(SBC_CFLAGS)
>  
>  module_bluez5_discover_la_SOURCES = modules/bluetooth/module-bluez5-discover.c
>  module_bluez5_discover_la_LDFLAGS = $(MODULE_LDFLAGS)
> @@ -2153,8 +2161,8 @@ module_bluez5_discover_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) -DPA_MODULE_NAME=
>  
>  module_bluez5_device_la_SOURCES = modules/bluetooth/module-bluez5-device.c
>  module_bluez5_device_la_LDFLAGS = $(MODULE_LDFLAGS)
> -module_bluez5_device_la_LIBADD = $(MODULE_LIBADD) $(SBC_LIBS) libbluez5-util.la
> -module_bluez5_device_la_CFLAGS = $(AM_CFLAGS) $(SBC_CFLAGS) -DPA_MODULE_NAME=module_bluez5_device
> +module_bluez5_device_la_LIBADD = $(MODULE_LIBADD) libbluez5-util.la
> +module_bluez5_device_la_CFLAGS = $(AM_CFLAGS) -DPA_MODULE_NAME=module_bluez5_device
>  
>  # Apple Airtunes/RAOP
>  module_raop_sink_la_SOURCES = modules/raop/module-raop-sink.c
> diff --git a/src/modules/bluetooth/a2dp-codec-api.h b/src/modules/bluetooth/a2dp-codec-api.h
> new file mode 100644
> index 000000000..d9cc1a58e
> --- /dev/null
> +++ b/src/modules/bluetooth/a2dp-codec-api.h
> @@ -0,0 +1,80 @@
> +#ifndef fooa2dpcodechfoo
> +#define fooa2dpcodechfoo
> +
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2018-2019 Pali Rohár <pali.rohar at gmail.com>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>;.
> +***/
> +
> +#include <pulsecore/core.h>
> +
> +typedef struct pa_a2dp_codec_capabilities {
> +    uint8_t size;
> +    uint8_t buffer[]; /* max size is 254 bytes */
> +} pa_a2dp_codec_capabilities;
> +
> +typedef struct pa_a2dp_codec_id {
> +    uint8_t codec_id;
> +    uint32_t vendor_id;
> +    uint16_t vendor_codec_id;
> +} pa_a2dp_codec_id;
> +
> +typedef struct pa_a2dp_codec {
> +    /* Unique name of the codec, lowercase and without whitespaces, used for constructing identifier, D-Bus paths, ... */
> +    const char *codec_name;
> +    /* Human readable codec description */
> +    const char *codec_description;
> +
> +    /* A2DP codec id */
> +    pa_a2dp_codec_id codec_id;

I agree with Luiz that it would be good to drop "codec_" from the names
of these variables.

> +
> +    /* True if codec is bi-directional and supports backchannel */
> +    bool support_backchannel;
> +
> +    /* Returns true if codec accepts capabilities, for_encoding is true when capabilities are used for encoding */
> +    bool (*accept_capabilities)(const uint8_t *capabilities_buffer, uint8_t capabilities_size, bool for_encoding);
> +    /* Choose preferred capabilities from hash map (const char * -> const pa_a2dp_codec_capabilities *) and returns corresponding key, for encoder is true when capabilities hash map is used for encoding */

Typo: "for encoder" should be "for_encoder".

Also, comments should be wrapped at 80 characters, that makes them
easier to read.

> +    const char *(*choose_capabilities)(const pa_hashmap *capabilities_hashmap, const pa_sample_spec *default_sample_spec, bool for_encoding);

Returning a string key rather than a pa_a2dp_codec_capabilities looked
weird, and the comment didn't explain what the hashmap keys should look
like. I looked up how choose_capabilities() is used, and it looks like
the returned key is an endpoint path. I think choose_remote_endpoint
would be a better name for this callback, and the documentation should
explain that the keys are remote endpoint paths.

It seems that returning NULL is allowed when none of the endpoints can
be accepted. It would be good to document that.

> +    /* Fill codec capabilities, returns size of filled buffer */
> +    uint8_t (*fill_capabilities)(uint8_t capabilities_buffer[254]);

I think we should use a define for the maximum capabilities buffer
size.

> +    /* Validate codec configuration, returns true on success */
> +    bool (*validate_configuration)(const uint8_t *config_buffer, uint8_t config_size);

I commented earlier:

    The bool return value is used for reporting failures, but according
    to our coding style failures should be reported using a negative
    int.

You replied:

    It is really needed to use integer return value for true/false?

I never answered that question (sorry). When the result is about
success/failure, then an int should be used. If the result is a yes/no
answer, then bool is correct. If the function name was
is_configuration_valid, then you could use bool here.

> +    /* Fill preferred codec configuration, returns size of filled buffer */
> +    uint8_t (*fill_preferred_configuration)(const pa_sample_spec *default_sample_spec, const uint8_t *capabilities_buffer, uint8_t capabilities_size, uint8_t config_buffer[254]);

It should be documented that the callback should return 0 on failure.

> +
> +    /* Initialize codec, returns codec info data and set sample_spec, for_encoding is true when codec_info is used for encoding, for_backchannel is true when codec_info is used for backchannel */
> +    void *(*init_codec)(bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec);

In order to be more consistent with other abstraction structs, I'd like
to add a void pointer variable called "userdata" to pa_a2dp_codec.
init_codec() would set that variable instead of returning the data. All
callbacks would receive a pa_a2dp_codec pointer as a parameter instead
of a void pointer.

> +    /* Deinitialize and release codec info data in codec_info */
> +    void (*finish_codec)(void *codec_info);
> +    /* Reset internal state of codec info data in codec_info */
> +    void (*reset_codec)(void *codec_info);
> +
> +    /* Get read block size for codec */
> +    size_t (*get_read_block_size)(void *codec_info, size_t read_link_mtu);
> +    /* Get write block size for codec */
> +    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 if not changed, called when socket is not accepting encoded data fast enough */
> +    size_t (*reduce_encoder_bitrate)(void *codec_info, size_t write_link_mtu);
> +
> +    /* Encode input_buffer of input_size to output_buffer of output_size, returns size of filled ouput_buffer and set processed to size of processed input_buffer */
> +    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);
> +    /* Decode input_buffer of input_size to output_buffer of output_size, returns size of filled ouput_buffer and set processed to size of processed input_buffer */
> +    size_t (*decode_buffer)(void *codec_info, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed);
> +} pa_a2dp_codec;
> +
> +#endif
> diff --git a/src/modules/bluetooth/a2dp-codec-sbc.c b/src/modules/bluetooth/a2dp-codec-sbc.c
> new file mode 100644
> index 000000000..3db827db3
> --- /dev/null
> +++ b/src/modules/bluetooth/a2dp-codec-sbc.c
> @@ -0,0 +1,638 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2018-2019 Pali Rohár <pali.rohar at gmail.com>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>;.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulsecore/core-util.h>
> +#include <pulsecore/log.h>
> +#include <pulsecore/macro.h>
> +#include <pulsecore/once.h>
> +#include <pulse/sample.h>
> +#include <pulse/xmalloc.h>
> +
> +#include <arpa/inet.h>
> +
> +#include <sbc/sbc.h>
> +
> +#include "a2dp-codecs.h"
> +#include "a2dp-codec-api.h"
> +#include "rtp.h"
> +
> +#define SBC_BITPOOL_DEC_LIMIT 32
> +#define SBC_BITPOOL_DEC_STEP 5
> +
> +struct sbc_info {
> +    sbc_t sbc;                           /* Codec data */
> +    size_t codesize, frame_length;       /* SBC Codesize, frame_length. We simply cache those values here */
> +    uint16_t seq_num;                    /* Cumulative packet sequence */
> +    uint8_t frequency;
> +    uint8_t blocks;
> +    uint8_t subbands;
> +    uint8_t mode;
> +    uint8_t allocation;
> +    uint8_t bitpool;
> +    uint8_t min_bitpool;
> +    uint8_t max_bitpool;
> +};
> +
> +static bool accept_capabilities(const uint8_t *capabilities_buffer, uint8_t capabilities_size, bool for_encoding) {
> +    const a2dp_sbc_t *capabilities = (const a2dp_sbc_t *) capabilities_buffer;
> +
> +    if (capabilities_size != sizeof(*capabilities))
> +        return false;
> +
> +    if (!(capabilities->frequency & (SBC_SAMPLING_FREQ_16000 | SBC_SAMPLING_FREQ_32000 | SBC_SAMPLING_FREQ_44100 | SBC_SAMPLING_FREQ_48000)))
> +        return false;
> +
> +    if (!(capabilities->channel_mode & (SBC_CHANNEL_MODE_MONO | SBC_CHANNEL_MODE_DUAL_CHANNEL | SBC_CHANNEL_MODE_STEREO | SBC_CHANNEL_MODE_JOINT_STEREO)))
> +        return false;
> +
> +    if (!(capabilities->allocation_method & (SBC_ALLOCATION_SNR | SBC_ALLOCATION_LOUDNESS)))
> +        return false;
> +
> +    if (!(capabilities->subbands & (SBC_SUBBANDS_4 | SBC_SUBBANDS_8)))
> +        return false;
> +
> +    if (!(capabilities->block_length & (SBC_BLOCK_LENGTH_4 | SBC_BLOCK_LENGTH_8 | SBC_BLOCK_LENGTH_12 | SBC_BLOCK_LENGTH_16)))
> +        return false;
> +
> +    return true;
> +}
> +
> +static const char *choose_capabilities(const pa_hashmap *capabilities_hashmap, bool for_encoding) {

The default_sample_spec argument is missing.

> +    const pa_a2dp_codec_capabilities *a2dp_capabilities;
> +    const char *key;
> +    void *state;
> +
> +    /* There is no preference, just choose random valid entry */
> +    PA_HASHMAP_FOREACH_KV(key, a2dp_capabilities, capabilities_hashmap, state) {
> +        if (accept_capabilities(a2dp_capabilities->buffer, a2dp_capabilities->size, for_encoding))
> +            return key;
> +    }
> +
> +    return NULL;
> +}
> +
> +static uint8_t fill_capabilities(uint8_t capabilities_buffer[254]) {
> +    a2dp_sbc_t *capabilities = (a2dp_sbc_t *) capabilities_buffer;
> +
> +    pa_zero(*capabilities);
> +
> +    capabilities->channel_mode = SBC_CHANNEL_MODE_MONO | SBC_CHANNEL_MODE_DUAL_CHANNEL | SBC_CHANNEL_MODE_STEREO |
> +                                 SBC_CHANNEL_MODE_JOINT_STEREO;
> +    capabilities->frequency = SBC_SAMPLING_FREQ_16000 | SBC_SAMPLING_FREQ_32000 | SBC_SAMPLING_FREQ_44100 |
> +                              SBC_SAMPLING_FREQ_48000;
> +    capabilities->allocation_method = SBC_ALLOCATION_SNR | SBC_ALLOCATION_LOUDNESS;
> +    capabilities->subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8;
> +    capabilities->block_length = SBC_BLOCK_LENGTH_4 | SBC_BLOCK_LENGTH_8 | SBC_BLOCK_LENGTH_12 | SBC_BLOCK_LENGTH_16;
> +    capabilities->min_bitpool = SBC_MIN_BITPOOL;
> +    capabilities->max_bitpool = 64;
> +
> +    return sizeof(*capabilities);
> +}
> +
> +static bool validate_configuration(const uint8_t *config_buffer, uint8_t config_size) {
> +    const a2dp_sbc_t *config = (const a2dp_sbc_t *) config_buffer;
> +
> +    if (config_size != sizeof(*config)) {
> +        pa_log_error("Invalid size of config buffer");
> +        return false;
> +    }
> +
> +    if (config->frequency != SBC_SAMPLING_FREQ_16000 && config->frequency != SBC_SAMPLING_FREQ_32000 &&
> +        config->frequency != SBC_SAMPLING_FREQ_44100 && config->frequency != SBC_SAMPLING_FREQ_48000) {
> +        pa_log_error("Invalid sampling frequency in configuration");
> +        return false;
> +    }
> +
> +    if (config->channel_mode != SBC_CHANNEL_MODE_MONO && config->channel_mode != SBC_CHANNEL_MODE_DUAL_CHANNEL &&
> +        config->channel_mode != SBC_CHANNEL_MODE_STEREO && config->channel_mode != SBC_CHANNEL_MODE_JOINT_STEREO) {
> +        pa_log_error("Invalid channel mode in configuration");
> +        return false;
> +    }
> +
> +    if (config->allocation_method != SBC_ALLOCATION_SNR && config->allocation_method != SBC_ALLOCATION_LOUDNESS) {
> +        pa_log_error("Invalid allocation method in configuration");
> +        return false;
> +    }
> +
> +    if (config->subbands != SBC_SUBBANDS_4 && config->subbands != SBC_SUBBANDS_8) {
> +        pa_log_error("Invalid SBC subbands in configuration");
> +        return false;
> +    }
> +
> +    if (config->block_length != SBC_BLOCK_LENGTH_4 && config->block_length != SBC_BLOCK_LENGTH_8 &&
> +        config->block_length != SBC_BLOCK_LENGTH_12 && config->block_length != SBC_BLOCK_LENGTH_16) {
> +        pa_log_error("Invalid block length in configuration");
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +static uint8_t default_bitpool(uint8_t freq, uint8_t mode) {
> +    /* These bitpool values were chosen based on the A2DP spec recommendation */
> +    switch (freq) {
> +        case SBC_SAMPLING_FREQ_16000:
> +        case SBC_SAMPLING_FREQ_32000:
> +            return 53;
> +
> +        case SBC_SAMPLING_FREQ_44100:
> +
> +            switch (mode) {
> +                case SBC_CHANNEL_MODE_MONO:
> +                case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> +                    return 31;
> +
> +                case SBC_CHANNEL_MODE_STEREO:
> +                case SBC_CHANNEL_MODE_JOINT_STEREO:
> +                    return 53;
> +            }
> +
> +            pa_log_warn("Invalid channel mode %u", mode);
> +            return 53;
> +
> +        case SBC_SAMPLING_FREQ_48000:
> +
> +            switch (mode) {
> +                case SBC_CHANNEL_MODE_MONO:
> +                case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> +                    return 29;
> +
> +                case SBC_CHANNEL_MODE_STEREO:
> +                case SBC_CHANNEL_MODE_JOINT_STEREO:
> +                    return 51;
> +            }
> +
> +            pa_log_warn("Invalid channel mode %u", mode);
> +            return 51;
> +    }
> +
> +    pa_log_warn("Invalid sampling freq %u", freq);
> +    return 53;
> +}
> +
> +static uint8_t fill_preferred_configuration(const pa_sample_spec *default_sample_spec, const uint8_t *capabilities_buffer, uint8_t capabilities_size, uint8_t config_buffer[254]) {
> +    a2dp_sbc_t *config = (a2dp_sbc_t *) config_buffer;
> +    const a2dp_sbc_t *capabilities = (const a2dp_sbc_t *) capabilities_buffer;
> +    int i;
> +
> +    static const struct {
> +        uint32_t rate;
> +        uint8_t cap;
> +    } freq_table[] = {
> +        { 16000U, SBC_SAMPLING_FREQ_16000 },
> +        { 32000U, SBC_SAMPLING_FREQ_32000 },
> +        { 44100U, SBC_SAMPLING_FREQ_44100 },
> +        { 48000U, SBC_SAMPLING_FREQ_48000 }
> +    };
> +
> +    if (capabilities_size != sizeof(*capabilities)) {
> +        pa_log_error("Invalid size of capabilities buffer");
> +        return 0;
> +    }
> +
> +    pa_zero(*config);
> +
> +    /* Find the lowest freq that is at least as high as the requested sampling rate */
> +    for (i = 0; (unsigned) i < PA_ELEMENTSOF(freq_table); i++)
> +        if (freq_table[i].rate >= default_sample_spec->rate && (capabilities->frequency & freq_table[i].cap)) {
> +            config->frequency = freq_table[i].cap;
> +            break;
> +        }
> +
> +    if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
> +        for (--i; i >= 0; i--) {
> +            if (capabilities->frequency & freq_table[i].cap) {
> +                config->frequency = freq_table[i].cap;
> +                break;
> +            }
> +        }
> +
> +        if (i < 0) {
> +            pa_log_error("Not suitable sample rate");
> +            return 0;
> +        }
> +    }
> +
> +    pa_assert((unsigned) i < PA_ELEMENTSOF(freq_table));
> +
> +    if (default_sample_spec->channels <= 1) {
> +        if (capabilities->channel_mode & SBC_CHANNEL_MODE_MONO)
> +            config->channel_mode = SBC_CHANNEL_MODE_MONO;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO)
> +            config->channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_STEREO)
> +            config->channel_mode = SBC_CHANNEL_MODE_STEREO;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL)
> +            config->channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
> +        else {
> +            pa_log_error("No supported channel modes");
> +            return 0;
> +        }
> +    } else {
> +        if (capabilities->channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO)
> +            config->channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_STEREO)
> +            config->channel_mode = SBC_CHANNEL_MODE_STEREO;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL)
> +            config->channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
> +        else if (capabilities->channel_mode & SBC_CHANNEL_MODE_MONO)
> +            config->channel_mode = SBC_CHANNEL_MODE_MONO;
> +        else {
> +            pa_log_error("No supported channel modes");
> +            return 0;
> +        }
> +    }
> +
> +    if (capabilities->block_length & SBC_BLOCK_LENGTH_16)
> +        config->block_length = SBC_BLOCK_LENGTH_16;
> +    else if (capabilities->block_length & SBC_BLOCK_LENGTH_12)
> +        config->block_length = SBC_BLOCK_LENGTH_12;
> +    else if (capabilities->block_length & SBC_BLOCK_LENGTH_8)
> +        config->block_length = SBC_BLOCK_LENGTH_8;
> +    else if (capabilities->block_length & SBC_BLOCK_LENGTH_4)
> +        config->block_length = SBC_BLOCK_LENGTH_4;
> +    else {
> +        pa_log_error("No supported block lengths");
> +        return 0;
> +    }
> +
> +    if (capabilities->subbands & SBC_SUBBANDS_8)
> +        config->subbands = SBC_SUBBANDS_8;
> +    else if (capabilities->subbands & SBC_SUBBANDS_4)
> +        config->subbands = SBC_SUBBANDS_4;
> +    else {
> +        pa_log_error("No supported subbands");
> +        return 0;
> +    }
> +
> +    if (capabilities->allocation_method & SBC_ALLOCATION_LOUDNESS)
> +        config->allocation_method = SBC_ALLOCATION_LOUDNESS;
> +    else if (capabilities->allocation_method & SBC_ALLOCATION_SNR)
> +        config->allocation_method = SBC_ALLOCATION_SNR;
> +
> +    config->min_bitpool = (uint8_t) PA_MAX(SBC_MIN_BITPOOL, capabilities->min_bitpool);
> +    config->max_bitpool = (uint8_t) PA_MIN(default_bitpool(config->frequency, config->channel_mode), capabilities->max_bitpool);
> +
> +    if (config->min_bitpool > config->max_bitpool)
> +        return 0;

Logging an error message would be helpful here.

> +
> +    return sizeof(*config);
> +}
> +
> +static void set_params(struct sbc_info *sbc_info) {
> +    sbc_info->sbc.frequency = sbc_info->frequency;
> +    sbc_info->sbc.blocks = sbc_info->blocks;
> +    sbc_info->sbc.subbands = sbc_info->subbands;
> +    sbc_info->sbc.mode = sbc_info->mode;
> +    sbc_info->sbc.allocation = sbc_info->allocation;
> +    sbc_info->sbc.bitpool = sbc_info->bitpool;
> +
> +    sbc_info->codesize = sbc_get_codesize(&sbc_info->sbc);
> +    sbc_info->frame_length = sbc_get_frame_length(&sbc_info->sbc);
> +}
> +
> +static void *init_codec(bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec) {
> +    struct sbc_info *sbc_info;
> +    const a2dp_sbc_t *config = (const a2dp_sbc_t *) config_buffer;
> +    int ret;
> +
> +    pa_assert(config_size == sizeof(*config));
> +    pa_assert(!for_backchannel);
> +
> +    sbc_info = pa_xnew0(struct sbc_info, 1);
> +
> +    ret = sbc_init(&sbc_info->sbc, 0);
> +    if (ret != 0) {
> +        pa_xfree(sbc_info);
> +        pa_log_error("SBC initialization failed: %d", ret);
> +        return NULL;
> +    }
> +
> +    sample_spec->format = PA_SAMPLE_S16LE;
> +
> +    switch (config->frequency) {
> +        case SBC_SAMPLING_FREQ_16000:
> +            sbc_info->frequency = SBC_FREQ_16000;
> +            sample_spec->rate = 16000U;
> +            break;
> +        case SBC_SAMPLING_FREQ_32000:
> +            sbc_info->frequency = SBC_FREQ_32000;
> +            sample_spec->rate = 32000U;
> +            break;
> +        case SBC_SAMPLING_FREQ_44100:
> +            sbc_info->frequency = SBC_FREQ_44100;
> +            sample_spec->rate = 44100U;
> +            break;
> +        case SBC_SAMPLING_FREQ_48000:
> +            sbc_info->frequency = SBC_FREQ_48000;
> +            sample_spec->rate = 48000U;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    switch (config->channel_mode) {
> +        case SBC_CHANNEL_MODE_MONO:
> +            sbc_info->mode = SBC_MODE_MONO;
> +            sample_spec->channels = 1;
> +            break;
> +        case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> +            sbc_info->mode = SBC_MODE_DUAL_CHANNEL;
> +            sample_spec->channels = 2;
> +            break;
> +        case SBC_CHANNEL_MODE_STEREO:
> +            sbc_info->mode = SBC_MODE_STEREO;
> +            sample_spec->channels = 2;
> +            break;
> +        case SBC_CHANNEL_MODE_JOINT_STEREO:
> +            sbc_info->mode = SBC_MODE_JOINT_STEREO;
> +            sample_spec->channels = 2;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    switch (config->allocation_method) {
> +        case SBC_ALLOCATION_SNR:
> +            sbc_info->allocation = SBC_AM_SNR;
> +            break;
> +        case SBC_ALLOCATION_LOUDNESS:
> +            sbc_info->allocation = SBC_AM_LOUDNESS;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    switch (config->subbands) {
> +        case SBC_SUBBANDS_4:
> +            sbc_info->subbands = SBC_SB_4;
> +            break;
> +        case SBC_SUBBANDS_8:
> +            sbc_info->subbands = SBC_SB_8;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    switch (config->block_length) {
> +        case SBC_BLOCK_LENGTH_4:
> +            sbc_info->blocks = SBC_BLK_4;
> +            break;
> +        case SBC_BLOCK_LENGTH_8:
> +            sbc_info->blocks = SBC_BLK_8;
> +            break;
> +        case SBC_BLOCK_LENGTH_12:
> +            sbc_info->blocks = SBC_BLK_12;
> +            break;
> +        case SBC_BLOCK_LENGTH_16:
> +            sbc_info->blocks = SBC_BLK_16;
> +            break;
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    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 */
> +    sbc_info->bitpool = for_encoding ? sbc_info->max_bitpool : sbc_info->min_bitpool;
> +
> +    set_params(sbc_info);
> +
> +    pa_log_info("SBC parameters: allocation=%s, subbands=%u, blocks=%u, mode=%s bitpool=%u codesize=%u frame_length=%u",
> +                sbc_info->sbc.allocation ? "SNR" : "Loudness", sbc_info->sbc.subbands ? 8 : 4,
> +                (sbc_info->sbc.blocks+1)*4, sbc_info->sbc.mode == SBC_MODE_MONO ? "Mono" :
> +                sbc_info->sbc.mode == SBC_MODE_DUAL_CHANNEL ? "DualChannel" :
> +                sbc_info->sbc.mode == SBC_MODE_STEREO ? "Stereo" : "JointStereo",
> +                sbc_info->sbc.bitpool, (unsigned)sbc_info->codesize, (unsigned)sbc_info->frame_length);
> +
> +    return sbc_info;
> +}
> +
> +static void finish_codec(void *codec_info) {
> +    struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> +
> +    sbc_finish(&sbc_info->sbc);
> +    pa_xfree(sbc_info);
> +}
> +
> +static void set_bitpool(struct sbc_info *sbc_info, uint8_t bitpool) {
> +    if (bitpool > sbc_info->max_bitpool)
> +        bitpool = sbc_info->max_bitpool;
> +    else if (bitpool < sbc_info->min_bitpool)
> +        bitpool = sbc_info->min_bitpool;
> +
> +    sbc_info->sbc.bitpool = bitpool;
> +
> +    sbc_info->codesize = sbc_get_codesize(&sbc_info->sbc);
> +    sbc_info->frame_length = sbc_get_frame_length(&sbc_info->sbc);
> +
> +    pa_log_debug("Bitpool has changed to %u", sbc_info->sbc.bitpool);
> +}
> +
> +static void reset_codec(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;
> +    }
> +
> +    /* sbc_reinit() sets also default parameters, so reset them back */
> +    set_params(sbc_info);
> +
> +    sbc_info->seq_num = 0;
> +}
> +
> +static size_t get_block_size(void *codec_info, size_t link_mtu) {
> +    struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> +
> +    return (link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
> +           / sbc_info->frame_length * sbc_info->codesize;
> +}
> +
> +static size_t reduce_encoder_bitrate(void *codec_info, size_t write_link_mtu) {
> +    struct sbc_info *sbc_info = (struct sbc_info *) codec_info;
> +    uint8_t bitpool;
> +
> +    /* Check if bitpool is already at its limit */
> +    if (sbc_info->sbc.bitpool <= SBC_BITPOOL_DEC_LIMIT)
> +        return 0;
> +
> +    bitpool = sbc_info->sbc.bitpool - SBC_BITPOOL_DEC_STEP;
> +
> +    if (bitpool < SBC_BITPOOL_DEC_LIMIT)
> +        bitpool = SBC_BITPOOL_DEC_LIMIT;
> +
> +    if (sbc_info->sbc.bitpool == bitpool)
> +        return 0;
> +
> +    set_bitpool(sbc_info, bitpool);
> +    return get_block_size(codec_info, 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;
> +    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));
> +
> +    frame_count = 0;
> +
> +    p = input_buffer;
> +    to_encode = input_size;
> +
> +    d = output_buffer + sizeof(*header) + sizeof(*payload);
> +    to_write = output_size - sizeof(*header) - sizeof(*payload);
> +
> +    while (PA_LIKELY(to_encode > 0 && to_write > 0)) {
> +        ssize_t written;
> +        ssize_t encoded;
> +
> +        encoded = sbc_encode(&sbc_info->sbc,
> +                             p, to_encode,
> +                             d, to_write,
> +                             &written);
> +
> +        if (PA_UNLIKELY(encoded <= 0)) {
> +            pa_log_error("SBC encoding error (%li)", (long) encoded);
> +            *processed = p - input_buffer;
> +            return 0;
> +        }
> +
> +        pa_assert_fp((size_t) encoded <= to_encode);
> +        pa_assert_fp((size_t) encoded == sbc_info->codesize);
> +
> +        pa_assert_fp((size_t) written <= to_write);
> +        pa_assert_fp((size_t) written == sbc_info->frame_length);
> +
> +        p += encoded;
> +        to_encode -= encoded;
> +
> +        d += written;
> +        to_write -= written;
> +
> +        frame_count++;
> +    }
> +
> +    PA_ONCE_BEGIN {
> +        pa_log_debug("Using SBC codec implementation: %s", pa_strnull(sbc_get_implementation_info(&sbc_info->sbc)));
> +    } PA_ONCE_END;
> +
> +    /* write it to the fifo */
> +    memset(output_buffer, 0, sizeof(*header) + sizeof(*payload));
> +    header->v = 2;
> +
> +    /* A2DP spec: "A payload type in the RTP dynamic range shall be chosen".
> +     * RFC3551 defines the dynamic range to span from 96 to 127, and 96 appears
> +     * to be the most common choice in A2DP implementations. */
> +    header->pt = 96;
> +
> +    header->sequence_number = htons(sbc_info->seq_num++);
> +    header->timestamp = htonl(timestamp);
> +    header->ssrc = htonl(1);
> +    payload->frame_count = frame_count;
> +
> +    *processed = p - input_buffer;
> +    return d - output_buffer;
> +}
> +
> +static size_t decode_buffer(void *codec_info, 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;
> +    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));
> +
> +    p = input_buffer + sizeof(*header) + sizeof(*payload);
> +    to_decode = input_size - sizeof(*header) - sizeof(*payload);
> +
> +    d = output_buffer;
> +    to_write = output_size;
> +
> +    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);
> +            *processed = p - input_buffer;
> +            return 0;
> +        }
> +
> +        /* 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);
> +
> +        p += decoded;
> +        to_decode -= decoded;
> +
> +        d += written;
> +        to_write -= written;
> +    }
> +
> +    *processed = p - input_buffer;
> +    return d - output_buffer;
> +}
> +
> +const pa_a2dp_codec pa_a2dp_codec_sbc = {
> +    .codec_name = "sbc",
> +    .codec_description = "SBC",
> +    .codec_id = { A2DP_CODEC_SBC, 0, 0 },
> +    .support_backchannel = false,
> +    .accept_capabilities = accept_capabilities,
> +    .choose_capabilities = choose_capabilities,
> +    .fill_capabilities = fill_capabilities,
> +    .validate_configuration = validate_configuration,
> +    .fill_preferred_configuration = fill_preferred_configuration,
> +    .init_codec = init_codec,
> +    .finish_codec = finish_codec,
> +    .reset_codec = reset_codec,
> +    .get_read_block_size = get_block_size,
> +    .get_write_block_size = get_block_size,
> +    .reduce_encoder_bitrate = reduce_encoder_bitrate,
> +    .encode_buffer = encode_buffer,
> +    .decode_buffer = decode_buffer,
> +};
> diff --git a/src/modules/bluetooth/a2dp-codec-util.c b/src/modules/bluetooth/a2dp-codec-util.c
> new file mode 100644
> index 000000000..27128d8ae
> --- /dev/null
> +++ b/src/modules/bluetooth/a2dp-codec-util.c
> @@ -0,0 +1,56 @@
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2019 Pali Rohár <pali.rohar at gmail.com>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>;.
> +***/
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <pulsecore/core.h>
> +#include <pulsecore/core-util.h>
> +
> +#include "a2dp-codec-util.h"
> +
> +extern const pa_a2dp_codec pa_a2dp_codec_sbc;
> +
> +/* This is list of supported codecs. Their order is important.
> + * Codec with higher index has higher priority. */
> +const pa_a2dp_codec *pa_a2dp_codecs[] = {
> +    &pa_a2dp_codec_sbc,
> +};
> +
> +unsigned int pa_bluetooth_a2dp_codec_count(void) {
> +    return PA_ELEMENTSOF(pa_a2dp_codecs);
> +}
> +
> +const pa_a2dp_codec *pa_bluetooth_a2dp_codec_iter(unsigned int i) {
> +    pa_assert(i < pa_bluetooth_a2dp_codec_count());
> +    return pa_a2dp_codecs[i];
> +}
> +
> +const pa_a2dp_codec *pa_bluetooth_a2dp_codec(const char *name) {

Some bikeshedding: I'd prefer pa_bluetooth_get_a2dp_codec as the name.

> +    unsigned int i;
> +    unsigned int count = pa_bluetooth_a2dp_codec_count();
> +
> +    for (i = 0; i < count; i++) {
> +        if (pa_streq(pa_a2dp_codecs[i]->codec_name, name))
> +            return pa_a2dp_codecs[i];
> +    }
> +
> +    return NULL;
> +}
> diff --git a/src/modules/bluetooth/a2dp-codec-util.h b/src/modules/bluetooth/a2dp-codec-util.h
> new file mode 100644
> index 000000000..84a9d55f5
> --- /dev/null
> +++ b/src/modules/bluetooth/a2dp-codec-util.h
> @@ -0,0 +1,34 @@
> +#ifndef fooa2dpcodecutilhfoo
> +#define fooa2dpcodecutilhfoo
> +
> +/***
> +  This file is part of PulseAudio.
> +
> +  Copyright 2019 Pali Rohár <pali.rohar at gmail.com>
> +
> +  PulseAudio is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU Lesser General Public License as
> +  published by the Free Software Foundation; either version 2.1 of the
> +  License, or (at your option) any later version.
> +
> +  PulseAudio is distributed in the hope that it will be useful, but
> +  WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +  General Public License for more details.
> +
> +  You should have received a copy of the GNU Lesser General Public
> +  License along with PulseAudio; if not, see <http://www.gnu.org/licenses/>;.
> +***/
> +
> +#include "a2dp-codec-api.h"
> +
> +/* Get number of supported A2DP codecs */
> +unsigned int pa_bluetooth_a2dp_codec_count(void);
> +
> +/* Get i-th codec. Codec with higher number has higher priority */
> +const pa_a2dp_codec *pa_bluetooth_a2dp_codec_iter(unsigned int i);
> +
> +/* Get codec by name */
> +const pa_a2dp_codec *pa_bluetooth_a2dp_codec(const char *name);
> +
> +#endif
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 485a57515..e4450cfe3 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -2,6 +2,7 @@
>    This file is part of PulseAudio.
>  
>    Copyright 2008-2013 João Paulo Rechi Vita
> +  Copyrigth 2018-2019 Pali Rohár <pali.rohar at gmail.com>
>  
>    PulseAudio is free software; you can redistribute it and/or modify
>    it under the terms of the GNU Lesser General Public License as
> @@ -33,6 +34,7 @@
>  #include <pulsecore/refcnt.h>
>  #include <pulsecore/shared.h>
>  
> +#include "a2dp-codec-util.h"
>  #include "a2dp-codecs.h"
>  
>  #include "bluez5-util.h"
> @@ -885,13 +887,19 @@ finish:
>      pa_xfree(endpoint);
>  }
>  
> -static void register_endpoint(pa_bluetooth_discovery *y, const char *path, const char *endpoint, const char *uuid) {
> +static void register_endpoint(pa_bluetooth_discovery *y, const pa_a2dp_codec *a2dp_codec, const char *path, const char *endpoint, const char *uuid) {
>      DBusMessage *m;
>      DBusMessageIter i, d;
> -    uint8_t codec = 0;
> +    uint8_t capabilities[254];
> +    size_t capabilities_size;
> +    uint8_t codec_id;
>  
>      pa_log_debug("Registering %s on adapter %s", endpoint, path);
>  
> +    codec_id = a2dp_codec->codec_id.codec_id;
> +    capabilities_size = a2dp_codec->fill_capabilities(capabilities);
> +    pa_assert(capabilities_size != 0);
> +
>      pa_assert_se(m = dbus_message_new_method_call(BLUEZ_SERVICE, path, BLUEZ_MEDIA_INTERFACE, "RegisterEndpoint"));
>  
>      dbus_message_iter_init_append(m, &i);
> @@ -899,23 +907,8 @@ static void register_endpoint(pa_bluetooth_discovery *y, const char *path, const
>      dbus_message_iter_open_container(&i, DBUS_TYPE_ARRAY, DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING DBUS_TYPE_STRING_AS_STRING
>                                           DBUS_TYPE_VARIANT_AS_STRING DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &d);
>      pa_dbus_append_basic_variant_dict_entry(&d, "UUID", DBUS_TYPE_STRING, &uuid);
> -    pa_dbus_append_basic_variant_dict_entry(&d, "Codec", DBUS_TYPE_BYTE, &codec);
> -
> -    if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE) || pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK)) {
> -        a2dp_sbc_t capabilities;
> -
> -        capabilities.channel_mode = SBC_CHANNEL_MODE_MONO | SBC_CHANNEL_MODE_DUAL_CHANNEL | SBC_CHANNEL_MODE_STEREO |
> -                                    SBC_CHANNEL_MODE_JOINT_STEREO;
> -        capabilities.frequency = SBC_SAMPLING_FREQ_16000 | SBC_SAMPLING_FREQ_32000 | SBC_SAMPLING_FREQ_44100 |
> -                                 SBC_SAMPLING_FREQ_48000;
> -        capabilities.allocation_method = SBC_ALLOCATION_SNR | SBC_ALLOCATION_LOUDNESS;
> -        capabilities.subbands = SBC_SUBBANDS_4 | SBC_SUBBANDS_8;
> -        capabilities.block_length = SBC_BLOCK_LENGTH_4 | SBC_BLOCK_LENGTH_8 | SBC_BLOCK_LENGTH_12 | SBC_BLOCK_LENGTH_16;
> -        capabilities.min_bitpool = SBC_MIN_BITPOOL;
> -        capabilities.max_bitpool = 64;
> -
> -        pa_dbus_append_basic_array_variant_dict_entry(&d, "Capabilities", DBUS_TYPE_BYTE, &capabilities, sizeof(capabilities));
> -    }
> +    pa_dbus_append_basic_variant_dict_entry(&d, "Codec", DBUS_TYPE_BYTE, &codec_id);
> +    pa_dbus_append_basic_array_variant_dict_entry(&d, "Capabilities", DBUS_TYPE_BYTE, &capabilities, capabilities_size);
>  
>      dbus_message_iter_close_container(&i, &d);
>  
> @@ -950,6 +943,7 @@ static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
>  
>          if (pa_streq(interface, BLUEZ_ADAPTER_INTERFACE)) {
>              pa_bluetooth_adapter *a;
> +            unsigned a2dp_codec_i, a2dp_codec_count;

The a2dp_codec_count variable doesn't have much value, since it's used
only once and can be replaced with the pa_bluetooth_a2dp_codec_count()
call.

>  
>              if ((a = pa_hashmap_get(y->adapters, path))) {
>                  pa_log_error("Found duplicated D-Bus path for adapter %s", path);
> @@ -964,8 +958,21 @@ static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
>              if (!a->valid)
>                  return;
>  
> -            register_endpoint(y, path, A2DP_SOURCE_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> -            register_endpoint(y, path, A2DP_SINK_ENDPOINT, PA_BLUETOOTH_UUID_A2DP_SINK);
> +            /* Order is important. bluez prefers endpoints registered earlier.
> +             * And codec with higher number has higher priority. So iterate in reverse order. */
> +            a2dp_codec_count = pa_bluetooth_a2dp_codec_count();
> +            for (a2dp_codec_i = a2dp_codec_count; a2dp_codec_i > 0; a2dp_codec_i--) {
> +                const pa_a2dp_codec *a2dp_codec = pa_bluetooth_a2dp_codec_iter(a2dp_codec_i-1);
> +                char *endpoint;
> +
> +                endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->codec_name);
> +                register_endpoint(y, a2dp_codec, path, endpoint, PA_BLUETOOTH_UUID_A2DP_SINK);
> +                pa_xfree(endpoint);
> +
> +                endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->codec_name);
> +                register_endpoint(y, a2dp_codec, path, endpoint, PA_BLUETOOTH_UUID_A2DP_SOURCE);
> +                pa_xfree(endpoint);
> +            }
>  
>          } else if (pa_streq(interface, BLUEZ_DEVICE_INTERFACE)) {
>  
> @@ -1257,48 +1264,6 @@ fail:
>      return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>  }
>  
> -static uint8_t a2dp_default_bitpool(uint8_t freq, uint8_t mode) {
> -    /* These bitpool values were chosen based on the A2DP spec recommendation */
> -    switch (freq) {
> -        case SBC_SAMPLING_FREQ_16000:
> -        case SBC_SAMPLING_FREQ_32000:
> -            return 53;
> -
> -        case SBC_SAMPLING_FREQ_44100:
> -
> -            switch (mode) {
> -                case SBC_CHANNEL_MODE_MONO:
> -                case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> -                    return 31;
> -
> -                case SBC_CHANNEL_MODE_STEREO:
> -                case SBC_CHANNEL_MODE_JOINT_STEREO:
> -                    return 53;
> -            }
> -
> -            pa_log_warn("Invalid channel mode %u", mode);
> -            return 53;
> -
> -        case SBC_SAMPLING_FREQ_48000:
> -
> -            switch (mode) {
> -                case SBC_CHANNEL_MODE_MONO:
> -                case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> -                    return 29;
> -
> -                case SBC_CHANNEL_MODE_STEREO:
> -                case SBC_CHANNEL_MODE_JOINT_STEREO:
> -                    return 51;
> -            }
> -
> -            pa_log_warn("Invalid channel mode %u", mode);
> -            return 51;
> -    }
> -
> -    pa_log_warn("Invalid sampling freq %u", freq);
> -    return 53;
> -}
> -
>  const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
>      switch(profile) {
>          case PA_BLUETOOTH_PROFILE_A2DP_SINK:
> @@ -1316,10 +1281,24 @@ const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
>      return NULL;
>  }
>  
> +static const pa_a2dp_codec *a2dp_endpoint_to_a2dp_codec(const char *endpoint) {
> +    const char *codec_name;
> +
> +    if (pa_startswith(endpoint, A2DP_SINK_ENDPOINT "/"))
> +        codec_name = endpoint + strlen(A2DP_SINK_ENDPOINT "/");
> +    else if (pa_startswith(endpoint, A2DP_SOURCE_ENDPOINT "/"))
> +        codec_name = endpoint + strlen(A2DP_SOURCE_ENDPOINT "/");
> +    else
> +        return NULL;
> +
> +    return pa_bluetooth_a2dp_codec(codec_name);
> +}
> +
>  static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage *m, void *userdata) {
>      pa_bluetooth_discovery *y = userdata;
>      pa_bluetooth_device *d;
>      pa_bluetooth_transport *t;
> +    const pa_a2dp_codec *a2dp_codec = NULL;
>      const char *sender, *path, *endpoint_path, *dev_path = NULL, *uuid = NULL;
>      const uint8_t *config = NULL;
>      int size = 0;
> @@ -1345,6 +1324,8 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>      if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
>          goto fail;
>  
> +    endpoint_path = dbus_message_get_path(m);
> +
>      /* Read transport properties */
>      while (dbus_message_iter_get_arg_type(&props) == DBUS_TYPE_DICT_ENTRY) {
>          const char *key;
> @@ -1367,16 +1348,13 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>  
>              dbus_message_iter_get_basic(&value, &uuid);
>  
> -            endpoint_path = dbus_message_get_path(m);
> -            if (pa_streq(endpoint_path, A2DP_SOURCE_ENDPOINT)) {
> -                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE))
> -                    p = PA_BLUETOOTH_PROFILE_A2DP_SINK;
> -            } else if (pa_streq(endpoint_path, A2DP_SINK_ENDPOINT)) {
> -                if (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK))
> -                    p = PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
> -            }
> +            if (pa_startswith(endpoint_path, A2DP_SINK_ENDPOINT "/"))
> +                p = PA_BLUETOOTH_PROFILE_A2DP_SOURCE;
> +            else if (pa_startswith(endpoint_path, A2DP_SOURCE_ENDPOINT "/"))
> +                p = PA_BLUETOOTH_PROFILE_A2DP_SINK;
>  
> -            if (p == PA_BLUETOOTH_PROFILE_OFF) {
> +            if ((pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SOURCE) && p != PA_BLUETOOTH_PROFILE_A2DP_SINK) ||
> +                (pa_streq(uuid, PA_BLUETOOTH_UUID_A2DP_SINK) && p != PA_BLUETOOTH_PROFILE_A2DP_SOURCE)) {
>                  pa_log_error("UUID %s of transport %s incompatible with endpoint %s", uuid, path, endpoint_path);
>                  goto fail;
>              }
> @@ -1389,7 +1367,6 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>              dbus_message_iter_get_basic(&value, &dev_path);
>          } else if (pa_streq(key, "Configuration")) {
>              DBusMessageIter array;
> -            a2dp_sbc_t *c;
>  
>              if (var != DBUS_TYPE_ARRAY) {
>                  pa_log_error("Property %s of wrong type %c", key, (char)var);
> @@ -1404,45 +1381,20 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>              }
>  
>              dbus_message_iter_get_fixed_array(&array, &config, &size);
> -            if (size != sizeof(a2dp_sbc_t)) {
> -                pa_log_error("Configuration array of invalid size");
> -                goto fail;
> -            }
>  
> -            c = (a2dp_sbc_t *) config;
> +            a2dp_codec = a2dp_endpoint_to_a2dp_codec(endpoint_path);
> +            pa_assert(a2dp_codec);
>  
> -            if (c->frequency != SBC_SAMPLING_FREQ_16000 && c->frequency != SBC_SAMPLING_FREQ_32000 &&
> -                c->frequency != SBC_SAMPLING_FREQ_44100 && c->frequency != SBC_SAMPLING_FREQ_48000) {
> -                pa_log_error("Invalid sampling frequency in configuration");
> +            if (!a2dp_codec->validate_configuration(config, size))
>                  goto fail;
> -            }
> -
> -            if (c->channel_mode != SBC_CHANNEL_MODE_MONO && c->channel_mode != SBC_CHANNEL_MODE_DUAL_CHANNEL &&
> -                c->channel_mode != SBC_CHANNEL_MODE_STEREO && c->channel_mode != SBC_CHANNEL_MODE_JOINT_STEREO) {
> -                pa_log_error("Invalid channel mode in configuration");
> -                goto fail;
> -            }
> -
> -            if (c->allocation_method != SBC_ALLOCATION_SNR && c->allocation_method != SBC_ALLOCATION_LOUDNESS) {
> -                pa_log_error("Invalid allocation method in configuration");
> -                goto fail;
> -            }
> -
> -            if (c->subbands != SBC_SUBBANDS_4 && c->subbands != SBC_SUBBANDS_8) {
> -                pa_log_error("Invalid SBC subbands in configuration");
> -                goto fail;
> -            }
> -
> -            if (c->block_length != SBC_BLOCK_LENGTH_4 && c->block_length != SBC_BLOCK_LENGTH_8 &&
> -                c->block_length != SBC_BLOCK_LENGTH_12 && c->block_length != SBC_BLOCK_LENGTH_16) {
> -                pa_log_error("Invalid block length in configuration");
> -                goto fail;
> -            }
>          }
>  
>          dbus_message_iter_next(&props);
>      }
>  
> +    if (!a2dp_codec)
> +        goto fail2;
> +
>      if ((d = pa_hashmap_get(y->devices, dev_path))) {
>          if (!d->valid) {
>              pa_log_error("Information about device %s is invalid", dev_path);
> @@ -1466,6 +1418,7 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>      dbus_message_unref(r);
>  
>      t = pa_bluetooth_transport_new(d, sender, path, p, config, size);
> +    t->a2dp_codec = a2dp_codec;
>      t->acquire = bluez5_transport_acquire_cb;
>      t->release = bluez5_transport_release_cb;
>      pa_bluetooth_transport_put(t);
> @@ -1484,21 +1437,17 @@ fail2:
>  
>  static DBusMessage *endpoint_select_configuration(DBusConnection *conn, DBusMessage *m, void *userdata) {
>      pa_bluetooth_discovery *y = userdata;
> -    a2dp_sbc_t *cap, config;
> -    uint8_t *pconf = (uint8_t *) &config;
> -    int i, size;
> +    const char *endpoint_path;
> +    uint8_t *cap;
> +    int size;
> +    const pa_a2dp_codec *a2dp_codec;
> +    uint8_t config[254];
> +    uint8_t *config_ptr = config;
> +    size_t config_size;
>      DBusMessage *r;
>      DBusError err;
>  
> -    static const struct {
> -        uint32_t rate;
> -        uint8_t cap;
> -    } freq_table[] = {
> -        { 16000U, SBC_SAMPLING_FREQ_16000 },
> -        { 32000U, SBC_SAMPLING_FREQ_32000 },
> -        { 44100U, SBC_SAMPLING_FREQ_44100 },
> -        { 48000U, SBC_SAMPLING_FREQ_48000 }
> -    };
> +    endpoint_path = dbus_message_get_path(m);
>  
>      dbus_error_init(&err);
>  
> @@ -1508,101 +1457,15 @@ static DBusMessage *endpoint_select_configuration(DBusConnection *conn, DBusMess
>          goto fail;
>      }
>  
> -    if (size != sizeof(config)) {
> -        pa_log_error("Capabilities array has invalid size");
> -        goto fail;
> -    }
> -
> -    pa_zero(config);
> -
> -    /* Find the lowest freq that is at least as high as the requested sampling rate */
> -    for (i = 0; (unsigned) i < PA_ELEMENTSOF(freq_table); i++)
> -        if (freq_table[i].rate >= y->core->default_sample_spec.rate && (cap->frequency & freq_table[i].cap)) {
> -            config.frequency = freq_table[i].cap;
> -            break;
> -        }
> -
> -    if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
> -        for (--i; i >= 0; i--) {
> -            if (cap->frequency & freq_table[i].cap) {
> -                config.frequency = freq_table[i].cap;
> -                break;
> -            }
> -        }
> -
> -        if (i < 0) {
> -            pa_log_error("Not suitable sample rate");
> -            goto fail;
> -        }
> -    }
> -
> -    pa_assert((unsigned) i < PA_ELEMENTSOF(freq_table));
> -
> -    if (y->core->default_sample_spec.channels <= 1) {
> -        if (cap->channel_mode & SBC_CHANNEL_MODE_MONO)
> -            config.channel_mode = SBC_CHANNEL_MODE_MONO;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO)
> -            config.channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_STEREO)
> -            config.channel_mode = SBC_CHANNEL_MODE_STEREO;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL)
> -            config.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
> -        else {
> -            pa_log_error("No supported channel modes");
> -            goto fail;
> -        }
> -    }
> -
> -    if (y->core->default_sample_spec.channels >= 2) {
> -        if (cap->channel_mode & SBC_CHANNEL_MODE_JOINT_STEREO)
> -            config.channel_mode = SBC_CHANNEL_MODE_JOINT_STEREO;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_STEREO)
> -            config.channel_mode = SBC_CHANNEL_MODE_STEREO;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_DUAL_CHANNEL)
> -            config.channel_mode = SBC_CHANNEL_MODE_DUAL_CHANNEL;
> -        else if (cap->channel_mode & SBC_CHANNEL_MODE_MONO)
> -            config.channel_mode = SBC_CHANNEL_MODE_MONO;
> -        else {
> -            pa_log_error("No supported channel modes");
> -            goto fail;
> -        }
> -    }
> -
> -    if (cap->block_length & SBC_BLOCK_LENGTH_16)
> -        config.block_length = SBC_BLOCK_LENGTH_16;
> -    else if (cap->block_length & SBC_BLOCK_LENGTH_12)
> -        config.block_length = SBC_BLOCK_LENGTH_12;
> -    else if (cap->block_length & SBC_BLOCK_LENGTH_8)
> -        config.block_length = SBC_BLOCK_LENGTH_8;
> -    else if (cap->block_length & SBC_BLOCK_LENGTH_4)
> -        config.block_length = SBC_BLOCK_LENGTH_4;
> -    else {
> -        pa_log_error("No supported block lengths");
> -        goto fail;
> -    }
> -
> -    if (cap->subbands & SBC_SUBBANDS_8)
> -        config.subbands = SBC_SUBBANDS_8;
> -    else if (cap->subbands & SBC_SUBBANDS_4)
> -        config.subbands = SBC_SUBBANDS_4;
> -    else {
> -        pa_log_error("No supported subbands");
> -        goto fail;
> -    }
> +    a2dp_codec = a2dp_endpoint_to_a2dp_codec(endpoint_path);
> +    pa_assert(a2dp_codec);
>  
> -    if (cap->allocation_method & SBC_ALLOCATION_LOUDNESS)
> -        config.allocation_method = SBC_ALLOCATION_LOUDNESS;
> -    else if (cap->allocation_method & SBC_ALLOCATION_SNR)
> -        config.allocation_method = SBC_ALLOCATION_SNR;
> -
> -    config.min_bitpool = (uint8_t) PA_MAX(SBC_MIN_BITPOOL, cap->min_bitpool);
> -    config.max_bitpool = (uint8_t) PA_MIN(a2dp_default_bitpool(config.frequency, config.channel_mode), cap->max_bitpool);
> -
> -    if (config.min_bitpool > config.max_bitpool)
> +    config_size = a2dp_codec->fill_preferred_configuration(&y->core->default_sample_spec, cap, size, config);
> +    if (config_size == 0)
>          goto fail;
>  
>      pa_assert_se(r = dbus_message_new_method_return(m));
> -    pa_assert_se(dbus_message_append_args(r, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &pconf, size, DBUS_TYPE_INVALID));
> +    pa_assert_se(dbus_message_append_args(r, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &config_ptr, config_size, DBUS_TYPE_INVALID));
>  
>      return r;
>  
> @@ -1677,7 +1540,7 @@ static DBusHandlerResult endpoint_handler(DBusConnection *c, DBusMessage *m, voi
>  
>      pa_log_debug("dbus: path=%s, interface=%s, member=%s", path, interface, member);
>  
> -    if (!pa_streq(path, A2DP_SOURCE_ENDPOINT) && !pa_streq(path, A2DP_SINK_ENDPOINT))
> +    if (!a2dp_endpoint_to_a2dp_codec(path))
>          return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>  
>      if (dbus_message_is_method_call(m, "org.freedesktop.DBus.Introspectable", "Introspect")) {
> @@ -1705,49 +1568,32 @@ static DBusHandlerResult endpoint_handler(DBusConnection *c, DBusMessage *m, voi
>      return DBUS_HANDLER_RESULT_HANDLED;
>  }
>  
> -static void endpoint_init(pa_bluetooth_discovery *y, pa_bluetooth_profile_t profile) {
> +static void endpoint_init(pa_bluetooth_discovery *y, const char *endpoint) {
>      static const DBusObjectPathVTable vtable_endpoint = {
>          .message_function = endpoint_handler,
>      };
>  
>      pa_assert(y);
> +    pa_assert(endpoint);
>  
> -    switch(profile) {
> -        case PA_BLUETOOTH_PROFILE_A2DP_SINK:
> -            pa_assert_se(dbus_connection_register_object_path(pa_dbus_connection_get(y->connection), A2DP_SOURCE_ENDPOINT,
> -                                                              &vtable_endpoint, y));
> -            break;
> -        case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
> -            pa_assert_se(dbus_connection_register_object_path(pa_dbus_connection_get(y->connection), A2DP_SINK_ENDPOINT,
> -                                                              &vtable_endpoint, y));
> -            break;
> -        default:
> -            pa_assert_not_reached();
> -            break;
> -    }
> +    pa_assert_se(dbus_connection_register_object_path(pa_dbus_connection_get(y->connection), endpoint,
> +                                                      &vtable_endpoint, y));
>  }
>  
> -static void endpoint_done(pa_bluetooth_discovery *y, pa_bluetooth_profile_t profile) {
> +static void endpoint_done(pa_bluetooth_discovery *y, const char *endpoint) {
>      pa_assert(y);
> +    pa_assert(endpoint);
>  
> -    switch(profile) {
> -        case PA_BLUETOOTH_PROFILE_A2DP_SINK:
> -            dbus_connection_unregister_object_path(pa_dbus_connection_get(y->connection), A2DP_SOURCE_ENDPOINT);
> -            break;
> -        case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
> -            dbus_connection_unregister_object_path(pa_dbus_connection_get(y->connection), A2DP_SINK_ENDPOINT);
> -            break;
> -        default:
> -            pa_assert_not_reached();
> -            break;
> -    }
> +    dbus_connection_unregister_object_path(pa_dbus_connection_get(y->connection), endpoint);
>  }
>  
>  pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backend) {
>      pa_bluetooth_discovery *y;
>      DBusError err;
>      DBusConnection *conn;
> -    unsigned i;
> +    unsigned i, count;
> +    const pa_a2dp_codec *a2dp_codec;
> +    char *endpoint;
>  
>      y = pa_xnew0(pa_bluetooth_discovery, 1);
>      PA_REFCNT_INIT(y);
> @@ -1799,8 +1645,18 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe
>      }
>      y->matches_added = true;
>  
> -    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SINK);
> -    endpoint_init(y, PA_BLUETOOTH_PROFILE_A2DP_SOURCE);
> +    count = pa_bluetooth_a2dp_codec_count();
> +    for (i = 0; i < count; i++) {
> +        a2dp_codec = pa_bluetooth_a2dp_codec_iter(i);
> +
> +        endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->codec_name);
> +        endpoint_init(y, endpoint);
> +        pa_xfree(endpoint);
> +
> +        endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->codec_name);
> +        endpoint_init(y, endpoint);
> +        pa_xfree(endpoint);
> +    }
>  
>      get_managed_objects(y);
>  
> @@ -1823,6 +1679,10 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_ref(pa_bluetooth_discovery *y) {
>  }
>  
>  void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
> +    unsigned i, count;
> +    const pa_a2dp_codec *a2dp_codec;
> +    char *endpoint;
> +
>      pa_assert(y);
>      pa_assert(PA_REFCNT_VALUE(y) > 0);
>  
> @@ -1868,8 +1728,18 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) {
>          if (y->filter_added)
>              dbus_connection_remove_filter(pa_dbus_connection_get(y->connection), filter_cb, y);
>  
> -        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SINK);
> -        endpoint_done(y, PA_BLUETOOTH_PROFILE_A2DP_SOURCE);
> +        count = pa_bluetooth_a2dp_codec_count();
> +        for (i = 0; i < count; i++) {
> +            a2dp_codec = pa_bluetooth_a2dp_codec_iter(i);
> +
> +            endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->codec_name);
> +            endpoint_done(y, endpoint);
> +            pa_xfree(endpoint);
> +
> +            endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->codec_name);
> +            endpoint_done(y, endpoint);
> +            pa_xfree(endpoint);
> +        }
>  
>          pa_dbus_connection_unref(y->connection);
>      }
> diff --git a/src/modules/bluetooth/bluez5-util.h b/src/modules/bluetooth/bluez5-util.h
> index ad30708f0..82739bffd 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -5,6 +5,7 @@
>    This file is part of PulseAudio.
>  
>    Copyright 2008-2013 João Paulo Rechi Vita
> +  Copyrigth 2018-2019 Pali Rohár <pali.rohar at gmail.com>
>  
>    PulseAudio is free software; you can redistribute it and/or modify
>    it under the terms of the GNU Lesser General Public License as
> @@ -22,6 +23,8 @@
>  
>  #include <pulsecore/core.h>
>  
> +#include "a2dp-codec-util.h"
> +
>  #define PA_BLUETOOTH_UUID_A2DP_SOURCE "0000110a-0000-1000-8000-00805f9b34fb"
>  #define PA_BLUETOOTH_UUID_A2DP_SINK   "0000110b-0000-1000-8000-00805f9b34fb"
>  
> @@ -82,6 +85,8 @@ struct pa_bluetooth_transport {
>      uint8_t *config;
>      size_t config_size;
>  
> +    const pa_a2dp_codec *a2dp_codec;
> +
>      uint16_t microphone_gain;
>      uint16_t speaker_gain;
>  
> diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
> index 3e72ac3c1..531bc25a5 100644
> --- a/src/modules/bluetooth/module-bluez5-device.c
> +++ b/src/modules/bluetooth/module-bluez5-device.c
> @@ -3,6 +3,7 @@
>  
>    Copyright 2008-2013 João Paulo Rechi Vita
>    Copyright 2011-2013 BMW Car IT GmbH.
> +  Copyright 2018-2019 Pali Rohár <pali.rohar at gmail.com>
>  
>    PulseAudio is free software; you can redistribute it and/or modify
>    it under the terms of the GNU Lesser General Public License as
> @@ -25,7 +26,6 @@
>  #include <errno.h>
>  
>  #include <arpa/inet.h>
> -#include <sbc/sbc.h>
>  
>  #include <pulse/rtclock.h>
>  #include <pulse/timeval.h>
> @@ -47,8 +47,8 @@
>  #include <pulsecore/time-smoother.h>
>  
>  #include "a2dp-codecs.h"
> +#include "a2dp-codec-util.h"
>  #include "bluez5-util.h"
> -#include "rtp.h"
>  
>  PA_MODULE_AUTHOR("João Paulo Rechi Vita");
>  PA_MODULE_DESCRIPTION("BlueZ 5 Bluetooth audio sink and source");
> @@ -62,8 +62,6 @@ PA_MODULE_USAGE("path=<device object path>"
>  #define FIXED_LATENCY_RECORD_A2DP   (25 * PA_USEC_PER_MSEC)
>  #define FIXED_LATENCY_RECORD_SCO    (25 * PA_USEC_PER_MSEC)
>  
> -#define BITPOOL_DEC_LIMIT 32
> -#define BITPOOL_DEC_STEP 5
>  #define HSP_MAX_GAIN 15
>  
>  static const char* const valid_modargs[] = {
> @@ -94,18 +92,6 @@ typedef struct bluetooth_msg {
>  PA_DEFINE_PRIVATE_CLASS(bluetooth_msg, pa_msgobject);
>  #define BLUETOOTH_MSG(o) (bluetooth_msg_cast(o))
>  
> -typedef struct sbc_info {
> -    sbc_t sbc;                           /* Codec data */
> -    bool sbc_initialized;                /* Keep track if the encoder is initialized */
> -    size_t codesize, frame_length;       /* SBC Codesize, frame_length. We simply cache those values here */
> -    uint16_t seq_num;                    /* Cumulative packet sequence */
> -    uint8_t min_bitpool;
> -    uint8_t max_bitpool;
> -
> -    void* buffer;                        /* Codec transfer buffer */
> -    size_t buffer_size;                  /* Size of the buffer */
> -} sbc_info_t;
> -
>  struct userdata {
>      pa_module *module;
>      pa_core *core;
> @@ -145,8 +131,18 @@ struct userdata {
>      pa_usec_t started_at;
>      pa_smoother *read_smoother;
>      pa_memchunk write_memchunk;
> -    pa_sample_spec sample_spec;
> -    struct sbc_info sbc_info;
> +
> +    const pa_a2dp_codec *a2dp_codec;
> +
> +    void *encoder_info;
> +    pa_sample_spec encoder_sample_spec;
> +    void *encoder_buffer;                        /* Codec transfer buffer */
> +    size_t encoder_buffer_size;                  /* Size of the buffer */
> +
> +    void *decoder_info;
> +    pa_sample_spec decoder_sample_spec;
> +    void *decoder_buffer;                        /* Codec transfer buffer */
> +    size_t decoder_buffer_size;                  /* Size of the buffer */
>  };
>  
>  typedef enum pa_bluetooth_form_factor {
> @@ -380,7 +376,7 @@ static int sco_process_push(struct userdata *u) {
>       * issues in our Bluetooth adapter. In these cases, in order to avoid
>       * an assertion failure due to unaligned data, just discard the whole
>       * packet */
> -    if (!pa_frame_aligned(l, &u->sample_spec)) {
> +    if (!pa_frame_aligned(l, &u->decoder_sample_spec)) {
>          pa_log_warn("SCO packet received of unaligned size: %zu", l);
>          pa_memblock_unref(memchunk.memblock);
>          return -1;
> @@ -403,7 +399,7 @@ static int sco_process_push(struct userdata *u) {
>          tstamp = pa_rtclock_now();
>      }
>  
> -    pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
> +    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_source_post(u->source, &memchunk);
> @@ -413,115 +409,41 @@ static int sco_process_push(struct userdata *u) {
>  }
>  
>  /* Run from IO thread */
> -static void a2dp_prepare_buffer(struct userdata *u) {
> +static void a2dp_prepare_encoder_buffer(struct userdata *u) {
>      size_t min_buffer_size = PA_MAX(u->read_link_mtu, u->write_link_mtu);

Since you added separate "prepare buffer" functions for encoding and
decoding, it seems that the PA_MAX() usage is unnecessary, and you
should just use u->write_link_mtu here (and u->read_link_mtu in
prepare_decoder_buffer).

>  
>      pa_assert(u);
>  
> -    if (u->sbc_info.buffer_size >= min_buffer_size)
> +    if (u->encoder_buffer_size >= min_buffer_size)
>          return;
>  
> -    u->sbc_info.buffer_size = 2 * min_buffer_size;
> -    pa_xfree(u->sbc_info.buffer);
> -    u->sbc_info.buffer = pa_xmalloc(u->sbc_info.buffer_size);
> +    u->encoder_buffer_size = 2 * min_buffer_size;
> +    pa_xfree(u->encoder_buffer);
> +    u->encoder_buffer = pa_xmalloc(u->encoder_buffer_size);
>  }
>  
>  /* Run from IO thread */
> -static int a2dp_process_render(struct userdata *u) {
> -    struct sbc_info *sbc_info;
> -    struct rtp_header *header;
> -    struct rtp_payload *payload;
> -    size_t nbytes;
> -    void *d;
> -    const void *p;
> -    size_t to_write, to_encode;
> -    unsigned frame_count;
> -    int ret = 0;
> +static void a2dp_prepare_decoder_buffer(struct userdata *u) {
> +    size_t min_buffer_size = PA_MAX(u->read_link_mtu, u->write_link_mtu);
>  
>      pa_assert(u);
> -    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK);
> -    pa_assert(u->sink);
> -
> -    /* First, render some data */
> -    if (!u->write_memchunk.memblock)
> -        pa_sink_render_full(u->sink, u->write_block_size, &u->write_memchunk);
> -
> -    pa_assert(u->write_memchunk.length == u->write_block_size);
> -
> -    a2dp_prepare_buffer(u);
> -
> -    sbc_info = &u->sbc_info;
> -    header = sbc_info->buffer;
> -    payload = (struct rtp_payload*) ((uint8_t*) sbc_info->buffer + sizeof(*header));
> -
> -    frame_count = 0;
> -
> -    /* Try to create a packet of the full MTU */
> -
> -    p = (const uint8_t *) pa_memblock_acquire_chunk(&u->write_memchunk);
> -    to_encode = u->write_memchunk.length;
> -
> -    d = (uint8_t*) sbc_info->buffer + sizeof(*header) + sizeof(*payload);
> -    to_write = sbc_info->buffer_size - sizeof(*header) - sizeof(*payload);
> -
> -    while (PA_LIKELY(to_encode > 0 && to_write > 0)) {
> -        ssize_t written;
> -        ssize_t encoded;
>  
> -        encoded = sbc_encode(&sbc_info->sbc,
> -                             p, to_encode,
> -                             d, to_write,
> -                             &written);
> -
> -        if (PA_UNLIKELY(encoded <= 0)) {
> -            pa_log_error("SBC encoding error (%li)", (long) encoded);
> -            pa_memblock_release(u->write_memchunk.memblock);
> -            return -1;
> -        }
> -
> -        pa_assert_fp((size_t) encoded <= to_encode);
> -        pa_assert_fp((size_t) encoded == sbc_info->codesize);
> -
> -        pa_assert_fp((size_t) written <= to_write);
> -        pa_assert_fp((size_t) written == sbc_info->frame_length);
> -
> -        p = (const uint8_t*) p + encoded;
> -        to_encode -= encoded;
> -
> -        d = (uint8_t*) d + written;
> -        to_write -= written;
> -
> -        frame_count++;
> -    }
> -
> -    pa_memblock_release(u->write_memchunk.memblock);
> -
> -    pa_assert(to_encode == 0);
> -
> -    PA_ONCE_BEGIN {
> -        pa_log_debug("Using SBC encoder implementation: %s", pa_strnull(sbc_get_implementation_info(&sbc_info->sbc)));
> -    } PA_ONCE_END;
> -
> -    /* write it to the fifo */
> -    memset(sbc_info->buffer, 0, sizeof(*header) + sizeof(*payload));
> -    header->v = 2;
> -
> -    /* A2DP spec: "A payload type in the RTP dynamic range shall be chosen".
> -     * RFC3551 defines the dynamic range to span from 96 to 127, and 96 appears
> -     * to be the most common choice in A2DP implementations. */
> -    header->pt = 96;
> +    if (u->decoder_buffer_size >= min_buffer_size)
> +        return;
>  
> -    header->sequence_number = htons(sbc_info->seq_num++);
> -    header->timestamp = htonl(u->write_index / pa_frame_size(&u->sample_spec));
> -    header->ssrc = htonl(1);
> -    payload->frame_count = frame_count;
> +    u->decoder_buffer_size = 2 * min_buffer_size;
> +    pa_xfree(u->decoder_buffer);
> +    u->decoder_buffer = pa_xmalloc(u->decoder_buffer_size);
> +}
>  
> -    nbytes = (uint8_t*) d - (uint8_t*) sbc_info->buffer;
> +/* Run from IO thread */
> +static int a2dp_write_buffer(struct userdata *u, size_t nbytes) {
> +    int ret = 0;
>  
>      for (;;) {
>          ssize_t l;
>  
> -        l = pa_write(u->stream_fd, sbc_info->buffer, nbytes, &u->stream_write_type);
> +        l = pa_write(u->stream_fd, u->encoder_buffer, nbytes, &u->stream_write_type);
>  
>          pa_assert(l != 0);
>  
> @@ -565,6 +487,40 @@ static int a2dp_process_render(struct userdata *u) {
>  }
>  
>  /* Run from IO thread */
> +static int a2dp_process_render(struct userdata *u) {
> +    const uint8_t *ptr;
> +    size_t processed;
> +    size_t length;
> +
> +    pa_assert(u);
> +    pa_assert(u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK);
> +    pa_assert(u->sink);
> +    pa_assert(u->a2dp_codec);
> +
> +    /* First, render some data */
> +    if (!u->write_memchunk.memblock)
> +        pa_sink_render_full(u->sink, u->write_block_size, &u->write_memchunk);
> +
> +    pa_assert(u->write_memchunk.length == u->write_block_size);
> +
> +    a2dp_prepare_encoder_buffer(u);
> +
> +    /* Try to create a packet of the full MTU */
> +    ptr = (const uint8_t *) pa_memblock_acquire_chunk(&u->write_memchunk);
> +
> +    length = u->a2dp_codec->encode_buffer(u->encoder_info, u->write_index / pa_frame_size(&u->encoder_sample_spec), ptr, u->write_memchunk.length, u->encoder_buffer, u->encoder_buffer_size, &processed);
> +
> +    pa_memblock_release(u->write_memchunk.memblock);
> +
> +    if (length == 0)
> +        return -1;
> +
> +    pa_assert(processed == u->write_memchunk.length);
> +
> +    return a2dp_write_buffer(u, length);
> +}
> +
> +/* Run from IO thread */
>  static int a2dp_process_push(struct userdata *u) {
>      int ret = 0;
>      pa_memchunk memchunk;
> @@ -573,6 +529,7 @@ static int a2dp_process_push(struct userdata *u) {
>      pa_assert(u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE);
>      pa_assert(u->source);
>      pa_assert(u->read_smoother);
> +    pa_assert(u->a2dp_codec);
>  
>      memchunk.memblock = pa_memblock_new(u->core->mempool, u->read_block_size);
>      memchunk.index = memchunk.length = 0;
> @@ -580,22 +537,14 @@ static int a2dp_process_push(struct userdata *u) {
>      for (;;) {
>          bool found_tstamp = false;
>          pa_usec_t tstamp;
> -        struct sbc_info *sbc_info;
> -        struct rtp_header *header;
> -        struct rtp_payload *payload;
> -        const void *p;
> -        void *d;
> +        uint8_t *ptr;
>          ssize_t l;
> -        size_t to_write, to_decode;
> +        size_t processed;
>          size_t total_written = 0;
>  
> -        a2dp_prepare_buffer(u);
> +        a2dp_prepare_decoder_buffer(u);
>  
> -        sbc_info = &u->sbc_info;
> -        header = sbc_info->buffer;
> -        payload = (struct rtp_payload*) ((uint8_t*) sbc_info->buffer + sizeof(*header));
> -
> -        l = pa_read(u->stream_fd, sbc_info->buffer, sbc_info->buffer_size, &u->stream_write_type);
> +        l = pa_read(u->stream_fd, u->decoder_buffer, u->decoder_buffer_size, &u->stream_write_type);
>  
>          if (l <= 0) {
>  
> @@ -612,7 +561,7 @@ static int a2dp_process_push(struct userdata *u) {
>              break;
>          }
>  
> -        pa_assert((size_t) l <= sbc_info->buffer_size);
> +        pa_assert((size_t) l <= u->decoder_buffer_size);
>  
>          /* TODO: get timestamp from rtp */
>          if (!found_tstamp) {
> @@ -620,50 +569,18 @@ static int a2dp_process_push(struct userdata *u) {
>              tstamp = pa_rtclock_now();
>          }
>  
> -        p = (uint8_t*) sbc_info->buffer + sizeof(*header) + sizeof(*payload);
> -        to_decode = l - sizeof(*header) - sizeof(*payload);
> -
> -        d = pa_memblock_acquire(memchunk.memblock);
> -        to_write = memchunk.length = pa_memblock_get_length(memchunk.memblock);
> -
> -        while (PA_LIKELY(to_decode > 0)) {
> -            size_t written;
> -            ssize_t decoded;
> +        ptr = pa_memblock_acquire(memchunk.memblock);
> +        memchunk.length = pa_memblock_get_length(memchunk.memblock);
>  
> -            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);
> -                pa_memblock_release(memchunk.memblock);
> -                pa_memblock_unref(memchunk.memblock);
> -                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);
> -
> -            p = (const uint8_t*) p + decoded;
> -            to_decode -= decoded;
> -
> -            d = (uint8_t*) d + written;
> -            to_write -= written;
> -        }
> +        total_written = u->a2dp_codec->decode_buffer(u->decoder_info, u->decoder_buffer, l, ptr, memchunk.length, &processed);
> +        if (total_written == 0)
> +            return 0;
>  
>          u->read_index += (uint64_t) total_written;
> -        pa_smoother_put(u->read_smoother, tstamp, pa_bytes_to_usec(u->read_index, &u->sample_spec));
> +        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);
>  
> -        memchunk.length -= to_write;
> +        memchunk.length -= l - processed;

This doesn't make sense to me, but maybe I'm missing something, since
you've probably tested that this works fine... memchunk.length should
be the length of uncompressed audio that was decoded, but
memchunk.length is initially set to read_block_size, i.e. the maximum
possible amount of compressed data that can be read in one go, and then
"l - processed" is subtracted, and "l - processed" is the amount of
compressed audio that was read but for some reason not processed. So
memchunk.length is set to the maximum amount of compressed data minus
received but unprocessed compressed data. That doesn't seem to have
anything to do with the length of uncompressed audio that was decoded.
Why is memchunk.length not simply set to the return value of
decode_buffer()?

>  
>          pa_memblock_release(memchunk.memblock);
>  
> @@ -678,7 +595,7 @@ static int a2dp_process_push(struct userdata *u) {
>      return ret;
>  }
>  
> -static void update_buffer_size(struct userdata *u) {
> +static void update_sink_buffer_size(struct userdata *u) {
>      int old_bufsize;
>      socklen_t len = sizeof(int);
>      int ret;
> @@ -710,72 +627,6 @@ static void update_buffer_size(struct userdata *u) {
>      }
>  }
>  
> -/* Run from I/O thread */
> -static void a2dp_set_bitpool(struct userdata *u, uint8_t bitpool) {
> -    struct sbc_info *sbc_info;
> -
> -    pa_assert(u);
> -
> -    sbc_info = &u->sbc_info;
> -
> -    if (sbc_info->sbc.bitpool == bitpool)
> -        return;
> -
> -    if (bitpool > sbc_info->max_bitpool)
> -        bitpool = sbc_info->max_bitpool;
> -    else if (bitpool < sbc_info->min_bitpool)
> -        bitpool = sbc_info->min_bitpool;
> -
> -    sbc_info->sbc.bitpool = bitpool;
> -
> -    sbc_info->codesize = sbc_get_codesize(&sbc_info->sbc);
> -    sbc_info->frame_length = sbc_get_frame_length(&sbc_info->sbc);
> -
> -    pa_log_debug("Bitpool has changed to %u", sbc_info->sbc.bitpool);
> -
> -    u->read_block_size =
> -        (u->read_link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
> -        / sbc_info->frame_length * sbc_info->codesize;
> -
> -    u->write_block_size =
> -        (u->write_link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
> -        / sbc_info->frame_length * sbc_info->codesize;
> -
> -    pa_sink_set_max_request_within_thread(u->sink, u->write_block_size);
> -    pa_sink_set_fixed_latency_within_thread(u->sink,
> -            FIXED_LATENCY_PLAYBACK_A2DP + pa_bytes_to_usec(u->write_block_size, &u->sample_spec));
> -
> -    /* If there is still data in the memchunk, we have to discard it
> -     * because the write_block_size may have changed. */
> -    if (u->write_memchunk.memblock) {
> -        pa_memblock_unref(u->write_memchunk.memblock);
> -        pa_memchunk_reset(&u->write_memchunk);
> -    }
> -
> -    update_buffer_size(u);
> -}
> -
> -/* Run from I/O thread */
> -static void a2dp_reduce_bitpool(struct userdata *u) {
> -    struct sbc_info *sbc_info;
> -    uint8_t bitpool;
> -
> -    pa_assert(u);
> -
> -    sbc_info = &u->sbc_info;
> -
> -    /* Check if bitpool is already at its limit */
> -    if (sbc_info->sbc.bitpool <= BITPOOL_DEC_LIMIT)
> -        return;
> -
> -    bitpool = sbc_info->sbc.bitpool - BITPOOL_DEC_STEP;
> -
> -    if (bitpool < BITPOOL_DEC_LIMIT)
> -        bitpool = BITPOOL_DEC_LIMIT;
> -
> -    a2dp_set_bitpool(u, bitpool);
> -}
> -
>  static void teardown_stream(struct userdata *u) {
>      if (u->rtpoll_item) {
>          pa_rtpoll_item_free(u->rtpoll_item);
> @@ -850,6 +701,24 @@ static void transport_release(struct userdata *u) {
>  }
>  
>  /* Run from I/O thread */
> +static void handle_sink_block_size_change(struct userdata *u) {
> +    pa_sink_set_max_request_within_thread(u->sink, u->write_block_size);
> +    pa_sink_set_fixed_latency_within_thread(u->sink,
> +                                            (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK ?
> +                                             FIXED_LATENCY_PLAYBACK_A2DP : FIXED_LATENCY_PLAYBACK_SCO) +
> +                                            pa_bytes_to_usec(u->write_block_size, &u->encoder_sample_spec));
> +
> +    /* If there is still data in the memchunk, we have to discard it
> +     * because the write_block_size may have changed. */
> +    if (u->write_memchunk.memblock) {
> +        pa_memblock_unref(u->write_memchunk.memblock);
> +        pa_memchunk_reset(&u->write_memchunk);
> +    }
> +
> +    update_sink_buffer_size(u);
> +}
> +
> +/* Run from I/O thread */
>  static void transport_config_mtu(struct userdata *u) {
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
>          u->read_block_size = u->read_link_mtu;
> @@ -865,28 +734,22 @@ static void transport_config_mtu(struct userdata *u) {
>              u->write_block_size = pa_frame_align(u->write_block_size, &u->sink->sample_spec);
>          }
>      } else {
> -        u->read_block_size =
> -            (u->read_link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
> -            / u->sbc_info.frame_length * u->sbc_info.codesize;
> -
> -        u->write_block_size =
> -            (u->write_link_mtu - sizeof(struct rtp_header) - sizeof(struct rtp_payload))
> -            / u->sbc_info.frame_length * u->sbc_info.codesize;
> +        pa_assert(u->a2dp_codec);
> +        if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
> +            u->write_block_size = u->a2dp_codec->get_write_block_size(u->encoder_info, u->write_link_mtu);
> +        } else {
> +            u->read_block_size = u->a2dp_codec->get_read_block_size(u->encoder_info, u->read_link_mtu);

Should be decoder_info instead of encoder_info.

> +        }
>      }
>  
> -    if (u->sink) {
> -        pa_sink_set_max_request_within_thread(u->sink, u->write_block_size);
> -        pa_sink_set_fixed_latency_within_thread(u->sink,
> -                                                (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK ?
> -                                                 FIXED_LATENCY_PLAYBACK_A2DP : FIXED_LATENCY_PLAYBACK_SCO) +
> -                                                pa_bytes_to_usec(u->write_block_size, &u->sample_spec));
> -    }
> +    if (u->sink)
> +        handle_sink_block_size_change(u);
>  
>      if (u->source)
>          pa_source_set_fixed_latency_within_thread(u->source,
>                                                    (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE ?
>                                                     FIXED_LATENCY_RECORD_A2DP : FIXED_LATENCY_RECORD_SCO) +
> -                                                  pa_bytes_to_usec(u->read_block_size, &u->sample_spec));
> +                                                  pa_bytes_to_usec(u->read_block_size, &u->decoder_sample_spec));
>  }
>  
>  /* Run from I/O thread */
> @@ -900,6 +763,14 @@ static void setup_stream(struct userdata *u) {
>  
>      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_codec(u->encoder_info);
> +    } else if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) {
> +        pa_assert(u->a2dp_codec);
> +        u->a2dp_codec->reset_codec(u->decoder_info);
> +    }
> +
>      transport_config_mtu(u);
>  
>      pa_make_fd_nonblock(u->stream_fd);
> @@ -911,11 +782,6 @@ static void setup_stream(struct userdata *u) {
>  
>      pa_log_debug("Stream properly set up, we're ready to roll!");
>  
> -    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
> -        a2dp_set_bitpool(u, u->sbc_info.max_bitpool);

It looks like resetting the bitpool isn't done anywhere. Should it be
done in the reset_codec() callback?

> -        update_buffer_size(u);
> -    }
> -
>      u->rtpoll_item = pa_rtpoll_item_new(u->rtpoll, PA_RTPOLL_NEVER, 1);
>      pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
>      pollfd->fd = u->stream_fd;
> @@ -957,7 +823,7 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
>  
>              if (u->read_smoother) {
>                  wi = pa_smoother_get(u->read_smoother, pa_rtclock_now());
> -                ri = pa_bytes_to_usec(u->read_index, &u->sample_spec);
> +                ri = pa_bytes_to_usec(u->read_index, &u->decoder_sample_spec);
>  
>                  *((int64_t*) data) = u->source->thread_info.fixed_latency + wi - ri;
>              } else
> @@ -1050,11 +916,11 @@ static void source_set_volume_cb(pa_source *s) {
>      if (volume < PA_VOLUME_NORM)
>          volume++;
>  
> -    pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume);
> +    pa_cvolume_set(&s->real_volume, u->decoder_sample_spec.channels, volume);
>  
>      /* Set soft volume when in headset role */
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
> -        pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, volume);
> +        pa_cvolume_set(&s->soft_volume, u->decoder_sample_spec.channels, volume);
>  
>      /* If we are in the AG role, we send a command to the head set to change
>       * the microphone gain. In the HS role, source and sink are swapped, so
> @@ -1075,7 +941,7 @@ static int add_source(struct userdata *u) {
>      data.name = pa_sprintf_malloc("bluez_source.%s.%s", u->device->address, pa_bluetooth_profile_to_string(u->profile));
>      data.namereg_fail = false;
>      pa_proplist_sets(data.proplist, "bluetooth.protocol", pa_bluetooth_profile_to_string(u->profile));
> -    pa_source_new_data_set_sample_spec(&data, &u->sample_spec);
> +    pa_source_new_data_set_sample_spec(&data, &u->decoder_sample_spec);
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
>          pa_proplist_sets(data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
>  
> @@ -1133,10 +999,10 @@ static int sink_process_msg(pa_msgobject *o, int code, void *data, int64_t offse
>  
>              if (u->read_smoother) {
>                  ri = pa_smoother_get(u->read_smoother, pa_rtclock_now());
> -                wi = pa_bytes_to_usec(u->write_index + u->write_block_size, &u->sample_spec);
> +                wi = pa_bytes_to_usec(u->write_index + u->write_block_size, &u->encoder_sample_spec);
>              } else if (u->started_at) {
>                  ri = pa_rtclock_now() - u->started_at;
> -                wi = pa_bytes_to_usec(u->write_index, &u->sample_spec);
> +                wi = pa_bytes_to_usec(u->write_index, &u->encoder_sample_spec);
>              }
>  
>              *((int64_t*) data) = u->sink->thread_info.fixed_latency + wi - ri;
> @@ -1224,11 +1090,11 @@ static void sink_set_volume_cb(pa_sink *s) {
>      if (volume < PA_VOLUME_NORM)
>          volume++;
>  
> -    pa_cvolume_set(&s->real_volume, u->sample_spec.channels, volume);
> +    pa_cvolume_set(&s->real_volume, u->encoder_sample_spec.channels, volume);
>  
>      /* Set soft volume when in headset role */
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY)
> -        pa_cvolume_set(&s->soft_volume, u->sample_spec.channels, volume);
> +        pa_cvolume_set(&s->soft_volume, u->encoder_sample_spec.channels, volume);
>  
>      /* If we are in the AG role, we send a command to the head set to change
>       * the speaker gain. In the HS role, source and sink are swapped, so
> @@ -1249,7 +1115,7 @@ static int add_sink(struct userdata *u) {
>      data.name = pa_sprintf_malloc("bluez_sink.%s.%s", u->device->address, pa_bluetooth_profile_to_string(u->profile));
>      data.namereg_fail = false;
>      pa_proplist_sets(data.proplist, "bluetooth.protocol", pa_bluetooth_profile_to_string(u->profile));
> -    pa_sink_new_data_set_sample_spec(&data, &u->sample_spec);
> +    pa_sink_new_data_set_sample_spec(&data, &u->encoder_sample_spec);
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
>          pa_proplist_sets(data.proplist, PA_PROP_DEVICE_INTENDED_ROLES, "phone");
>  
> @@ -1295,117 +1161,38 @@ static int add_sink(struct userdata *u) {
>  }
>  
>  /* Run from main thread */
> -static void transport_config(struct userdata *u) {
> +static int transport_config(struct userdata *u) {
>      if (u->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT || u->profile == PA_BLUETOOTH_PROFILE_HEADSET_AUDIO_GATEWAY) {
> -        u->sample_spec.format = PA_SAMPLE_S16LE;
> -        u->sample_spec.channels = 1;
> -        u->sample_spec.rate = 8000;
> +        u->encoder_sample_spec.format = PA_SAMPLE_S16LE;
> +        u->encoder_sample_spec.channels = 1;
> +        u->encoder_sample_spec.rate = 8000;
> +        u->decoder_sample_spec.format = PA_SAMPLE_S16LE;
> +        u->decoder_sample_spec.channels = 1;
> +        u->decoder_sample_spec.rate = 8000;
> +        return 0;
>      } else {
> -        sbc_info_t *sbc_info = &u->sbc_info;
> -        a2dp_sbc_t *config;
> +        bool is_a2dp_sink = u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK;
> +        void *info;
>  
>          pa_assert(u->transport);
>  
> -        u->sample_spec.format = PA_SAMPLE_S16LE;
> -        config = (a2dp_sbc_t *) u->transport->config;
> -
> -        if (sbc_info->sbc_initialized)
> -            sbc_reinit(&sbc_info->sbc, 0);
> -        else
> -            sbc_init(&sbc_info->sbc, 0);
> -        sbc_info->sbc_initialized = true;
> -
> -        switch (config->frequency) {
> -            case SBC_SAMPLING_FREQ_16000:
> -                sbc_info->sbc.frequency = SBC_FREQ_16000;
> -                u->sample_spec.rate = 16000U;
> -                break;
> -            case SBC_SAMPLING_FREQ_32000:
> -                sbc_info->sbc.frequency = SBC_FREQ_32000;
> -                u->sample_spec.rate = 32000U;
> -                break;
> -            case SBC_SAMPLING_FREQ_44100:
> -                sbc_info->sbc.frequency = SBC_FREQ_44100;
> -                u->sample_spec.rate = 44100U;
> -                break;
> -            case SBC_SAMPLING_FREQ_48000:
> -                sbc_info->sbc.frequency = SBC_FREQ_48000;
> -                u->sample_spec.rate = 48000U;
> -                break;
> -            default:
> -                pa_assert_not_reached();
> -        }
> -
> -        switch (config->channel_mode) {
> -            case SBC_CHANNEL_MODE_MONO:
> -                sbc_info->sbc.mode = SBC_MODE_MONO;
> -                u->sample_spec.channels = 1;
> -                break;
> -            case SBC_CHANNEL_MODE_DUAL_CHANNEL:
> -                sbc_info->sbc.mode = SBC_MODE_DUAL_CHANNEL;
> -                u->sample_spec.channels = 2;
> -                break;
> -            case SBC_CHANNEL_MODE_STEREO:
> -                sbc_info->sbc.mode = SBC_MODE_STEREO;
> -                u->sample_spec.channels = 2;
> -                break;
> -            case SBC_CHANNEL_MODE_JOINT_STEREO:
> -                sbc_info->sbc.mode = SBC_MODE_JOINT_STEREO;
> -                u->sample_spec.channels = 2;
> -                break;
> -            default:
> -                pa_assert_not_reached();
> -        }
> +        pa_assert(!u->a2dp_codec);
> +        pa_assert(!u->encoder_info);
> +        pa_assert(!u->decoder_info);
>  
> -        switch (config->allocation_method) {
> -            case SBC_ALLOCATION_SNR:
> -                sbc_info->sbc.allocation = SBC_AM_SNR;
> -                break;
> -            case SBC_ALLOCATION_LOUDNESS:
> -                sbc_info->sbc.allocation = SBC_AM_LOUDNESS;
> -                break;
> -            default:
> -                pa_assert_not_reached();
> -        }
> +        u->a2dp_codec = u->transport->a2dp_codec;
> +        pa_assert(u->a2dp_codec);
>  
> -        switch (config->subbands) {
> -            case SBC_SUBBANDS_4:
> -                sbc_info->sbc.subbands = SBC_SB_4;
> -                break;
> -            case SBC_SUBBANDS_8:
> -                sbc_info->sbc.subbands = SBC_SB_8;
> -                break;
> -            default:
> -                pa_assert_not_reached();
> -        }
> -
> -        switch (config->block_length) {
> -            case SBC_BLOCK_LENGTH_4:
> -                sbc_info->sbc.blocks = SBC_BLK_4;
> -                break;
> -            case SBC_BLOCK_LENGTH_8:
> -                sbc_info->sbc.blocks = SBC_BLK_8;
> -                break;
> -            case SBC_BLOCK_LENGTH_12:
> -                sbc_info->sbc.blocks = SBC_BLK_12;
> -                break;
> -            case SBC_BLOCK_LENGTH_16:
> -                sbc_info->sbc.blocks = SBC_BLK_16;
> -                break;
> -            default:
> -                pa_assert_not_reached();
> -        }
> -
> -        sbc_info->min_bitpool = config->min_bitpool;
> -        sbc_info->max_bitpool = config->max_bitpool;
> +        info = u->a2dp_codec->init_codec(is_a2dp_sink, false, u->transport->config, u->transport->config_size, is_a2dp_sink ? &u->encoder_sample_spec : &u->decoder_sample_spec);
> +        if (is_a2dp_sink)
> +            u->encoder_info = info;
> +        else
> +            u->decoder_info = info;
>  
> -        /* Set minimum bitpool for source to get the maximum possible block_size */
> -        sbc_info->sbc.bitpool = u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK ? sbc_info->max_bitpool : sbc_info->min_bitpool;
> -        sbc_info->codesize = sbc_get_codesize(&sbc_info->sbc);
> -        sbc_info->frame_length = sbc_get_frame_length(&sbc_info->sbc);
> +        if (!info)
> +            return -1;
>  
> -        pa_log_info("SBC parameters: allocation=%u, subbands=%u, blocks=%u, bitpool=%u",
> -                    sbc_info->sbc.allocation, sbc_info->sbc.subbands ? 8 : 4, sbc_info->sbc.blocks, sbc_info->sbc.bitpool);
> +        return 0;
>      }
>  }
>  
> @@ -1436,9 +1223,7 @@ static int setup_transport(struct userdata *u) {
>              return -1; /* We need to fail here until the interactions with module-suspend-on-idle and alike get improved */
>      }
>  
> -    transport_config(u);
> -
> -    return 0;
> +    return transport_config(u);
>  }
>  
>  /* Run from main thread */
> @@ -1618,12 +1403,12 @@ static void thread_func(void *userdata) {
>  
>                      if (u->started_at) {
>                          time_passed = pa_rtclock_now() - u->started_at;
> -                        audio_sent = pa_bytes_to_usec(u->write_index, &u->sample_spec);
> +                        audio_sent = pa_bytes_to_usec(u->write_index, &u->encoder_sample_spec);
>                      }
>  
>                      /* A new block needs to be sent. */
>                      if (audio_sent <= time_passed) {
> -                        size_t bytes_to_send = pa_usec_to_bytes(time_passed - audio_sent, &u->sample_spec);
> +                        size_t bytes_to_send = pa_usec_to_bytes(time_passed - audio_sent, &u->encoder_sample_spec);
>  
>                          /* There are more than two blocks that need to be written. It seems that
>                           * the socket has not been accepting data fast enough (could be due to
> @@ -1636,7 +1421,7 @@ static void thread_func(void *userdata) {
>                              pa_usec_t skip_usec;
>  
>                              skip_bytes = bytes_to_send - 2 * u->write_block_size;
> -                            skip_usec = pa_bytes_to_usec(skip_bytes, &u->sample_spec);
> +                            skip_usec = pa_bytes_to_usec(skip_bytes, &u->encoder_sample_spec);
>  
>                              pa_log_debug("Skipping %llu us (= %llu bytes) in audio stream",
>                                          (unsigned long long) skip_usec,
> @@ -1656,8 +1441,13 @@ static void thread_func(void *userdata) {
>                                  skip_bytes -= bytes_to_render;
>                              }
>  
> -                            if (u->write_index > 0 && u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK)
> -                                a2dp_reduce_bitpool(u);
> +                            if (u->write_index > 0 && u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK) {
> +                                size_t new_write_block_size = u->a2dp_codec->reduce_encoder_bitrate(u->encoder_info, u->write_link_mtu);
> +                                if (new_write_block_size) {
> +                                    u->write_block_size = new_write_block_size;
> +                                    handle_sink_block_size_change(u);
> +                                }
> +                            }
>                          }
>  
>                          blocks_to_write = 1;
> @@ -1686,7 +1476,7 @@ static void thread_func(void *userdata) {
>                          if (writable) {
>                              /* There was no write pending on this iteration of the loop.
>                               * Let's estimate when we need to wake up next */
> -                            next_write_at = pa_bytes_to_usec(u->write_index, &u->sample_spec);
> +                            next_write_at = pa_bytes_to_usec(u->write_index, &u->encoder_sample_spec);
>                              sleep_for = time_passed < next_write_at ? next_write_at - time_passed : 0;
>                              /* pa_log("Sleeping for %lu; time passed %lu, next write at %lu", (unsigned long) sleep_for, (unsigned long) time_passed, (unsigned long)next_write_at); */
>                          } else
> @@ -1830,6 +1620,20 @@ static void stop_thread(struct userdata *u) {
>          pa_smoother_free(u->read_smoother);
>          u->read_smoother = NULL;
>      }
> +
> +    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) {
> +        if (u->encoder_info) {
> +            u->a2dp_codec->finish_codec(u->encoder_info);
> +            u->encoder_info = NULL;
> +        }
> +
> +        if (u->decoder_info) {
> +            u->a2dp_codec->finish_codec(u->decoder_info);
> +            u->decoder_info = NULL;
> +        }
> +
> +        u->a2dp_codec = NULL;
> +    }
>  }
>  
>  /* Run from main thread */
> @@ -2309,7 +2113,7 @@ static pa_hook_result_t transport_speaker_gain_changed_cb(pa_bluetooth_discovery
>      if (volume < PA_VOLUME_NORM)
>          volume++;
>  
> -    pa_cvolume_set(&v, u->sample_spec.channels, volume);
> +    pa_cvolume_set(&v, u->encoder_sample_spec.channels, volume);
>      if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
>          pa_sink_volume_changed(u->sink, &v);
>      else
> @@ -2336,7 +2140,7 @@ static pa_hook_result_t transport_microphone_gain_changed_cb(pa_bluetooth_discov
>      if (volume < PA_VOLUME_NORM)
>          volume++;
>  
> -    pa_cvolume_set(&v, u->sample_spec.channels, volume);
> +    pa_cvolume_set(&v, u->decoder_sample_spec.channels, volume);
>  
>      if (t->profile == PA_BLUETOOTH_PROFILE_HEADSET_HEAD_UNIT)
>          pa_source_volume_changed(u->source, &v);
> @@ -2497,11 +2301,18 @@ void pa__done(pa_module *m) {
>      if (u->transport_microphone_gain_changed_slot)
>          pa_hook_slot_free(u->transport_microphone_gain_changed_slot);
>  
> -    if (u->sbc_info.buffer)
> -        pa_xfree(u->sbc_info.buffer);
> +    if (u->encoder_buffer)
> +        pa_xfree(u->encoder_buffer);
>  
> -    if (u->sbc_info.sbc_initialized)
> -        sbc_finish(&u->sbc_info.sbc);
> +    if (u->decoder_buffer)
> +        pa_xfree(u->decoder_buffer);
> +
> +    if (u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK || u->profile == PA_BLUETOOTH_PROFILE_A2DP_SOURCE) {
> +        if (u->encoder_info)
> +            u->a2dp_codec->finish_codec(u->encoder_info);
> +        if (u->decoder_info)
> +            u->a2dp_codec->finish_codec(u->decoder_info);

stop_thread() ensures that encoder_info and decoder_info are NULL, so
this code is unnecessary. You can add assertions if you like.

> +    }
>  
>      if (u->msg)
>          pa_xfree(u->msg);
-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk



More information about the pulseaudio-discuss mailing list