Add support for choosing custom audio input#531
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 838862350d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if nextMode == .manual { | ||
| if self.selectedInputUID.isEmpty, | ||
| let defaultUID = currentSystemInputUID | ||
| { | ||
| self.selectedInputUID = defaultUID | ||
| SettingsStore.shared.recordInputDeviceSelection(defaultUID) | ||
| } else { | ||
| SettingsStore.shared.recordInputDeviceSelection(self.selectedInputUID) | ||
| } |
There was a problem hiding this comment.
When the Settings view is in system mode, selectedInputUID tracks the current macOS default, so turning off “Use macOS default microphone” here overwrites any previously saved manual microphone with that system default. This differs from the menu path, which only initializes preferredInputDeviceUID when it is empty, and it means a user who temporarily switches back to macOS default loses their custom mic preference as soon as they re-enable manual mode unless they reselect it.
Useful? React with 👍 / 👎.
grohith327
force-pushed
the
allow-default-audio-input
branch
from
8388623 to
457af51
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 457af51ff5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SettingsStore.shared.preferredInputDeviceUID = newUID | ||
| // Only change system default if sync is enabled | ||
| if SettingsStore.shared.syncAudioDevicesWithSystem { | ||
| SettingsStore.shared.recordInputDeviceSelection(newUID) |
There was a problem hiding this comment.
In manual mode this onChange fires for programmatic selection changes too, not just user picker choices. For example, SettingsView.onAppear still replaces an invalid/unavailable selectedInputUID with the current default; this line then persists that fallback as PreferredInputDeviceUID, so simply opening Settings while the saved mic is disconnected loses the custom mic that manual mode is meant to preserve. Gate this write to user-initiated picker changes or make the fallback path mode-aware.
Useful? React with 👍 / 👎.
| if let microphoneSelectionMode = payload.microphoneSelectionMode { | ||
| self.microphoneSelectionMode = microphoneSelectionMode | ||
| } |
There was a problem hiding this comment.
When restoring a backup from before this field existed while the current install is in manual mode, the if let leaves microphoneSelectionMode unchanged. Those older backups came from builds where audio selection always followed macOS defaults, so after restore future recordings can keep forcing the restored PreferredInputDeviceUID as a manual mic instead of following the legacy system-mode behavior. Treat nil as .system (or remove the key) during restore.
Useful? React with 👍 / 👎.
grohith327
force-pushed
the
allow-default-audio-input
branch
from
62a0a77 to
6dff573
Compare
|
@altic-dev Good to review, tested with the new audio optimized pipeline |
|
@greptileai review |
Greptile SummaryThis PR replaces the previously no-op
|
Greptile SummaryThis PR introduces a
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case .manual: | ||
| if let prefIn = SettingsStore.shared.preferredInputDeviceUID { | ||
| self.selectedInputUID = prefIn | ||
| } else if let sysIn = AudioDevice.getDefaultInputDevice()?.uid { | ||
| // Fallback to system default if preferred device disconnected | ||
| self.selectedInputUID = sysIn | ||
| SettingsStore.shared.preferredInputDeviceUID = sysIn | ||
| } |
There was a problem hiding this comment.
In .manual mode the hardware-change handler now sets selectedInputUID to the stored preferredInputDeviceUID unconditionally, even when the device was just disconnected. Because refreshDevices() runs first, inputDevices is already up-to-date, so the picker immediately shows a UID that is not in the list — the selection renders blank/missing. The old implementation guarded against this with an availability check and fell back to the system default, which needs to be restored here. Note: MicrophonePreferenceCoordinator.inputDeviceForCapture() does fall back correctly for the actual recording path, but the UI state is left stale.
| case .manual: | |
| if let prefIn = SettingsStore.shared.preferredInputDeviceUID { | |
| self.selectedInputUID = prefIn | |
| } else if let sysIn = AudioDevice.getDefaultInputDevice()?.uid { | |
| // Fallback to system default if preferred device disconnected | |
| self.selectedInputUID = sysIn | |
| SettingsStore.shared.preferredInputDeviceUID = sysIn | |
| } | |
| case .manual: | |
| if let prefIn = SettingsStore.shared.preferredInputDeviceUID, | |
| self.inputDevices.contains(where: { $0.uid == prefIn }) | |
| { | |
| self.selectedInputUID = prefIn | |
| } else if let sysIn = AudioDevice.getDefaultInputDevice()?.uid { | |
| self.selectedInputUID = sysIn | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/ContentView.swift
Line: 365-368
Comment:
**Preferred-device unavailability not handled after hardware change**
In `.manual` mode the hardware-change handler now sets `selectedInputUID` to the stored `preferredInputDeviceUID` unconditionally, even when the device was just disconnected. Because `refreshDevices()` runs first, `inputDevices` is already up-to-date, so the picker immediately shows a UID that is not in the list — the selection renders blank/missing. The old implementation guarded against this with an availability check and fell back to the system default, which needs to be restored here. Note: `MicrophonePreferenceCoordinator.inputDeviceForCapture()` does fall back correctly for the actual recording path, but the UI state is left stale.
```suggestion
case .manual:
if let prefIn = SettingsStore.shared.preferredInputDeviceUID,
self.inputDevices.contains(where: { $0.uid == prefIn })
{
self.selectedInputUID = prefIn
} else if let sysIn = AudioDevice.getDefaultInputDevice()?.uid {
self.selectedInputUID = sysIn
}
```
How can I resolve this? If you propose a fix, please make it concise.| private var _microphonePreferenceCoordinator: MicrophonePreferenceCoordinator? | ||
| var microphonePreferenceCoordinator: MicrophonePreferenceCoordinator { | ||
| if let existing = self._microphonePreferenceCoordinator { | ||
| return existing | ||
| } | ||
| let coordinator = MicrophonePreferenceCoordinator() | ||
| self._microphonePreferenceCoordinator = coordinator | ||
| return coordinator | ||
| } |
There was a problem hiding this comment.
lazy var
The same manual double-check pattern is used for _asr, so it's consistent with the existing style, but MicrophonePreferenceCoordinator is @MainActor-isolated and is accessed from multiple call sites. Swift's lazy var is automatically initialized on first access and is thread-safe for value-type initialization, making intent clearer and removing the possibility of a race if the surrounding class ever stops being exclusively main-actor-accessed.
| private var _microphonePreferenceCoordinator: MicrophonePreferenceCoordinator? | |
| var microphonePreferenceCoordinator: MicrophonePreferenceCoordinator { | |
| if let existing = self._microphonePreferenceCoordinator { | |
| return existing | |
| } | |
| let coordinator = MicrophonePreferenceCoordinator() | |
| self._microphonePreferenceCoordinator = coordinator | |
| return coordinator | |
| } | |
| lazy var microphonePreferenceCoordinator: MicrophonePreferenceCoordinator = MicrophonePreferenceCoordinator() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/AppServices.swift
Line: 71-79
Comment:
**Manual lazy-init pattern is not thread-safe; prefer `lazy var`**
The same manual double-check pattern is used for `_asr`, so it's consistent with the existing style, but `MicrophonePreferenceCoordinator` is `@MainActor`-isolated and is accessed from multiple call sites. Swift's `lazy var` is automatically initialized on first access and is thread-safe for value-type initialization, making intent clearer and removing the possibility of a race if the surrounding class ever stops being exclusively main-actor-accessed.
```suggestion
lazy var microphonePreferenceCoordinator: MicrophonePreferenceCoordinator = MicrophonePreferenceCoordinator()
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| return SettingsStore.shared.preferredInputDeviceUID ?? defaultInputUID | ||
| } | ||
| } | ||
|
|
||
| @objc private func selectMicrophone(_ sender: NSMenuItem) { | ||
| guard self.isRecording == false else { return } | ||
| guard let uid = sender.representedObject as? String, !uid.isEmpty else { return } | ||
|
|
||
| SettingsStore.shared.preferredInputDeviceUID = uid | ||
|
|
||
| if SettingsStore.shared.syncAudioDevicesWithSystem { | ||
| SettingsStore.shared.recordInputDeviceSelection(uid) | ||
| if SettingsStore.shared.shouldSyncInputSelectionToSystemDefault() { | ||
| _ = AudioDevice.setDefaultInputDevice(uid: uid) | ||
| } | ||
|
|
||
| self.refreshMicrophoneMenu() | ||
| } | ||
|
|
||
| @objc private func toggleMicrophoneSelectionMode(_ sender: NSMenuItem) { | ||
| guard self.isRecording == false else { return } | ||
|
|
||
| let nextMode: SettingsStore.MicrophoneSelectionMode = | ||
| SettingsStore.shared.microphoneSelectionMode == .system ? .manual : .system | ||
| let currentSystemInputUID = AudioDevice.getDefaultInputDevice()?.uid | ||
| let availableInputUIDs = Set(AudioDevice.listInputDevices().map(\.uid)) | ||
| let restoredSystemInputUID = SettingsStore.shared.setMicrophoneSelectionMode( | ||
| nextMode, | ||
| currentSystemInputUID: currentSystemInputUID, | ||
| availableInputUIDs: availableInputUIDs |
There was a problem hiding this comment.
MenuBarManager.toggleMicrophoneSelectionMode and SettingsView.updateMicrophoneSelectionMode implement the same "switch mode and seed the manual preference" flow independently. The key difference: the menu-bar path checks SettingsStore.shared.preferredInputDeviceUID to decide whether to seed a default, while the settings-view path checks the local UI binding selectedInputUID. This means flipping the toggle from the menu bar when preferredInputDeviceUID already holds a value will skip seeding (correct), but if the state somehow diverges the two call-sites will behave differently. Extracting the shared transition logic into SettingsStore.setMicrophoneSelectionMode (or a dedicated coordinator method) would remove this maintenance risk.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/MenuBarManager.swift
Line: 621-647
Comment:
**Duplicated mode-toggle logic diverges from SettingsView**
`MenuBarManager.toggleMicrophoneSelectionMode` and `SettingsView.updateMicrophoneSelectionMode` implement the same "switch mode and seed the manual preference" flow independently. The key difference: the menu-bar path checks `SettingsStore.shared.preferredInputDeviceUID` to decide whether to seed a default, while the settings-view path checks the local UI binding `selectedInputUID`. This means flipping the toggle from the menu bar when `preferredInputDeviceUID` already holds a value will skip seeding (correct), but if the state somehow diverges the two call-sites will behave differently. Extracting the shared transition logic into `SettingsStore.setMicrophoneSelectionMode` (or a dedicated coordinator method) would remove this maintenance risk.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Description
Add support for choosing custom audio input separate from MacOS default. The following is the behavior
Also, add a menu bar setting to toggle micrphone
Type of Change
Related Issue or Discussion
Closes #136
Testing
swiftlint --strict --config .swiftlint.yml SourcesScreenshots / Video
fv_audio_input_compressed.mp4