Skip to content

Commit

Permalink
Merge pull request #13312 from ronso0/startup-no-sound-IO-fix
Browse files Browse the repository at this point in the history
(fix) Sound config: fix UX when applying config with missing/busy devices
  • Loading branch information
daschuer authored Jun 9, 2024
2 parents ab87f0e + ddb4bad commit 3acd833
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 10 deletions.
3 changes: 3 additions & 0 deletions src/preferences/dialog/dlgpreferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,9 @@ void DlgPreferences::slotButtonPressed(QAbstractButton* pButton) {
case QDialogButtonBox::AcceptRole:
// Same as Apply but close the dialog
emit applyPreferences();
// TODO Unfortunately this will accept() even if DlgPrefSound threw a warning
// due to inaccessible device(s) or inapplicable samplerate.
// https://github.com/mixxxdj/mixxx/issues/6077
accept();
break;
case QDialogButtonBox::RejectRole:
Expand Down
27 changes: 20 additions & 7 deletions src/preferences/dialog/dlgprefsound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
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
// apply only in slotApply(). Add hint to UI?
connect(latencyCompensationSpinBox,
QOverload<double>::of(&QDoubleSpinBox::valueChanged),
this,
Expand Down Expand Up @@ -263,18 +266,12 @@ DlgPrefSound::DlgPrefSound(QWidget* pParent,
MIXXX_WIKI_HARDWARE_COMPATIBILITY_URL)));
}

/// Slot called when the preferences dialog is opened or this pane is
/// selected.
/// Slot called when the preferences dialog is opened.
void DlgPrefSound::slotUpdate() {
// this is unfortunate, because slotUpdate is called every time
// we change to this pane, we lose changed and unapplied settings
// every time. There's no real way around this, just another argument
// for a prefs rewrite -- bkgood
m_bSkipConfigClear = true;
loadSettings();
checkLatencyCompensation();
m_bSkipConfigClear = false;
m_settingsModified = false;
}

/// Slot called when the Apply or OK button is pressed.
Expand Down Expand Up @@ -386,6 +383,10 @@ void DlgPrefSound::connectSoundItem(DlgPrefSoundItem* pItem) {
&DlgPrefSoundItem::selectedChannelsChanged,
this,
&DlgPrefSound::deviceChannelsChanged);
connect(pItem,
&DlgPrefSoundItem::configuredDeviceNotFound,
this,
&DlgPrefSound::configuredDeviceNotFound);
connect(this, &DlgPrefSound::loadPaths, pItem, &DlgPrefSoundItem::loadPath);
connect(this, &DlgPrefSound::writePaths, pItem, &DlgPrefSoundItem::writePath);
if (pItem->isInput()) {
Expand Down Expand Up @@ -678,6 +679,18 @@ void DlgPrefSound::settingChanged() {
m_settingsModified = true;
}

/// Slot called when a device from the config can not be selected, i.e. is
/// currently not available. This may happen during startup when MixxxMainWindow
/// 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
/// apply the (seemingly) unchanged configuration if users simply click Apply/Okay
/// because they are okay to continue without these devices.
void DlgPrefSound::configuredDeviceNotFound() {
m_settingsModified = true;
}

void DlgPrefSound::deviceChanged() {
if (m_loading) {
return;
Expand Down
1 change: 1 addition & 0 deletions src/preferences/dialog/dlgprefsound.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class DlgPrefSound : public DlgPreferencePage, public Ui::DlgPrefSoundDlg {
void settingChanged();
void deviceChanged();
void deviceChannelsChanged();
void configuredDeviceNotFound();
void queryClicked();

private:
Expand Down
10 changes: 7 additions & 3 deletions src/preferences/dialog/dlgprefsounditem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,18 @@ SoundDevicePointer DlgPrefSoundItem::getDevice() const {
return SoundDevicePointer();
}

/// Selects a device in the device combo box given a SoundDevice
/// internal name, or selects "None" if the device isn't found.
/// Selects a device in the device combo box given a SoundDevice' internal name,
/// or selects "None" if the device is nullptr or isn't found.
/// Called only internally via DlPrefSound::loadPaths()
void DlgPrefSoundItem::setDevice(const SoundDeviceId& device) {
int index = deviceComboBox->findData(QVariant::fromValue(device));
//qDebug() << "DlgPrefSoundItem::setDevice" << device;
if (index == -1) {
deviceComboBox->setCurrentIndex(0); // None
emit selectedDeviceChanged();
if (device != SoundDeviceId()) {
// Notify DlgPrefSound that the device that can't be found.
emit configuredDeviceNotFound();
}
} else {
m_emitSettingChanged = false;
deviceComboBox->setCurrentIndex(index);
Expand Down
1 change: 1 addition & 0 deletions src/preferences/dialog/dlgprefsounditem.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class DlgPrefSoundItem : public QWidget, public Ui::DlgPrefSoundItem {
signals:
void selectedDeviceChanged();
void selectedChannelsChanged();
void configuredDeviceNotFound();

public slots:
void refreshDevices(const QList<SoundDevicePointer>& devices);
Expand Down

0 comments on commit 3acd833

Please sign in to comment.