On Wed, 2013-09-18 at 16:17 -0500, jprvita at gmail.com wrote:
> From: Jo?o Paulo Rechi Vita <jprvita at openbossa.org>
>
> ---
> src/modules/bluetooth/bluez5-util.c | 176 +++++++++++++++++++++++++++++++++++-
> src/modules/bluetooth/bluez5-util.h | 5 +
> 2 files changed, 179 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 983f707..b0d3864 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -33,6 +33,8 @@
> #include <pulsecore/refcnt.h>
> #include <pulsecore/shared.h>
>
> +#include "a2dp-codecs.h"
> +
> #include "bluez5-util.h"
>
> #define BLUEZ_SERVICE "org.bluez"
> @@ -406,12 +408,182 @@ fail:
> return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> }
>
> +const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
> + switch(profile) {
> + case PA_BLUETOOTH_PROFILE_A2DP_SINK:
> + return "a2dp_sink";
> + case PA_BLUETOOTH_PROFILE_A2DP_SOURCE:
> + return "a2dp_source";
> + case PA_BLUETOOTH_PROFILE_OFF:
> + return "off";
> + }
> +
> + return NULL;
> +}
> +
> 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 char *sender, *path, *endpoint_path, *dev_path = NULL, *uuid = NULL;
> + const uint8_t *config = NULL;
> + int size = 0;
> + pa_bluetooth_profile_t p = PA_BLUETOOTH_PROFILE_OFF;
> + DBusMessageIter args, props;
> DBusMessage *r;
>
> - pa_assert_se(r = dbus_message_new_error(m, BLUEZ_MEDIA_ENDPOINT_INTERFACE ".Error.NotImplemented",
> - "Method not implemented"));
> + if (!dbus_message_iter_init(m, &args) || !pa_streq(dbus_message_get_signature(m), "oa{sv}")) {
> + pa_log_error("Invalid signature for method SetConfiguration()");
> + goto fail2;
> + }
> +
> + dbus_message_iter_get_basic(&args, &path);
> +
> + if (pa_hashmap_get(y->transports, path)) {
> + pa_log_error("Endpoint SetConfiguration(): Transport %s is already configured.", path);
> + goto fail2;
> + }
> +
> + pa_assert_se(dbus_message_iter_next(&args));
> +
> + dbus_message_iter_recurse(&args, &props);
> + if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> + goto fail;
> +
> + /* Read transport properties */
> + while (dbus_message_iter_get_arg_type(&props) == DBUS_TYPE_DICT_ENTRY) {
> + const char *key;
> + DBusMessageIter value, entry;
> + int var;
> +
> + dbus_message_iter_recurse(&props, &entry);
> + dbus_message_iter_get_basic(&entry, &key);
> +
> + dbus_message_iter_next(&entry);
> + dbus_message_iter_recurse(&entry, &value);
> +
> + var = dbus_message_iter_get_arg_type(&value);
> +
> + if (pa_streq(key, "UUID")) {
> + if (var != DBUS_TYPE_STRING) {
> + pa_log_error("Property %s of wrong type %c", key, (char)var);
> + goto fail;
> + }
> +
> + 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 (p == PA_BLUETOOTH_PROFILE_OFF) {
> + pa_log_error("UUID %s of transport %s incompatible with endpoint %s", uuid, path, endpoint_path);
> + goto fail;
> + }
> + } else if (pa_streq(key, "Device")) {
> + if (var != DBUS_TYPE_OBJECT_PATH) {
> + pa_log_error("Property %s of wrong type %c", key, (char)var);
> + goto fail;
> + }
> +
> + 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);
> + goto fail;
> + }
> +
> + dbus_message_iter_recurse(&value, &array);
> + var = dbus_message_iter_get_arg_type(&array);
> + if (var != DBUS_TYPE_BYTE) {
> + pa_log_error("%s is an array of wrong type %c", key, (char)var);
> + goto fail;
> + }
> +
> + dbus_message_iter_get_fixed_array(&array, &config, &size);
> + if (size != sizeof(a2dp_sbc_t)) {
> + pa_log_error("Properties array of invalid size");
"Properties array" is not very accurate choice of words. "Configuration
array" or "Configuration blob" would be better.
> + goto fail;
> + }
> +
> + c = (a2dp_sbc_t *) config;
> +
> + 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");
> + 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;
> + }
My earlier criticism is still valid: "It would better to do the config
validation later, after checking that the UUID matches the endpoint
type, because if the UUID doesn't match, then the config is probably not
what you expect it to be either. Checking the config too early may
result in less clear error messages (it's better to complain about UUID
mismatch than errors in the config)."
You moved the UUID check to happen earlier, but there's still no
guarantee that the UUID check happens before the configuration
validation, because the "Configuration" property may be located at an
earlier position in the D-Bus message than the "UUID" property.
--
Tanu
