[pulseaudio-discuss] [PATCH] echo-cancel: add api support for webrtc echo suppression

[Tanu Kaskinen]

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?

-- 
Tanu

https://www.patreon.com/tanuk