Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

PulseAudio: Cork audio input when it isn't needed #171

Open
wants to merge 5 commits into from

8 participants

@cbs228

As noted in https://sourceforge.net/p/mumble/bugs/868/ , mumble consumes a great deal of CPU when it is idle. The vast majority of this usage can be traced to the audio input DSP. When mumble is self-muted, these operations are largely unnecessary and unhelpful. To save CPU, the audio input stream can be "corked" (muted) when it is not needed. This blocks the audio input DSP when it isn't needed.

This update automatically corks the PulseAudio input when the user is self-muted. In order to do this, MainWindow emits a new signal, corkStream(bool), whenever the audio input is or is not required. This signal is slotted by the PulseAudio source, which passes it to the underlying PulseAudio server. This implementation will enable the fix to be ported to other sound servers and systems (i.e., Windows).

A downside to this fix is that the AEC will stop updating while Mumble is muted. If the user is truly idling or AFK, however, there probably won't be much sound with which to update it. While this patch appears to work, I would welcome more widespread testing.

@Natenom
Owner

Thank you for this patch; I use it for two weeks now without problems.

Is it possible to also disable the PulseAudio source when using push to talk?

@cbs228

@Natenom: Muting the audio stops the echo canceler from updating. Such a patch, if implemented, would probably effectively break echo cancellation with PTT. Perhaps we can disable audio input for PTT if echo cancellation is also disabled.

I'm also not sure how long it takes PA to uncork a stream, and that might make the PTT "keyup" take a bit longer. If PA is slow to uncork, we might risk chopping off the first syllable of outgoing speech.

Thanks for testing this!

@ngollan

Echo cancellation should be mostly fine (AFAIK it's just acting on a delayed version of output, so it should only really "fail" for a few milliseconds). I'm more worried about noise filtering, which depends on constant updates to keep the noise power spectrum in order. So if noise changes significantly while the stream is inactive, e.g. from a printer starting to run, a window being opened/closed, or whatever else, there's a chance that things sound quite ugly after input resumes, possibly throwing off voice activation too. Having the client's audio processing always-on is (partly) to avoid that issue and always provide a cleaned input.

@cbs228

Interesting. The patch was not written with rapid toggling in mind: it was intended more for people who go AFK and idle, and it only kicks in when you self-mute. Some additional enhancements could include:

  • A timer, which will only turn off the audio when the user has been Self-Muted for so long
  • A preferences setting to turn this behavior on (or off)
  • A small delay when un-muting to allow the AGC to re-converge.

I've not observed any obnoxious behavior with the AGC while using this patch, but I tend to use PulseAudio's own echo canceler (AEC) and AGC, instead of Mumble's (see http://sourceforge.net/p/mumble/discussion/492607/thread/81d0b264/). The PulseAudio code has an input/output synchronizer and runs with real-time priority, and I suspect one or both of these things makes it work. I've been considering trying to patch Mumble to use PA's canceler directly, via filter.want=echo-cancel. That would, however, be a separate patch.

@ei-grad ei-grad commented on the diff
src/mumble/AudioWizard.cpp
@@ -55,6 +55,7 @@ bool CompletablePage::isComplete() const {
bInit = true;
bLastActive = false;
g.bInAudioWizard = true;
+ g.mw->onChangeMute();
@ei-grad
ei-grad added a note

Why there are 4 spaces instead of 2 as in previous line?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cbs228

@ei-grad It's probably just what my text editor was set to. If the patch is something that the mumble team wants to include, I can re-format it.

@Kissaki
Owner

We discussed this PR. We think that this should definitely be implemented - but for all audio systems (as mentioned in the newly created #1089 ) if noise suppression effectiveness is not noticeable.

This PR will be a good starting- and testing-base.

Thank you so far for your suggestion and implementation. We prioritized other PRs for 1.2.5 though, since this needs some more work and testing, so none of us will concretely work on this at this point in time.

@mkrautz
Owner

Clarification: the mechanism which activates the "corking" should be compatible with all audio systems. That is, it should be implementable by subclassing AudioInput. Not all of them need to implement it, necessarily.

@cbs228

I definitely agree on the need to support all audio systems. I really only develop for linux, but please let me know if I can be of further assistance.

@Vaskozl

I've been running this patched version for over a month now and I can confirm that it significantly decreases CPU usage while idling in a channel muted. This functionality should definitely be part of core mumble. I have not experienced any issues and it works marvellous.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 11, 2013
  1. PulseAudio: substantially reduce CPU usage when muted by corking Puls…

    Colin Stagner authored
    …eAudio source.
  2. Never mute PA source initially.

    Colin Stagner authored
  3. cork/uncork logic now handled by new function, MainWindow::onChangeMu…

    Colin Stagner authored
    …te()
    
    audio input now uncorked for audio setup wizard
Commits on Aug 12, 2013
Commits on Oct 31, 2013
  1. Merge remote-tracking branch 'upstream/master'

    Colin Stagner authored
This page is out of date. Refresh to see the latest.
View
3  src/mumble/AudioWizard.cpp
@@ -55,6 +55,7 @@ AudioWizard::AudioWizard(QWidget *p) : QWizard(p) {
bInit = true;
bLastActive = false;
g.bInAudioWizard = true;
+ g.mw->onChangeMute();
@ei-grad
ei-grad added a note

Why there are 4 spaces instead of 2 as in previous line?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
ticker = new QTimer(this);
ticker->setObjectName(QLatin1String("Ticker"));
@@ -410,6 +411,7 @@ void AudioWizard::reject() {
ao->wipe();
aosSource = NULL;
g.bInAudioWizard = false;
+ g.mw->onChangeMute();
QWizard::reject();
}
@@ -445,6 +447,7 @@ void AudioWizard::accept() {
g.bPosTest = false;
restartAudio();
g.bInAudioWizard = false;
+ g.mw->onChangeMute();
QWizard::accept();
}
View
22 src/mumble/MainWindow.cpp
@@ -1916,7 +1916,7 @@ void MainWindow::talkingChanged() {
}
void MainWindow::on_qaAudioReset_triggered() {
- AudioInputPtr ai = g.ai;
+ AudioInputPtr ai = g.ai;
if (ai)
ai->bResetProcessor = true;
}
@@ -1953,7 +1953,8 @@ void MainWindow::on_qaAudioMute_triggered() {
if (g.sh) {
g.sh->setSelfMuteDeafState(g.s.bMute, g.s.bDeaf);
}
-
+
+ onChangeMute();
updateTrayIcon();
}
@@ -1965,7 +1966,7 @@ void MainWindow::on_qaAudioDeaf_triggered() {
if (! qaAudioDeaf->isChecked() && bAutoUnmute) {
qaAudioDeaf->setChecked(true);
- qaAudioMute->setChecked(false);
+ qaAudioMute->setChecked(false);
on_qaAudioMute_triggered();
return;
}
@@ -1990,6 +1991,7 @@ void MainWindow::on_qaAudioDeaf_triggered() {
g.sh->setSelfMuteDeafState(g.s.bMute, g.s.bDeaf);
}
+ onChangeMute();
updateTrayIcon();
}
@@ -2346,6 +2348,19 @@ void MainWindow::whisperReleased(QVariant scdata) {
updateTarget();
}
+void MainWindow::onChangeMute()
+{
+ // This function will attempt to cork the underlying
+ // audio device if it is not needed and uncork it if it is.
+ if (! g.ai)
+ return;
+
+ if (g.s.bMute && !g.bInAudioWizard)
+ emit corkStream(true);
+ else
+ emit corkStream(false);
+}
+
void MainWindow::onResetAudio()
{
qWarning("MainWindow: Start audio reset");
@@ -2399,6 +2414,7 @@ void MainWindow::serverConnected() {
if (g.s.bMute || g.s.bDeaf) {
g.sh->setSelfMuteDeafState(g.s.bMute, g.s.bDeaf);
+ onChangeMute();
}
// Update QActions and menues
View
4 src/mumble/MainWindow.h
@@ -267,8 +267,12 @@ class MainWindow : public QMainWindow, public MessageHandler, public Ui::MainWin
void pttReleased();
void whisperReleased(QVariant scdata);
void onResetAudio();
+ void onChangeMute();
void on_qaFilterToggle_triggered();
+ signals:
+ void corkStream(const bool corked);
+
public:
MainWindow(QWidget *parent);
~MainWindow();
View
18 src/mumble/PulseAudio.cpp
@@ -303,6 +303,12 @@ void PulseAudioSystem::eventCallback(pa_mainloop_api *api, pa_defer_event *) {
qsInputCache = idev;
pa_stream_connect_record(pasInput, qPrintable(idev), &buff, PA_STREAM_ADJUST_LATENCY);
+
+ // Ensure stream is initially un-muted
+ pa_stream_cork(pasInput, 0, NULL, NULL);
+
+ // connect PulseAudioInput corkStream signal to here
+ QObject::connect(g.mw, SIGNAL(corkStream(const bool)), this, SLOT(corkStream(const bool)), Qt::QueuedConnection);
}
}
@@ -371,6 +377,18 @@ void PulseAudioSystem::eventCallback(pa_mainloop_api *api, pa_defer_event *) {
}
}
+void PulseAudioSystem::corkStream(const bool corked)
+{
+ // TODO: Do I need this lock?
+ if (pasInput)
+ {
+ pa_threaded_mainloop_lock(pam);
+ pa_stream_cork(pasInput, corked, NULL, NULL);
+ pa_threaded_mainloop_unlock(pam);
+ }
+ return;
+}
+
void PulseAudioSystem::context_state_callback(pa_context *c, void *userdata) {
PulseAudioSystem *pas = reinterpret_cast<PulseAudioSystem *>(userdata);
pas->contextCallback(c);
View
5 src/mumble/PulseAudio.h
@@ -99,7 +99,10 @@ class PulseAudioSystem : public QObject {
void setVolumes();
PulseAttenuation* getAttenuation(QString stream_restore_id);
-
+
+ public slots:
+ void corkStream(const bool corked);
+
public:
QHash<QString, QString> qhInput;
QHash<QString, QString> qhOutput;
View
1  src/mumble/main.cpp
@@ -439,6 +439,7 @@ int main(int argc, char **argv) {
g.p->rescanPlugins();
Audio::start();
+ g.mw->onChangeMute();
a.setQuitOnLastWindowClosed(false);
Something went wrong with that request. Please try again.