On 08.05.2017 12:09, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Mon, May 8, 2017 at 11:53 AM, Tanu Kaskinen <ta...@iki.fi> wrote:
On Mon, 2017-05-08 at 11:26 +0300, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Sat, May 6, 2017 at 7:20 PM, Tanu Kaskinen <ta...@iki.fi> wrote:
On Fri, 2017-05-05 at 16:41 +0300, Luiz Augusto von Dentz wrote:
Hi Tanu,
On Thu, May 4, 2017 at 3:29 PM, Tanu Kaskinen <ta...@iki.fi> wrote:
On Thu, 2017-05-04 at 12:58 +0300, Luiz Augusto von Dentz wrote:
From: Luiz Augusto von Dentz <luiz.von.de...@intel.com>
This means something went wrong, which in case of ofono backend it is
probably due to the profile not connecting immediately, but it can be
safely restored in that case the transport is playing which means the
profile has recovered connectivity.
---
src/modules/bluetooth/module-bluez5-device.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/modules/bluetooth/module-bluez5-device.c
b/src/modules/bluetooth/module-bluez5-device.c
index a96da17..2f0ec97 100644
--- a/src/modules/bluetooth/module-bluez5-device.c
+++ b/src/modules/bluetooth/module-bluez5-device.c
@@ -2060,8 +2060,14 @@ static pa_hook_result_t
transport_state_changed_cb(pa_bluetooth_discovery *y, pa
if (t == u->transport && t->state <=
PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED)
pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles,
"off"), false) >= 0);
- if (t->device == u->device)
+ if (t->device == u->device) {
+ /* Auto recover from errors causing the profile to be set to off */
+ if (u->profile == PA_BLUETOOTH_PROFILE_OFF && t->state ==
PA_BLUETOOTH_TRANSPORT_STATE_PLAYING) {
+ pa_log_debug("Switching to profile %s",
pa_bluetooth_profile_to_string(t->profile));
+ pa_assert_se(pa_card_set_profile(u->card,
pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)), false)
>= 0);
Regarding the assertion, how do you know that the card profile switch
will always succeed? Is there no IO involved? If you somehow know that
it will always succeed, there should be a comment explaining how you
know that.
The playing state should guarantee the fd is available, so there
shouldn't be any problem in this regard.
At least start_thread() can fail, and that will make the profile change
fail.
I know there are already assertions when switching to the off profile,
but that's a special profile - activating it should never fail.
This change seems dubious in another way too. So far we've kept all
automatic profile switch code out of module-bluez5-device (apart from
switches to "off" when things fail). Automatic switching is handled by
module-bluetooth-policy instead. If I understood correctly, this is a
fix for a situation where we already tried to switch to a profile, but
it failed first. The code is lacking any checks that would do the
profile change only if there really was a prior profile switch attempt.
It is exactly to counter the profiles switching 'off' which is done in
the device itself, we could of course move this to policy as well
though. Note that in most cases it is better to switch to a working
profile than 'off' since that for sure won't render anything.
If we try to switch to some profile and it fails and as a result the
profile gets set to "off", then I think it's fine to fix that problem
in module-bluez5-device, but as I said, there's nothing in this patch
that would ensure that this is done *only* when this situation occurs.
If there was no problem earlier, then module-bluez5-device shouldn't
automatically change its profile, that kind of stuff belongs to module-
bluetooth-policy.
Profiles can be connected from both ends, lets say the policy attempt
to switch profile, lets say from A2DP to HFP, and for some reason it
fails, the result would be "off" profile but then at some point the
remote device decides to play some audio, not A2DP is playing but
because the current profile is 'off' nothing is heard.
module-bluetooth-policy already handles this case.
It only does this for specific roles:
/* Do not automatically switch profiles for headsets, just in case */
/* TODO: remove a2dp and hsp when we remove BlueZ 4 support */
if (pa_streq(profile->name, "hsp") || pa_streq(profile->name,
"a2dp") || pa_streq(profile->name, "a2dp_sink") ||
pa_streq(profile->name, "headset_head_unit"))
return PA_HOOK_OK;
Georg told me that this patch isn't needed anyway, after the "bluez5-
device: Correctly handle suspend/resume cyle for audio gateway role of
ofono backend" patch. If that's true, then can we just revert this?
It is not needed for regular cases, though what I just said above is
still valid,
See the above comment.
also note that in both A2DP and HFP the device may refuse
to resume
Are you talking specifically about the case where pulseaudio acts as an
A2DP source or HFP audio gateway? In these cases switching to off seems
like the right approach. We don't know when the remote will again allow
streaming, so the user has to manually try again.
Here there something that perhaps is not clear, both A2DP and HFP are
symmetric in terms of audio control, both sides can suspend and resume
the stream regardless of their role. Perhaps this comes because it
seems unlikely that a sink would want to suspend/resume, but this is a
common practice in case of HFP HF and it is even documented in the
spec:
'4.17 Audio Connection Transfer towards the AG
v1.7.1
The audio paths of an ongoing call may be transferred from the HF to
the AG. This procedure
represents a particular case of an “Audio Connection release”
procedure, as described in
Section 4.12.
The call connection transfer from the HF to the AG is initiated by a
user action in the HF or due
to an internal event or user action on the AG side. This results in an
“Audio Connection release”
procedure being initiated either by the HF or the AG respectively,
with the current call kept and
its audio paths routed to the AG.'
If resuming fails, can the remote even initiate streaming without
pulseaudio doing anything? If the remote can initiate streaming, then
maybe module-bluetooth-policy should do something, although I don't see
what the use case would be.
or it can sometimes have a different policy, disabling
inband ringtone for example can prevent SCO to connect until the call
is actually in place but once it has been answer it would resume SCO
but at that point the profile would be set to 'off'.
Can you explain this "disable inband ringtone" case in more detail?
What role (GW or HF) does pulseaudio have? What steps are in the
process?
>From the spec:
'Depending on whether in-band ringing is enabled or disabled, there
may or may not be a
synchronous connection established between the HF and AG. The
synchronous connection
state (enabled or disabled) shall not be changed when an incoming call
is placed on hold.'
In practice this probably means that both the AG and the HF will play
their own ringtone while the call is alerting and only establish the
SCO connection once the the call has been accepted, which means the BT
card are switched to 'off', doesn't matter if AG or HF, and the
default card would play the ringtone locally. Though we lack proper
policy based on in-band ringtone and call states it seems quite clear
that sinks can self resume, in fact this is what the spec refer as
audio transfer and it clearly states that this is no limited to AG
only.
A2DP is even more generic as the protocol is completely symmetric both
sinks and source can do suspend and resume at will, and there is even
test cases in the qualification to ensure this:
'4.4.1 Suspend Audio Streaming Initiated by SRC
Expected Outcome
Pass verdict:
SRC:
– If there is a corresponding indicator, suspend audio streaming is indicated.
It is possible to resume audio streaming.
SNK:
– The user action suspends audio streaming connection.
–
If there is a corresponding indicator, suspend audio streaming is indicated.
It is possible to resume audio streaming.'
The patch was only introduced, because the profile was set to off when
handling the SET_STATE message failed. That failed, because the connection
could only be requested, but not established. When then the connection was
finally established and the NewConnection signal was received the profile
needed to be set correctly again.
Now requesting the connection is also considered a success in SET_STATE,
so the profile is not set to off anymore between the connection request
and the connection establishment. Therefore I think the patch is not
necessary.
_______________________________________________
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
