Add AudioRecorder with startup test recording#3226
Conversation
Introduce AudioRecorder, a thread-safe wrapper around AudioRecord that captures 16 kHz mono PCM from the device microphone on a dedicated HandlerThread and delivers chunks via AudioChunkCallback. Wire MicrophoneCommandHandler (previously stubbed) to start/stop the recorder and toggle VAD mode in response to BLE set_mic_state and set_mic_vad_state commands. Register the handler in CommandProcessor. Add a startup test: 10 s after AsgClientService.onCreate() completes, the recorder runs for 30 s then stops automatically. Co-authored-by: Cursor <cursoragent@cursor.com>
📋 PR Review Helper📱 Mobile App Build⏳ Waiting for build... 🕶️ ASG Client Build✅ Ready to test! (commit 🔀 Test Locallygh pr checkout 3226 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
AudioRecord.startRecording() is void and silently discards native errors. Add a getRecordingState() check immediately after the call; if the recorder is not in RECORDSTATE_RECORDING, release the instance and return false instead of posting the read loop on a dead recorder. Root cause observed on device: native start() returned -1 (mic resource conflict with I2S audio path), but the Java layer logged "Audio capture started" and the read loop immediately hit ERROR_INVALID_OPERATION (-3) on every read. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Deploying mentra-live-ota-site with
|
| Latest commit: |
16df055
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1ddd7ecf.mentra-live-ota-site.pages.dev |
| Branch Preview URL: | https://cursor-08828ca3.mentra-live-ota-site.pages.dev |
PR Agent Orchestrator State{
"cycle": 7,
"fixRound": 0,
"totalReviewerRuns": 7,
"consecutiveNoNewReviews": 0,
"openFindings": [
{
"id": "6f4cef",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/asgclientservice.java:993b2871236a",
"source": "depth",
"severity": "blocking",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/AsgClientService.java",
"line": 219,
"message": "scheduleStartupTestRecording() is debug/test code in the production startup path: it unconditionally records the microphone for 30s on every service start, 10s after init, with no user action. Privacy/behavioral regression and contends with the legitimate set_mic_state and I2S audio paths for the single mic resource. Remove before merge.",
"status": "open",
"introducedCycle": 4,
"lastSeenCycle": 4
},
{
"id": "48b16b",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/audio/audiorecorder.java:dc87a9654e6c",
"source": "depth",
"severity": "blocking",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/audio/AudioRecorder.java",
"line": 83,
"message": "AudioRecorder.setCallback() is never called anywhere, so mCallback is always null in readLoop() and all captured PCM is discarded. set_mic_state:ON starts capture and returns true but delivers no audio downstream — the mic feature is non-functional end-to-end.",
"status": "open",
"introducedCycle": 4,
"lastSeenCycle": 4
},
{
"id": "d19c45",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/asgclientservice.java:2e1ed91b9b51",
"source": "depth",
"severity": "blocking",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/AsgClientService.java",
"line": 219,
"message": "scheduleStartupTestRecording() still in production startup path: unconditionally records the mic for 30s on every service start (20s after init) with no user action. Privacy/behavioral regression; contends with set_mic_state and the I2S path for the single mic resource. Remove before merge. (open finding 6f4cef, unresolved)",
"status": "open",
"introducedCycle": 6,
"lastSeenCycle": 6
},
{
"id": "382352",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/audio/audiorecorder.java:0da221804568",
"source": "depth",
"severity": "blocking",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/audio/AudioRecorder.java",
"line": 102,
"message": "AudioRecorder.setCallback() is never called anywhere in production, so mCallback is always null in readLoop() and every PCM chunk is discarded. set_mic_state:ON starts capture and returns true but delivers no audio downstream — mic feature non-functional end-to-end. (open finding 48b16b, unresolved)",
"status": "open",
"introducedCycle": 6,
"lastSeenCycle": 6
},
{
"id": "659daa",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/audio/audiorecorder.java:453953746e46",
"source": "depth",
"severity": "blocking",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/audio/AudioRecorder.java",
"line": 183,
"message": "Race between unsynchronized deferred start runnable and synchronized stop(): if stop() runs after the runnable's mRecording guard but before mAudioRecord is assigned (during buildAudioRecord's probing), stop() releases nothing and disables I2S, then the runnable startRecording()s an AudioRecord that readLoop never reads and nobody releases — leaked open mic handle with I2S gate torn down beneath it; next start() leaks another.",
"status": "open",
"introducedCycle": 6,
"lastSeenCycle": 6
}
],
"resolvedFindings": [],
"nitFindings": [
{
"id": "a82fee",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/audio/audiorecorder.java:3fe468f6e68d",
"source": "depth",
"severity": "nit",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/audio/AudioRecorder.java",
"line": 93,
"message": "setVadEnabled() only sets a flag that readLoop() never reads; VAD mode (set_mic_vad_state) has no functional effect despite reporting success.",
"status": "open",
"introducedCycle": 4,
"lastSeenCycle": 4
},
{
"id": "a26109",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/audio/audiorecorder.java:04e481fe0e34",
"source": "depth",
"severity": "nit",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/audio/AudioRecorder.java",
"line": 282,
"message": "readLoop() allocates a new byte[mChunkBytes] every iteration, causing avoidable GC churn on the audio hot path; reuse a single buffer.",
"status": "open",
"introducedCycle": 4,
"lastSeenCycle": 4
},
{
"id": "03200f",
"fingerprint": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/processors/commandprocessor.java:eb9dbb9ca234",
"source": "depth",
"severity": "nit",
"file": "asg_client/app/src/main/java/com/mentra/asg_client/service/core/processors/CommandProcessor.java",
"line": 112,
"message": "I2S gate is a single non-refcounted boolean (lastI2sPlaying) shared by mic capture, I2SAudioCommandHandler playback, and I2SAudioController. Mic stop() can issue mh_stopi2s while playback is still active (and vice-versa a prior enable makes the mic's mh_starti2s a no-op). Add refcounting/ownership before mic + playback ship together.",
"status": "open",
"introducedCycle": 6,
"lastSeenCycle": 6
}
],
"frozenPair": [
"depth",
"bugbot"
],
"phase": "convergence",
"status": "in_progress",
"lastPair": [
"depth",
"bugbot"
],
"stagnationFixRounds": 1,
"lastOpenCount": 5,
"fingerprintReopenCounts": {},
"mutedFingerprints": []
} |
Prefer MediaRecorder.AudioSource.UNPROCESSED when the device advertises PROPERTY_SUPPORT_AUDIO_SOURCE_UNPROCESSED, bypassing vendor DSP (beamforming, noise suppression, AGC, echo cancellation) that would otherwise be applied upstream by the MTK audio stack. Falls back to MIC on devices that do not support the unprocessed path. Also pass Context through AudioRecorder constructor (required for AudioManager property query) and log the active source on start. Co-authored-by: Cursor <cursoragent@cursor.com>
🤖 PR Agent Review — cycle 7
Claude — depth —
|
|
bugbot run |
Deploying mentra-store-dev with
|
| Latest commit: |
16df055
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d9d48a67.augmentos-appstore-2.pages.dev |
| Branch Preview URL: | https://cursor-08828ca3.augmentos-appstore-2.pages.dev |
Deploying dev-augmentos-console with
|
| Latest commit: |
16df055
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e171eab1.dev-augmentos-console.pages.dev |
| Branch Preview URL: | https://cursor-08828ca3.dev-augmentos-console.pages.dev |
| this.fileManager = fileManager; | ||
| this.rgbLedCommandHandler = rgbLedCommandHandler; | ||
| this.otaCommandHandler = otaCommandHandler; | ||
| this.audioRecorder = new AudioRecorder(context); |
There was a problem hiding this comment.
No audio chunk consumer wired
High Severity
AudioRecorder is created and started from BLE set_mic_state, but nothing in this change calls setCallback. Capture runs and PCM is read on the recorder thread, yet every chunk is dropped because mCallback stays null, so microphone streaming to the phone cannot work.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f510c72. Configure here.
| } | ||
|
|
||
| // Schedule a 30-second test recording that starts 10 seconds after service init. | ||
| scheduleStartupTestRecording(); |
There was a problem hiding this comment.
Validation recording left in onCreate
Medium Severity
scheduleStartupTestRecording() is invoked from onCreate() for every production boot, starting unsolicited 30-second microphone capture ~10 seconds after startup. The PR test plan says to remove this call before merge, but it is still wired into the main service path.
Reviewed by Cursor Bugbot for commit f510c72. Configure here.
Deploying prod-augmentos-account with
|
| Latest commit: |
16df055
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6a02a51b.augmentos-e84.pages.dev |
| Branch Preview URL: | https://cursor-08828ca3.augmentos-e84.pages.dev |
…te control for Mentra Live Changed the audio source to MediaRecorder.AudioSource.MIC, as the unprocessed path is not available on Mentra Live devices. Introduced optional I2S gate callbacks to manage the microphone routing through the BES MCU's I2S bus, ensuring proper initialization before audio recording starts. Updated sample rate to 44.1 kHz to match factory test configurations and adjusted buffer handling accordingly.
Updated the startup test recording to begin 20 seconds after service initialization instead of 10 seconds, ensuring better verification of microphone availability. Adjusted logging to provide clearer error messages in case of microphone unavailability. This change enhances the reliability of audio recording during service startup.
Implemented I2S audio callbacks in the CommandProcessor to manage microphone routing through the BES MCU's I2S bus. This ensures proper initialization of audio recording by notifying the AsgClientService when the I2S audio state changes. This change enhances the audio processing capabilities for Mentra Live devices.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ccf4855. Configure here.
| + " " + record.getSampleRate() + " Hz, " + record.getBufferSizeInFrames() | ||
| + " frames buffer"); | ||
| readLoop(); | ||
| }, I2S_OPEN_DELAY_MS); |
There was a problem hiding this comment.
Stop races deferred AudioRecord init
High Severity
start() sets mRecording and posts delayed work on the recorder thread, but that runnable only checks mRecording once at entry. If stop() runs while buildAudioRecord() is still probing, mAudioRecord may still be null so stop() skips release, yet the runnable can still call startRecording() afterward with mRecording already false, leaving an active AudioRecord that nothing stops.
Reviewed by Cursor Bugbot for commit ccf4855. Configure here.
| }, | ||
| () -> { | ||
| AsgClientService svc = AsgClientService.getInstance(); | ||
| if (svc != null) svc.handleI2SAudioState(false); |
There was a problem hiding this comment.
Mic stop closes shared I2S path
High Severity
The recorder’s I2S disable callback invokes handleI2SAudioState(false), which sends mh_stopi2s and updates global lastI2sPlaying. Stopping the mic after or during I2S speaker playback can tear down the same BES I2S route playback still relies on, with no reference counting between mic capture and I2SAudioController playback.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ccf4855. Configure here.
1 participant
Summary
AudioRecorder— a thread-safe wrapper around Android'sAudioRecordthat captures 16 kHz mono PCM on a dedicatedHandlerThreadand delivers chunks via the existingAudioChunkCallbackinterfaceMicrophoneCommandHandlerto start/stop the recorder and toggle VAD mode in response to BLEset_mic_state/set_mic_vad_statecommands; register the handler inCommandProcessorAsgClientService.onCreate()completes the recorder runs for 30 s then stops automaticallyTest plan
Audio capture started~10 s after boot, thenAudio capture stopped~30 s laterset_mic_state { "enabled": true }from the phone and confirm recording startsset_mic_state { "enabled": false }and confirm recording stopsset_mic_vad_state { "enabled": true/false }and confirm VAD flag toggles in logcatscheduleStartupTestRecording()call before merging if this was only for validationMade with Cursor