Fixed ScreenAudioCapturer crash, stale audio replay, and AudioRecord leak#982
Conversation
…fter MediaProjection stops
🦋 Changeset detectedLatest commit: 80760fc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
initAudioRecord runs on WebRTC's audio record thread, while releaseAudioResources is called by the app from a thread of its choosing. A release landing between AudioRecord creation and its assignment to the field saw a null audioRecord, did nothing, and left init to publish a recording AudioRecord that nothing owned. Publication and release now share a lock, and a released capturer stays released, so an init that finishes after a release discards its AudioRecord instead of stranding it. Leaked recorders hold the playback capture input open: with 100 racing release/init pairs on an Android 17 emulator, 9 recorders were stranded and the remaining 91 creation attempts failed with "could not open input for device AUDIO_DEVICE_IN_REMOTE_SUBMIX". After the fix all 100 discard cleanly and no creation fails.
adrian-niculescu
changed the title
| val audioCaptureConfig = AudioPlaybackCaptureConfiguration.Builder(mediaProjection) | ||
| .apply(captureConfigurator) | ||
| .build() | ||
| val channelMask = if (channelCount == 1) AudioFormat.CHANNEL_IN_MONO else AudioFormat.CHANNEL_IN_STEREO |
There was a problem hiding this comment.
nit, can you add an assert in this function that channelCount is either 1 or 2 ?
| val channelMask = if (channelCount == 1) AudioFormat.CHANNEL_IN_MONO else AudioFormat.CHANNEL_IN_STEREO | ||
|
|
||
| val minBufferSize = AudioRecord.getMinBufferSize(sampleRate, channelMask, audioFormat) | ||
| if (minBufferSize == AudioRecord.ERROR || minBufferSize == AudioRecord.ERROR_BAD_VALUE) { |
There was a problem hiding this comment.
nit, will be be slightly less error prone to do
if (minBufferSize <= 0) {
LKLog.e { "AudioRecord.getMinBufferSize error: $minBufferSize" }
return false
}
| } | ||
| LKLog.v { "AudioRecord.getMinBufferSize: $minBufferSize" } | ||
| val audioRecord = try { | ||
| val audioCaptureConfig = AudioPlaybackCaptureConfiguration.Builder(mediaProjection) |
There was a problem hiding this comment.
I think AudioPlaybackCaptureConfiguration requires secific Android version, any idea if that is higher than our SDK's minimal supported version ?
| synchronized(audioRecordLock) { | ||
| isReleased = true | ||
| audioRecord?.release() | ||
| audioRecord = null |
There was a problem hiding this comment.
rather than setting isReleased to true here, should we allow the app to auto recover from some read failures?
Like if (!isReleased) { hasInitialized = false }
So that the onBufferRequest() will recreate the buffer again.
| // On a short or failed read, the buffer contents are stale; skip mixing rather than replay them. | ||
| if (readResult < 0) { | ||
| LKLog.w { "AudioRecord.read failed: $readResult. Stopping screen share audio capture." } | ||
| releaseAudioResources() |
There was a problem hiding this comment.
From this code, it seems that one negative read will permanently disable screen share audio until the apps restart the screen audio capturer ?
What negative errors audioRecord.read() can give out ? should some of the errors be recoverable ?
2 participants
When the MediaProjection backing a ScreenAudioCapturer stops (the user ends the share from the status bar chip, or another app takes over the projection), two things go wrong:
initAudioRecordthrows on WebRTC's audio record thread:AudioRecord.Builder.build()fails withUnsupportedOperationException, and the playback capture config builder can throwIllegalStateException. OnlystartRecording()was guarded, and the audio record thread has no handler around the buffer callback, so the process dies.AudioRecord.read()starts failing, but the return value was ignored and the buffer still holds the previous frame. That frame kept getting mixed into the mic track on every callback, so listeners heard the last captured 10 ms looping until the callback was detached.initAudioRecordnow returns false on any creation failure, matching its existing degrade paths, and releases the record if it never reaches the recording state.onBufferRequestonly mixes fully read buffers; on a read error it logs, releases the AudioRecord, and the track degrades to mic-only audio. Read errors are not alwaysERROR_DEAD_OBJECT(Android 17 returnsERROR_BAD_VALUEafter the record restore fails), so any negative result is treated as terminal.The second commit fixes a related problem in the teardown path rather than in projection loss.
releaseAudioResources()is called by the app from a thread of its choosing, whileinitAudioRecordruns on the audio record thread. A release landing betweenAudioRecord.Builder.build()and the assignment to the field saw a nullaudioRecord, did nothing, and left init to publish a recordingAudioRecordthat nothing owned. Publication and release now share a lock, and a released capturer stays released, so an init finishing after a release discards its record instead of stranding it.Verified with the screenshare-audio example on an Android 17 Pixel 6a and and an emulator: stopping the share from the status bar chip while the capturer is attached logs a single
AudioRecord.read failed: -2. Stopping screen share audio capture.and the session continues on mic audio with no crash. For the teardown race, driving 100 racing release/init pairs against a real MediaProjection stranded 9 recorders before the fix, and the remaining 91 creation attempts then failed withcould not open input for device AUDIO_DEVICE_IN_REMOTE_SUBMIX, so the leak also breaks later capture attempts. After the fix all 100 discard cleanly and none fail../gradlew test, spotless, and detekt pass.