Add adaptive notch filter to remove narrowband echo components in AEC3

Add adaptive notch filter to remove narrowband echo components in AEC3

This CL adds detection of components in the render signal that are of 
strong narrowband nature and therefore may cause problems for the AEC.
This CL also adds functionality in the echo suppressor to suppress 
these signals

BUG=webrtc:7967
Review-Url: https://codereview.webrtc.org/2980493002
Cr-Commit-Position: refs/heads/master@{#18968}
Committed: https://chromium.googlesource.com/external/webrtc/+/14c11a47123bcdf833eeb1f6a62521b69df4d463

[peah-webrtc, 2017-07-10 22:55:12]

Description was changed from

==========
Narrowband echo

BUG=
==========

to

==========
Add adaptive notch filter to remove narrowband echo components in AEC3

This CL adds detection of components in the render signal that are of 
strong narrowband nature and therefore may cause problems for the AEC.
This CL also adds functionality in the echo suppressor to suppress 
these signals

BUG=webrtc:7967
==========

[peah-webrtc, 2017-07-10 22:55:25]

Patchset #1 (id:1) has been deleted

[peah-webrtc, 2017-07-10 22:55:32]

Patchset #1 (id:20001) has been deleted

[peah-webrtc, 2017-07-10 22:56:07]

peah@webrtc.org changed reviewers:
+ ivoc@webrtc.org

[peah-webrtc, 2017-07-10 22:56:07]

Hi ivoc@
Could you please take a look at this CL?

[ivoc, 2017-07-11 11:06:51]

lgtm, but see some comments below.

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec3/render_signal_analyzer.cc (right):

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/render_signal_analyzer.cc:83:
*narrow_peak_band = rtc::Optional<int>();
After narrow_peak_counter > 7 this code will create a new empty optional on each
iteration afterwards. You can add a check in the if statement to avoid doing it
over and over:
if (*narrow_peak_band && ++(*narrow_peak_counter) > 7)

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec3/suppression_gain.cc (right):

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/suppression_gain.cc:67: if
(render_signal_analyzer.NarrowPeakBand() &&
I think it makes sense to store this in a temporary variable.

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/suppression_gain.cc:287: const
RenderSignalAnalyzer& render_signal_analyzer,
This object is passed around a lot, but it seems like only
render_signal_analyzer.NarrowPeakBand() is actually used. I think it makes sense
to pass this directly as a bool instead (here and in the other functions), wdyt?

[peah-webrtc, 2017-07-11 11:54:18]

Thanks for the review and the comments!
I've uploaded a new patchset with changes done according to the comments!

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec3/render_signal_analyzer.cc (right):

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/render_signal_analyzer.cc:83:
*narrow_peak_band = rtc::Optional<int>();
On 2017/07/11 11:06:50, ivoc wrote:
> After narrow_peak_counter > 7 this code will create a new empty optional on
each
> iteration afterwards. You can add a check in the if statement to avoid doing
it
> over and over:
> if (*narrow_peak_band && ++(*narrow_peak_counter) > 7)

Great point!
Done

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec3/suppression_gain.cc (right):

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/suppression_gain.cc:67: if
(render_signal_analyzer.NarrowPeakBand() &&
On 2017/07/11 11:06:50, ivoc wrote:
> I think it makes sense to store this in a temporary variable.

I changed it to a separate input argument. Unfortunately, the double check of
the optional cannot be avoided though.

Done.

https://codereview.webrtc.org/2980493002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec3/suppression_gain.cc:287: const
RenderSignalAnalyzer& render_signal_analyzer,
On 2017/07/11 11:06:50, ivoc wrote:
> This object is passed around a lot, but it seems like only
> render_signal_analyzer.NarrowPeakBand() is actually used. I think it makes
sense
> to pass this directly as a bool instead (here and in the other functions),
wdyt?

Good point! I changed the code along that, but as it return an
rtc::Optional<int> I needed to pass that around. But regardless of that, with
this suggestion the code becomes much better.

Done.

[peah-webrtc, 2017-07-11 11:54:21]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2017-07-11 11:54:22]

The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2980493002/#ps80001 (title:
"Changes in response to reviewer commments")

[commit-bot: I haz the power, 2017-07-11 11:54:27]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-11 11:57:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-11 11:57:39]

Try jobs failed on following builders:
  linux32_arm_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux32_arm_rel/builds/...)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[peah-webrtc, 2017-07-11 12:09:54]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2017-07-11 12:09:55]

The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2980493002/#ps100001 (title:
"Fixed build error in debug mode")

[commit-bot: I haz the power, 2017-07-11 12:09:58]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-11 12:26:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-11 12:26:08]

Try jobs failed on following builders:
  win_clang_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_clang_dbg/builds/15315)

[peah-webrtc, 2017-07-11 12:52:40]

The CQ bit was checked by peah@webrtc.org

[commit-bot: I haz the power, 2017-07-11 12:52:48]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-11 13:13:43]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1499777560399540,
"parent_rev": "5e6685ff3584c3990febd040359cd9258fc3fa74", "commit_rev":
"14c11a47123bcdf833eeb1f6a62521b69df4d463"}

[commit-bot: I haz the power, 2017-07-11 13:13:49]

Description was changed from

==========
Add adaptive notch filter to remove narrowband echo components in AEC3

This CL adds detection of components in the render signal that are of 
strong narrowband nature and therefore may cause problems for the AEC.
This CL also adds functionality in the echo suppressor to suppress 
these signals

BUG=webrtc:7967
==========

to

==========
Add adaptive notch filter to remove narrowband echo components in AEC3

This CL adds detection of components in the render signal that are of 
strong narrowband nature and therefore may cause problems for the AEC.
This CL also adds functionality in the echo suppressor to suppress 
these signals

BUG=webrtc:7967

Review-Url: https://codereview.webrtc.org/2980493002
Cr-Commit-Position: refs/heads/master@{#18968}
Committed:
https://chromium.googlesource.com/external/webrtc/+/14c11a47123bcdf833eeb1f6a...
==========

[commit-bot: I haz the power, 2017-07-11 13:13:50]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/14c11a47123bcdf833eeb1f6a...