New issue
Advanced search Search tips

Issue 811658 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

The AEC3 delay estimator sometime is slow at tracking audio path changes

Project Member Reported by peah@chromium.org, Feb 13 2018

Issue description

On some systems with audio path changes, the delay estimator does not track the changes as fast as would be desired which may cause echo leakage for up to 10 seconds.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 13 2018

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

commit 4712776bf43f9dd3cf93368d0f4bfd892e32aebe
Author: Per Åhgren <peah@webrtc.org>
Date: Tue Feb 13 12:52:58 2018

Leveraging the skew in API call order to a boost AEC3 signal realignment

This CL resets the AEC3 realignment functionality when a significant
and persistent skew in the number of render and capture API calls is
detected.

Bug:  chromium:811658 , webrtc:8879 
Change-Id: Ib5c727b38f427da2a7d25eac7c939a17bdaabe74
Reviewed-on: https://webrtc-review.googlesource.com/52260
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21997}
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/block_processor.cc
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/echo_path_delay_estimator.cc
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/echo_path_delay_estimator.h
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/mock/mock_render_delay_controller.h
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/render_delay_controller.cc
[modify] https://crrev.com/4712776bf43f9dd3cf93368d0f4bfd892e32aebe/modules/audio_processing/aec3/render_delay_controller.h

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 13 2018

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

commit fdd4400ef48f17ed2a8bcf841938c30f1d89588e
Author: Per Åhgren <peah@webrtc.org>
Date: Tue Feb 13 14:29:23 2018

Removed hysteresis in the delay estimation offset


Bug:  chromium:811658 , webrtc:8879 
Change-Id: I9e67fd9aaae4b85e344b9b40ca6bcf9a8fe1eec1
Reviewed-on: https://webrtc-review.googlesource.com/52480
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22002}
[modify] https://crrev.com/fdd4400ef48f17ed2a8bcf841938c30f1d89588e/modules/audio_processing/aec3/render_delay_controller.cc

Comment 3 by peah@chromium.org, Feb 15 2018

Labels: Merge-Request-65
Project Member

Comment 4 by sheriffbot@chromium.org, Feb 15 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: M65 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), bhthompson@(ChromeOS), govind@(Desktop)

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

Comment 5 by gov...@chromium.org, Feb 15 2018

Before we approve merge to M65, could you pls confirm followings?

Is this M65 regression and critical to merge?
Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge?
Any other imp details to justify the merge

Please note M65 is already promoted to Beta so merge bar is very high. Thank you.

Comment 6 by peah@chromium.org, Feb 15 2018

The merge is aims to reduce the recently identified effects of audio playout glitches (among other places reported in https://bugs.chromium.org/p/chromium/issues/detail?id=798690) which are causing echo issues in VoIP calls.

The change has been baked in Canary for 1.5 days and the metrics looks good. Apart from that it has been extensively tested on the devices in our test lab.

Furthermore, the code resides beneath an experimental flag which makes the merge safe.

Comment 7 by peah@chromium.org, Feb 15 2018

What I meant with the experimental flag is that it can be used to turn off the merged code if anything would go wrong with the merge.

Comment 8 by gov...@chromium.org, Feb 15 2018

Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch based on comments #6 and #7. Please merge ASAP. Also pls mark bug 798690 as fixed after merge if nothing is pending. Thank you. 
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 18 2018

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

commit fc67520ded384429039770ec0b449aa3e601a249
Author: Per Åhgren <peah@webrtc.org>
Date: Sun Feb 18 23:00:40 2018

Merge to M65: Leveraging the skew in API call order to a boost AEC3 ...

This CL resets the AEC3 realignment functionality when a significant
and persistent skew in the number of render and capture API calls is
detected.

TBR=gustaf@webrtc.org,henrik.lundin@webrtc.org
(cherry picked from commit 4712776bf43f9dd3cf93368d0f4bfd892e32aebe)

Bug:  chromium:811658 , webrtc:8879 
Change-Id: Ib5c727b38f427da2a7d25eac7c939a17bdaabe74
Reviewed-on: https://webrtc-review.googlesource.com/52260
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#21997}
Reviewed-on: https://webrtc-review.googlesource.com/54312
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/65@{#21}
Cr-Branched-From: 3ac67a736bb200ecf7c116a88b2f8d5c542973c8-refs/heads/master@{#21637}
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/block_processor.cc
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/echo_path_delay_estimator.cc
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/echo_path_delay_estimator.h
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/mock/mock_render_delay_controller.h
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/render_delay_controller.cc
[modify] https://crrev.com/fc67520ded384429039770ec0b449aa3e601a249/modules/audio_processing/aec3/render_delay_controller.h

Comment 10 Deleted

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 18 2018

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

commit 53941ef41f884ea13c0593e1c3645af3ab50c2e3
Author: Per Åhgren <peah@webrtc.org>
Date: Sun Feb 18 23:45:59 2018

Merge to M65: Removed hysteresis in the delay estimation offset

(cherry picked from commit fdd4400ef48f17ed2a8bcf841938c30f1d89588e)

TBR: gustaf@webrtc.org,henrik.lundin@webrtc.org
Bug:  chromium:811658 , webrtc:8879 
Change-Id: I9e67fd9aaae4b85e344b9b40ca6bcf9a8fe1eec1
Reviewed-on: https://webrtc-review.googlesource.com/52480
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22002}
Reviewed-on: https://webrtc-review.googlesource.com/54313
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/65@{#22}
Cr-Branched-From: 3ac67a736bb200ecf7c116a88b2f8d5c542973c8-refs/heads/master@{#21637}
[modify] https://crrev.com/53941ef41f884ea13c0593e1c3645af3ab50c2e3/modules/audio_processing/aec3/render_delay_controller.cc

Is anything pending for M65?

Comment 13 by peah@chromium.org, Feb 19 2018

Not for this issue. There is, however, a related issue for which a merge is requested (https://bugs.chromium.org/p/chromium/issues/detail?id=812524) which addresses a delay buildup which has been shown to happen due to the audio path glitches. But the fixing CL for that addresses the size of the look window rather than the speed of tracking for the delay estimator so therefore it merits a different issue.

Comment 14 by peah@chromium.org, Feb 19 2018

Status: Fixed (was: Assigned)

Sign in to add a comment