Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose custom configuration of internal AEC3 #44

Open
wants to merge 1 commit into
base: upgrade-v1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ impl From<HighPassFilter> for ffi::AudioProcessing_Config_HighPassFilter {
}
}

/// [Highly Experimental] Configurations of internal AEC3 implementation.
/// TODO(skywhale): Add more parameters from:
/// https://gitlab.freedesktop.org/pulseaudio/webrtc-audio-processing/-/blob/master/webrtc/api/audio/echo_canceller3_config.h
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
pub struct EchoCanceller3Config {
/// Number of linear filters to apply.
pub num_filters: usize,
}

impl Default for EchoCanceller3Config {
fn default() -> Self {
Self { num_filters: 5 }
}
}

impl From<EchoCanceller3Config> for ffi::EchoCanceller3ConfigOverride {
fn from(other: EchoCanceller3Config) -> Self {
Self { num_filters: other.num_filters as i32 }
}
}

/// AEC (acoustic echo cancellation) configuration.
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "derive_serde", derive(Serialize, Deserialize))]
Expand Down
40 changes: 34 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
mod config;
mod stats;

use std::{error, fmt, sync::Arc};
use std::{error, fmt, ptr::null, sync::Arc};
use webrtc_audio_processing_sys as ffi;

pub use config::*;
Expand Down Expand Up @@ -49,7 +49,15 @@ impl Processor {
/// instantiation, however new configs can be be passed to `set_config()`
/// at any time during processing.
pub fn new(config: &InitializationConfig) -> Result<Self, Error> {
let inner = Arc::new(AudioProcessing::new(config)?);
Self::with_aec3_config(config, None)
}

/// Creates a new `Processor` with custom AEC3 configuration.
pub fn with_aec3_config(
config: &InitializationConfig,
aec3_config: Option<EchoCanceller3Config>,
) -> Result<Self, Error> {
let inner = Arc::new(AudioProcessing::new(config, aec3_config)?);
let num_samples = inner.num_samples_per_frame();
Ok(Self {
inner,
Expand All @@ -64,6 +72,13 @@ impl Processor {
})
}

/// Initializes internal states, while retaining all user settings. This should be called before
/// beginning to process a new audio stream. However, it is not necessary to call before processing
/// the first stream after creation.
pub fn initialize(&mut self) {
self.inner.initialize()
Comment on lines +75 to +79
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I guess the idea is we could use this from our portal code?

}

/// Processes and modifies the audio frame from a capture device by applying
/// signal processing as specified in the config. `frame` should hold an
/// interleaved f32 audio frame, with NUM_SAMPLES_PER_FRAME samples.
Expand Down Expand Up @@ -182,13 +197,23 @@ struct AudioProcessing {
}

impl AudioProcessing {
fn new(config: &InitializationConfig) -> Result<Self, Error> {
fn new(
config: &InitializationConfig,
aec3_config: Option<EchoCanceller3Config>,
) -> Result<Self, Error> {
let aec3_config = if let Some(aec3_config) = aec3_config {
&aec3_config.into() as *const ffi::EchoCanceller3ConfigOverride
} else {
null()
};

let mut code = 0;
let inner = unsafe {
ffi::audio_processing_create(
config.num_capture_channels as i32,
config.num_render_channels as i32,
config.sample_rate_hz as i32,
aec3_config,
&mut code,
)
};
Expand All @@ -199,6 +224,10 @@ impl AudioProcessing {
}
}

fn initialize(&self) {
unsafe { ffi::initialize(self.inner) }
}

fn process_capture_frame(&self, frame: &mut Vec<Vec<f32>>) -> Result<(), Error> {
let mut frame_ptr = frame.iter_mut().map(|v| v.as_mut_ptr()).collect::<Vec<*mut f32>>();
unsafe {
Expand Down Expand Up @@ -412,8 +441,8 @@ mod tests {
..InitializationConfig::default()
};
let mut ap = Processor::new(&config).unwrap();
// tweak params outside of config

// tweak params outside of config
ap.set_output_will_be_muted(true);
ap.set_stream_key_pressed(true);

Expand All @@ -427,5 +456,4 @@ mod tests {

// it shouldn't crash
}

}
5 changes: 5 additions & 0 deletions webrtc-audio-processing-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ fn out_dir() -> PathBuf {
std::env::var("OUT_DIR").expect("OUT_DIR environment var not set.").into()
}

fn src_dir() -> PathBuf {
std::env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR environment var not set.").into()
}

#[cfg(not(feature = "bundled"))]
mod webrtc {
use super::*;
Expand Down Expand Up @@ -62,6 +66,7 @@ mod webrtc {
let mut include_paths = vec![
out_dir().join("include"),
out_dir().join("include").join("webrtc-audio-processing-1"),
src_dir().join("webrtc-audio-processing").join("webrtc"),
];
let mut lib_paths = vec![out_dir().join("lib")];

Expand Down
42 changes: 41 additions & 1 deletion webrtc-audio-processing-sys/src/wrapper.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
#include "wrapper.hpp"

// These definitions shouldn't affect the implementation of AEC3.
// We are defining them to work around some build-time assertions
// when including the internal header file of echo_canceller3.h
#define WEBRTC_APM_DEBUG_DUMP 0
#define WEBRTC_POSIX
#include <modules/audio_processing/aec3/echo_canceller3.h>

#include <algorithm>
#include <memory>

Expand Down Expand Up @@ -29,6 +36,32 @@ OptionalBool from_absl_optional(const absl::optional<bool>& optional) {
return rv;
}

webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& override) {
webrtc::EchoCanceller3Config config;
config.delay.num_filters = override.num_filters;
return config;
}
Comment on lines +39 to +43
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supernit: override seems to be a C++ keyword (as GitHub highlights it and as I rembember). Even though it compiles, it may be better can name it differently:

Suggested change
webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& override) {
webrtc::EchoCanceller3Config config;
config.delay.num_filters = override.num_filters;
return config;
}
webrtc::EchoCanceller3Config build_aec3_config(const EchoCanceller3ConfigOverride& config_override) {
webrtc::EchoCanceller3Config config;
config.delay.num_filters = config_override.num_filters;
return config;
}


class EchoCanceller3Factory : public webrtc::EchoControlFactory {
public:
explicit EchoCanceller3Factory(const webrtc::EchoCanceller3Config& config)
: config_(config) {}

std::unique_ptr<webrtc::EchoControl> Create(
int sample_rate_hz,
int num_render_channels,
int num_capture_channels) override {
return std::unique_ptr<webrtc::EchoControl>(new webrtc::EchoCanceller3(
config_,
sample_rate_hz,
num_render_channels,
num_capture_channels));
}

private:
webrtc::EchoCanceller3Config config_;
};

} // namespace

struct AudioProcessing {
Expand All @@ -43,9 +76,16 @@ AudioProcessing* audio_processing_create(
int num_capture_channels,
int num_render_channels,
int sample_rate_hz,
const EchoCanceller3ConfigOverride* aec3_config_override,
int* error) {
AudioProcessing* ap = new AudioProcessing;
ap->processor.reset(webrtc::AudioProcessingBuilder().Create());

webrtc::AudioProcessingBuilder builder;
if (aec3_config_override != nullptr) {
auto* factory = new EchoCanceller3Factory(build_aec3_config(*aec3_config_override));
builder.SetEchoControlFactory(std::unique_ptr<webrtc::EchoControlFactory>(factory));
}
ap->processor.reset(builder.Create());

const bool has_keyboard = false;
ap->capture_stream_config = webrtc::StreamConfig(
Expand Down
7 changes: 7 additions & 0 deletions webrtc-audio-processing-sys/src/wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,18 @@ struct Stats {
OptionalInt delay_ms;
};

// A slimmed-down version of webrtc::EchoCanceller3Config.
// We can not just expose the webrtc variant as the binding loses all the default values.
struct EchoCanceller3ConfigOverride {
int num_filters;
};

// Creates a new instance of AudioProcessing.
AudioProcessing* audio_processing_create(
int num_capture_channels,
int num_render_channels,
int sample_rate_hz,
const EchoCanceller3ConfigOverride* aec3_config_override,
int* error);

// Processes and modifies the audio frame from a capture device.
Expand Down