feat(android): add input_preset to StreamConfig#995
feat(android): add input_preset to StreamConfig#995TD-Sky wants to merge 8 commits intoRustAudio:masterfrom
input_preset to StreamConfig#995Conversation
|
Looking at the Github Actions results, it seems you left out some platform specific flags. Unfortunately, I can't edit your code, but it may be best to ensure this doesn't fail other systems. Take a look at this: MacOS: Would it be possible to go back and prevent other platforms from attempting to use this code, or fix the cross-platform issues? |
All fixed. |
roderickvd
left a comment
roderickvd
left a comment
There was a problem hiding this comment.
Nice addition. If you would take a look at the review comments and add a changelog entry?
|
|
||
| [features] | ||
| asio = ["asio-sys", "num-traits"] # Only available on Windows. See README for setup instructions. | ||
| android-input-preset = ["ndk/api-level-28"] |
There was a problem hiding this comment.
I see this requires api-level-28 where the normal feature requires api-level-26. That OK?
There was a problem hiding this comment.
I don't see any conflicts between the features in ndk document and didn't find any errors about api levels when I used the branch of this PR in my own project.
There was a problem hiding this comment.
Well, if it works, it works. As it does bump the minimum Android version, it would be a good thing to document. Would you document the feature flag, as well as add an entry to the changelog?
| channels: self.channels, | ||
| sample_rate: self.sample_rate, | ||
| buffer_size: BufferSize::Default, | ||
| #[cfg(all(target_os = "android", feature = "android-input-preset"))] |
There was a problem hiding this comment.
Would it make sense to rename the feature flag to something more specific like "android-voice-recognition" or "android-aec"? Because it seems like there are more input presets that one could choose, if not today then in the future.
There was a problem hiding this comment.
I think it better to keep the name because AEC is not the only function of the flag.
There was a problem hiding this comment.
That's what I was trying to say. Looking at the AudioInputPreset enum there are six variants today. How can we make the feature flag clearly convey which preset gets enabled?
Edit: what would be a good way going forward to support the other variants as well?
There was a problem hiding this comment.
Pull Request Overview
This PR adds Android-specific audio input preset support to enable Acoustic Echo Canceler (AEC) functionality through the VOICE_COMMUNICATION setting. This allows developers to leverage Android's built-in audio preprocessing features for better audio quality in voice applications.
- Adds
input_presetfield toStreamConfigwith Android-specific feature gating - Integrates the preset configuration into the AAudio stream builder
- Includes necessary feature flag and dependency configuration
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/platform/mod.rs | Exports AudioInputPreset type under feature flag |
| src/lib.rs | Adds input_preset field to StreamConfig with default value |
| src/host/aaudio/mod.rs | Integrates input preset into stream builder and includes code cleanup |
| Cargo.toml | Adds android-input-preset feature flag with NDK dependency |
Comments suppressed due to low confidence (1)
src/host/aaudio/mod.rs:1
- This import removal appears unrelated to the input preset feature. Consider separating cleanup changes into a separate commit to keep the feature implementation focused.
use std::cmp;
|
This is a candidate for opt-in with #1010 together with the other audio input presets. |
3 participants
Users can enable AEC (Acoustic Echo Canceler) via setting
VOICE_COMMUNICATIONfor recording audio on Android (refer to here), which is a powerful and important feature for developing Android applications. However, I find no ways to enable this feature in cpal at present. So I add it here.