[pulseaudio-discuss] [PATCH next v0 01/10] bluetooth: Abstract microphone gain in transport

[Tanu Kaskinen]

On Wed, 2012-12-12 at 13:17 +0100, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> 
> The microphone gain represents the volume of the incoming audio stream
> from the headset. This can be nicely abstracted inside the transport
> object in bluetooth-util, so the modules don't have to take care about
> the D-Bus details.
> ---
>  src/modules/bluetooth/bluetooth-util.c          | 60 +++++++++++++++++++++++++
>  src/modules/bluetooth/bluetooth-util.h          |  6 +++
>  src/modules/bluetooth/module-bluetooth-device.c |  1 -
>  3 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index f3e9b53..a319db3 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -528,6 +528,27 @@ static int parse_audio_property(pa_bluetooth_device *d, const char *interface, D
>  
>              break;
>          }
> +
> +        case DBUS_TYPE_UINT16: {
> +            uint16_t value;
> +
> +            dbus_message_iter_get_basic(&variant_i, &value);
> +
> +            if (pa_streq(key, "MicrophoneGain")) {
> +                pa_log_debug("dbus: property 'State' changed to value '%u'", value);

Copy-paste error.

> +
> +                if (!transport) {
> +                    pa_log("Volume change does not have an associated transport");
> +                    return -1;
> +                }
> +
> +                transport->microphone_gain = PA_MIN(value, HSP_MAX_GAIN);
> +
> +                pa_hook_fire(&d->discovery->hooks[PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED], transport);

I'd like to send change notifications only when there's an actual
change.

> +            }
> +
> +            break;
> +        }
>      }
>  
>      return 0;
> @@ -1086,6 +1107,45 @@ void pa_bluetooth_transport_release(pa_bluetooth_transport *t, const char *acces
>          pa_log_info("Transport %s released", t->path);
>  }
>  
> +static void pa_bt_set_property(pa_bluetooth_discovery *y, const char *path, const char *interface, const char *prop_name,
> +                               int prop_type, void *prop_value) {

Don't use unnecessary prefixes with private functions. "set_property" is
sufficient.

> +    DBusMessage *m;
> +    DBusMessageIter i;
> +    DBusMessageIter variant;
> +    const char *variant_signature;
> +
> +    pa_assert(y);
> +    pa_assert(path);
> +    pa_assert(interface);
> +    pa_assert(prop_name);
> +
> +    pa_assert_se(m = dbus_message_new_method_call("org.bluez", path, interface, "SetProperty"));

Messages to transports may need different target than org.bluez.

> +    dbus_message_iter_init_append(m, &i);
> +    dbus_message_iter_append_basic(&i, DBUS_TYPE_STRING, &prop_name);
> +
> +    switch (prop_type) {
> +        case DBUS_TYPE_UINT16:
> +            variant_signature = DBUS_TYPE_UINT16_AS_STRING;
> +            break;
> +        default:
> +            pa_assert_not_reached(); /* Not implemented */
> +    }
> +
> +    dbus_message_iter_open_container(&i, DBUS_TYPE_VARIANT, variant_signature, &variant);
> +    dbus_message_iter_append_basic(&variant, prop_type, prop_value);
> +    dbus_message_iter_close_container(&i, &variant);

pa_dbus_append_basic_variant() is available.

> +
> +    pa_assert_se(dbus_connection_send(pa_dbus_connection_get(y->connection), m, NULL));
> +    dbus_message_unref(m);

Since we're not handling the reply, I think it would be good to call
dbus_message_set_no_reply(true).

-- 
Tanu