New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 833801 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

The AEC3 echo removal is too aggressive for low-level echoes during nearend-speech

Project Member Reported by peah@chromium.org, Apr 17 2018

Issue description

During doubletalk, the AEC3 applies too much suppression of low-level echoes. That sometimes causes the nearend to be perceived a "choppy" as low-level parts of the nearend speech are removed due to it being closer to the residual echo. This echo removal behavior should not be necessary when the low-level echoes occur after strong nearend signals.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/47d7fbd8fe4077b425c90b580d74a624276bfe7a

commit 47d7fbd8fe4077b425c90b580d74a624276bfe7a
Author: Per Åhgren <peah@webrtc.org>
Date: Tue Apr 24 11:24:44 2018

Reuse the AEC2 coherence-based gain for the lower bands in AEC3.

This CL overrides the power-based suppressor gain decision with
a coherence based descision for the cases when that indicates a
higher suppressor gain.

Bug:  webrtc:9159 , chromium:833801 
Change-Id: I0e7d82ac1b8c70ffe9d45907559bb14b1b849d71
Reviewed-on: https://webrtc-review.googlesource.com/71660
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22997}
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/api/audio/echo_canceller3_config.h
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/BUILD.gn
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/aec3_fft.cc
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/aec3_fft.h
[add] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/coherence_gain.cc
[add] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/coherence_gain.h
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/echo_remover.cc
[delete] https://crrev.com/f0e88d4601a7cdbb97359dae18bb1f01ee18e99f/modules/audio_processing/aec3/output_selector.cc
[delete] https://crrev.com/f0e88d4601a7cdbb97359dae18bb1f01ee18e99f/modules/audio_processing/aec3/output_selector.h
[delete] https://crrev.com/f0e88d4601a7cdbb97359dae18bb1f01ee18e99f/modules/audio_processing/aec3/output_selector_unittest.cc
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_filter.cc
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_filter.h
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_filter_unittest.cc
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_gain.cc
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_gain.h
[modify] https://crrev.com/47d7fbd8fe4077b425c90b580d74a624276bfe7a/modules/audio_processing/aec3/suppression_gain_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce5f1458fb12d06ef4a31cda4dfd9544730effae

commit ce5f1458fb12d06ef4a31cda4dfd9544730effae
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Apr 24 14:42:50 2018

Roll src/third_party/webrtc/ b04e5cae0..29e865a5d (8 commits)

https://webrtc.googlesource.com/src.git/+log/b04e5cae08b8..29e865a5d8a5

$ git log b04e5cae0..29e865a5d --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:833801 ,chromium:None,chromium:834875,chromium:None


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Iab31fc923dd915e26178e425bfda37c9a6dd4f67
Reviewed-on: https://chromium-review.googlesource.com/1025320
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#553098}
[modify] https://crrev.com/ce5f1458fb12d06ef4a31cda4dfd9544730effae/DEPS

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 26 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/0cb4a25e432f90dab773f41c9fe9eebb8c36b163

commit 0cb4a25e432f90dab773f41c9fe9eebb8c36b163
Author: Gustaf Ullberg <gustaf@webrtc.org>
Date: Thu Apr 26 15:43:27 2018

Apply upper gain limit after coherence gains in AEC3

This CL makes sure that the coherence-based gains are affected by the
upper gain limit during call start-up and after resets.

Bug:  webrtc:9159 , chromium:833801 
Change-Id: I93fdd173b6e11ea861d0e01e12c048ec0a91db70
Reviewed-on: https://webrtc-review.googlesource.com/72841
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23039}
[modify] https://crrev.com/0cb4a25e432f90dab773f41c9fe9eebb8c36b163/modules/audio_processing/aec3/suppression_gain.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5bb157b379b5695efd06b22d78746195a7d16630

commit 5bb157b379b5695efd06b22d78746195a7d16630
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 27 00:59:53 2018

Roll src/third_party/webrtc/ 775d07e27..e782abab1 (13 commits)

https://webrtc.googlesource.com/src.git/+log/775d07e27769..e782abab190e

$ git log 775d07e27..e782abab1 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:835281 ,chromium:None,chromium:none,chromium:833801,chromium:None,chromium:836344


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I13862900de88f1441e209ddca59dd0a2984d7810
Reviewed-on: https://chromium-review.googlesource.com/1031110
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#554243}
[modify] https://crrev.com/5bb157b379b5695efd06b22d78746195a7d16630/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/658ad8816b7ef9dafef041a4632a4b106f79ff39

commit 658ad8816b7ef9dafef041a4632a4b106f79ff39
Author: Per Åhgren <peah@webrtc.org>
Date: Fri Apr 27 19:26:03 2018

Removed the updating of the padding data buffer in the AEC3 FFT

This CL removes the updating of the buffered data used to to pad the
64 sample blocks to 128 samples FFTs. As that padding was used
incorrectly in one place this resolves an important issue.


Bug:  webrtc:9159 , chromium:833801 , webrtc:9206 
Change-Id: Ie6830878ebec6130b61d4e7e3169357f2e253073
Reviewed-on: https://webrtc-review.googlesource.com/73240
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23059}
[modify] https://crrev.com/658ad8816b7ef9dafef041a4632a4b106f79ff39/modules/audio_processing/aec3/aec3_fft.cc
[modify] https://crrev.com/658ad8816b7ef9dafef041a4632a4b106f79ff39/modules/audio_processing/aec3/aec3_fft.h
[modify] https://crrev.com/658ad8816b7ef9dafef041a4632a4b106f79ff39/modules/audio_processing/aec3/aec3_fft_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad5f0089cde37854d35976056157a5675e632e1d

commit ad5f0089cde37854d35976056157a5675e632e1d
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 27 22:52:34 2018

Roll src/third_party/webrtc/ 169c7fd52..658ad8816 (2 commits)

https://webrtc.googlesource.com/src.git/+log/169c7fd521da..658ad8816b7e

$ git log 169c7fd52..658ad8816 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:833801 ,chromium:None


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I7b0d1b8892886b8bc7c593cf407e147e3fcee689
Reviewed-on: https://chromium-review.googlesource.com/1033812
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#554546}
[modify] https://crrev.com/ad5f0089cde37854d35976056157a5675e632e1d/DEPS

Comment 7 by peah@chromium.org, Apr 29 2018

Cc: gustaf@chromium.org hlundin@chromium.org
Labels: Merge-Request-67
We would like to merge this into M67. The code changes has been tested both in Canary and during manual testing of devices. The metrics looks good and the changes have performed as designed during manual testing.

Please note that one CL (https://webrtc-review.googlesource.com/c/src/+/70421) for this issue is not linked to the issue to to a misspelled Chromium issue link.

All the changes for this issue are beneath an experimental flag which can be used as a switch to turn off the code. 
This makes the merge safe.
Project Member

Comment 8 by sheriffbot@chromium.org, Apr 29 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: Less than 26 days to go before AppStore submit on M67
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 9 by gov...@chromium.org, Apr 29 2018

Could you pls provide CLs you're planning to merge? Also could you pls justify why multiple Cls needed to be merged after M67 branch for one bug report?

Comment 10 by peah@chromium.org, Apr 30 2018

The CLs we would like to merge are:
1) https://webrtc-review.googlesource.com/c/src/+/70421 
  (AEC3 transparency improvements through refined echo audibility analysis)
2) https://webrtc-review.googlesource.com/c/src/+/71660 
  (Reuse the AEC2 coherence-based gain for the lower bands in AEC3.)
3) https://webrtc-review.googlesource.com/c/src/+/72841 
  (Apply upper gain limit after coherence gains in AEC3)
4) https://webrtc-review.googlesource.com/c/src/+/73240 
  (Removed the updating of the padding data buffer in the AEC3 FFT)

CLs 1)+2) address the issue herein described issue of excessive suppression of low level echoes, which were identified during the release testing of M66: 
-CL 1) was the first attempt which turned out to solve the issue for all frequencies above 700 Hz. 
-CL 2) extends CL 1) to also work for frequencies below 700 Hz.

CLs 3)+4) corrects two issues with CL 2) identified during testing: 
-CL 3) moves the effect introduced by CL 2) to avoid overruling a call-startup gain limit.
-CL 4) corrects a long-standing bug in the FFT computation which affected CL 2).


The reason for asking for a merge of 4 CLs to address one issue is that all 4 CLs are needed to fully address the issue. The CLs were not bundled into one CL as CLs 2)-4) were devised based on real-call testing of CL 1)-2) in Chrome Canary.
 

Note that the CLs listed in #2, #4, #6 are automated messages generated by the WebRTC importer bots that import WebRTC into Chromium and are not something that will be merged.

Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 for CLs listed at #10 based on comment #7 & #10. Please merge before 1:00 PM PT, Monday (04/30) so we can pick them up for next beta release. Thank you.

Comment 12 by peah@chromium.org, Apr 30 2018

All the CLs (the ones listed in #10) for this issue are now merged into M67.

Comment 13 by peah@chromium.org, Apr 30 2018

Labels: Merge-Merged

Comment 14 by peah@chromium.org, Apr 30 2018

Labels: M-67

Comment 15 by peah@chromium.org, Apr 30 2018

Status: Fixed (was: Assigned)
Could you pls list merged CLs here and remove "Merge-Approved-67" label?

Comment 17 by peah@chromium.org, Apr 30 2018

The CLs merged were:
1) https://webrtc-review.googlesource.com/c/src/+/70421 
  (AEC3 transparency improvements through refined echo audibility analysis)
2) https://webrtc-review.googlesource.com/c/src/+/71660 
  (Reuse the AEC2 coherence-based gain for the lower bands in AEC3.)
3) https://webrtc-review.googlesource.com/c/src/+/72841 
  (Apply upper gain limit after coherence gains in AEC3)
4) https://webrtc-review.googlesource.com/c/src/+/73240 
  (Removed the updating of the padding data buffer in the AEC3 FFT)

The CLs for performing the merge of those were
a) https://webrtc-review.googlesource.com/c/src/+/73360
b) https://webrtc-review.googlesource.com/c/src/+/73362
c) https://webrtc-review.googlesource.com/c/src/+/73363
d) https://webrtc-review.googlesource.com/c/src/+/73364

Comment 18 by peah@chromium.org, Apr 30 2018

Labels: -Merge-Approved-67
Labels: -Merge-Merged merge-merged-67

Sign in to add a comment