New issue
Advanced search Search tips

Issue 812524 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

The AEC3 look window is too short to cover the very long audio buffer delays seen on some platforms

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

Issue description

On some platforms, audio buffer delays of up to 360 ms has been seen. The current AEC3 look window size is too short to handle those, resulting in full echo leakage.
 
Project Member

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

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

commit 75b57d3bb894aa51da3a4bcf1bb455785b7a4098
Author: Per Åhgren <peah@webrtc.org>
Date: Thu Feb 15 09:28:14 2018

Increased the size of the delay estimation look window

Bug:  webrtc:8889 , chromium:812524 
Change-Id: I508a0d3619b90f6a8851db66c151a980b14adb55
Reviewed-on: https://webrtc-review.googlesource.com/53640
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22030}
[modify] https://crrev.com/75b57d3bb894aa51da3a4bcf1bb455785b7a4098/api/audio/echo_canceller3_config.h

Comment 2 by peah@chromium.org, Feb 18 2018

The theory is that the issue is related to https://bugs.chromium.org/p/chromium/issues/detail?id=798690 in that the increased delay occurred some time into the call on a platform which is known to suffer from audio path glitches.

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

I would like to merge the above CL to M65. It fixes the problem seen above of an increased delay due to echo issues causing echoes running AEC3.

The change has been baked in Canary for 3 days and the metrics looks good and no issues has been found. Furthermore, the change has since before full unit test coverage (as it is only a reconfiguration which resides within the standard operating conditions for the AEC3). Furthermore, it has been extensively been tested on customized setups..

On top of that, the code resides beneath an experimental flag which makes the merge very safe in that the experimental flag can be used to turn off the merged code if anything would go wrong with the merge.

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

Labels: Merge-Request-65
Project Member

Comment 5 by sheriffbot@chromium.org, Feb 19 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 6 by gov...@chromium.org, Feb 19 2018

Labels: -Merge-Review-65 Merge-Approved-65
Approving merge to M65 branch based on comment #3. Pleas merge ASAP. Thank you.
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 19 2018

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

commit 36af4e9614f707f733eb2340fae66d6325aaac5b
Author: Per Åhgren <peah@webrtc.org>
Date: Mon Feb 19 14:48:27 2018

Merge to M65: Increased the size of the delay estimation look window

(cherry picked from commit 75b57d3bb894aa51da3a4bcf1bb455785b7a4098)

TBR: gustaf@webrtc.org,henrik.lundin@webrtc.org
Bug:  webrtc:8889 , chromium:812524 
Change-Id: I508a0d3619b90f6a8851db66c151a980b14adb55
Reviewed-on: https://webrtc-review.googlesource.com/53640
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22030}
Reviewed-on: https://webrtc-review.googlesource.com/54900
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/65@{#23}
Cr-Branched-From: 3ac67a736bb200ecf7c116a88b2f8d5c542973c8-refs/heads/master@{#21637}
[modify] https://crrev.com/36af4e9614f707f733eb2340fae66d6325aaac5b/modules/audio_processing/include/audio_processing.h

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

Status: Fixed (was: Assigned)
Project Member

Comment 9 by sheriffbot@chromium.org, Feb 22 2018

Cc: gov...@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
Project Member

Comment 10 by sheriffbot@chromium.org, Feb 26 2018

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-65
Removing "Merge-Approved-65" label as this is already merged at #7.

Sign in to add a comment