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

Issue 819240 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

The abrupt transition from initial to normal mode in AEC3 may cause a small amount of echo leakage

Project Member Reported by peah@chromium.org, Mar 6 2018

Issue description

In AEC3 more course settings are used in the startup phase of the call. After the startup phase, when the internal models are fairly converged,, a more refined set of settings is applied.

Currently, the transition between the course and refined set of parameters is instantaneous which has been shown to sometimes cause small amounts of echo leakage.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 15 2018

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

commit 5f1a31c565a5e35a7c1806d4e1b3449b823daab0
Author: Per Åhgren <peah@webrtc.org>
Date: Thu Mar 15 13:38:16 2018

Adding a smooth transition from the startup phase parameter set in AEC3

This CL ensures a smooth transition from the parameters used during
the startup phase in the call to the parameters used in the rest of the
call. This is achieved by slowly transitioning between the parameter
sets via interpolation.

Bug:  chromium:819240 , webrtc:8983 
Change-Id: Ifbac4b93fc6ad6efc441f41fb88ef09e8ee3d669
Reviewed-on: https://webrtc-review.googlesource.com/60360
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22451}
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/api/audio/echo_canceller3_config.h
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/adaptive_fir_filter.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/adaptive_fir_filter.h
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/adaptive_fir_filter_unittest.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/echo_remover.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/main_filter_update_gain.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/main_filter_update_gain.h
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/main_filter_update_gain_unittest.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/shadow_filter_update_gain.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/shadow_filter_update_gain.h
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/shadow_filter_update_gain_unittest.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/subtractor.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/suppression_gain.cc
[modify] https://crrev.com/5f1a31c565a5e35a7c1806d4e1b3449b823daab0/modules/audio_processing/aec3/suppression_gain.h

Comment 2 by peah@chromium.org, Mar 23 2018

Cc: tlegrand@chromium.org hlundin@chromium.org
Labels: Merge-Request-66
I would like to merge this to M66.

The reason is to simplify the approved merge of  issue 824111  which have been tested together with this the code for this issue. 
Furthermore, the merge of 824111 will be much more straightforward if this issue is also merged.

The code that addressed this issue has been active in Chrome Canary for 16 days and the metrics looks good.

Similar as for 824111, this feature is only active beneath an experimental flag so the merge is safe in the sense that the code can be turned off in case any issue would arise. 
Project Member

Comment 3 by sheriffbot@chromium.org, Mar 23 2018

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 23 2018

Labels: merge-merged-66
The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8

commit ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8
Author: Per Åhgren <peah@webrtc.org>
Date: Fri Mar 23 22:35:12 2018

Merge to M66: Adding a smooth transition from the startup phase ...

This CL ensures a smooth transition from the parameters used during
the startup phase in the call to the parameters used in the rest of the
call. This is achieved by slowly transitioning between the parameter
sets via interpolation.

(cherry picked from commit 5f1a31c565a5e35a7c1806d4e1b3449b823daab0)

TBR: devicentepena@webrtc.org,henrik.lundin@webrtc.org
Bug:  chromium:819240 , webrtc:8983 
Change-Id: Ifbac4b93fc6ad6efc441f41fb88ef09e8ee3d669
Reviewed-on: https://webrtc-review.googlesource.com/60360
Reviewed-by: Jesus de Vicente Pena <devicentepena@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22451}
Reviewed-on: https://webrtc-review.googlesource.com/64480
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/66@{#10}
Cr-Branched-From: 12c8110e8c717b7f0f87615d3b99caac2a69fa6c-refs/heads/master@{#22215}
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/api/audio/echo_canceller3_config.h
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/adaptive_fir_filter.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/adaptive_fir_filter.h
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/adaptive_fir_filter_unittest.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/echo_remover.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/main_filter_update_gain.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/main_filter_update_gain.h
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/main_filter_update_gain_unittest.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/shadow_filter_update_gain.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/shadow_filter_update_gain.h
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/shadow_filter_update_gain_unittest.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/subtractor.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/suppression_gain.cc
[modify] https://crrev.com/ddf2cf3ba2a78ffe4412060c174f26dd2ee685b8/modules/audio_processing/aec3/suppression_gain.h

Comment 6 by peah@chromium.org, Mar 23 2018

Status: Fixed (was: Assigned)

Comment 7 by peah@chromium.org, Mar 23 2018

Labels: M-66
Project Member

Comment 8 by sheriffbot@chromium.org, Mar 27 2018

Cc: abdulsyed@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Approved-66
Looks to have been merged already, as per comment 5.

Sign in to add a comment