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

Issue 841193 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

The AEC3 transparent mode is too aggressive

Project Member Reported by peah@chromium.org, May 9 2018

Issue description

The transparent mode in AEC3, which is designed to handle closed headsets and tandem operation, is currently too easily activated in scenarios such a when there is near-end noise, or when the farend is fairly, but not fully, silent.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 9 2018

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

commit d18e87edd49b89f07a0e93d63652f6ffc6666913
Author: Per Åhgren <peah@webrtc.org>
Date: Wed May 09 12:36:41 2018

Correcting the AEC3 transparent mode behavior avoid incorrect activation

This CL adds robustness to avoid the AEC3 transparent mode to be
incorrectly activated when
-there is strong near-end noise
-there is only low-level nearend activity.

Bug:  webrtc:9256 , chromium:841193 
Change-Id: I26c2759d163914eb85dc3d863da8acbf28cbb88d
Reviewed-on: https://webrtc-review.googlesource.com/75511
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23191}
[modify] https://crrev.com/d18e87edd49b89f07a0e93d63652f6ffc6666913/modules/audio_processing/aec3/aec_state.cc
[modify] https://crrev.com/d18e87edd49b89f07a0e93d63652f6ffc6666913/modules/audio_processing/aec3/aec_state.h
[modify] https://crrev.com/d18e87edd49b89f07a0e93d63652f6ffc6666913/modules/audio_processing/aec3/subtractor.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 9 2018

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

commit 0ca6086c6e395647e883c8ade6a63a51bd84d033
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 09 17:14:33 2018

Roll src/third_party/webrtc/ c6c44268b..a29b14855 (4 commits)

https://webrtc.googlesource.com/src.git/+log/c6c44268bcb5..a29b148557b0

$ git log c6c44268b..a29b14855 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:826914, chromium:841193 ,chromium:851187, chromium:839860 


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: Ia175141797bf07db2094a2b43e74a71db4377ecc
Reviewed-on: https://chromium-review.googlesource.com/1052216
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@{#557224}
[modify] https://crrev.com/0ca6086c6e395647e883c8ade6a63a51bd84d033/DEPS

Comment 3 by peah@chromium.org, May 11 2018

Cc: gustaf@chromium.org huib@chromium.org hlundin@chromium.org
Labels: Merge-Request-67
We would like to merge this into M67. 

The CL addresses problems with echo leakage during VoIP calls in noisy environments that was identified during testing of the last M67 Beta.

The CL we would like to merge is:
-https://webrtc-review.googlesource.com/75511 (Correcting the AEC3 transparent mode behavior avoid incorrect activation)

The CL has been tested both in Canary and in offline testing and works well.

The merge is safe in the sense that the code affected is beneath an experimental flag. Furthermore, a field_trial switch was added for this in another CL that can be used to turn off the functionality if needed. 
Project Member

Comment 4 by sheriffbot@chromium.org, May 11 2018

Labels: -Merge-Request-67 Merge-Review-67 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
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 5 by gov...@chromium.org, May 11 2018

Re #3, is this behind finch and can be easily disabled if something goes wrong?

Also may I pls know why multiple merge requests are coming for M67? We already took few merges in for M67 earlier and now 3 more (current bug + 2 below). 

 * https://bugs.chromium.org/p/chromium/issues/detail?id=841187#c3
 * https://bugs.chromium.org/p/chromium/issues/detail?id=840347#c5

Pls not M67 is going to stable very soon and we're only taking truly critical and safe merges in. Thank you.

Comment 6 by peah@chromium.org, May 11 2018

Yes, this is (as well as the other requests) are active beneath the WebRtcUseEchoCanceller3 finch flag and can be disabled using that if anything would go wrong.

The reason for the simultaneous merge requests is that a major testing session was this week done with the current Beta M67 in order to find any remaining issues with the WebRTC echo canceller solution we are trying to launch.
The findings from that, together with the results from the experiment in M66 Stable ended up in these changes.

The changes are fairly small, but since they address separate issues they ended up in several merge requests.

The changes have been verified in Canary, but a major test session is scheduled for the upcoming last M67 Beta. If the changes then turn out to not perform well, the finch experiment can be used to fully deactivate this code. 



 


Comment 7 by gov...@chromium.org, May 11 2018

Labels: -Merge-Review-67 Merge-Approved-67
Approving merge to M67 branch 3396 based on comment #6. Pls merge. Thank you.

Comment 8 by peah@chromium.org, May 12 2018

Labels: -Merge-Approved-67 Merge-Merged M-67 merge-merged-67
This issue was merged.
The merge CL was:
-https://webrtc-review.googlesource.com/c/src/+/76322

Comment 9 by peah@chromium.org, May 12 2018

Status: Fixed (was: Assigned)

Sign in to add a comment