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

Issue 902262 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 7
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

AEC3: Comfort noise is too low

Project Member Reported by gustaf@chromium.org, Nov 6

Issue description

The power of the comfort noise that is estimated and applied during
suppression in AEC3 is too low. This makes it more obvious when
suppression is applied and also prevents the noise from masking low
volume echo.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 6

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

commit 83b00f020e2feb9360d74db65573f5442e1b6494
Author: Gustaf Ullberg <gustaf@webrtc.org>
Date: Tue Nov 06 16:10:52 2018

AEC3: Computation of comfort noise gains from suppression gains

This change corrects the computation of the comfort noise gains.

Previously the comfort noise gain of band k, CG_k, was computed
from suppression gain of band k, SG_k, as:
CG_k = 1 - SG_k

But since the two signals are uncorrelated (the comfort noise
is randomly generated), the correct gain to maintain power is:
CG_k = sqrt(1 - SG_k^2).

Bug:  webrtc:9967 , chromium:902262 
Change-Id: I393495742163d5e658bca4ab2f7a5067ab15af01
Reviewed-on: https://webrtc-review.googlesource.com/c/109580
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25525}
[modify] https://crrev.com/83b00f020e2feb9360d74db65573f5442e1b6494/modules/audio_processing/aec3/echo_remover.cc
[modify] https://crrev.com/83b00f020e2feb9360d74db65573f5442e1b6494/modules/audio_processing/aec3/suppression_filter.cc
[modify] https://crrev.com/83b00f020e2feb9360d74db65573f5442e1b6494/modules/audio_processing/aec3/suppression_filter.h
[modify] https://crrev.com/83b00f020e2feb9360d74db65573f5442e1b6494/modules/audio_processing/aec3/suppression_filter_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 6

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

commit 020e583291a8026b77e01d6e878a760977c24b32
Author: Gustaf Ullberg <gustaf@webrtc.org>
Date: Tue Nov 06 16:17:02 2018

AEC3: Compensate comfort noise level for loss due to filter bank

The analysis and synthesis windowing cause loss of power when
cross-fading the noise where frames are completely uncorrelated
(generated with random phase).

This CL also removes duplicate code and enables platform specific
optimizations for ARM in the comfort noise generation.

Bug:  webrtc:9967 , chromium:902262 
Change-Id: Iffd59b301876442079d4a5f2c7fac55a3522397c
Reviewed-on: https://webrtc-review.googlesource.com/c/109581
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25526}
[modify] https://crrev.com/020e583291a8026b77e01d6e878a760977c24b32/modules/audio_processing/aec3/comfort_noise_generator.cc
[modify] https://crrev.com/020e583291a8026b77e01d6e878a760977c24b32/modules/audio_processing/aec3/comfort_noise_generator_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 7

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

commit 9eae3e2be6375d5c22a8227064476018cdbf70d3
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Wed Nov 07 00:52:43 2018

Roll src/third_party/webrtc 34fc346a0c77..dc98b9b975fa (5 commits)

https://webrtc.googlesource.com/src.git/+log/34fc346a0c77..dc98b9b975fa


git log 34fc346a0c77..dc98b9b975fa --date=short --no-merges --format='%ad %ae %s'
2018-11-06 peah@webrtc.org AEC3: Corrected include
2018-11-06 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 7841106b37..793c8566ab (605607:605715)
2018-11-06 sprang@webrtc.org Update Android encoder to use GetEncoderInfo()
2018-11-06 gustaf@webrtc.org AEC3: Compensate comfort noise level for loss due to filter bank
2018-11-06 gustaf@webrtc.org AEC3: Computation of comfort noise gains from suppression gains


Created with:
  gclient setdep -r src/third_party/webrtc@dc98b9b975fa

The AutoRoll server is located here: https://autoroll.skia.org/r/webrtc-chromium-autoroll

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=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG=chromium:None,chromium:902262,chromium:902262
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I3e5de56c6457fe51e999dd549f6f476401e4f482
Reviewed-on: https://chromium-review.googlesource.com/c/1320613
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#605883}
[modify] https://crrev.com/9eae3e2be6375d5c22a8227064476018cdbf70d3/DEPS

Status: Fixed (was: Started)
Labels: Merge-Request-71
We would like to merge this into M71. It addresses an issue identified during release testing of the WebRTC echo canceller (AEC) functionality in Chrome M71.

This solves an issue where the noise levels were wrong, causing bad audio quality.

We consider this is a low risk CL to merge, but in the very unlikely event that something goes wrong AEC3 can be disabled remotely.

The fixes have been verified in Canary.

The CLs we would like to merge are:
https://webrtc-review.googlesource.com/c/109580
https://webrtc-review.googlesource.com/c/109581


Project Member

Comment 6 by sheriffbot@chromium.org, Nov 9

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #5. Pls merge ASAP.

Sign in to add a comment