[4.7.0] Audio API section: various fixes#32945
[4.7.0] Audio API section: various fixes#32945RomanPudashkin wants to merge 2 commits intomusescore:4.7from
Conversation
📝 WalkthroughWalkthroughAdds logic to resolve ASIO driver device IDs before loading, and adds UI-model validation when switching audio APIs: if the newly selected API has no output devices, an interactive warning is shown and the selection reverts to the previous API. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Prefs as Preferences UI
participant Model as AudioMidiPreferencesModel
participant Controller as AudioDriverController
participant Driver as AsioAudioDriver
participant Dialog as Interactive Warning
User->>Prefs: Select audio API
Prefs->>Model: setCurrentAudioApiIndex()
Model->>Model: Save fallback API
Model->>Model: Register one-shot availableOutputDevicesChanged listener
Model->>Controller: changeCurrentAudioApi(spec)
Controller->>Driver: open(spec)
Driver->>Driver: Resolve deviceId (use defaultDevice() if DEFAULT_DEVICE_ID)
Driver->>Driver: loadDriver(resolvedDeviceId)
Controller-->>Model: Driver initialized / opened
Model->>Model: availableOutputDevicesChanged() triggers
alt no output devices
Model->>Dialog: Show interactive warning
Dialog->>User: Inform & prompt
User->>Dialog: Confirm
Model->>Model: Revert to fallback API
Model->>Prefs: Emit currentAudioApiIndexChanged()
else devices present
Model->>Prefs: Complete selection
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
RomanPudashkin
force-pushed
the
audio_api_section_fixes_470
branch
from
033687c to
7b880aa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/framework/audio/main/internal/audiodrivercontroller.cpp (1)
227-235:⚠️ Potential issue | 🟠 MajorDo not persist unavailable API selection.
Line 234 updates config even when no device exists (Lines 230-232) or open fails, so the UI can still remain on an unavailable API.
💡 Proposed fix
- if (!spec.deviceId.empty()) { + bool opened = false; + if (!spec.deviceId.empty()) { spec.deviceId = DEFAULT_DEVICE_ID; - m_audioDriver->open(spec, nullptr); + opened = m_audioDriver->open(spec, nullptr); } else { LOGW() << "No devices for " << name; } - configuration()->setCurrentAudioApi(name); + if (!opened) { + LOGW() << "Audio API switch aborted, unavailable device(s) for " << name; + return; + } + + configuration()->setCurrentAudioApi(name);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 58d4b731-f8ae-4f06-86a9-1f088ac44117
📒 Files selected for processing (14)
src/framework/audio/driver/platform/win/asio/asioaudiodriver.cppsrc/framework/audio/main/internal/audiodrivercontroller.cppsrc/preferences/CMakeLists.txtsrc/preferences/internal/preferencesconfiguration.cppsrc/preferences/internal/preferencesconfiguration.hsrc/preferences/ipreferencesconfiguration.hsrc/preferences/preferencesmodule.cppsrc/preferences/preferencesmodule.hsrc/preferences/qml/MuseScore/Preferences/AudioMidiPreferencesPage.qmlsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cppsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.hsrc/preferences/qml/MuseScore/Preferences/internal/AudioApiSection.qmlsrc/preferences/qml/MuseScore/Preferences/internal/ComboBoxWithTitle.qmlsrc/preferences/qml/MuseScore/Preferences/internal/CommonAudioApiConfiguration.qml
src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cpp
Outdated
Show resolved
Hide resolved
src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cpp
Outdated
Show resolved
Hide resolved
src/preferences/qml/MuseScore/Preferences/internal/AudioApiSection.qml
Outdated
Show resolved
Hide resolved
RomanPudashkin
force-pushed
the
audio_api_section_fixes_470
branch
from
7b880aa to
47132cd
Compare
RomanPudashkin
force-pushed
the
audio_api_section_fixes_470
branch
from
47132cd to
8f49b9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/preferences/qml/MuseScore/Preferences/internal/AudioApiSection.qml (1)
60-75:⚠️ Potential issue | 🟠 MajorThe warning button approach may not fully address issue
#31664.The button only appears when
outputDeviceCount === 0, but users must manually discover and click it to see the warning. Issue#31664requests that ASIO selection should be prevented or a warning shown proactively when no ASIO device is present.Consider automatically triggering
showNoOutputDevicesWarningRequested()when:
- The audio API is changed to one with no available devices, or
- The component loads with an API that has no devices
This could be done via a
Component.onCompletedcheck or anonOutputDeviceCountChangedhandler that auto-fires the warning under specific conditions (respecting the "don't show again" preference).src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cpp (1)
159-172:⚠️ Potential issue | 🟠 MajorMove the opt-out guard into
showNoOutputDevicesWarning()itself.This method is
Q_INVOKABLEand can be called directly from QML (e.g.,AudioMidiPreferencesPage.qml) without going throughsetCurrentAudioApiIndex(). TheshouldShowNoOutputDevicesWarning()guard at line 54 only protects the C++ call path, allowing the QML call path to bypass the user's "don't show again" preference.,
🛡️ Proposed fix to add internal guard
void AudioMidiPreferencesModel::showNoOutputDevicesWarning() { + if (!preferencesConfiguration()->shouldShowNoOutputDevicesWarning()) { + return; + } + auto promise = interactive()->warning( muse::trc("preferences", "No output devices available"),
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e7bce1ef-5524-40f2-9b18-7d21620efb91
📒 Files selected for processing (12)
src/preferences/CMakeLists.txtsrc/preferences/internal/preferencesconfiguration.cppsrc/preferences/internal/preferencesconfiguration.hsrc/preferences/ipreferencesconfiguration.hsrc/preferences/preferencesmodule.cppsrc/preferences/preferencesmodule.hsrc/preferences/qml/MuseScore/Preferences/AudioMidiPreferencesPage.qmlsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cppsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.hsrc/preferences/qml/MuseScore/Preferences/internal/AudioApiSection.qmlsrc/preferences/qml/MuseScore/Preferences/internal/ComboBoxWithTitle.qmlsrc/preferences/qml/MuseScore/Preferences/internal/CommonAudioApiConfiguration.qml
RomanPudashkin
force-pushed
the
audio_api_section_fixes_470
branch
from
8f49b9b to
d23ce05
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a448c34a-bb5f-4717-8ef5-59d6ce5ec6ee
📒 Files selected for processing (2)
src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cppsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.h
RomanPudashkin
force-pushed
the
audio_api_section_fixes_470
branch
from
d23ce05 to
f904dbe
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cpp (1)
70-72:⚠️ Potential issue | 🟡 MinorRemove the redundant rollback signal emission.
changeCurrentAudioApi(fallbackApi)already triggerscurrentAudioApiChanged(), andinit()forwards that tocurrentAudioApiIndexChanged()at Lines 134-136. Emitting it again here double-notifies the UI on the rollback path.♻️ Suggested simplification
promise.onResolve(this, [this, fallbackApi](const muse::IInteractive::Result&) { audioDriverController()->changeCurrentAudioApi(fallbackApi); - emit currentAudioApiIndexChanged(currentAudioApiIndex()); });
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 79bc7be8-7b70-4690-b3cd-73910e68ac61
📒 Files selected for processing (3)
src/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.cppsrc/preferences/qml/MuseScore/Preferences/audiomidipreferencesmodel.hsrc/preferences/qml/MuseScore/Preferences/internal/AudioApiSection.qml
|
One observation:
In this case, the ASIO driver can only be selected after restarting MuseScore Studio. |
|
Second observation: When selecting ASIO as the driver, the device field remains empty, even if there's only one available device. Is it possible to enable an available device as soon as ASIO is selected? If there's only one, enable it. If there's more than one, just pick one :-) Screen.Recording.2026-04-09.161603.mp4 |
3 participants
Resolves: #31663
Resolves: #31664
Summary by CodeRabbit
New Features
Bug Fixes