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

Issue 839860 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

AEC3: External delay is not applied correctly

Project Member Reported by gustaf@chromium.org, May 4 2018

Issue description

When an audio delay is reported to AEC3 via SetAudioBufferDelay it is not correctly applied in AEC3. This can cause unnecessary echo in the beginning of a call.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2018

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

commit a49eacb30a7126577bffe8577061aeccbc991a67
Author: Gustaf Ullberg <gustaf@webrtc.org>
Date: Fri May 04 16:52:24 2018

AEC3: External delay - Fix mismatch in time units

Fixes a confusion of time units (milliseconds vs blocks) of externally
reported audio delay. This fix reduces the risk of echo in the beginning
of a call.

Bug:  webrtc:9241 , chromium:839860 
Change-Id: I534cc15d6b215a5881ae46759f573a56871170a3
Reviewed-on: https://webrtc-review.googlesource.com/74589
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23128}
[modify] https://crrev.com/a49eacb30a7126577bffe8577061aeccbc991a67/modules/audio_processing/aec3/render_delay_buffer.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 7 2018

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

commit 931c6c067b2891c89c9cbd2d3831e38f5d3e0b80
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon May 07 12:11:56 2018

Roll src/third_party/webrtc/ e6256055e..823f9135f (18 commits)

https://webrtc.googlesource.com/src.git/+log/e6256055e7db..823f9135f858

$ git log e6256055e..823f9135f --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:None,chromium:None,chromium:839860,chromium:none


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

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=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ief34c2bd69b2feb972c8ffa05f039c8b34c87a25
Reviewed-on: https://chromium-review.googlesource.com/1046925
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#556419}
[modify] https://crrev.com/931c6c067b2891c89c9cbd2d3831e38f5d3e0b80/DEPS

Project Member

Comment 3 by bugdroid1@chromium.org, May 7 2018

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

commit 623d28179239fa5f31fdf6c706cc5de6bcc5f3a7
Author: Gustaf Ullberg <gustaf@webrtc.org>
Date: Mon May 07 13:49:28 2018

Correcting the use of externally reported delay in AEC3

Externally reported delay affects internal delay of the render delay buffer.

Bug:  webrtc:9241 , chromium:839860 
Change-Id: Ia4e67eaea739e732dd6dcfec343dd7ee37ef883f
Reviewed-on: https://webrtc-review.googlesource.com/74704
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Gustaf Ullberg <gustaf@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23144}
[modify] https://crrev.com/623d28179239fa5f31fdf6c706cc5de6bcc5f3a7/modules/audio_processing/aec3/render_delay_buffer.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 7 2018

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

commit 71098015063adea710cce4616686d08d65fc1940
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Mon May 07 20:43:08 2018

Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits)

https://webrtc.googlesource.com/src.git/+log/823f9135f858..d5ef6ff258da

$ git log 823f9135f..d5ef6ff25 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402


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

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=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ibaedd51469f75941f90aed9f4ba6bb686cd79f6c
Reviewed-on: https://chromium-review.googlesource.com/1048112
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#556561}
[modify] https://crrev.com/71098015063adea710cce4616686d08d65fc1940/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, May 8 2018

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

commit d1429ef7c2699d69eec780f6f8beba5ecab9e312
Author: Kunihiko Sakamoto <ksakamoto@chromium.org>
Date: Tue May 08 03:27:37 2018

Revert "Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits)"

This reverts commit 71098015063adea710cce4616686d08d65fc1940.

Reason for revert: ios build is still broken.
https://ci.chromium.org/buildbot/chromium.mac/ios-device-xcode-clang/59679

Original change's description:
> Roll src/third_party/webrtc/ 823f9135f..d5ef6ff25 (15 commits)
> 
> https://webrtc.googlesource.com/src.git/+log/823f9135f858..d5ef6ff258da
> 
> $ git log 823f9135f..d5ef6ff25 --date=short --no-merges --format='%ad %ae %s'
> 
> Created with:
>   roll-dep src/third_party/webrtc
> BUG=chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402
> 
> 
> The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org
> 
> 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=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
> TBR=webrtc-chromium-sheriffs-robots@google.com
> 
> Change-Id: Ibaedd51469f75941f90aed9f4ba6bb686cd79f6c
> Reviewed-on: https://chromium-review.googlesource.com/1048112
> Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
> Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/master@{#556561}

TBR=webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com,webrtc-chromium-sheriffs-robots@google.com

Change-Id: I2823387c7aadad799befbf88cc5b4503a379dd3c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:None, chromium:840347, chromium:839860, chromium:838402
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1049345
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556675}
[modify] https://crrev.com/d1429ef7c2699d69eec780f6f8beba5ecab9e312/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, May 8 2018

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

commit ebc7dc2c2cccb2e04ed594fa68c0a1fb0d7a59e6
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue May 08 14:16:42 2018

Roll src/third_party/webrtc/ 823f9135f..826738b78 (29 commits)

https://webrtc.googlesource.com/src.git/+log/823f9135f858..826738b78c6a

$ git log 823f9135f..826738b78 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:840347,chromium:839860,chromium:838402


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

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=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: I555e922aefb9dac0e262c32fe8562a47f7a15144
Reviewed-on: https://chromium-review.googlesource.com/1049945
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#556789}
[modify] https://crrev.com/ebc7dc2c2cccb2e04ed594fa68c0a1fb0d7a59e6/DEPS

Project Member

Comment 7 by bugdroid1@chromium.org, May 9 2018

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

commit e05c43cc396818059ffae4552f6c5a133d99e24b
Author: Per Åhgren <peah@webrtc.org>
Date: Wed May 09 12:34:26 2018

Remove the headroom and delay estimation feedback loop in AEC3

This CL ensures that the external audio buffer delay is correctly used
by removing the applied headroom and avoiding that the delay estimation
feedback fromt the echo remover overrules the external delay
information.

Bug:  webrtc:9241 , chromium:839860 
Change-Id: I53cc78ace34a71994ab24a3b552f29979e2aae78
Reviewed-on: https://webrtc-review.googlesource.com/75513
Reviewed-by: Gustaf Ullberg <gustaf@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23189}
[modify] https://crrev.com/e05c43cc396818059ffae4552f6c5a133d99e24b/modules/audio_processing/aec3/echo_remover.cc
[modify] https://crrev.com/e05c43cc396818059ffae4552f6c5a133d99e24b/modules/audio_processing/aec3/render_delay_buffer.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 9 2018

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

commit 0ca6086c6e395647e883c8ade6a63a51bd84d033
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed May 09 17:14:33 2018

Roll src/third_party/webrtc/ c6c44268b..a29b14855 (4 commits)

https://webrtc.googlesource.com/src.git/+log/c6c44268bcb5..a29b148557b0

$ git log c6c44268b..a29b14855 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:826914, chromium:841193 ,chromium:851187, chromium:839860 


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

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=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ia175141797bf07db2094a2b43e74a71db4377ecc
Reviewed-on: https://chromium-review.googlesource.com/1052216
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#557224}
[modify] https://crrev.com/0ca6086c6e395647e883c8ade6a63a51bd84d033/DEPS

Comment 9 by peah@chromium.org, May 11 2018

Cc: huib@chromium.org hlundin@chromium.org peah@chromium.org
Labels: Merge-Request-67
We would like to merge this into M67. 

The CLs address problems with the usage of the externally reported audio buffer delay during VoIP calls that was identified during testing of the last M67 Beta. 

The CLs related to this that we would like to merge are:
-https://webrtc-review.googlesource.com/c/src/+/74589 (AEC3: External delay - Fix mismatch in time units)
-https://webrtc-review.googlesource.com/c/src/+/74704 (Correcting the use of externally reported delay in AEC3)
-https://webrtc-review.googlesource.com/c/src/+/75513 (Remove the headroom and delay estimation feedback loop in AEC3)

The CLs have been tested both in Canary and in offline testing and work well.

The merge is safe in the sense that the code affected is beneath an experimental flag. Furthermore, a field_trial switch was added that can be used to turn back on the effect of the headroom. 
Project Member

Comment 10 by sheriffbot@chromium.org, May 11 2018

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

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls apply appropriate OSs label. Thank you.
Labels: -Merge-Review-67 Merge-Approved-67 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Approving merge to M67 branch 3396 based on https://bugs.chromium.org/p/chromium/issues/detail?id=841193#c6. 

Comment 13 by peah@chromium.org, May 13 2018

The CLs for this issue have now been merged.

The merging CLs (for the CLs listed in #9) are:
-https://webrtc-review.googlesource.com/c/src/+/76324
-https://webrtc-review.googlesource.com/c/src/+/76325
-https://webrtc-review.googlesource.com/c/src/+/76326


Comment 14 by peah@chromium.org, May 13 2018

Labels: -Merge-Approved-67 Merge-Merged M-67 merge-merged-67

Comment 15 by peah@chromium.org, May 13 2018

Status: Fixed (was: Started)

Sign in to add a comment