Issue: When HFP/HSP profile is used with certain BT chipsets, the
audio sounds heavily distorted, with very slow playback full of noise.
During recording, the samples are dropped and it distorts the recorded
audio samples.
The root cause of both the issues are related to the fixed MTU sizes
in the PA stack, which is 48 bytes. Here, the BT chipset CC256x had
180 bytes MTU and it was being under-utilized and the rate at which
the samples were being accepted where not matching the expected rate,
and hence the distortion.
Solution: The appropriate solution to this problem is by reading the
MTU size of the SCO socket using getsockopts dynamically.
BugLink: http://bit.ly/2gDpGPv
BugLink: http://bit.ly/2hQsARK
Not all VOIP applications (specially those which use alsa) set media.role to
phone. This means we need some heuristic to determinate if we want to switch
from a2dp to hsp profile based on number and types of source output (recording)
streams.
And also some people want to use their bluetooth headset (with microphone) as
their default recording device but some do not want to because of low quality.
This patch implements optional heuristic which is disabled by default. It is
disabled by default to not break experience of current pulseaudio users because
heuristic cannot be optimal. Heuristic is implemented in module-bluetooth-policy
module and decide if pulseaudio should switch to a hsp profile or not. It checks
if there is some source output with pass all these conditions:
* does not have set media.role
* does not use peak resample method (which is used by desktop volume programs)
* has assigned client/application (non virtual stream)
* does not record from monitor of sink
And if yes it switch to hsp profile.
By default this heuristic is disabled and can be enabled when loading module
module-bluetooth-policy with specifying parameter auto_switch=2
Because it is disabled by default nobody will be affected by this change unless
manually change auto_switch parameter.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
This fixes a crash that happens if the bluetooth headset is the only
non-monitor source in the system and the last "phone" stream dies.
When the stream dies, the native protocol calls pa_source_output_unlink()
and would call pa_source_output_unref() next, but without this patch,
things happen during the unlinking, and the unreffing ends up being
performed on a stream that is already freed.
pa_source_output_unlink() fires the "unlink" hook before doing anything
else. module-bluetooth-policy then switches the headset profile from HSP
to A2DP within that hook. The HSP source gets removed, and at this point
the dying stream is still connected to it, and needs to be rescued.
Rescuing fails, because there are no other sources in the system, so the
stream gets killed. The native protocol has a kill callback, which again
calls pa_source_output_unlink() and pa_source_output_unref(). This is
the point where the native protocol drops its own reference to the
stream, but another unref call is waiting to be executed once we return
from the original unlink call.
I first tried to avoid the double unreffing by making it safe to do
unlinking recursively, but I found out that there's code that assumes
that once unlink() returns, unlinking has actually occurred (a
reasonable assumption), and at least with my implementation this was not
guaranteed. I now think that we must avoid situations where unlinking
happens recursively. It's just too hairy to deal with. This patch moves
the bluetooth profile switch to happen at a time when the dead stream
isn't any more connected to the source, so it doesn't have to be
rescued or killed.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=97906
Bug 96741 shows a case where an assertion is hit, because
pa_asyncq_new() failed due to running out of file descriptors.
pa_asyncq_new() is used in only one place (not counting the call in
asyncq-test): pa_asyncmsgq_new(). Now pa_asyncmsgq_new() can fail too,
which requires error handling in many places. One of those places is
pa_thread_mq_init(), which can now fail too, and that needs additional
error handling in many more places. Luckily there weren't any places
where adding better error handling wouldn't have been easy, so there are
many changes in this patch, but they are not complicated.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=96741
SW: Pulseaudio 8.0 / BlueZ 5.39
Symptoms:
While disconnecting/reconnecting a paired bluetooth headset (LG HBS750)
audio fails roughly on every other connection.
On a failed connection "pactl list cards" shows the bluetooth device's
card but "Active Profile: off". Issuing "pacmd set-card-profile X
a2dp_sink" makes audio work immediately.
I realized that when this happened, the previous disconnection did not
remove the card, instead it was only configured for "Active Profile:
off" but otherwise left in place.
Upon looking at PA debug logs I saw that the transport for the a2dp_sink
was first set into disconnected state and then into idle state. In
"device_connection_changed_cb()" this causes the
"pa_bluetooth_device_any_transport_connected()" return true and the
module-bluez5-device is not unloaded.
Further investigation shows that this is caused by a race of
module-bluez5-device.c:thread_func() and
MediaPoint1::ClearConfiguration().
When the FD in thread_func() is closed (POLLHUP) an
BLUETOOTH_MESSAGE_STREAM_FD_HUP message is sent into the main thread.
The handler of this message unconditionally sets the transport into IDLE
state. This is a problem if it has already been set into DISCONNECTED
state.
Bluez5 uses different profile names as bluez4, so we need to check for
a2dp_sink and headset_head_unit too for bluez5 support.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
CID 1353122
this is a false-positive because
if (dbus_message_has_interface(p->message, "org.bluez.Manager") ||
dbus_message_has_interface(p->message, "org.bluez.Adapter"))
d = NULL;
else if (!(d = pa_hashmap_get(y->devices, dbus_message_get_path(p->message)))) {
pa_log_warn("Received GetProperties() reply from unknown device: %s (device removed?)",
dbus_message_get_path(p->message));
goto finish2;
}
d can be NULL only if p->message interface is org.bluez.Manager or
org.bluez.Adapter. If
dbus_message_is_method_call(p->message, "org.bluez.Device", "GetProperties")
returns true, we know that the interface is org.bluez.Device.
thanks, Tanu!
create_card_profile() used to get called separately for HSP and HFP,
so if a headset supports both profiles, a profile named
"headset_head_unit" would get created twice. The second instance would
get immediately freed, so that wasn't a particularly serious problem.
However, I think it makes more sense to create the profile only once.
This patch makes things so that before a profile is created, we check
what name that profile would have, and if a profile with that name
already exists, we don't create the profile.
A couple of Yocto releases (jethro and krogoth) have non-upstream
patches that suffer from this double creation. The patches add
associations between profiles and ports, and those associations use
the profile name as the key. When the second profile gets freed, the
associations between the profile and its ports get removed, and since
the profile name is used as the key, this erroneously affects the
first profile too. Crashing ensues.
BugLink: https://bugzilla.yoctoproject.org/show_bug.cgi?id=10018
Add transport_set_state() that encapsulates changing the variable,
logging and firing the change hook.
I also made a cosmetic change to the corresponding BlueZ 5 log
message so that both messages have the format that I like.
A hashmap is more convenient than a linked list for storing the UUIDs,
so change the BlueZ 4 code accordingly.
Rename the BlueZ 4 UUID constants to match the BlueZ 5 naming.
The only changes to the BlueZ 5 code are the addition of one comment
and making another comment a bit clearer.
The properties_received flag affects whether the device should be
considered valid, so let's update the valid flag after setting the
properties_received flag.
There's a call to device_update_valid() anyway later when setting
the device adapters, so this change isn't strictly necessary, but
this makes it more obvious that the code is correct (and less
fragile).
The CONNECTION_CHANGED hook is used to notify the discovery module
about new and removed devices. When a bluetooth device connects, the
hook used to be called immediately when the first profile connected.
That meant that only one profile was marked as available during the
card creation, other profiles would get marked as available later.
That makes it hard for module-card-restore to restore the saved
profile, if the saved profile becomes available with some delay.
module-card-restore has a workaround for this problem, but that turned
out to interfere with module-bluetooth-policy, so the workaround will
be removed in the next patch.
The BlueZ 4 code doesn't need changes, because we use the
org.bluez.Audio interface to get a notification when all profiles are
connected.
With this patch module-bluetooth-policy automatically switch from a2dp profile
to hsp profile if some VOIP application with media.role=phone wants to start
recording audio.
By default a2dp profile is used for listening music, but for VOIP calls is
needed profile with microphone support (hsp). So this patch will switch to
hsp profile if some application want to use microphone (and specify it in
media.role as "phone). After recording is stopped profile is switched back
to a2dp. So this patch allows to use bluetooth microphone for VOIP applications
with media.role=phone automatically without need of user interaction.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
I want module-alsa-card to set the availability of unavailable
profiles before the initial card profile gets selected, so that the
selection logic can use correct availability information.
module-alsa-card initializes the jack state after calling
pa_card_new(), however, and the profile selection happens in
pa_card_new(). This patch solves that by moving parts of pa_card_new()
to pa_card_choose_initial_profile() and pa_card_put().
pa_card_choose_initial_profile() applies the profile selection policy,
so module-alsa-card can first call pa_card_new(), then initialize the
jack state, and then call pa_card_choose_initial_profile(). After that
module-alsa-card can still override the profile selection policy, in
case module-alsa-card was loaded with the "profile" argument. Finally,
pa_card_put() finalizes the card creation.
An alternative solution would have been to move the jack
initialization to happen before pa_card_new() and use pa_card_new_data
instead of pa_card in the jack initialization code, but I disliked
that idea (I want to get rid of the "new data" pattern eventually).
The order in which the initial profile policy is applied is reversed
in this patch. Previously the first one to set it won, now the last
one to set it wins. I think this is better, because if you have N
parties that want to set the profile, we avoid checking N times
whether someone else has already set the profile.
If 'pa_modargs_new' returns a NULL, we need to be careful to not call
'pa_modargs_free' in the failure path since it requires that we pass it
a non-null argument. Also updates 'module-bluetooth-policy.c:pa__init'
to follow the standard "goto fail" pattern used everywhere else.
Signed-off-by: Jason Gerecke <killertofu@gmail.com>
pa_module_unload() takes two pointers: pa_module and pa_core.
The pa_core pointer is also available via the pa_module object,
so the pa_core argument is redundant
[David Henningsson: Rebased to git HEAD]
Flushing the asyncmsgq can cause arbitrarily callbacks to run, potentially
causing recursion into pa_thread_mq_done again. Because of this; rtpoll which
is cleared in the second iteration is tried to free once again by the first
iteration leading to PA crash.
FSF addresses used in PA sources are no longer valid and rpmlint
generates numerous warnings during packaging because of this.
This patch changes all FSF addresses to FSF web page according to
the GPL how-to: https://www.gnu.org/licenses/gpl-howto.en.html
Done automatically by sed-ing through sources.
In case a transport is currently disconnected and transitions to
idle, that should not count as a "remote hang up" event.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
Because the adapters reference the devices hashmap on free, we mush
free the adapters hashmap first and then the devices hashmap.
Reported-by: Alexander Patrakov <patrakov@gmail.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
Send the right command to set the speaker and microphone gain.
Note that setting the volume on the Headset should use the unsolicited
result code. Receiving the volume from the Headset uses the AT
command.
supresses a warning when compiling with NDEBUG:
pulsecore/aupdate.c: In function 'pa_aupdate_read_end':
pulsecore/aupdate.c:82:14: warning: variable 'n' set but not used [-Wunused-but-set-variable]
unsigned n;
pulsecore/sink-input.c: In function 'pa_sink_input_unlink':
pulsecore/sink-input.c:648:27: warning: variable 'p' set but not used [-Wunused-but-set-variable]
pa_source_output *o, *p = NULL;
pulsecore/sink-input.c: In function 'find_filter_sink_input':
pulsecore/sink-input.c:1523:14: warning: unused variable 'i' [-Wunused-variable]
unsigned i = 0;
pulsecore/sink-input.c: In function 'pa_sink_input_start_move':
pulsecore/sink-input.c:1569:27: warning: variable 'p' set but not used [-Wunused-but-set-variable]
pa_source_output *o, *p = NULL;
CC pulsecore/libpulsecore_5.0_la-sink.lo
pulsecore/sink.c: In function 'pa_sink_unlink':
pulsecore/sink.c:673:24: warning: variable 'j' set but not used [-Wunused-but-set-variable]
pa_sink_input *i, *j = NULL;
pulsecore/source-output.c: In function 'find_filter_source_output':
pulsecore/source-output.c:1179:9: warning: unused variable 'i' [-Wunused-variable]
int i = 0;
CC pulsecore/libpulsecore_5.0_la-source.lo
pulsecore/source.c: In function 'pa_source_unlink':
pulsecore/source.c:616:27: warning: variable 'j' set but not used [-Wunused-but-set-variable]
pa_source_output *o, *j = NULL;
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
This patch adds a module argument "headset=ofono|native|auto" to
module-bluetooth-discover and module-bluez5-discover.
To make Arun's happy, the default is 'native' if compiled in, otherwise
'ofono'. 'Auto' will try to autoswitch depending on whether ofono is
running or not.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
This implements some autodetect if both headset backends are compiled in:
First we try to contact the oFono service, if that's not working,
then we start the native backend instead.
Likewise if the oFono service is going offline/online, we load/unload
the native backend accordingly.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
Enable both ofono and native backends to be built into the same
libbluez5-util. Never build the null backend.
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
We weren't writing out one character from the "OK" response, and the
"AT" part of the "+VGS" and "+VGM" commands was missing. Also, the spec
says that the command is terminated by only a CR and not an LF (probably
doesn't hurt, but let's adhere to the spec for now).
Parse the gain changed AT commands from the headset and fire 2 new
hooks as a result. The device will connect to those hooks and change the
source/sink volumes.
When the source/sink volume changes, set the gain on the microphone or
speaker respectively. Make sure we do nothing if the transport can not
handle the gain changes.
Add a simple native headset backend that implements support for the
blutooth HSP profile.
This allows pulseaudio to output audio to a Headset using the HSP profile.
Make the native backend the default.
This should not have any effect on behaviour. The goal is to align
with the pattern that I think we should follow:
Object initialization:
- put() is the place to create references from other objects to the
newly created object. In this case, adding the transport to
discovery->transports was moved from new() to put, and adding the
transport to device->transports was moved from set_state() to
put().
Object destruction:
- unlink() undoes put() and removes all references from other objects
to the object being unlinked. In this case setting the
device->transports pointer to NULL was moved from set_state() to
unlink(), and setting the discovery->transports pointer to NULL was
moved from free() to unlink().
- free() undoes new(), but also calls unlink() so that object owners
don't need to remember to call unlink() before free().
Now a2dp and hsp sinks and sources will have different names which means that
applications and other modules can use sink/source to distinguish selected
profile.
Module module-device-restore uses sink/source name and port name as identifier,
so if different profiles have different names module-device-restore can store
volume settings for each profile.
So with this patch it is possible to configure different volume settings for
a2dp and hsp profiles.
This patch does not change port names so gnome applications will be happy.
Note that similar patch is needed also for bluez5, but I'm not using bluez5
so I cannot write or test it.
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
This allow 'off' profile to be choosen when no other profile is available
which is considered better since it requires less resources than other
profiles.
