The percent calculation could overflow in the pa_*volume_snprint*() functions.
For large volumes, volume * 100 can exceed UINT32_MAX.
This patch adds appropriate type casts.
When the volume exceeds PA_VOLUME_MAX in pa_sw_volume_multiply() or
pa_sw_volume_divide(), volume settings are insanely high and the
user should be notified about it.
This patch adds volume clamping to pa_sw_volume_divide() and prints
a warning when the volume is clipped in both functions.
In pa_{source,sink}_new() and pa_{source,sink}_put() the current hardware
volume was miscalculated:
hw volume (dB) = real volume (dB) + soft volume (dB)
was used instead of
hw volume (dB) = real volume (dB) - soft volume (dB)
This lead to a crash in pa_alsa_path_set_volume() if high volumes were
set and the port was changed.
This patch fixes the calculation. Thanks to Tanu for pointing out
the correct solution.
Bug link: https://bugs.freedesktop.org/show_bug.cgi?id=65520
Several virtual sources and sinks apart from module-echo-cancel also query the master
sink or source to estimate the current latency. Those modules might potentially show
the bug that is described for module-echo-cancel in bug 100277.
This patch checks in the message handlers for the PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY
if the master sink or source is valid and returns 0 as latency if not. This is however
not yet sufficient to solve the issue. Additional patches will follow.
When module-echo-cancel is loaded and there is only one sound card, then during a
profile switch, all sinks and sources can become temporarily unavailable. If
module-always sink is loaded, it will load a null-sink in that situation. If
also module-switch-on-connect is loaded, it will try to move the sink-inputs to
the new null-sink. If a sink-input was connected to the echo-cancel sink,
pa_sink_input_start_move() will send a PA_SINK_GET_LATENCY message to the
echo-cancel sink. The message handler will then in turn call
pa_sink_get_latency_within_thread() for the invalid master sink of module-echo-cancel.
This lead to a segfault.
This patch checks in the message handler if the master sink (or source) is valid and
returns 0 if not.
If a HFP audio gateway was connected via the ofono backend, pulse would
segfault during shutdown of the daemon. pa_bluetooth_discovery_unref()
removed the devices and transports before the ofono backend was freed.
Because the ofono backend keeps its own list of transports, transport_free()
was then called during termination of the ofono backend with an invalid
transport. Bug reported by Andrew Hlynskyi.
This patch moves the termination of the ofono and native backends before
freeing the devices.
The 'portable' form factor was currently missing meaning it is not
getting any form-factor priority at all and it would therefore always
be ranked lower then internal devices (which receive 400 form factor
priority). The priority 450 is smaller then 'speaker', based on the
idea that a portable device might have less quality then a dedicated
'speaker' device (some Yamaha amplifiers announce themselves as such).
https://bugs.freedesktop.org/show_bug.cgi?id=100579
The reported latency of source or sink is based on measured initial conditions.
If the conditions contain an error, the estimated latency values may become negative.
This does not indicate that the latency is indeed negative but can be considered
merely an offset error. The current get_latency_in_thread() calls and the
implementations of the PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY messages truncate negative
latencies because they do not make sense from a physical point of view. In fact,
the values are truncated twice, once in the message handler and a second time in
the pa_{source,sink}_get_latency_within_thread() call itself.
This leads to two problems for the latency controller within module-loopback:
- Truncating leads to discontinuities in the latency reports which then trigger
unwanted end to end latency corrections.
- If a large negative port latency offsets is set, the reported latency is always 0,
making it impossible to control the end to end latency at all.
This patch is a pre-condition for solving these problems.
It adds a new flag to pa_{sink,source}_get_latency_within_thread() to allow
negative return values. Truncating is also removed in all implementations of the
PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY message handlers. The allow_negative flag
is set to false for all calls of pa_{sink,source}_get_latency_within_thread()
except when used within PA_{SINK,SOURCE}_MESSAGE_GET_LATENCY. This means that the
original behavior is not altered in most cases. Only if a positive latency offset
is set and the message returns a negative value, the reported latency is smaller
because the values are not truncated twice.
Additionally let PA_SOURCE_MESSAGE_GET_LATENCY return -pa_sink_get_latency_within_thread()
for monitor sources because the source gets the data before it is played.
The old code worked incorrectly in several situations. For example,
trying to use the "master" argument wouldn't work, because if
"sink_master" wasn't specified, pa_namereg_get() would pick the default
sink as the master sink.
The latency controller will try to adjust to the configured latency regardless
of underruns. If the configured latency is set too small, it will lead to
periodically occuring underruns. Therefore an underrun protection is implemented
which will increase the target latency if too many underruns are detected.
Underruns are tracked and if more than 3 underruns occur, the target latency
is increased in increments of 5 ms. One underrun per hour is accepted.
The protection ensures, that independent from the configured latency the
module will converge to a stable latency if the configured latency is too
small.
The print_msg argument to update_minimum_latency() had to be re-introduced,
because there is one place where the message should not be logged.
Currently passing parameters to a filter loaded by module-filter-apply is
not possible.
To enable passing parameters to a filter this patch uses an additional property
filter.apply.{MODULE_NAME}.parameters. This way, filters like virtual-surround-sink
and ladspa-sink are fully supported. For example:
paplay file.wav --property=filter.apply=ladspa-sink \
--property=filter.apply.ladspa-sink.parameters="plugin=ladspa \
label=ladspa_stereo control=0"
Currently, module-filter-apply cannot load module-ladspa-sink because filter-apply
provides the argument "sink_master" but ladspa-sink expects "master" instead.
Therefore this patch adds the sink_master argument to module-ladspa-sink.
Additionally, the autoloaded argument was also added.
Currently, module-filter-apply cannot load module-virtual-surround-sink because filter-apply
provides the argument "sink_master" but virtual-surround-sink expects "master" instead.
Therefore this patch adds the sink_master argument to module-virtual-surround-sink.
Additionally, the autoloaded argument was also added.
fltr->name should be freed before freeing fltr. Because filter_free()
can never be called from other places without f set, the pa_assert()
can be removed and filter_free() can be used in process() as well.
When the specified pid no longer exists as a child of the process (since
it was already reaped by the SIGCHLD handler), errno is set to ECHILD, not
to ESRCH.
If source or sink are changed, the current sink input rate may be different
from the default rate. Switch sink input rate back to default to avoid the
influence of the previous combination of source and sink.
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.
