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
When autoloaded, module-echo-cancel doesn't support moving the sink
input and source output that it creates, but the move prevention was
implemented by manually requesting module unloading in the middle of
the stream move procedure, rather than by just setting the DONT_MOVE
flags. This patch removes the module unloading code from the moving()
callbacks and adds the DONT_MOVE flags. In addition to saving some
code, this also prevents problems related to trying to move streams
connected to the echo cancel sink or source while the echo cancel sink
or source is in the middle of a move too (a crash will happen in such
situation, as demonstrated in
https://bugs.freedesktop.org/show_bug.cgi?id=93443).
It is expected that the underlying AGC mechanism will likely provide a
single volume for the source rather than a per-channel volume. Dealing
with per-channel volumes just adds complexity with regards to the
actual volume setting (depending on whether volume sharing is enabled or
not, we would set the volume on the source output of the virtual source,
and their sample specs may be different).
Using a single volume allows us to sidestep this problem entirely.
This is required to have unequal channel counts on capture in and out
streams, which is needed for beamforming to work. The deinterleaved API
only works with floating point samples.
The calculations around how many samples were sent to the canceller
engine was not updated when we started supporting different channel
counts for playback and capture.
The AGC code no longer seems to honour the analog volume limits we set,
and internally uses 0-255 as the volume range. So we switch to use that
(keeping the old API usage as is in case this gets fixed upstream).
This allows us to inherit the sample spec parameters from the sink and
source master (rather than forcing 32 kHz / mono). It is still possible
to override some of the parameters for the source side with modargs.
My original testing showed that these parameters provided a decent
perf/quality trade-off on lower end hardware (which I no longer have
access to). I figure it makes sense to continue with that for now, and
in the future this can be relaxed (use_master_format=yes could be the
default, and resource-constrained systems can disable it).
In the refactoring, I'm expressing the constraints in what I see to be a
more natural way -- rec_ss expresses what we're feeding the canceller,
so it makes sense to apply the constraints on what the canceller accepts
there. This then propagates to the output spec.
This also exposes the range of sample rates that the library actually
supports (8, 16, 32 and 48 kHz).
The original intention was to configure low enough parameters to keep
CPU consumption down. Prior to this change, we assumed that the EC
backend would override the sink parameters based on the source
parameters to achieve this goal, and with this change we remove that
assumption by forcing the default parameters for the sink to be low
enough.
It's not possible to enable the intelligibility enhancer at the
moment, because the feature would require modifying the audio that we
play to speakers, which we don't do currently. All audio processing is
done at the source side, and it's not easy to change that.
This patch is based on Arun Raghavan's code, I just reordered things
a bit and reworded the FIXME comment.
This creates a longer filter that is more complex and less sensitive to
incorrect delay reporting from the hardware. There is also a
delay-agnostic mode that can eventually be enabled if required.
In some very quick testing, not enabling this seems to provide better
results during double-talk.
This forces the canceller engine to be invoked even if playback is not
currently active. We need to do this for cases where the engine provides
additional processing that is independent of playback, such as noise
suppression and AGC.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=83557
Adding AGC broke this test, so we hard-disable the volume code in test
mode. This is probably okay for now, since at least with analog AGC, the
source volume changes and the data we get is going to be with AGC
applied, but digital gain won't be encapsulated here.
Long term, we might need to figure out how to deal with this properly.
When we the underlying sink/source goes away, there is an intermediate
state where the asyncmsgqs that we were using for the sink-input and
source-output go away. This is usually okay if the sink-input and
source-output are moved to another device, but can be problematic if we
don't support moving (which is the case when the filter is autoloaded).
This becomes a problem because of the following chain of events:
* The underlying sink goes away
* Moving the filter sink-input fails (because it is autloaded)
* At this point the sink-input has no underlying sink, and thus
no underlying asyncmsgq
* This also applies to all sink-inputs connected to the echo-cancel
module
* The sink-input is killed, triggering a module unload
* On unlink, module-rescue-streams tries to move sink-inputs to
another sink, starting with a START_MOVE message
* There is no asyncmsgq for the message, so we crash
* We can't just perform a NULL check for the asyncmsgq, since there
are state changes we need to effect during the move
To fix this, we pretend to allow the move to the new sink, and then
unlink ourselves *after* the move is complete. This ensures that we
never find ourselves in a position where we need the underlying
sink/asyncmsgq to be present when it is not.
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=90416
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.
The IS_ACTIVE() macro does a pa_sink/source_get_state() on our sink and
source, which does not work in the state change callback, since the
state is not actually committed at that point.
The callback just called pa_source_output_get_mute(), which doesn't
have any side effects, and the return value wasn't used either, so
the callback was essentially a no-op.
This patch removes all occurrences of double and triple
newlines.
Command used for this:
find . -type d \( -name ffmpeg \) -prune -o \
-regex '\(.*\.[hc]\|.*\.cc\)' \
-a -not -name 'adrian-aec.*' -a -not \
-name reserve.c -a -not -name 'rtkit.*' \
-exec sed -i -e '/^$/{N;s/^\n$//}' {} \;
Two passes were needed to remove triple newlines.
The excluded files are mirrored files from external sources.
This patch replaces every occurrence of ')\n{' with ') {'.
Command used for this:
find . -type d \( -name ffmpeg \) -prune -o \
-regex '\(.*\.[hc]\|.*\.cc\)' \
-a -not -name core-util.c -a -not \
-name adrian-aec.c -a -not -name g711.c \
-exec sed -i -e '/)$/{N;s/)\n{$/) {/}' {} \;
The excluded files are mirrored files from external sources.
send_counter/recv_counter relate to the bytes (play stream) passed
through the queue, hence the same sample spec must be used
Signed-off-by: Peter Meerwald <p.meerwald@bct-electronic.com>
Acked-by: Stefan Huber <shuber@sthu.org>
Enable advanced AEC methods to use different specs (i.e., number of
channels) for rec and out stream. A typical application is beam forming
resp. multi-channel AEC, which takes multiple record channels to produce
an echo-canceled output stream.
This commit alters the EC API as follows: the EC's init() used to get
source and sink's sample spec/channel map. The new interface renamed
source to rec and sink to play and additionally passes sample spec and
channel map of the out stream. The new parameter names of init()
{rec,play,out}_{ss,map} are more intuitive and also resemble to the
parameter names known from run(). Both rec_{ss,map} and out_{ss,map} are
initialized as we knew it from source_{ss,map} before being passed to
init(). The previous EC implementations only require trivial changes,
i.e., setting rec_{ss,map} to out_{ss,map} at the end of init() in case
that out_{ss,map} is modified in init().
When the play stream from the EC sink has not enough data available then
the EC implementation is currently bypassed by directly forwarding the
record bytes to the EC source. Since EC implementations maintain their
own buffers and cause certain latencies, a bypass leads to glitches as
the out stream stream jumps forth and back in time. Furthermore, some
EC implementations may also apply noise reduction or other sound
enhancing techniques, which are therefore bypassed, too.
Fix this by passing silence bytes to the EC implementation if the play
stream runs empty. Hence, this patch keeps the EC implementation running
even if the play stream has no data available.
The echo canceller module can pass arguments to the EC implementation
via the module parameter aec_args. However, the echo-cancel-test passes
EC arguments via a separate argv[] option, which is inconsistent. Fix
this.
Initialize the variable to zero by using pa_xnew0() instead of
pa_xnew(). This also allows us to remove a bunch of other zero
initialization statements.
Reported-by: Peter Meerwald <p.meerwald@bct-electronic.com>
bug probably caused by alignment requirement; sizeof(a->w) is a pointer, sizeof(a->w_arr) is an array
Signed-off-by: Peter Meerwald <p.meerwald@bct-electronic.com>
