[pulseaudio-discuss] [PATCH next v0 07/10] bluetooth: Abstract transport access types inside bluetooth-util

[Tanu Kaskinen]

On Wed, 2012-12-12 at 13:17 +0100, Mikel Astiz wrote:
>  static bool bt_transport_is_acquired(struct userdata *u) {
> -    if (u->accesstype == NULL) {
> +    if (!u->transport_acquired) {
>          pa_assert(u->stream_fd < 0);
>          return FALSE;
>      } else {

Now that we have the transport_acquired variable in userdata, isn't
bt_transport_is_acquired() pretty useless? It contains an assertion,
which has some value, but I don't think it's valuable enough to justify
the existence of this function.

> @@ -397,21 +394,7 @@ static int bt_transport_acquire(struct userdata *u, pa_bool_t start) {
>  
>      pa_log_debug("Acquiring transport %s", u->transport->path);
>  
> -    if (!start) {
> -        /* 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 (u->device->profile_state[u->profile] < PA_BT_AUDIO_STATE_PLAYING) {
> -            pa_log_info("Failed optional acquire of transport %s", u->transport->path);
> -            return -1;
> -        }
> -    }
> -
> -    u->stream_fd = pa_bluetooth_transport_acquire(u->transport, accesstype, &u->read_link_mtu, &u->write_link_mtu);
> +    u->stream_fd = pa_bluetooth_transport_acquire(u->transport, !start, &u->read_link_mtu, &u->write_link_mtu);

The start parameter could be changed from "start" to "optional" to be
consistent with pa_bluetooth_transport_acquire(). On the other hand,
"start" describes better the behavior regarding whether setup_stream()
will be called or not. On the third hand, does setup_stream() really
belong in this function? Could it be moved out? In that case, could this
whole function be removed in favor of using
pa_bluetooth_transport_acquire() directly?

-- 
Tanu