On 07.12.19 15:47, Pali Rohár wrote:
On Saturday 07 December 2019 15:37:15 Georg Chini wrote:
On 06.10.19 19:58, Pali Rohár wrote:
This patch provides support for FastStream codec in bluetooth A2DP profile.
FastStream codec is bi-directional, which means that it supports both music
playback and microphone voice at the same time.
FastStream codec is just SBC codec with fixed parameters. For playback are
used following parameters: 48.0kHz or 44.1kHz, Blocks 16, Sub-bands 8,
Joint Stereo, Loudness, Bitpool = 29 (data rate = 212kbps, packet size =
(71+1)*3 <= DM5 = 220, with 3 SBC frames). SBC frame size is 71 bytes, but
FastStream is zero-padded to the even size (72). For microphone are used
following SBC parameters: 16kHz, Mono, Blocks 16, Sub-bands 8, Loudness,
Bitpool = 32 (data rate = 72kbps, packet size = 72*3 <= DM5 = 220, with
3 SBC frames).
So FastStream codec is equivalent to SBC in Low Quality settings. But the
main benefit of FastStream codec is support for microphone voice channel
for audio calls. Compared to bluetooth HSP profile (with CVSD codec), it
provides better audio quality for both playback and recording.
---
src/Makefile.am | 2 +
src/modules/bluetooth/a2dp-codec-faststream.c | 554 ++++++++++++++++++++++++++
src/modules/bluetooth/a2dp-codec-util.c | 4 +
src/modules/bluetooth/meson.build | 1 +
4 files changed, 561 insertions(+)
create mode 100644 src/modules/bluetooth/a2dp-codec-faststream.c
...
+static bool is_configuration_valid_common(const a2dp_faststream_t *config, uint8_t config_size) {
+ uint8_t sink_frequency;
+
+ if (config_size != sizeof(*config)) {
+ pa_log_error("Invalid size of config buffer");
+ return false;
+ }
+
+ if (A2DP_GET_VENDOR_ID(config->info) != FASTSTREAM_VENDOR_ID || A2DP_GET_CODEC_ID(config->info) != FASTSTREAM_CODEC_ID) {
+ pa_log_error("Invalid vendor codec information in configuration");
+ return false;
+ }
+
+ if (!(config->direction & FASTSTREAM_DIRECTION_SINK)) {
+ pa_log_error("Invalid direction in configuration");
+ return false;
+ }
+
+ sink_frequency = config->sink_frequency;
+
+ /* Some headsets are buggy and set both 48 kHz and 44.1 kHz in
+ * the config. In such situation trying to send audio at 44.1 kHz
+ * results in choppy audio, so we have to assume that the headset
+ * actually wants 48 kHz audio. */
+ if (sink_frequency == (FASTSTREAM_SINK_SAMPLING_FREQ_44100 | FASTSTREAM_SINK_SAMPLING_FREQ_48000))
+ sink_frequency = FASTSTREAM_SINK_SAMPLING_FREQ_48000;
+
+ if (sink_frequency != FASTSTREAM_SINK_SAMPLING_FREQ_44100 && sink_frequency != FASTSTREAM_SINK_SAMPLING_FREQ_48000) {
+ pa_log_error("Invalid sink sampling frequency in configuration");
+ return false;
+ }
+
+ return true;
+}
Why not simply
if (config->sink_frequency & (FASTSTREAM_SINK_SAMPLING_FREQ_44100 | FASTSTREAM_SINK_SAMPLING_FREQ_48000))
return true;
return false;
This change suggested Tanu in email Message-ID: <4622e48b2c79eaab2486cdc43a5fdf430859345e.camel@xxxxxx>
Can't find the e-mail with that reference.
+
+static bool is_configuration_valid(const uint8_t *config_buffer, uint8_t config_size) {
+ const a2dp_faststream_t *config = (const a2dp_faststream_t *) config_buffer;
+
+ if (!is_configuration_valid_common(config, config_size))
+ return false;
+
+ if (config->direction & FASTSTREAM_DIRECTION_SOURCE) {
+ pa_log_error("Invalid direction in configuration");
+ return false;
+ }
+
+ return true;
+}
+
+static bool is_configuration_valid_mic(const uint8_t *config_buffer, uint8_t config_size) {
+ const a2dp_faststream_t *config = (const a2dp_faststream_t *) config_buffer;
+
+ if (!is_configuration_valid_common(config, config_size))
+ return false;
+
+ if (!(config->direction & FASTSTREAM_DIRECTION_SOURCE)) {
+ pa_log_error("Invalid direction in configuration");
+ return false;
+ }
+
+ if (config->source_frequency != FASTSTREAM_SOURCE_SAMPLING_FREQ_16000) {
+ pa_log_error("Invalid source sampling frequency in configuration");
+ return false;
+ }
+
+ return true;
+}
+
+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[MAX_A2DP_CAPS_SIZE]) {
+ a2dp_faststream_t *config = (a2dp_faststream_t *) config_buffer;
+ const a2dp_faststream_t *capabilities = (const a2dp_faststream_t *) capabilities_buffer;
+ int i;
+
+ static const struct {
+ uint32_t rate;
+ uint8_t cap;
+ } freq_table[] = {
+ { 44100U, FASTSTREAM_SINK_SAMPLING_FREQ_44100 },
+ { 48000U, FASTSTREAM_SINK_SAMPLING_FREQ_48000 }
+ };
+
+ if (capabilities_size != sizeof(*capabilities)) {
+ pa_log_error("Invalid size of capabilities buffer");
+ return 0;
+ }
+
+ pa_zero(*config);
+
+ if (A2DP_GET_VENDOR_ID(capabilities->info) != FASTSTREAM_VENDOR_ID || A2DP_GET_CODEC_ID(capabilities->info) != FASTSTREAM_CODEC_ID) {
+ pa_log_error("No supported vendor codec information");
+ return 0;
+ }
+
+ config->info = A2DP_SET_VENDOR_ID_CODEC_ID(FASTSTREAM_VENDOR_ID, FASTSTREAM_CODEC_ID);
+
+ /* 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->sink_frequency & freq_table[i].cap)) {
+ config->sink_frequency = freq_table[i].cap;
+ break;
+ }
+
+ if ((unsigned) i == PA_ELEMENTSOF(freq_table)) {
This "if" does not seem necessary, we know that the table has some elements.
It is necessary. It is fallback when first for loop (above) did not
choose suitable sample rate. "i" is set to elements count when previous
for loop did not break.
Yes, sorry. Misread the code.
+ for (--i; i >= 0; i--) {
+ if (capabilities->sink_frequency & freq_table[i].cap) {
+ config->sink_frequency = freq_table[i].cap;
+ break;
+ }
+ }
+
+ if (i < 0) {
+ pa_log_error("Not suitable sample rate");
+ return 0;
As said in the previous patch, I think an assertion would be OK here.
This error can happen. E.g. when remote side advertise frequencies
different then 44.1kHz and 48kHz. We do not want to crash pulseaudio
(with assertion) when some bluetooth headset either send garbage or send
list of frequencies which pulseaudio does not support.
Can that really happen? Do you not check that the capabilities
are valid before filling the configuration?
+ }
+ }
+
+ pa_assert((unsigned) i < PA_ELEMENTSOF(freq_table));
+
+ if (!(capabilities->direction & FASTSTREAM_DIRECTION_SINK)) {
+ pa_log_error("No sink support");
+ return 0;
+ }
+
+ config->direction = FASTSTREAM_DIRECTION_SINK;
+
+ return sizeof(*config);
+}
+
...
+
+static void *init(bool for_encoding, bool for_backchannel, const uint8_t *config_buffer, uint8_t config_size, pa_sample_spec *sample_spec) {
+ struct faststream_info *faststream_info;
+ const a2dp_faststream_t *config = (const a2dp_faststream_t *) config_buffer;
+ int ret;
+
+ pa_assert(config_size == sizeof(*config));
+
+ faststream_info = pa_xnew0(struct faststream_info, 1);
+ faststream_info->is_microphone = for_backchannel;
+
+ ret = sbc_init(&faststream_info->sbc, 0);
+ if (ret != 0) {
+ pa_xfree(faststream_info);
+ pa_log_error("SBC initialization failed: %d", ret);
+ return NULL;
+ }
+
+ sample_spec->format = PA_SAMPLE_S16LE;
+
+ if (faststream_info->is_microphone) {
+ if (config->source_frequency == FASTSTREAM_SOURCE_SAMPLING_FREQ_16000) {
+ faststream_info->frequency = SBC_FREQ_16000;
+ sample_spec->rate = 16000U;
+ } else {
+ pa_assert_not_reached();
+ }
+
+ sample_spec->channels = 1;
+ } else {
+ uint8_t sink_frequency = config->sink_frequency;
+
+ /* Some headsets are buggy and set both 48 kHz and 44.1 kHz in
+ * the config. In such situation trying to send audio at 44.1 kHz
+ * results in choppy audio, so we have to assume that the headset
+ * actually wants 48 kHz audio. */
+ if (sink_frequency == (FASTSTREAM_SINK_SAMPLING_FREQ_44100 | FASTSTREAM_SINK_SAMPLING_FREQ_48000))
+ sink_frequency = FASTSTREAM_SINK_SAMPLING_FREQ_48000;
+
+ if (sink_frequency == FASTSTREAM_SINK_SAMPLING_FREQ_48000) {
Why not simply
if (config->sink_frequency & FASTSTREAM_SINK_SAMPLING_FREQ_48000) {
This would take care of the case where both bits are set, because 48k is
preferred.
See Message-ID: <4622e48b2c79eaab2486cdc43a5fdf430859345e.camel@xxxxxx>
As said, can't find it.
+ faststream_info->frequency = SBC_FREQ_48000;
+ sample_spec->rate = 48000U;
+ } else if (config->sink_frequency == FASTSTREAM_SINK_SAMPLING_FREQ_44100) {
and
} else if (config->sink_frequency & FASTSTREAM_SINK_SAMPLING_FREQ_44100) {
+ faststream_info->frequency = SBC_FREQ_44100;
+ sample_spec->rate = 44100U;
+ } else {
+ pa_assert_not_reached();
+ }
+
+ sample_spec->channels = 2;
+ }
+
+ set_params(faststream_info);
+
+ pa_log_info("SBC parameters: allocation=%s, subbands=%u, blocks=%u, mode=%s bitpool=%u codesize=%u frame_length=%u",
+ faststream_info->sbc.allocation ? "SNR" : "Loudness", faststream_info->sbc.subbands ? 8 : 4,
+ (faststream_info->sbc.blocks+1)*4, faststream_info->sbc.mode == SBC_MODE_MONO ? "Mono" :
+ faststream_info->sbc.mode == SBC_MODE_DUAL_CHANNEL ? "DualChannel" :
+ faststream_info->sbc.mode == SBC_MODE_STEREO ? "Stereo" : "JointStereo",
+ faststream_info->sbc.bitpool, (unsigned)faststream_info->codesize, (unsigned)faststream_info->frame_length);
+
+ return faststream_info;
+}
+
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
