fix(audio): play transcription cues with app-local gain - #867
Conversation
Independent Volume wrote the Mac output volume before each cue and restored it on a timer afterwards. That is audible in every other application, which is the spike reported in altic-dev#522, and overlapping cues could restore the wrong level or strand the output at a cue level. Compensate the player toward the selected level instead. When the output scalar is readable and above the mute threshold the player volume becomes min(1, desired / output), so the audible result is min(output, desired) and the current output stays the ceiling. Cues are suppressed at mute or near-zero output rather than raising anything, and an unreadable scalar falls back to the ordinary selected level. The saved-volume state, the delayed restoration, and setSystemVolume are removed outright, so no code path can write the output volume. Cached players and overlap behaviour are unchanged.
Greptile SummaryThis PR replaces temporary system-output-volume writes with app-local transcription-cue gain, preventing cues from changing other applications’ audio.
|
![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: 40dd7c5e56
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The cap is what keeps the audible result at or below the current | ||
| // output: `output * min(1, desired / output)` is `min(output, desired)`. | ||
| return .play(playerVolume: min(1, desired / output)) |
There was a problem hiding this comment.
On output devices whose CoreAudio scalar-to-decibel mapping is nonlinear, dividing desired by the hardware volume scalar does not produce the gain needed to reach the desired audible level: the hardware scalar and AVAudioPlayer.volume are different gain domains. For example, an output scalar of 0.8 and desired scalar of 0.4 need not correspond to a player gain of 0.5, so Independent Volume can play substantially above or below the selected level. Convert the device scalars to decibels using its CoreAudio conversion property, then derive the player gain from the dB difference.
Useful? React with 👍 / 👎.
| /// Independent Volume compensates the player toward the selected level instead | ||
| /// of moving the Mac output volume, so the current output stays the ceiling and | ||
| /// other audio is never touched. |
There was a problem hiding this comment.
When Independent Volume is enabled and system output is below the selected cue level, this new ceiling deliberately makes the cue quieter, but SettingsView.swift:375-377 still promises that the sound stays constant regardless of system volume and warns that playback temporarily changes system volume. Users therefore see a description that is now false in both directions; update that copy alongside this behavioral change.
Useful? React with 👍 / 👎.
|
CI note: the Build FluidVoice failure is |
1 participant
Description
With Independent Volume enabled,
TranscriptionSoundPlayerread the Mac output volume, wrote it to the selected cue level, played the cue at full player gain, and restored the output on a timer. Writing the output volume is globally audible, and that is what #522 reports: with YouTube playing in a tab, the reporter hears a few milliseconds of raised output before it settles.The overlap behaviour made it worse. Both cues share one
savedSystemVolumeslot, so a cue that starts while another is still playing reads the output while it is pinned at the first cue's level and saves that as though it were the user's setting. Depending on which cue finishes first, the output is either stranded at a cue level permanently or snapped back to the pre-cue level while an earlier cue is still audible.This stops writing the output volume at all. When the output scalar is readable and above the mute threshold, the player volume becomes
min(1, desired / output), so the audible result ismin(output, desired)and the current output stays the ceiling. At mute or near zero the cue is suppressed rather than raising anything. If the scalar cannot be read the cue falls back to the plain selected level, which is also what ordinary mode does.savedSystemVolume, the delayed restoration, andsetSystemVolumeare removed rather than patched, so no code path is left that can write the output volume. Cached players and the overlap behaviour of the audio itself are unchanged.The tradeoff, and I think it is the right one: the cue can no longer be louder than the current output. If you keep system audio low and relied on an independently louder cue, the cue is now quieter. I would rather state that limitation than deliver it by moving everyone's volume.
One thing I deliberately left out. The Independent Volume setting copy still describes the old behaviour, and its footnote ("Temporarily changes system volume during playback") is now false. I have that correction ready but kept it out so this PR stays on one surface. Happy to fold it in here if you would rather have it in one piece.
Type of Change
Related Issue or Discussion
Closes #522. This is the app local approach I sketched in that thread.
Testing
swiftlint --strict --config .swiftlint.yml Sourcesxcodebuild test -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS,arch=arm64'swiftformat --config .swiftformat SourcesI ran the linter in the form CI runs it,
swiftlint lint --strict --config .swiftlint.ymlover the whole repository rather than justSources. Clean, 0 violations across 154 files.Eight new tests cover the gain policy as a pure function: ordinary mode, compensation, the
1.0cap, the exact mute threshold and just above it, an unreadable scalar, out of range and NaN inputs, order independence across two overlapping cues, and a grid assertion that the audible level equalsmin(output, desired)and never exceeds the pre cue output. All eight pass.Worth being precise about what those tests do not cover: they exercise the resolver, not production playback. They would not catch a save and restore path being reintroduced around
player.play()later. What rules that out today is thatsetSystemVolumeno longer exists to call.One thing to flag about the local run. The full
xcodebuild testexits 65 on my machine, and it does the same onmainwithout this change. Neither run has a failing assertion. The cause is a double free during test host teardown,pointer being freed was not allocated, which crashes and restarts the runner. I ran the same command againstmainto check: 27 crash and restart cycles in both, the same tests in the same order, the same address every time. So it is pre existing and unrelated, but you will see that exit code if you run the suite locally. It shows up inDictationE2ETests.testAutomaticDictionarySuggestion*and acrossHotkeyShortcutTests, on macOS 15.7.7 with Xcode 26.3. Happy to open a separate issue for it if it is not already tracked.Screenshots / Video
Notes
The capability that goes away is worth naming: previously, when system output was below the selected cue level, the app raised the output so the cue always hit that level. It cannot do that now without affecting other audio, which was the bug. At output at or above the selected level the effective cue loudness is unchanged.
getSystemVolumereturned1.0on failure, which made a read error indistinguishable from full volume. It now returnsFloat?ascurrentOutputVolume, which is what lets the unreadable case fall back deliberately rather than by accident.