From 519052e77e65e27f77b59aa79b599bff06ce0b88 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 19 Jan 2021 23:20:07 +0100 Subject: [PATCH] bluetooth: Check support for encoding and decoding separately As suggested in [1]: This way it is possible for a codec to have both the encoding and decoding part optional, instead of getting both or nothing (where PA theoretically supports both). In addition this cleans up code that was previously checking the existence of a function pointer, or nothing at all (switch_codec). [1]: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/440#note_768146 Part-of: --- src/modules/bluetooth/a2dp-codec-api.h | 2 +- src/modules/bluetooth/a2dp-codec-aptx-gst.c | 32 +++++++++++--------- src/modules/bluetooth/a2dp-codec-ldac-gst.c | 5 ++- src/modules/bluetooth/a2dp-codec-sbc.c | 2 +- src/modules/bluetooth/bluez5-util.c | 26 ++++++---------- src/modules/bluetooth/module-bluez5-device.c | 8 ++--- 6 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/modules/bluetooth/a2dp-codec-api.h b/src/modules/bluetooth/a2dp-codec-api.h index b7396c05d..744e5664a 100644 --- a/src/modules/bluetooth/a2dp-codec-api.h +++ b/src/modules/bluetooth/a2dp-codec-api.h @@ -49,7 +49,7 @@ typedef struct pa_a2dp_codec { bool support_backchannel; /* Returns true if the codec can be supported on the system */ - bool (*can_be_supported)(void); + bool (*can_be_supported)(bool for_encoding); /* Returns true if codec accepts capabilities, for_encoding is true when * capabilities are used for encoding */ diff --git a/src/modules/bluetooth/a2dp-codec-aptx-gst.c b/src/modules/bluetooth/a2dp-codec-aptx-gst.c index d04880326..f949115fa 100644 --- a/src/modules/bluetooth/a2dp-codec-aptx-gst.c +++ b/src/modules/bluetooth/a2dp-codec-aptx-gst.c @@ -33,25 +33,27 @@ #include "a2dp-codec-gst.h" #include "rtp.h" -static bool can_be_supported(void) { +static bool can_be_supported(bool for_encoding) { GstElementFactory *element_factory; - element_factory = gst_element_factory_find("openaptxenc"); - if (element_factory == NULL) { - pa_log_info("aptX encoder not found"); - return false; + if (for_encoding) { + element_factory = gst_element_factory_find("openaptxenc"); + if (element_factory == NULL) { + pa_log_info("aptX encoder not found"); + return false; + } + + gst_object_unref(element_factory); + } else { + element_factory = gst_element_factory_find("openaptxdec"); + if (element_factory == NULL) { + pa_log_info("aptX decoder not found"); + return false; + } + + gst_object_unref(element_factory); } - gst_object_unref(element_factory); - - element_factory = gst_element_factory_find("openaptxdec"); - if (element_factory == NULL) { - pa_log_info("aptX decoder not found"); - return false; - } - - gst_object_unref(element_factory); - return true; } diff --git a/src/modules/bluetooth/a2dp-codec-ldac-gst.c b/src/modules/bluetooth/a2dp-codec-ldac-gst.c index 70f38d208..313ff5bf2 100644 --- a/src/modules/bluetooth/a2dp-codec-ldac-gst.c +++ b/src/modules/bluetooth/a2dp-codec-ldac-gst.c @@ -33,9 +33,12 @@ #include "a2dp-codec-gst.h" #include "rtp.h" -static bool can_be_supported(void) { +static bool can_be_supported(bool for_encoding) { GstElementFactory *element_factory; + if (!for_encoding) + return false; + element_factory = gst_element_factory_find("ldacenc"); if (element_factory == NULL) { pa_log_info("LDAC encoder not found"); diff --git a/src/modules/bluetooth/a2dp-codec-sbc.c b/src/modules/bluetooth/a2dp-codec-sbc.c index e0114e792..fc3027b37 100644 --- a/src/modules/bluetooth/a2dp-codec-sbc.c +++ b/src/modules/bluetooth/a2dp-codec-sbc.c @@ -53,7 +53,7 @@ struct sbc_info { uint8_t max_bitpool; }; -static bool can_be_supported(void) { +static bool can_be_supported(bool for_encoding) { return true; } diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c index c5694aef7..e49145ab7 100644 --- a/src/modules/bluetooth/bluez5-util.c +++ b/src/modules/bluetooth/bluez5-util.c @@ -2108,21 +2108,21 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage * char *endpoint; a2dp_codec = pa_bluetooth_a2dp_codec_iter(i); - if (!a2dp_codec->can_be_supported()) - continue; codec_id = a2dp_codec->id.codec_id; - capabilities_size = a2dp_codec->fill_capabilities(capabilities); - pa_assert(capabilities_size != 0); - if (a2dp_codec->decode_buffer != NULL) { + if (a2dp_codec->can_be_supported(false)) { + capabilities_size = a2dp_codec->fill_capabilities(capabilities); + pa_assert(capabilities_size != 0); endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->name); append_a2dp_object(&array, endpoint, PA_BLUETOOTH_UUID_A2DP_SINK, codec_id, capabilities, capabilities_size); pa_xfree(endpoint); } - if (a2dp_codec->encode_buffer != NULL) { + if (a2dp_codec->can_be_supported(true)) { + capabilities_size = a2dp_codec->fill_capabilities(capabilities); + pa_assert(capabilities_size != 0); endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->name); append_a2dp_object(&array, endpoint, PA_BLUETOOTH_UUID_A2DP_SOURCE, codec_id, capabilities, capabilities_size); @@ -2222,16 +2222,13 @@ pa_bluetooth_discovery* pa_bluetooth_discovery_get(pa_core *c, int headset_backe count = pa_bluetooth_a2dp_codec_count(); for (i = 0; i < count; i++) { a2dp_codec = pa_bluetooth_a2dp_codec_iter(i); - if (!a2dp_codec->can_be_supported()) - continue; - - if (a2dp_codec->decode_buffer != NULL) { + if (a2dp_codec->can_be_supported(false)) { endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->name); endpoint_init(y, endpoint); pa_xfree(endpoint); } - if (a2dp_codec->encode_buffer != NULL) { + if (a2dp_codec->can_be_supported(true)) { endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->name); endpoint_init(y, endpoint); pa_xfree(endpoint); @@ -2316,16 +2313,13 @@ void pa_bluetooth_discovery_unref(pa_bluetooth_discovery *y) { for (i = 0; i < count; i++) { a2dp_codec = pa_bluetooth_a2dp_codec_iter(i); - if (!a2dp_codec->can_be_supported()) - continue; - - if (a2dp_codec->decode_buffer != NULL) { + if (a2dp_codec->can_be_supported(false)) { endpoint = pa_sprintf_malloc("%s/%s", A2DP_SINK_ENDPOINT, a2dp_codec->name); endpoint_done(y, endpoint); pa_xfree(endpoint); } - if (a2dp_codec->encode_buffer != NULL) { + if (a2dp_codec->can_be_supported(true)) { endpoint = pa_sprintf_malloc("%s/%s", A2DP_SOURCE_ENDPOINT, a2dp_codec->name); endpoint_done(y, endpoint); pa_xfree(endpoint); diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index b2113228f..54471324a 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -2307,7 +2307,7 @@ static char *list_codecs(struct userdata *u) { a2dp_codec = pa_bluetooth_a2dp_codec_iter(i); if (memcmp(key, &a2dp_codec->id, sizeof(pa_a2dp_codec_id)) == 0) { - if (a2dp_codec->can_be_supported()) { + if (a2dp_codec->can_be_supported(is_a2dp_sink)) { pa_message_params_begin_list(param); pa_message_params_write_string(param, a2dp_codec->name); @@ -2383,13 +2383,13 @@ static int bluez5_device_message_handler(const char *object_path, const char *me return -PA_ERR_INVALID; } - if (!codec->can_be_supported()) { + is_a2dp_sink = u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK; + + if (!u->a2dp_codec->can_be_supported(is_a2dp_sink)) { pa_log_info("Codec not found on system"); return -PA_ERR_NOTSUPPORTED; } - is_a2dp_sink = u->profile == PA_BLUETOOTH_PROFILE_A2DP_SINK; - /* * We need to check if we have valid sink or source endpoints which * were registered during the negotiation process. If we do, then we