echo-cancel: add api support for webrtc echo suppression

Submitted by James Huang on Sept. 14, 2017, 7:21 p.m.

Details

Message ID 20170914192139.34750-1-hng.jms@gmail.com
State New
Headers show
Series "echo-cancel: add api support for webrtc echo suppression" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

James Huang Sept. 14, 2017, 7:21 p.m.
---
 src/modules/echo-cancel/webrtc.cc | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
index aadb1af2..107d2301 100644
--- a/src/modules/echo-cancel/webrtc.cc
+++ b/src/modules/echo-cancel/webrtc.cc
@@ -55,6 +55,7 @@  PA_C_DECL_END
 #define DEFAULT_AGC_START_VOLUME 85
 #define DEFAULT_BEAMFORMING false
 #define DEFAULT_TRACE false
+#define DEFAULT_ECHO_SUPPRESSION_LEVEL 0
 
 #define WEBRTC_AGC_MAX_VOLUME 255
 
@@ -76,6 +77,7 @@  static const char* const valid_modargs[] = {
     "mic_geometry", /* documented in parse_mic_geometry() */
     "target_direction", /* documented in parse_mic_geometry() */
     "trace",
+    "echo_suppression_level",
     NULL
 };
 
@@ -240,7 +242,7 @@  bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
     webrtc::Config config;
     bool hpf, ns, agc, dgc, mobile, cn, vad, ext_filter, intelligibility, experimental_agc, beamforming;
     int rm = -1, i;
-    uint32_t agc_start_volume;
+    uint32_t agc_start_volume, echo_suppression_level;
     pa_modargs *ma;
     bool trace = false;
 
@@ -290,6 +292,12 @@  bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
         goto fail;
     }
 
+    echo_suppression_level = DEFAULT_ECHO_SUPPRESSION_LEVEL;
+    if (pa_modargs_get_value_u32(ma, "echo_suppression_level", &echo_suppression_level) < 0) {
+        pa_log("Failed to parse echo_suppression_level value");
+        goto fail;
+    }
+
     if (mobile) {
         if (ec->params.drift_compensation) {
             pa_log("Can't use drift_compensation in mobile mode");
@@ -439,6 +447,7 @@  bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
     if (hpf)
         apm->high_pass_filter()->Enable(true);
 
+    apm->echo_cancellation()->set_suppression_level((webrtc::EchoCancellation::SuppressionLevel) echo_suppression_level);
     if (!mobile) {
         apm->echo_cancellation()->enable_drift_compensation(ec->params.drift_compensation);
         apm->echo_cancellation()->Enable(true);

Comments

On Thu, 2017-09-14 at 12:21 -0700, James Huang wrote:
> ---
>  src/modules/echo-cancel/webrtc.cc | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
> index aadb1af2..107d2301 100644
> --- a/src/modules/echo-cancel/webrtc.cc
> +++ b/src/modules/echo-cancel/webrtc.cc
> @@ -55,6 +55,7 @@ PA_C_DECL_END
>  #define DEFAULT_AGC_START_VOLUME 85
>  #define DEFAULT_BEAMFORMING false
>  #define DEFAULT_TRACE false
> +#define DEFAULT_ECHO_SUPPRESSION_LEVEL 0
>  
>  #define WEBRTC_AGC_MAX_VOLUME 255
>  
> @@ -76,6 +77,7 @@ static const char* const valid_modargs[] = {
>      "mic_geometry", /* documented in parse_mic_geometry() */
>      "target_direction", /* documented in parse_mic_geometry() */
>      "trace",
> +    "echo_suppression_level",
>      NULL
>  };
>  
> @@ -240,7 +242,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>      webrtc::Config config;
>      bool hpf, ns, agc, dgc, mobile, cn, vad, ext_filter, intelligibility, experimental_agc, beamforming;
>      int rm = -1, i;
> -    uint32_t agc_start_volume;
> +    uint32_t agc_start_volume, echo_suppression_level;
>      pa_modargs *ma;
>      bool trace = false;
>  
> @@ -290,6 +292,12 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>          goto fail;
>      }
>  
> +    echo_suppression_level = DEFAULT_ECHO_SUPPRESSION_LEVEL;
> +    if (pa_modargs_get_value_u32(ma, "echo_suppression_level", &echo_suppression_level) < 0) {
> +        pa_log("Failed to parse echo_suppression_level value");
> +        goto fail;
> +    }
> +
>      if (mobile) {
>          if (ec->params.drift_compensation) {
>              pa_log("Can't use drift_compensation in mobile mode");
> @@ -439,6 +447,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>      if (hpf)
>          apm->high_pass_filter()->Enable(true);
>  
> +    apm->echo_cancellation()->set_suppression_level((webrtc::EchoCancellation::SuppressionLevel) echo_suppression_level);
>      if (!mobile) {
>          apm->echo_cancellation()->enable_drift_compensation(ec->params.drift_compensation);
>          apm->echo_cancellation()->Enable(true);

The code looks fine, but the commit message should explain the
following things:

What problem does the patch solve? What does the "echo suppression
level" mean? What's the range of valid values for the level? (Oh, now I
realized that the code needs some changes too: it should check that the
passed value is in the valid range, unless the range covers all
possible u32 values.) If the set_suppression_level() function is not
called, is that equivalent to setting the level to 0?