Skip to content

Commit

Permalink
Merge pull request #13450 from ronso0/soundpref-apply-keylockdualthread
Browse files Browse the repository at this point in the history 
(fix) Sound preferences: don't set m_settingsModified in update slots
  • Loading branch information
@daschuer
daschuer committed Jul 19, 2024
2 parents fe878b9 + 5321f91 commit fd4d342
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 59 deletions.
105 changes: 56 additions & 49 deletions src/preferences/dialog/dlgprefsound.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
namespace {

const QString kAppGroup = QStringLiteral("[App]");
const QString kMasterGroup = QStringLiteral("[Master]");
const ConfigKey kKeylockEngingeCfgkey =
ConfigKey(kAppGroup, QStringLiteral("keylock_engine"));
const ConfigKey kKeylockMultiThreadingCfgkey =
ConfigKey(kAppGroup, QStringLiteral("keylock_multithreading"));

bool soundItemAlreadyExists(const AudioPath& output, const QWidget& widget) {
for (const QObject* pObj : widget.children()) {
Expand Down Expand Up @@ -69,6 +74,12 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
m_pSoundManager(pSoundManager),
m_pSettings(pSettings),
m_config(pSoundManager.get()),
m_pLatencyCompensation(kMasterGroup, QStringLiteral("microphoneLatencyCompensation")),
m_pMainDelay(kMasterGroup, QStringLiteral("delay")),
m_pHeadDelay(kMasterGroup, QStringLiteral("headDelay")),
m_pBoothDelay(kMasterGroup, QStringLiteral("boothDelay")),
m_pMicMonitorMode(kMasterGroup, QStringLiteral("talkover_mix")),
m_pKeylockEngine(kKeylockEngingeCfgkey),
m_settingsModified(false),
m_bLatencyChanged(false),
m_bSkipConfigClear(true),
Expand Down Expand Up @@ -137,16 +148,11 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
}
}

m_pLatencyCompensation = new ControlProxy("[Master]", "microphoneLatencyCompensation", this);
m_pMainDelay = new ControlProxy("[Master]", "delay", this);
m_pHeadDelay = new ControlProxy("[Master]", "headDelay", this);
m_pBoothDelay = new ControlProxy("[Master]", "boothDelay", this);

latencyCompensationSpinBox->setValue(m_pLatencyCompensation->get());
latencyCompensationSpinBox->setValue(m_pLatencyCompensation.get());
latencyCompensationWarningLabel->setWordWrap(true);
mainDelaySpinBox->setValue(m_pMainDelay->get());
headDelaySpinBox->setValue(m_pHeadDelay->get());
boothDelaySpinBox->setValue(m_pBoothDelay->get());
mainDelaySpinBox->setValue(m_pMainDelay.get());
headDelaySpinBox->setValue(m_pHeadDelay.get());
boothDelaySpinBox->setValue(m_pBoothDelay.get());

// TODO These settings are applied immediately via ControlProxies.
// While this is handy for testing the delays, it breaks the rule to
Expand All @@ -168,15 +174,14 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
this,
&DlgPrefSound::boothDelaySpinboxChanged);

m_pMicMonitorMode = new ControlProxy("[Master]", "talkover_mix", this);
micMonitorModeComboBox->addItem(tr("Main output only"),
QVariant(static_cast<int>(EngineMixer::MicMonitorMode::Main)));
micMonitorModeComboBox->addItem(tr("Main and booth outputs"),
QVariant(static_cast<int>(EngineMixer::MicMonitorMode::MainAndBooth)));
micMonitorModeComboBox->addItem(tr("Direct monitor (recording and broadcasting only)"),
QVariant(static_cast<int>(EngineMixer::MicMonitorMode::DirectMonitor)));
int modeIndex = micMonitorModeComboBox->findData(
static_cast<int>(m_pMicMonitorMode->get()));
static_cast<int>(m_pMicMonitorMode.get()));
micMonitorModeComboBox->setCurrentIndex(modeIndex);
micMonitorModeComboBoxChanged(modeIndex);
connect(micMonitorModeComboBox,
Expand Down Expand Up @@ -211,6 +216,10 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
QOverload<int>::of(&QComboBox::currentIndexChanged),
this,
&DlgPrefSound::settingChanged);
connect(keylockComboBox,
QOverload<int>::of(&QComboBox::currentIndexChanged),
this,
&DlgPrefSound::updateKeylockDualThreadingCheckbox);
#ifdef __RUBBERBAND__
connect(keylockDualthreadedCheckBox,
&QCheckBox::clicked,
Expand Down Expand Up @@ -240,16 +249,18 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
loadSettings();
});

m_pAudioLatencyOverloadCount =
new ControlProxy(kAppGroup, QStringLiteral("audio_latency_overload_count"), this);
m_pAudioLatencyOverloadCount = make_parented<ControlProxy>(
kAppGroup, QStringLiteral("audio_latency_overload_count"), this);
m_pAudioLatencyOverloadCount->connectValueChanged(this, &DlgPrefSound::bufferUnderflow);

m_pOutputLatencyMs = new ControlProxy(kAppGroup, QStringLiteral("output_latency_ms"), this);
m_pOutputLatencyMs = make_parented<ControlProxy>(
kAppGroup, QStringLiteral("output_latency_ms"), this);
m_pOutputLatencyMs->connectValueChanged(this, &DlgPrefSound::outputLatencyChanged);

// TODO: remove this option by automatically disabling/enabling the main mix
// when recording, broadcasting, headphone, and main outputs are enabled/disabled
m_pMainEnabled = new ControlProxy("[Master]", "enabled", this);
m_pMainEnabled =
make_parented<ControlProxy>(kMasterGroup, QStringLiteral("enabled"), this);
mainMixComboBox->addItem(tr("Disabled"));
mainMixComboBox->addItem(tr("Enabled"));
mainMixComboBox->setCurrentIndex(m_pMainEnabled->toBool() ? 1 : 0);
Expand All @@ -259,7 +270,8 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
&DlgPrefSound::mainMixChanged);
m_pMainEnabled->connectValueChanged(this, &DlgPrefSound::mainEnabledChanged);

m_pMainMonoMixdown = new ControlProxy("[Master]", "mono_mixdown", this);
m_pMainMonoMixdown =
make_parented<ControlProxy>(kMasterGroup, QStringLiteral("mono_mixdown"), this);
mainOutputModeComboBox->addItem(tr("Stereo"));
mainOutputModeComboBox->addItem(tr("Mono"));
mainOutputModeComboBox->setCurrentIndex(m_pMainMonoMixdown->toBool() ? 1 : 0);
Expand All @@ -269,9 +281,6 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
&DlgPrefSound::mainOutputModeComboBoxChanged);
m_pMainMonoMixdown->connectValueChanged(this, &DlgPrefSound::mainMonoMixdownChanged);

m_pKeylockEngine =
new ControlProxy(kAppGroup, QStringLiteral("keylock_engine"), this);

#ifdef __LINUX__
qDebug() << "RLimit Cur " << RLimit::getCurRtPrio();
qDebug() << "RLimit Max " << RLimit::getMaxRtPrio();
Expand Down Expand Up @@ -306,7 +315,6 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
void DlgPrefSound::slotUpdate() {
m_bSkipConfigClear = true;
loadSettings();
settingChanged();
checkLatencyCompensation();
m_bSkipConfigClear = false;
}
Expand All @@ -326,14 +334,14 @@ void DlgPrefSound::slotApply() {
ScopedWaitCursor cursor;
const auto keylockEngine =
keylockComboBox->currentData().value<EngineBuffer::KeylockEngine>();
m_pKeylockEngine->set(static_cast<double>(keylockEngine));
m_pSettings->set(ConfigKey("[Master]", "keylock_engine"),
m_pKeylockEngine.set(static_cast<double>(keylockEngine));
m_pSettings->set(kKeylockEngingeCfgkey,
ConfigValue(static_cast<int>(keylockEngine)));

#ifdef __RUBBERBAND__
bool keylockMultithreading = m_pSettings->getValue(
ConfigKey(kAppGroup, "keylock_multithreading"), false);
m_pSettings->setValue(ConfigKey(kAppGroup, "keylock_multithreading"),
kKeylockMultiThreadingCfgkey, false);
m_pSettings->setValue(kKeylockMultiThreadingCfgkey,
keylockDualthreadedCheckBox->isChecked() &&
keylockDualthreadedCheckBox->isEnabled());
if (keylockMultithreading !=
Expand All @@ -357,6 +365,7 @@ void DlgPrefSound::slotApply() {
m_bSkipConfigClear = true;
loadSettings(); // in case SM decided to change anything it didn't like
checkLatencyCompensation();
updateKeylockDualThreadingCheckbox();
m_bSkipConfigClear = false;
}

Expand Down Expand Up @@ -527,7 +536,7 @@ void DlgPrefSound::loadSettings(const SoundManagerConfig& config) {

// Default keylock engine is Rubberband Faster (v2)
const auto keylockEngine = static_cast<EngineBuffer::KeylockEngine>(
m_pSettings->getValue(ConfigKey("[Master]", "keylock_engine"),
m_pSettings->getValue(kKeylockEngingeCfgkey,
static_cast<int>(EngineBuffer::defaultKeylockEngine())));
const auto keylockEngineVariant = QVariant::fromValue(keylockEngine);
const int index = keylockComboBox->findData(keylockEngineVariant);
Expand All @@ -542,7 +551,7 @@ void DlgPrefSound::loadSettings(const SoundManagerConfig& config) {
#ifdef __RUBBERBAND__
// Default is no multi threading on keylock
keylockDualthreadedCheckBox->setChecked(m_pSettings->getValue(
ConfigKey(kAppGroup, QStringLiteral("keylock_multithreading")),
kKeylockMultiThreadingCfgkey,
false));
#endif

Expand Down Expand Up @@ -742,6 +751,9 @@ void DlgPrefSound::settingChanged() {
m_settingsModified = true;

#ifdef __RUBBERBAND__
}

void DlgPrefSound::updateKeylockDualThreadingCheckbox() {
bool supportedScaler = keylockComboBox->currentData()
.value<EngineBuffer::KeylockEngine>() !=
EngineBuffer::KeylockEngine::SoundTouch;
Expand Down Expand Up @@ -776,6 +788,8 @@ void DlgPrefSound::updateKeylockMultithreading(bool enabled) {
msg.setDefaultButton(pNoBtn);
msg.exec();
keylockDualthreadedCheckBox->setChecked(msg.clickedButton() == pYesBtn);

updateKeylockDualThreadingCheckbox();
#endif
}

Expand All @@ -784,7 +798,7 @@ void DlgPrefSound::updateKeylockMultithreading(bool enabled) {
/// opens this page to allow users to make adjustments in case configured
/// devices are busy/missing.
/// The issue is that the visual state (combobox(es) with 'None') does not match
/// the untouched config state. This set the modified flag so slotApply() will
/// the untouched config state. This sets the modified flag so slotApply() will
/// apply the (seemingly) unchanged configuration if users simply click Apply/Okay
/// because they are okay to continue without these devices.
void DlgPrefSound::configuredDeviceNotFound() {
Expand Down Expand Up @@ -871,22 +885,22 @@ void DlgPrefSound::slotResetToDefaults() {
if (index >= 0) {
keylockComboBox->setCurrentIndex(index);
}
m_pKeylockEngine->set(static_cast<double>(keylockEngine));
m_pKeylockEngine.set(static_cast<double>(keylockEngine));

mainMixComboBox->setCurrentIndex(1);
m_pMainEnabled->set(1.0);

mainDelaySpinBox->setValue(0.0);
m_pMainDelay->set(0.0);
m_pMainDelay.set(0.0);

headDelaySpinBox->setValue(0.0);
m_pHeadDelay->set(0.0);
m_pHeadDelay.set(0.0);

boothDelaySpinBox->setValue(0.0);
m_pBoothDelay->set(0.0);
m_pBoothDelay.set(0.0);

// Enable talkover main output
m_pMicMonitorMode->set(
m_pMicMonitorMode.set(
static_cast<double>(
static_cast<int>(EngineMixer::MicMonitorMode::Main)));
micMonitorModeComboBox->setCurrentIndex(
Expand All @@ -895,7 +909,8 @@ void DlgPrefSound::slotResetToDefaults() {

latencyCompensationSpinBox->setValue(latencyCompensationSpinBox->minimum());

settingChanged(); // force the apply button to enable
settingChanged();
updateKeylockDualThreadingCheckbox();
}

void DlgPrefSound::bufferUnderflow(double count) {
Expand All @@ -909,20 +924,20 @@ void DlgPrefSound::outputLatencyChanged(double latency) {
}

void DlgPrefSound::latencyCompensationSpinboxChanged(double value) {
m_pLatencyCompensation->set(value);
m_pLatencyCompensation.set(value);
checkLatencyCompensation();
}

void DlgPrefSound::mainDelaySpinboxChanged(double value) {
m_pMainDelay->set(value);
m_pMainDelay.set(value);
}

void DlgPrefSound::headDelaySpinboxChanged(double value) {
m_pHeadDelay->set(value);
m_pHeadDelay.set(value);
}

void DlgPrefSound::boothDelaySpinboxChanged(double value) {
m_pBoothDelay->set(value);
m_pBoothDelay.set(value);
}

void DlgPrefSound::mainMixChanged(int value) {
Expand All @@ -938,15 +953,7 @@ void DlgPrefSound::mainOutputModeComboBoxChanged(int value) {
m_pMainMonoMixdown->set(static_cast<double>(value));

#ifdef __RUBBERBAND__
bool supportedScaler = keylockComboBox->currentData()
.value<EngineBuffer::KeylockEngine>() !=
EngineBuffer::KeylockEngine::SoundTouch;
keylockDualthreadedCheckBox->setEnabled(!value && supportedScaler);
keylockDualthreadedCheckBox->setToolTip(
value ? kKeylockMultiThreadedUnavailableMono
: (supportedScaler
? kKeylockMultiThreadedAvailable
: kKeylockMultiThreadedUnavailableRubberband));
updateKeylockDualThreadingCheckbox();
#endif
}

Expand All @@ -960,15 +967,15 @@ void DlgPrefSound::micMonitorModeComboBoxChanged(int value) {
static_cast<EngineMixer::MicMonitorMode>(
micMonitorModeComboBox->itemData(value).toInt());

m_pMicMonitorMode->set(static_cast<double>(newMode));
m_pMicMonitorMode.set(static_cast<double>(newMode));

checkLatencyCompensation();
}

void DlgPrefSound::checkLatencyCompensation() {
EngineMixer::MicMonitorMode configuredMicMonitorMode =
static_cast<EngineMixer::MicMonitorMode>(
static_cast<int>(m_pMicMonitorMode->get()));
static_cast<int>(m_pMicMonitorMode.get()));

// Do not clear the SoundManagerConfig on startup, from slotApply, or from slotUpdate
if (!m_bSkipConfigClear) {
Expand All @@ -984,7 +991,7 @@ void DlgPrefSound::checkLatencyCompensation() {
latencyCompensationSpinBox->setEnabled(true);
QString lineBreak("<br/>");
// TODO(Be): Make the "User Manual" text link to the manual.
if (m_pLatencyCompensation->get() == 0.0) {
if (m_pLatencyCompensation.get() == 0.0) {
latencyCompensationWarningLabel->setText(kWarningIconHtmlString +
tr("Microphone inputs are out of time in the record & "
"broadcast signal compared to what you hear.") +
Expand Down
26 changes: 16 additions & 10 deletions src/preferences/dialog/dlgprefsound.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

#include <memory>

#include "control/pollingcontrolproxy.h"
#include "defs_urls.h"
#include "preferences/dialog/dlgpreferencepage.h"
#include "preferences/dialog/ui_dlgprefsounddlg.h"
#include "preferences/usersettings.h"
#include "soundio/sounddevice.h"
#include "soundio/sounddevicestatus.h"
#include "soundio/soundmanagerconfig.h"
#include "util/parented_ptr.h"

class SoundManager;
class PlayerManager;
Expand Down Expand Up @@ -73,6 +75,7 @@ class DlgPrefSound : public DlgPreferencePage, public Ui::DlgPrefSoundDlg {
void configuredDeviceNotFound();
void queryClicked();
#ifdef __RUBBERBAND__
void updateKeylockDualThreadingCheckbox();
void updateKeylockMultithreading(bool enabled);
#endif

Expand All @@ -86,16 +89,19 @@ class DlgPrefSound : public DlgPreferencePage, public Ui::DlgPrefSoundDlg {
std::shared_ptr<SoundManager> m_pSoundManager;
UserSettingsPointer m_pSettings;
SoundManagerConfig m_config;
ControlProxy* m_pAudioLatencyOverloadCount;
ControlProxy* m_pOutputLatencyMs;
ControlProxy* m_pHeadDelay;
ControlProxy* m_pMainDelay;
ControlProxy* m_pBoothDelay;
ControlProxy* m_pLatencyCompensation;
ControlProxy* m_pKeylockEngine;
ControlProxy* m_pMainEnabled;
ControlProxy* m_pMainMonoMixdown;
ControlProxy* m_pMicMonitorMode;

PollingControlProxy m_pLatencyCompensation;
PollingControlProxy m_pMainDelay;
PollingControlProxy m_pHeadDelay;
PollingControlProxy m_pBoothDelay;
PollingControlProxy m_pMicMonitorMode;
PollingControlProxy m_pKeylockEngine;

parented_ptr<ControlProxy> m_pAudioLatencyOverloadCount;
parented_ptr<ControlProxy> m_pOutputLatencyMs;
parented_ptr<ControlProxy> m_pMainEnabled;
parented_ptr<ControlProxy> m_pMainMonoMixdown;

QList<SoundDevicePointer> m_inputDevices;
QList<SoundDevicePointer> m_outputDevices;
QHash<DlgPrefSoundItem*, QPair<SoundDeviceId, int>> m_selectedOutputChannelIndices;
Expand Down

0 comments on commit fd4d342

Please sign in to comment.