On Wed, 2012-12-19 at 13:58 +0100, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
>
> The new D-Bus API doesn't support access rights, which weren't used by
> PulseAudio anyway, but it does solve a race condition: now optional
> acquires can be implemented by bluetooth-util atomically using the D-Bus
> TryAcquire() method.
> ---
> src/modules/bluetooth/bluetooth-util.c | 64 +++++++++++++++++++++-------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/src/modules/bluetooth/bluetooth-util.c b/src/modules/bluetooth/bluetooth-util.c
> index 365a546..001e179 100644
> --- a/src/modules/bluetooth/bluetooth-util.c
> +++ b/src/modules/bluetooth/bluetooth-util.c
> @@ -1440,8 +1440,6 @@ bool pa_bluetooth_device_any_audio_connected(const pa_bluetooth_device *d) {
> }
>
> int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, bool optional, size_t *imtu, size_t *omtu) {
> - const char *accesstype = "rw";
> - const char *interface;
> DBusMessage *m, *r;
> DBusError err;
> int ret;
> @@ -1451,29 +1449,41 @@ int pa_bluetooth_transport_acquire(pa_bluetooth_transport *t, bool optional, siz
> pa_assert(t->device);
> pa_assert(t->device->discovery);
>
> - interface = t->device->discovery->version == BLUEZ_VERSION_4 ? "org.bluez.MediaTransport" : "org.bluez.MediaTransport1";
> -
> - if (optional) {
> - /* FIXME: we are trying to acquire the transport only if the stream is
> - playing, without actually initiating the stream request from our side
> - (which is typically undesireable specially for hfgw use-cases.
> - However this approach is racy, since the stream could have been
> - suspended in the meantime, so we can't really guarantee that the
> - stream will not be requested until BlueZ's API supports this
> - atomically. */
> - if (t->state < PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
> - pa_log_info("Failed optional acquire of transport %s", t->path);
> - return -1;
> + dbus_error_init(&err);
> +
> + if (t->device->discovery->version == BLUEZ_VERSION_4) {
> + const char *accesstype = "rw";
> +
> + if (optional) {
> + /* We are trying to acquire the transport only if the stream is
> + playing, without actually initiating the stream request from our side
> + (which is typically undesireable specially for hfgw use-cases.
> + However this approach is racy, since the stream could have been
> + suspended in the meantime, so we can't really guarantee that the
> + stream will not be requested with the API in BlueZ 4.x */
> + if (t->state < PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
> + pa_log_info("Failed optional acquire of transport %s", t->path);
> + return -1;
> + }
> }
> - }
>
> - dbus_error_init(&err);
> + pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.bluez.MediaTransport", "Acquire"));
> + pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, &accesstype, DBUS_TYPE_INVALID));
> + } else {
> + pa_assert(t->device->discovery->version == BLUEZ_VERSION_5);
> +
> + if (optional)
> + pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.bluez.MediaTransport1", "TryAcquire"));
> + else
> + pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, "org.bluez.MediaTransport1", "Acquire"));
> + }
>
> - pa_assert_se(m = dbus_message_new_method_call(t->owner, t->path, interface, "Acquire"));
> - pa_assert_se(dbus_message_append_args(m, DBUS_TYPE_STRING, &accesstype, DBUS_TYPE_INVALID));
> r = dbus_connection_send_with_reply_and_block(pa_dbus_connection_get(t->device->discovery->connection), m, -1, &err);
>
> if (dbus_error_is_set(&err) || !r) {
Checking both things is redundant, and raises the question whether it's
possible that one of the conditions is true and the other is false.
That's not possible.
> + if (optional)
> + pa_log_info("Failed optional acquire of transport %s", t->path);
An error message should be printed if this is not an optional acquire.
The messages would be more informative if they included the error name
and message from err.
--
Tanu
