[pulseaudio-discuss] [PATCH v3 03/24] echo-cancel: Add support for the webrtc intelligibility enhancer

[Arun Raghavan]

On 24 January 2016 at 22:00, Tanu Kaskinen <tanuk at iki.fi> wrote:
> On Mon, 2016-01-18 at 13:06 +0530, arun at accosted.net wrote:
>> From: Arun Raghavan <git at arunraghavan.net>
>>
>> Just exposing this, disabled by default. It's not used by Chromium at
>> the moment.
>> ---
>>  src/modules/echo-cancel/webrtc.cc | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/modules/echo-cancel/webrtc.cc b/src/modules/echo-cancel/webrtc.cc
>> index f4f1395..bbfa43f 100644
>> --- a/src/modules/echo-cancel/webrtc.cc
>> +++ b/src/modules/echo-cancel/webrtc.cc
>> @@ -47,6 +47,7 @@ PA_C_DECL_END
>>  #define DEFAULT_COMFORT_NOISE true
>>  #define DEFAULT_DRIFT_COMPENSATION false
>>  #define DEFAULT_EXTENDED_FILTER false
>> +#define DEFAULT_INTELLIGIBILITY_ENHANCER false
>>
>>  static const char* const valid_modargs[] = {
>>      "high_pass_filter",
>> @@ -58,6 +59,7 @@ static const char* const valid_modargs[] = {
>>      "comfort_noise",
>>      "drift_compensation",
>>      "extended_filter",
>> +    "intelligibility_enhancer",
>>      NULL
>>  };
>>
>> @@ -84,7 +86,7 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>>      webrtc::AudioProcessing *apm = NULL;
>>      webrtc::ProcessingConfig pconfig;
>>      webrtc::Config config;
>> -    bool hpf, ns, agc, dgc, mobile, cn, ext_filter;
>> +    bool hpf, ns, agc, dgc, mobile, cn, ext_filter, intelligibility;
>>      int rm = -1;
>>      pa_modargs *ma;
>>
>> @@ -163,8 +165,16 @@ bool pa_webrtc_ec_init(pa_core *c, pa_echo_canceller *ec,
>>          goto fail;
>>      }
>>
>> +    intelligibility = DEFAULT_INTELLIGIBILITY_ENHANCER;
>> +    if (pa_modargs_get_value_boolean(ma, "intelligibility_enhancer", &intelligibility) < 0) {
>> +        pa_log("Failed to parse intelligibility_enhancer value");
>> +        goto fail;
>> +    }
>> +
>>      if (ext_filter)
>>          config.Set(new webrtc::ExtendedFilter(true));
>> +    if (intelligibility)
>> +        config.Set<webrtc::Intelligibility>(new webrtc::Intelligibility(true));
>
> The same comment as in the previous patch: could the if statement be
> eliminated?

Ack.

>>
>>      apm = webrtc::AudioProcessing::Create(config);
>>
>> @@ -253,7 +263,7 @@ void pa_webrtc_ec_play(pa_echo_canceller *ec, const uint8_t *play) {
>>      pa_assert(play_frame.samples_per_channel_ <= webrtc::AudioFrame::kMaxDataSizeSamples);
>>      memcpy(play_frame.data_, play, ec->params.priv.webrtc.blocksize);
>>
>> -    apm->AnalyzeReverseStream(&play_frame);
>> +    apm->ProcessReverseStream(&play_frame);
>
> This looks like a potentially unrelated change. Why is this change
> done?

It is needed for the intelligibility enhancer to work, but I later
realised we don't actually support this -- unlike
AnalyzeReverseStream(), ProcessReverseStream() modifies the playback
samples. This is something that needs to be done in the future
(there's a later commit that disables this that I could not squash due
to some intermediate changes).

-- Arun