This allows us to restore the default device properly when a
hotpluggable device (e.g. a USB sound card) is set as the default, but
unplugged temporarily. Previously we would forget that the unplugged
device was ever set as the default, because we had to set
configured_default_sink to NULL to avoid having a stale pa_sink pointer,
and also because module-default-device-restore couldn't resolve the name
of a currently-unplugged device to a pa_sink pointer.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=89934
In sink_put() and source_put(), pa_core_update_default_{sink,source}() was called
before the PA_CORE_HOOK_{SINK,SOURCE}_PUT hook. Therefore module-switch-on-connect
could not correctly determine the old default sink/source if no user default was
set and a sink/source with higher priority than any other sink/source turned up.
This patch corrects the problem by swapping the order of the hook call and the
pa_core_update_default_sink() call.
Additionally it corrects a problem in module-switch-on-connect. If, after the
change above, the new sink/source was the first sink/source to appear, pulseaudio
would crash because module-switch-on-connect assumed that the default sink/source
was not NULL. The patch checks if the default sink/source is NULL and only sets
the new default sink/source in that case.
When sinks are compared during the default sink selection, the active
port's availability is inspected. Therefore, the default sink should be
updated when the active port changes, because the new port may have
different availability status than the old port.
For example, let's say that a laptop has an analog sink with a speaker
and a headphone port, and headphones are initially plugged in, so both
ports can be used[1]. The headphone port is initially the active port.
There's also a null sink in the system. When the headphones are
unplugged, the headphone port becomes unavailable, and the null sink
becomes the new default sink. Then module-switch-on-port-available
changes the analog sink port to speakers. Now the default sink should
change back to the analog sink, but that doesn't happen without this
patch.
[1] Actually we currently mark speakers as unavailable when headphones
are plugged in, but that's not strictly necessary. My example relies on
both ports being available initially, so the bug can't be reproduced
with the current mixer configuration.
Currently the default sink policy is simple: either the user has
configured it explicitly, in which case we always use that as the
default, or we pick the sink with the highest priority. The sink
priorities are currently static, so there's no need to worry about
updating the default sink when sink priorities change.
I intend to make things a bit more complex: if the active port of a sink
is unavailable, the sink should not be the default sink, and I also want
to make sink priorities dependent on the active port, so changing the
port should cause re-evaluation of which sink to choose as the default.
Currently the default sink choice is done only when someone calls
pa_namereg_get_default_sink(), and change notifications are only sent
when a sink is created or destroyed. That makes it hard to add new rules
to the default sink selection policy.
This patch moves the default sink selection to
pa_core_update_default_sink(), which is called whenever something
happens that can affect the default sink choice. That function needs to
know the previous choice in order to send change notifications as
appropriate, but previously pa_core.default_sink was only set when the
user had configured it explicitly. Now pa_core.default_sink is always
set (unless there are no sinks at all), so pa_core_update_default_sink()
can use that to get the previous choice. The user configuration is saved
in a new variable, pa_core.configured_default_sink.
pa_namereg_get_default_sink() is now unnecessary, because
pa_core.default_sink can be used directly to get the
currently-considered-best sink. pa_namereg_set_default_sink() is
replaced by pa_core_set_configured_default_sink().
I haven't confirmed it, but I expect that this patch will fix problems
in the D-Bus protocol related to default sink handling. The D-Bus
protocol used to get confused when the current default sink gets
removed. It would incorrectly think that if there's no explicitly
configured default sink, then there's no default sink at all. Even
worse, when the D-Bus thinks that there's no default sink, it concludes
that there are no sinks at all, which made it impossible to configure
the default sink via the D-Bus interface. Now that pa_core.default_sink
is always set, except when there really aren't any sinks, the D-Bus
protocol should behave correctly.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=99425
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
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 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.
This serves to explicitly document the various cases we deal with in
pa_sink_update_rate()/pa_source_update_rate() rather than have some of
them hidden behind the initialisation of desired_rate.
This adds an "avoid-resampling" option to daemon.conf that makes the
daemon try to use the stream sample rate if possible (the device needs
to support it, which currently only ALSA does), and there should not be
any other stream connected).
This should enable some of the "audiophile" use-cases where users wish
to play high sample rate audio files without resampling.
We still will do conversion if sample formats don't match, though. This
means that if you want to play 96 kHz/24 bit audio without any
modification the default format will need to be set to be 24-bit as
well. This will force all streams to be upconverted, which, other than
the wasted resources, should be relatively harmless.
Streams are detached when they are removed or moved away from a device,
or when a filter device that they're connected to is removed or moved.
If these cases overlap, a crash will happen due to "double-detaching".
This can happen if a filter sink is removed, and a stream connected to
that filter sink removes itself when its sink goes away.
Here are the steps in more detail: When a filter sink is unloaded, first
it will unlink its own sink input. This will cause the filter sink's
input to be detached. The filter sink propagates the detachment to all
inputs connected to it using pa_sink_detach_within_thread(). After the
filter sink is done unlinking its own sink input, it will unlink the
sink. This will cause at least module-combine-sink to remove its sink
input if it had one connected to the removed filter sink. When the
combine sink removes its sink input, that input will get detached again,
and a crash follows.
We can relax the assertions a bit, and skip the detach() call if the
sink input is already detached.
I think a better fix would be to unlink the sink before the sink input
when unloading a filter sink - that way we could avoid the
double-detaching - but that would be a much more complicated change. I
decided to go with this simple fix for now.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=98617
The functions that call attach()/detach() for all streams on a sink or
source didn't update the "attached" flag accordingly. Since the flag is
only used in assertions, this omission didn't cause any harm in normal
use.
The "attached" flag is only used for asserting that the stream is in the
expected state when attaching or detaching.
Sometimes the flag was checked and updated before calling the attach or
detach callback, and sometimes after. I think it makes more sense to
always check it before calling the callback.
In the "unlink post" hook it's not guaranteed that the stream's old
device exists any more, so let's use the "unlink" hook that is safer.
For example, module-bluetooth-policy does a card profile change in the
source-output "unlink post" hook, which invalidates the source-output's
source pointer.
When the "unlink" hook is fired, the stream is still linked to its
device, which affects the return values of the check_suspend()
functions. The unlinked streams should be ignored by the check_suspend()
functions, so I had to add extra parameters to those functions.
Renamed all variables pertaining to latency offsets of sinks and sources,
calling them "port_latency_offset" or similar instead. All of these variables
refer to latency offsets inherited from ports, rather than being unique to
the sinks or sources themselves.
This change is to pave the way for additional functionality for setting
latency offsets on sources and sinks independenly from the value they inherit
from their port. In order to implement them we first need this rename so that
the two latency offsets can be stored individually and summed when reporting
the total latency of the source or sink.
The renames made are:
pa_sink_set_latency_offset() -> pa_sink_set_port_latency_offset()
pa_source_set_latency_offset() -> pa_source_set_port_latency_offset()
sink->latency_offset -> sink->port_latency_offset
sink->thread_info.latency_offset -> sink->thread_info.port_latency_offset
source->latency_offset -> source->port_latency_offset
source->thread_info.latency_offset -> source->thread_info.port_latency_offset
PA_SINK_MESSAGE_SET_LATENCY_OFFSET -> PA_SINK_MESSAGE_SET_PORT_LATENCY_OFFSET
PA_SOURCE_MESSAGE_SET_LATENCY_OFFSET -> PA_SOURCE_MESSAGE_SET_PORT_LATENCY_OFFSET
Unlike pa_sink_set_port(), which calls pa_sink_set_latency_offset() to update
the latency offset of the sink to match that of its newly set port,
pa_source_set_port() did not do so. This patch adds the appropriate call to
pa_source_set_latency_offset() in pa_source_set_port() to fix this.
Sink(-input) and source(-output) called unlink function when reference
count dropped to zero. This would result in unlink hooks being called
with an object having a reference count of zero, and this is not a
situation we want modules to have to deal with. It is better to just
remove the redundant unlinking code from sink(-input) and
source(-output) and assert on reference count in unlink functions as well.
It is expected that in well behaving code the owner of an object will
always unlink the object before unreferencing.
Signed-off-by: Arun Raghavan <arun@arunraghavan.net>
Before a device is unlinked, the unlink hook is fired, and it's
possible that a routing module tries to move streams to the unlinked
device in that hook, because it doesn't know that the device is being
unlinked. Of course, the unlinking is obvious when the code is in an
unlink hook callback, but it's possible that some other module does
something in the unlink hook that in turn triggers some other hook,
and it's this second hook where the routing module may get confused.
This patch adds an "unlink_requested" flag that is set before the
unlink hook is fired, and moving streams to a device with that flag
set is prevented.
This patch is motivated by seeing module-device-manager moving a
stream to a sink that was being unlinked. It was a complex case where
an alsa card was changing its profile, while an echo-cancel sink was
connected to the old alsa sink. module-always-sink loaded a null sink
in the middle of the profile change, and after a stream had been
rescued to the null sink, module-device-manager decided to move it
back to the old alsa sink that was being unlinked. That move made no
sense, so I came up with this patch.
Without this, we hit an assert because the channel count in
new_reference (which was inherited from the master) is lower than the
channel count of the filter.
pulsecore/sink.c: In function 'pa_sink_put':
pulsecore/sink.c:648:53: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
pa_assert(!(s->flags & PA_SINK_DYNAMIC_LATENCY) == (s->thread_info.fixed_latency != 0));
^
pulsecore/source.c: In function 'pa_source_put':
pulsecore/source.c:599:55: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
pa_assert(!(s->flags & PA_SOURCE_DYNAMIC_LATENCY) == (s->thread_info.fixed_latency != 0));
^
rewrite expression to suppress warning:
!(x & MASK) == (y != 0)
<->
!(x & MASK) == !(y == 0)
Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
When a sink or source is freed, there may be pending volume changes that
didn't get applied before the IO thread got torn down. Those pending
changes need to be freed.
The memory leak was reported here:
http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/23162/focus=23169
Reported-by: Alexander E. Patrakov <patrakov@gmail.com>
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.
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>
Module-device-restore sets reference_volume, but soft_volume remains at
zero dB, so if a device only has soft_volume (i e no hw volume controls),
its volume was not restored correctly.
Reported-by: Richardo Salveti de Araujo <ricardo.salveti@canonical.com>
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
This makes it easy to log a message every time the reference ratio
changes. I also need to add a hook for reference ratio changes, but
that need will go away if the stream relative volume controls will be
created by the core in the future.
This fixes assertion failures that manifest themselves with cards that
support only weird rates such as 37286Hz. Tested with snd-pcsp.
Signed-off-by: Alexander E. Patrakov <patrakov@gmail.com>
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=48109
Forcing all mute changes to go through set_mute() makes it easier to
check where the muted field is changed, and it also allows us to have
only one place where notifications for changed mute are sent.
This refactoring reduces duplication, as mute_changed() used to do the
same things as set_mute(). Other benefits are improved logging
(set_mute() logs the mute change, mute_changed() used to not do that)
and the soft mute state is kept up to date, because set_mute() sends
the SET_MUTE message to the IO thread.
The set_mute_in_progress flag is an extra precaution for preventing
recursion in case a sink/source implementation's set_mute() callback
causes mute_changed() to be called. Currently there are no such
implementations, but I think that would be a valid thing to do, so
some day there might be such implementation.
Currently the alsa sink and source write directly to s->muted during
initialization, but I think it's better to avoid direct writes, and
use the set_mute() function instead, because that makes it easier to
figure out where s->muted is modified. This patch prevents the
set_mute() call from crashing in the state assertion.
Forcing all volume changes to go through set_volume_direct() makes
it easier to check where the stream volume is changed, and it also
allows us to have only one place where notifications for changed
volume are sent.
Forcing all reference volume changes to go through
set_reference_volume_direct() makes it easier to check where the
reference volume is changed, and it also allows us to have only one
place where notifications for changed reference volume are sent.
In case a port has not yet been saved, which is e g often the case
if a sink/source has only one port, reading volume/mute will be done
without port, whereas writing volume/mute will be done with port.
Work around this by setting a default port before the fixate hook,
so module-device-restore can read volume/mute for the correct port.
BugLink: https://bugs.launchpad.net/bugs/1289515
Signed-off-by: David Henningsson <david.henningsson@canonical.com>
When given an explicit device.description in card_properties, prefer
this information over other default prefixes (e.g. 'Built-in Audio')
when constructing sink/source descriptions.
For example, if I manually configure the card description to be
"FooBar", I then expect that the sinks and created by the card also
have "FooBar" in their description instead of generic "Built-in
Audio".
I think this makes the code a bit nicer to read and write. This also
reduces the chances of off-by-one errors when checking the bounds of
sample rate values.
Since the hashmap stores a pointer to the key provided at pa_hashmap_put()
time, it make sense to allow the hashmap to be given ownership of the key and
have it free it at pa_hashmap_remove/free time.
To do this cleanly, we now provide the key and value free functions at hashmap
creation time with a pa_hashmap_new_full. With this, we do away with the free
function that was provided at remove/free time for freeing the value.
If the sink rate is not updated, then the monitor source will appear
to have a different rate than the sink, but in reality there's never
any resampling done when moving data from the sink to the monitor
source, so it's a lie that the monitor source has a different rate.
The result of lying is that clients that capture from the monitor
source will have streams that run too fast or slow.
When a sink changes its sample rate, also the monitor source rate
needs to be changed. In order to determine whether a source supports
rate changing, the code checks if the update_rate() callback is set,
but monitor sources don't have that callback set, so the old code
always failed to change the monitor source rate.
This patch fixes the monitor source rate changing by handling monitor
sources as a special case in pa_source_update_rate(): if the source is
a monitor source, then the update_rate() callback is not required.
BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=66424
