Expose a real microphone source instead of the sink monitor - #1804
Expose a real microphone source instead of the sink monitor#1804Zum0DePapaya wants to merge 6 commits into
Conversation
On audioDriver=pulseaudio the only recording device available to games is AAudioSink.monitor, which PulseAudio creates automatically for every sink and which Wine enumerates as a Windows recording device. Games take it for a microphone, so anything with voice chat transmits its own output - in R.E.P.O. that means rebroadcasting the whole lobby back into the lobby. Switching to the ALSA driver gives no capture device at all. Load module-aaudio-source alongside the sink so there is a real capture device. The module makes itself the default source, which is what actually outranks the monitor; PulseAudio 13 cannot suppress a sink's monitor, so it stays in the device list, but it is no longer what a game gets handed by default. The module is only loaded when RECORD_AUDIO has actually been granted. When it has not, the daemon starts exactly as before - it runs with --fail=false, so a missing capture source degrades rather than taking audio down. updateSink() now suspends and resumes the source alongside the sink. Without that a paused game keeps holding the microphone open, which leaves the Android privacy indicator lit and runs into the Android 14+ background-capture restrictions. A failure to suspend the source is logged rather than blocking the pause/resume transition, since the source is optional. RECORD_AUDIO was already declared in the manifest but never requested - there is no AudioRecord anywhere in the codebase. It is now requested from XServerScreen, in context, and only for containers actually using the pulseaudio driver, rather than at app startup. modernXr removes the permission via tools:node="remove", so the request is skipped there. Requires module-aaudio-source in the pulseaudio asset: GameNative/pulseaudio-android#2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesMicrophone audio support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant XServerScreen
participant AndroidPermissionAPI
participant PulseAudioComponent
participant PulseAudioServer
XServerScreen->>AndroidPermissionAPI: Request RECORD_AUDIO when needed
AndroidPermissionAPI-->>XServerScreen: Return permission result
XServerScreen->>PulseAudioComponent: Enable microphone when granted
PulseAudioComponent->>AndroidPermissionAPI: Check RECORD_AUDIO
AndroidPermissionAPI-->>PulseAudioComponent: Return permission state
PulseAudioComponent->>PulseAudioServer: Load optional AAudio capture source
PulseAudioComponent->>PulseAudioServer: Suspend or resume sink and capture source
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 392-411: Update the RECORD_AUDIO result callback in
XServerScreen’s micPermissionLauncher to refresh the active PulseAudioComponent
when permission is granted, ensuring default.pa gains module-aaudio-source for
the current container session; preserve the existing logging and no-op behavior
for denial. Add a test covering permission grant and verifying the PulseAudio
source refresh.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 021ef643-c045-4fe7-8f07-e0e67ee99165
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Two issues raised in review. The daemon reads default.pa once, when it spawns. On a first launch the RECORD_AUDIO dialog is answered while the container is still booting, so the permission often arrives after startPulseAudio() has already decided not to load the capture source - and nothing reloaded it afterwards, leaving no microphone until the game was relaunched. The permission callback now loads the module into the running daemon via pactl, so both orderings work: granted before the daemon spawns goes through the config as before, granted after loads at runtime. The suspend-source failure warning could also never fire for the case it exists to report. micEnabled tracked whether permission was granted, not whether the module actually loaded, and a missing source makes pactl print "Failure: No such entity" - which contains neither "process timeout" nor anything else the check looked for, so a failure read as success. That matters today, since the shipped asset does not carry module-aaudio-source yet. The check now also looks for a pactl failure, and on "no such entity" it stops issuing suspend-source rather than warning on every pause for the session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both findings were valid. Fixed in f703cdb. The dead warning path (cubic,
The first-launch race (both reviewers, Rather than gating startup on the permission result, the callback now loads the module into the already-running daemon with On the suggested unit test — skipping this one deliberately. Both paths are a |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java`:
- Around line 267-273: Update the successful load path in the runtime
capture-source logic to check isPaused.get() and suspend the newly loaded source
when the sink is already paused. Reuse updateSink()’s missing-source failure
handling and only set micEnabled after the source is successfully suspended or
when no suspension is needed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d050b202-c62c-4b0d-a6cb-2f8e14064ede
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Two more issues raised in review, both in the runtime load path added in the previous commit. enableMicrophone() loaded an active source without checking isPaused. Granting RECORD_AUDIO pauses the activity while the dialog is up, so the grant callback can easily fire while audio is suspended - and the new source would then hold the microphone open until the next pause/resume cycle. That is exactly the privacy problem the suspend handling exists to prevent. It now suspends the source immediately when isPaused is set. The success check also treated anything that was not a timeout or an explicit pactl failure as success. ProcessHelper reports a failed exec as "Error: ..." and returns an empty string when nothing was captured, so a module that never loaded could set micEnabled and log that the capture source was ready. Blank and "error"-prefixed output are now failures too. Suspend/resume handling is extracted into updateSource() so both callers share one implementation of the missing-source path rather than duplicating it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both valid again. Fixed in 2a52ee6. Capture source loaded while paused — a fair hit, and the more serious of the two. The comment three lines away in It now suspends the source immediately when Loose success detection — also correct. Confirmed against Worth noting for whoever reviews next: each round has found a real defect in the previous round's fix, which suggests this component has more edge cases than its size implies. None of it is genuinely validated until it runs on a device against an asset containing |
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java">
<violation number="1" location="app/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java:309">
P3: When the capture source is loaded at runtime while audio is already paused (the permission dialog can pause the activity, so this is a real path), `load-module module-aaudio-source` first brings the source up active and only afterwards is it suspended by a separate best-effort `updateSource(true)` call. Because loading an active source while the game/activity is suspended is exactly the background-capture the PR is trying to avoid, there is a brief window where the mic/privacy indicator is open during a paused state; and if the follow-up suspend fails (the suspend command is optional and only logged), the freshly loaded source stays active while the game remains paused for the rest of the session. Consider loading the module directly into a suspended state (if module-aaudio-source supports it) or treating a failed suspend after a successful load as a reason to unload the module again, so the paused/background guarantee holds. This is a minor edge concern given the explicitly non-blocking source handling.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // the microphone open until the next pause/resume cycle. | ||
| if (isPaused.get()) { | ||
| Timber.tag("PulseAudioComponent").d("Audio is paused, suspending the new capture source"); | ||
| updateSource(true); |
There was a problem hiding this comment.
P3: When the capture source is loaded at runtime while audio is already paused (the permission dialog can pause the activity, so this is a real path), load-module module-aaudio-source first brings the source up active and only afterwards is it suspended by a separate best-effort updateSource(true) call. Because loading an active source while the game/activity is suspended is exactly the background-capture the PR is trying to avoid, there is a brief window where the mic/privacy indicator is open during a paused state; and if the follow-up suspend fails (the suspend command is optional and only logged), the freshly loaded source stays active while the game remains paused for the rest of the session. Consider loading the module directly into a suspended state (if module-aaudio-source supports it) or treating a failed suspend after a successful load as a reason to unload the module again, so the paused/background guarantee holds. This is a minor edge concern given the explicitly non-blocking source handling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/xenvironment/components/PulseAudioComponent.java, line 309:
<comment>When the capture source is loaded at runtime while audio is already paused (the permission dialog can pause the activity, so this is a real path), `load-module module-aaudio-source` first brings the source up active and only afterwards is it suspended by a separate best-effort `updateSource(true)` call. Because loading an active source while the game/activity is suspended is exactly the background-capture the PR is trying to avoid, there is a brief window where the mic/privacy indicator is open during a paused state; and if the follow-up suspend fails (the suspend command is optional and only logged), the freshly loaded source stays active while the game remains paused for the rest of the session. Consider loading the module directly into a suspended state (if module-aaudio-source supports it) or treating a failed suspend after a successful load as a reason to unload the module again, so the paused/background guarantee holds. This is a minor edge concern given the explicitly non-blocking source handling.</comment>
<file context>
@@ -264,13 +287,26 @@ public void enableMicrophone() {
+ // the microphone open until the next pause/resume cycle.
+ if (isPaused.get()) {
+ Timber.tag("PulseAudioComponent").d("Audio is paused, suspending the new capture source");
+ updateSource(true);
}
});
</file context>
|
Not fixing this one, because the premise doesn't hold for this particular module — but chasing it turned up a real bug elsewhere, so thanks for it. Why the window doesn't exist: So a freshly loaded source with no client attached is Unloading the module on a failed suspend would also be a net negative: a transient What it did surface. Checking the "does loading actually open the mic" question properly, I found that Fixed in the module PR by resolving it through |
|
@joshuatam , can you help with this one? Requires changes to pulseaudio |
Yes, there is a discussion on code-changes |
Rebuilt from GameNative/pulseaudio-android main with the AAudio capture source module added, so the load-module line added in this branch has something to load. Without it the daemon silently comes up without a source, since it runs with --fail=false. pactl, libprotocol-native.so and module-native-protocol-unix.so are byte-identical to the 20260612 asset. module-aaudio-sink.so is rebuilt and so picks up the 16 KB page alignment from pulseaudio-android#1 (LOAD align goes from 0x1000 to 0x4000); its exported symbols are unchanged. module-aaudio-source.so is new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
module-aaudio-source now resolves AAudioStreamBuilder_setInputPreset via a weak declaration rather than dlsym, matching the sink, so the module binary changed. Only modules/module-aaudio-source.so differs from the previous asset; pactl, libprotocol-native.so, module-native-protocol-unix.so and module-aaudio-sink.so are byte-identical. Filename is unchanged since it is a same-day rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Requesting it from XServerScreen raised a system dialog at the worst possible moment. The LaunchedEffect fired on screen entry, while the container was still booting and before the AndroidView factory had created xServerView, and the dialog backgrounds the activity for as long as it is up. Two consequences: - setupXEnvironment finishes with isActivityInForeground false, so the branch right after it suspends the environment immediately; on the manual-resume suspend policy the user has to resume a game they just launched. - onActivityDestroyed calls exit(xServerView!!...) with no null guard, and it is registered during composition, well before xServerView is assigned. Anything that destroys the activity while the dialog is up (don't-keep-activities, memory pressure, a configuration change) lands on that non-null assertion. The sharp edge already existed; putting a dialog there made it far easier to reach. Moving the request next to the existing POST_NOTIFICATIONS one resolves the permission before any container exists, so default.pa is written correctly at daemon spawn and neither problem is reachable. The two are sequenced rather than fired together, since the platform drops a permission request made while another dialog is showing. The trade-off is that the prompt no longer carries launch context and is shown to users who never touch voice chat. Reported by @joshuatam. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Good catch on the permission request location — you were right, and for two reasons rather than one. Both come from the dialog backgrounding the activity while the container is still booting:
Being straight about ownership on the second one: that null window exists on Moved the request to
The trade-off is the prompt losing its launch context and being shown to people who never touch voice chat. If you'd prefer it elsewhere, happy to move it again. One caveat: I couldn't reproduce the crash directly — that needs an install under the real |
3 participants
Description
On
audioDriver=pulseaudiothe only recording device available to games isAAudioSink.monitor— the loopback PulseAudio creates automatically for every sink, which Wine enumerates as a Windows recording device. Games take it for a microphone, so anything with voice chat transmits its own output. In R.E.P.O. that means rebroadcasting the whole lobby back into the lobby. Switching to the ALSA driver gives no capture device at all (Microphone.devices is empty).This loads
module-aaudio-sourcealongside the sink so there is a real capture device. The module makes itself the default source, which is what actually outranks the monitor.Discussed in
#code-changesbeforehand.Paired with GameNative/pulseaudio-android#2, which is where the module itself lives. The compiled asset is bundled here (see below) so this branch is testable on its own, but #2 should land first so the asset is reproducible from source rather than being a binary nobody can regenerate.
What changed
app/src/main/assets/pulseaudio-gamenative-20260810.tzst(replaces-20260612)pulseaudio-androidmainwithmodule-aaudio-source.soadded. Without it theload-moduleline below has nothing to load, and since the daemon runs with--fail=falsethat failure is silent.pactl,libprotocol-native.soandmodule-native-protocol-unix.soare byte-identical.module-aaudio-sink.sois rebuilt, so it picks up the 16 KB page alignment from pulseaudio-android#1 —LOAD aligngoes0x1000→0x4000— with its exported symbols unchanged.module-aaudio-source.sois new, also 16 KB aligned.pa_*symbols the new module needs resolve against the shippedlibpulsecore/libpulsecommon/libpulse.XServerScreen.kt'srefreshComponentsFiles()reference is bumped to the new filename, following the existing convention of date-stamping the asset and dropping the old one.PulseAudioComponent.javaload-module module-aaudio-sourceline to the generateddefault.pa, only whenRECORD_AUDIOhas actually been granted. When it hasn't, the config is byte-identical to today's.updateSink()now suspends and resumes the source alongside the sink. Without that a paused game keeps holding the microphone open, leaving the Android privacy indicator lit and running into Android 14+ background-capture restrictions. A failure to suspend the source is logged rather than blocking the pause/resume transition, since the source is optional.MainActivity.ktRECORD_AUDIOat startup, next to the existingPOST_NOTIFICATIONSrequest. It was already declared in the manifest but never requested anywhere — there is noAudioRecordin the codebase at all.modernXr, which removes the permission viatools:node="remove".No
set-default-sourcecall is needed; the module does it itself.Why the prompt moved out of
XServerScreen(@joshuatam)An earlier revision requested the permission from
XServerScreen, gated on the container actually using the pulseaudio driver, so that the prompt carried launch context. @joshuatam flagged that location as crash-prone, and he was right — for two separate reasons, both stemming from the dialog backgrounding the activity while the container is mid-boot:setupXEnvironmentfinishes withisActivityInForegroundfalse, so the branch immediately after it suspends the environment. On the manual-resume suspend policy that leaves the user having to resume a game they just launched.onActivityDestroyedcallsexit(xServerView!!…)with no null guard, and it is registered during composition — well before theAndroidViewfactory assignsxServerView. Anything that destroys the activity while the dialog is up (don't-keep-activities, memory pressure, a configuration change) lands on that non-null assertion.To be clear about ownership: that second one is a pre-existing sharp edge, not something this PR introduced — the null window between registering the handler and creating the view is there on
mastertoday. But putting a system dialog at exactly that moment made it far easier to reach, which is a good enough reason not to do it. Worth hardening separately if you agree; I've left it alone here to keep this PR to its own scope.Requesting at startup resolves the permission before any container exists, so
default.pais written correctly at daemon spawn and neither problem is reachable. The cost is that the prompt no longer carries context and is shown to users who will never touch voice chat. If you'd rather have it somewhere else — a container setting would be the nicest UX, though that's UI and so out of scope perCONTRIBUTING.md— say the word.I have not been able to reproduce the crash directly, since testing needs an install under the real
app.gamenativeID; the reasoning above is from the code paths rather than a captured stack trace.Testing
Verified on a Poco X4 GT (Android 14, bionic container,
proton-10.0-arm64ec-2) using an equivalent build withmodule-pipe-sourcestanding in for the real module, since the asset doesn't carrymodule-aaudio-sourceyet:Separately confirmed that Wine enumerates such a source as a Windows recording device and that its default capture endpoint follows PulseAudio's default source (tested with a mingw-built winmm + mmdevapi probe under Wine 9.0 / PA 16.1 on x86_64, not arm64ec Proton, so ordering could differ there).
Also confirmed AAudio input works from a bare native process with no Java context — 48 kHz mono PCM_I16 with the
VOICE_COMMUNICATIONpreset, 144000/144000 frames over 3s — which is whatmodule-aaudio-sourcerelies on, since the daemon is a forked child of the app.The bundled asset itself is verified as far as packaging goes: I read the
.tzstback out of the built APK and confirmed it matches what I built (rather than a stale merged copy — Gradle has bitten me on that here before), and thatmodule-aaudio-source.sois present inside it and 16 KB aligned.Not yet verified end to end: voice actually transmitting in R.E.P.O. with the real module — that is the one remaining gap, and it needs an install under the real
app.gamenativeapplication ID. Worth knowing that the monitor remains in the device list at a lower index than the real source — PulseAudio 13 can't suppress a sink's monitor — so an app that picks by index rather than using the default endpoint could still land on it.Recording
Still to come. Now that the asset is in the branch this is unblocked, so I'll add one from a real R.E.P.O. session rather than the
module-pipe-sourcestand-in.Draft
Left as a draft at @joshuatam's request so the changes can be reviewed piece by piece.
Type of Change
Checklist
#code-changesand green-litCONTRIBUTING.mdSummary by cubic
Expose a real microphone to games by loading a PulseAudio capture source instead of the
AAudioSink.monitorloopback. The source becomes default whenRECORD_AUDIOis granted, can be loaded into a running daemon, and is immediately suspended if audio is already paused.Bug Fixes
module-aaudio-sourcewith the sink whenRECORD_AUDIOis granted; if granted late, load it viapactland suspend it if audio is paused (no relaunch).pactlfailures, blank/error output, or “No such entity”, log and stop issuing source suspend calls; never block pause/resume.RECORD_AUDIOat container launch only forpulseaudiocontainers; skipped onmodernXr.Dependencies
pulseaudio-gamenative-20260810.tzstwithmodule-aaudio-source; rebuilt same day after switching the module to a weak symbol forAAudioStreamBuilder_setInputPreset(only the source module changed; filename unchanged). Older assets still fall back to current behavior.Written for commit 80448b7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes