feat(android): AudioRecorder input preset option (androidInputPreset) - #1210
feat(android): AudioRecorder input preset option (androidInputPreset)#1210lpmv wants to merge 3 commits into
Conversation
The Oboe input stream builder never calls setInputPreset, so every capture
stream runs on Oboe's implicit default, InputPreset::VoiceRecognition - the
speech-recognition preprocessing chain, which applies no acoustic echo
cancellation. For duplex voice apps (playing audio through the same device
while recording, VoIP-style) that makes Android capture echo-raw, while the
equivalent iOS setup gets AEC from the voiceChat session mode.
This adds an optional constructor option to AudioRecorder:
new AudioRecorder({ androidInputPreset: 'voiceCommunication' })
mapping to Oboe's InputPreset on the capture stream builder. When the option
is omitted (or names an unknown preset) no setInputPreset call is made, so
existing behavior is preserved exactly. iOS ignores the option; its input
chain is selected by the AVAudioSession mode instead.
Presets exposed: generic, camcorder, voiceRecognition, voiceCommunication,
unprocessed, voicePerformance.
|
looks similar to #1216, will probably try to merge your both approaches into one common api, but need some time to test it |
| var createAudioRecorder: ( | ||
| androidInputPreset: string, | ||
| iosVoiceProcessing: boolean | ||
| ) => IAudioRecorder; |
There was a problem hiding this comment.
Wouldn't it be better if we passed an object instead of individual props? In case we would add more options.
There was a problem hiding this comment.
withDefaultOptions helper seems ambiguous now. I think it could handle both file and preset options or split into two verbose helpers then merge, but now the logic is a bit scattered.
| IOSAudioRecorder::IOSAudioRecorder( | ||
| const std::shared_ptr<IAudioEventHandlerRegistry> &audioEventHandlerRegistry) | ||
| const std::shared_ptr<IAudioEventHandlerRegistry> &audioEventHandlerRegistry, | ||
| bool voiceProcessingEnabled) |
There was a problem hiding this comment.
Again, shoudn't this be a member of some struct IOSAudioRecorderOptions?
| @@ -29,7 +29,9 @@ class AudioFileWriter; | |||
|
|
|||
| class IOSAudioRecorder : public AudioRecorder { | |||
| std::string androidInputPreset; | ||
| if (count > 0 && args[0].isString()) { | ||
| androidInputPreset = args[0].getString(runtime).utf8(runtime); | ||
| } | ||
|
|
||
| bool iosVoiceProcessing = false; | ||
| if (count > 1 && args[1].isBool()) { | ||
| iosVoiceProcessing = args[1].getBool(); | ||
| } | ||
|
|
||
| auto audioRecorderHostObject = std::make_shared<AudioRecorderHostObject>( | ||
| audioEventHandlerRegistry, &runtime, jsCallInvoker); | ||
| audioEventHandlerRegistry, | ||
| &runtime, | ||
| jsCallInvoker, | ||
| androidInputPreset, | ||
| iosVoiceProcessing); |
There was a problem hiding this comment.
Do we have to do this parsing inline? Maybe we could extract this logic?
| explicit AndroidAudioRecorder( | ||
| const std::shared_ptr<IAudioEventHandlerRegistry> &audioEventHandlerRegistry); | ||
| const std::shared_ptr<IAudioEventHandlerRegistry> &audioEventHandlerRegistry, | ||
| const std::string &inputPreset = ""); |
There was a problem hiding this comment.
Why is this a reference? If AndroidAudioRecorder is the utlimate owner of inputPreset I think it should just use std::move here.
| | `options` <Optional /> | [`AudioRecorderStartOptions`](#audiorecorderstartoptions) | Optional recording start configuration. | | ||
|
|
||
| #### Returns `Promise<Result<{}>>`. | ||
| ###### Returns `Promise<Result<{}>>`. |
There was a problem hiding this comment.
Is there a reason for incrementing the heading depth?
There was a problem hiding this comment.
yes, with those options
---
sidebar_position: 4
toc_min_heading_level: 2
toc_max_heading_level: 5
---
h5 also is rendered on the right side as "clickable" shortcut which for sure we don't want to have
| | 'voicePerformance'; | ||
| ``` | ||
|
|
||
| Maps to the Android [audio input preset](https://developer.android.com/ndk/reference/group/audio#anonymous-enum-9) of the capture stream. |
| export { default as Audio } from './Audio'; | ||
| export { default as AudioControls } from './Audio/controls/AudioControls'; | ||
| export type { MediaElementAudioSourceOptions } from './core/MediaElementAudioSourceNode'; | ||
| export type { | ||
| AudioRecorderOptions, | ||
| AndroidInputPreset, | ||
| } from './core/AudioRecorder'; | ||
| export type { default as AudioEventSubscription } from './events/AudioEventSubscription'; | ||
| export { default as FilePreset } from './utils/filePresets'; | ||
|
|
There was a problem hiding this comment.
Is it really a react component used in <audio>?
Why
The Oboe input stream builder in
AndroidAudioRecorder::openAudioStreamnever callssetInputPreset, so every capture stream runs on Oboe's implicit default,InputPreset::VoiceRecognition— the speech-recognition preprocessing chain, which applies no acoustic echo cancellation. For duplex voice apps (playing TTS/voice through the same device while recording, VoIP-style — the use case asked about in #670), that makes Android capture echo-raw, while the equivalent iOS setup gets AEC from thevoiceChatsession mode.What
An optional constructor option on
AudioRecorder:mapped to Oboe's
InputPreseton the capture stream builder.voiceCommunicationengages the platform AEC/NS chain — the Android twin of iOSvoiceChat.setInputPresetcall is made, so existing behavior is preserved exactly.generic,camcorder,voiceRecognition,voiceCommunication,unprocessed,voicePerformance.Notes
We measured the difference in a duplex probe (1024-sample pcm16 frames at 24 kHz, speaker playback at full volume while recording): on iOS under
voiceChatthe played audio re-enters the mic at +1.9 dB over the room floor; Android capture through the current builder has no platform AEC path at all. Happy to adjust the option's shape (e.g. a generalAudioRecorderOptionsbag was chosen so future per-platform stream options have a home).