During a move sink_input->sink is not valid. This leads to a crash when
sink_input_set_rate() is called from the moving() callback. This patch
fixes the problem.
The previous patch assumed constant port latency offsets. The offsets can
however be changed by the user, therefore these changes need to be tracked
as well. This patch adds the necessary hooks.
Also the print_msg argument was removed from update_minimum_latency() and
update_latency_boundaries() because the message should always be logged.
With the current code, the user can request any end-to-end latency. Because there
is no protection against underruns, setting the latency too small will result in
repetitive underruns.
This patch tries to mitigate the problem by calculating the minimum possible latency
for the current combination of source and sink. The actual calculation has been put
in a separate function so it can easily be changed. To keep the values up to date,
changes in the latency ranges have to be tracked.
The calculated minimum latency is used to limit the configured latency.
The minimum latency is only a "best guess", so the actual minimum may be much
larger (for example for USB devices) or much smaller than the calculated value.
Changes of the port latency offsets are not yet handled, this will be done in a
separate patch.
The old code makes no sense to me. Why would multiple references mean
that a previously read-only memblock is suddenly writable? I'm pretty
sure that the original intention was to treat multi-referenced blocks
as read-only. I don't have any examples where the old code would have
caused bad behaviour, however.
The old pa_sink_set_fixed_latency() call didn't take into account that
other places use pa_frame_align() on the pa_pipe_buf() result, so the
configured latency could be sometimes slightly too high.
Adding a buffer_size variable in userdata makes it a bit easier to keep
all places that deal with the buffer size in sync.
Users may configure the device alias to have characters outside the
ASCII range, so our name cleanup routine was too aggressive. Let's just
make sure that the device description is a valid UTF-8 string.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=98160
Currently internal > speaker > headphone and pci > usb > bluetooth.
Invert both of these sets, with the reasoning that a headphone and
speakers are something that a user has actively attached and should
therefore get a higher priority. The same reasoning is applied for
the bus type, i.e. bluetooth and usb should be higher than pci,
because they most likely have been actively attached be a user.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99222
The function can only fail if there's not enough memory available, and
if that happens, the convention in PulseAudio is to abort.
CID: 1353106, 1353108, 1353140
We don't always know whether the in-flight memory chunks will be
rendered or skipped (if the source is not in RUNNING). This can cause us
to have an erroneous estimate of drift, particularly when the canceller
starts.
To avoid this, we explicitly flush out the send and receive sides of the
message queue of audio chunks going from the sink to the source before
trying to perform a resync.
When moving from a user suspended source or sink to an idle suspended source or sink
the sink input or source output would not be uncorked because we did not check for
the suspend cause.
Uncorking also would not be possible in that situation because the state change callback
of the source output or sink input is called before the new source or sink is attached,
leading to a crash of pulseaudio due to a cork() call without valid source or sink.
The previous patch fixes this problem, therefore sink input or source output can now also
be uncorked when the destination is idle suspended.
If pa_sink_input_cork() or pa_source_output_cork() were called without a sink
or source attached, the calls would crash pulseaudio.
This patch fixes the problem, so that a source output or sink input can still
be corked or uncorked while source or sink are invalid. This is needed to
correct the corking logic in module-loopback.
There were two bugs in the old logic. The first one:
If a device has two profiles, the old code would start the wait timer
when the first profile connects, but when the second profile connects,
the timer would not get stopped and the CONNECTION_CHANGED hook would
not get fired, because the code for that was inside an if block that
only gets executed when the first profile connects. As a result,
module-bluez5-device loading would always be delayed until the wait
timeout expires.
The second bug:
A crash was observed in device_start_waiting_for_profiles(). That
function is called whenever the connected profile count changes from 0
to 1. The function also has an assertion that checks that the timer is
not running when the function is called. That assertion crashed in the
following scenario with a headset that supports HSP and A2DP:
1. First HSP gets connected. The timer is started.
2. Then HSP gets disconnected for some reason. The timer is still
running.
3. Then A2DP gets connected. device_start_waiting_for_profiles() is
called, because the connected profile count changed from 0 to 1 again.
The timer is already running, so the assertion fails.
First I thought I'd remove the assertion from
device_start_waiting_for_profiles() and just restart the timer on the
second call, but then I figured that when the device returns to the
"everything disconnected" state in step 2, it would be better to stop
the timer. The purpose of the timer is to delay the notification of the
device becoming connected, but if the device becomes disconnected during
the waiting period, the notification doesn't make sense any more, and
therefore the timer doesn't make sense either.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100237
The webrtc canceller seems to have changed to require that the
set_stream_drift_samples() method be called before every call of
ProcessStream().
So we now call ec->set_stream_drift_samples() before calling
ec->record() by:
1. Always calling do_push_drift_comp() instead of only when the sink is
running
2. Calling set_stream_drift_samples() in the loop with record() instead
of outside
We do kind of leak this quirk of the webrtc canceller into the generic
bits of module-echo-cancel, but this should not be harmful in the
general case either.
The auto_switch argument was added in PulseAudio 10.0. In that release
the argument type was boolean. The type was changed to integer in commit
3397127f00. This patch adds backwards compatibility so that old
configuration files won't break when upgrading PulseAudio to 11.0.
With headset=auto it is possible that AG devices are connected and handled
via the native backend when ofono is started. Because the HS role will then
be disabled in the native backend, AG devices must be disconnected and any
future connections will be handled by ofono.
This patch changes the behavior of the headset=auto switch for module-bluez5-discover.
With headset=auto now both backends will be active at the same time for the AG role and
the switching between the backends is only done for the HS role.
headset=ofono and headset=native remain unchanged.
This allows to use old HSP only headsets while running ofono and to have headset support
via pulseaudio if ofono is started with the --noplugin=hfp_ag_bluez5 option.
document behaviour of pa_shared_remove() in case name does not exist
Coverity ID: #1380672
thanks to Georg Chini for suggesting to swap patch title and commit message
Signed-off-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
The "profile->card != c->card" check always evaluated to false, so the
CardProfileUpdated signal was never sent. The reason: call_data was
assigned to a pa_card_profile pointer, but the correct type is a pa_card
pointer.
readdir_r() was supposed to be a thread-safe version of readdir(), but
the interface turned out to be problematic. Due to the problems and the
fact that readdir() is safe enough on modern libc implementations, glibc
deprecated readdir_r() in version 2.24.
The man page contains more information about what's wrong with
readdir_r(): http://man7.org/linux/man-pages/man3/readdir_r.3.html
On systems with constrained CPUs, we might run into a situation where
the master source/sink is configured to have too high a latency.
On the source side, this would cause us to wake up with a large chunk of
data to process, which might cause us to exhust our RT limit and thus be
killed.
So it makes sense to limit the overall latency that we request from the
source (and correspondingly, the sink, so we don't starve for playback
data on the source side).
The 10 blocks maximum is somewhat arbitrary (I'm assuming the system has
enough headroom to process 10 chunks through the canceller without
getting close to the RT limit). This might make sense to make tunable in
the future.
If the ALSA device supports granular pointer reporting, we end up in a
situation where we write out a bunch of data, iterate, and then find a
small amount of data available in the buffer (consumed while we were
writing data into the available buffer space). We do this 10 times
before quitting the write loop.
This is inefficient in itself, but can also have wider consequences. For
example, with module-combine-sink, this will end up pushing the same
small chunks to all other devices too.
Given both of these, it just makes sense to not try to write out data
unless a minimum threshold is available. This could potentially be a
fragment, but it's likely most robust to just work with a fraction of
the total available buffer size.
don't ignore server port parsing errors as suggested by Hajime Fujita
Signed-off-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Hajime Fujita <crisp.fujita@nifty.com>
wath may be NULL, as suggested by Hajime Fujita
Coverity ID: #1398156
setting val = NULL is not needed
Signed-off-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Cc: Hajime Fujita <crisp.fujita@nifty.com>
for example, in case HAVE_MEMFD is #undef, checking with #if HAVE_MEMFD
gives a warning (gcc 5.4.1, Ubuntu)
pulsecore/shm.c: In function 'sharedmem_create':
pulsecore/shm.c:208:5: warning: "HAVE_MEMFD" is not defined [-Wundef]
#if HAVE_MEMFD
use #ifdef or #if defined() to check for presence of a #define
Signed-off-by: Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Both input and output device were chosen with the same device number.
This is problematic as those numbers don't have to correspond.
Additionally the input device was named after the output device. This
commit adresses both issues by providing specific parameters for each
type.
This is a rebase of Wim Taymans patch to support the HSP headset role that has
somehow been forgotten. Original patch can be found at
https://lists.freedesktop.org/archives/pulseaudio-discuss/2015-February/023242.html
Rebase and minor changes by Georg Chini.
In addition to the HSP Audio Gateway, also add support for the headset
role in the native bluetooth backend. In this role, pulseaudio is used as
headset.
In the headset role, we create source and sink to receive and send the samples
from the gateway, respectively. Module-bluetooth-policy will automatically load
loopback modules to link these to a sink and source for playback. Because this
makes the source the speaker and the sink the microphone, we need to reverse the
roles of source and sink compared to the gateway role.
In the gateway role, adjusting the sink volume generates a +VGS command to set
the volume on the headset. Likewise, receiving AT+VGS updates the sink volume.
In the headset role, receiving a +VGS should set the source volume and any
source volume changes should be reported back to the gateway with AT+VGS.
Clang didn't like the variable length array:
pulsecore/iochannel.c:358:17: error: fields must have a constant size:
'variable length array in structure' extension will never be supported
uint8_t data[CMSG_SPACE(sizeof(int) * nfd)];
^
Commit 451d1d6762 introduced the variable length array in order to have
the correct value in msg_controllen. This patch reverts that commit and
uses a different way to achieve the same goal.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99458
