New issue
Advanced search Search tips

Issue 775653 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

AEC3 sometimes fails to cancel echoes for highly saturated echoes

Project Member Reported by peah@chromium.org, Oct 17 2017

Issue description

When the echoes are so strong that they are saturated in the microphone signal, AEC3 sometimes fails to remove them.
 

Comment 1 by sczs@chromium.org, Oct 18 2017

Labels: -OS-iOS
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25 2017

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

commit 7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7
Author: Per Åhgren <peah@webrtc.org>
Date: Wed Oct 25 01:36:59 2017

Balancing the transparency in AEC3 between saturating and low echo paths

This CL balances the NLP tradeoff in AEC3 to properly handle the cases
when the echo path is so strong that it saturates the echo and when it
is so weak that the echo is very low compared to nearend.

Bug:  webrtc:8411 ,  webrtc:8412 ,  chromium:775653 
Change-Id: I5aff74dfadd51cac1ce71b1cb935d68a5be6918d
Reviewed-on: https://webrtc-review.googlesource.com/14120
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20418}
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/aec_state.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/aec_state.h
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/echo_remover.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/matched_filter.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/residual_echo_estimator.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/subtractor.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/subtractor.h
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/suppression_gain.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/suppression_gain.h
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/aec3/suppression_gain_unittest.cc
[modify] https://crrev.com/7ddd46386a24b4d2f3064c3fdca0e124d8bb38b7/modules/audio_processing/include/audio_processing.h

Comment 3 by ossu@chromium.org, Oct 25 2017

Status: Assigned (was: Untriaged)
Triaging bugs... setting this to assigned since, well, it looks like it's assigned already. :)
Flip it to Started or whatever other state best reflects the current status, peah!

Comment 4 by peah@chromium.org, Oct 25 2017

Status: Started (was: Assigned)

Comment 5 by peah@chromium.org, Oct 26 2017

Labels: Merge-Request-63
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 26 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

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

Comment 7 by gov...@chromium.org, Oct 26 2017

M63 is already promoted to Beta and we're only taking critical merges in. This doesn't seem like M63 regression, can this wait until M64? 

Please provide merge justification if is critical to merge to M63. Thank you.

Comment 8 by peah@chromium.org, Oct 30 2017

The merge was important, but the stats still looks good. Therefore, considering the that M63 is already in beta I think it is safer to wait with that until M64.

Thanks for looking into this!

Comment 9 by gov...@chromium.org, Oct 30 2017

Labels: -Merge-Review-63 Merge-Rejected-63
Thank you peah@. Rejecting merge to M63 based on comment #8.
Per, is this fixed? Please set a milestone also.

Comment 11 by peah@chromium.org, Dec 12 2017

Labels: M-64
Status: Fixed (was: Started)

Sign in to add a comment