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

Issue 826720 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

Headsets performance is worse in AEC3 is it is not used initially in the call

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

Issue description

When a call is started without a headset, and a headset is introduced during a call, the AEC3 transparency is not as good as if the headset is used from the start of the call.
 
Project Member

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

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

commit 8131eb0667793312004f64fa5548b31c618360a7
Author: Per Åhgren <peah@webrtc.org>
Date: Wed Mar 28 17:28:46 2018

Allow the headset mode to be entered after the call has started

This CL adds a timeout for the detection of the headset mode that
allows it to be entered also for the cases where a headset is
inserted during the call.

Bug:  chromium:826720 , webrtc:9083 
Change-Id: Ic3cb4cc0258997a74eccd1bcdf65765e44016ad8
Reviewed-on: https://webrtc-review.googlesource.com/65240
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22658}
[modify] https://crrev.com/8131eb0667793312004f64fa5548b31c618360a7/modules/audio_processing/aec3/aec_state.cc
[modify] https://crrev.com/8131eb0667793312004f64fa5548b31c618360a7/modules/audio_processing/aec3/aec_state.h

Project Member

Comment 2 by bugdroid1@chromium.org, Mar 28 2018

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

commit 85eef49fa2cd44657bd0ce8508ba8b8ce0514fbd
Author: Per Åhgren <peah@webrtc.org>
Date: Wed Mar 28 18:15:57 2018

Further decrease the AEC3 look window in the nonlinear mode

This CL further decreases the look window size, as well
as the effect of the look window used by AEC3 when is is
in the nonlinear mode.

Bug:  chromium:826720 , webrtc:9083 
Change-Id: I193539c0af74eea18d2821a3b7e1fae2f783d38a
Reviewed-on: https://webrtc-review.googlesource.com/65161
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22659}
[modify] https://crrev.com/85eef49fa2cd44657bd0ce8508ba8b8ce0514fbd/api/audio/echo_canceller3_config.h

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

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

commit 2e39e58de6dc45d9ed16c3e878c969e96af23649
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Mar 29 03:34:36 2018

Roll src/third_party/webrtc/ 467057ec7..7abc9a07d (18 commits)

https://webrtc.googlesource.com/src.git/+log/467057ec7fe7..7abc9a07d774

$ git log 467057ec7..7abc9a07d --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG=chromium:None,chromium:None,chromium:826720,chromium:826720,chromium:826655,chromium:None,chromium:b/77199993,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;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ib24def67d5d129b206f2dc5b390e371701b028e3
Reviewed-on: https://chromium-review.googlesource.com/984728
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@{#546726}
[modify] https://crrev.com/2e39e58de6dc45d9ed16c3e878c969e96af23649/DEPS

Comment 4 by peah@chromium.org, Mar 29 2018

Status: Fixed (was: Assigned)

Comment 5 by peah@chromium.org, Apr 4 2018

Cc: huib@chromium.org hlundin@chromium.org
Labels: Merge-Request-66
We would like this to be merged to M66. 

These chances removes the last problem remaining from M65 of echo canceller headset performance  and is needed to properly address the case when a headset is inserted during a call.
The issue it addressed was introduced in M64/65.

The merge is safe in that the code changed is beneath an experimental flag which can be used to turn the code off if needed.
  
The change has been verified in Canary and performance looks good.
Project Member

Comment 6 by sheriffbot@chromium.org, Apr 4 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Less than 9 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

Comment 7 by peah@chromium.org, Apr 4 2018

I just want to add that if it helps for the next beta release, I'm on standby for doing the merge immediately if it is approved.

Comment 8 by cmasso@google.com, Apr 4 2018

Do you mean in #5 that no CL will be merged into the release if we wanted to turn this off on stable? That turning this off will completely be done via finch?

Comment 9 by peah@chromium.org, Apr 4 2018

Sorry for the confusion!

What I meant is that all the code affected by the CLs is only active beneath a finch flag (WebRtcUseEchoCanceller3) and that the merge is safe in the sense that the merged code can be deactivated by that flag in case anything would go wrong with that code.

The CLs in question will, however, still be merged into the release.

Comment 10 by cmasso@google.com, Apr 5 2018

I will approve this merge if after merging #5 into M66 now, there will no be a need to merge any additional cl in stable if something wrong was to happen with this code. Please confirm.

Comment 11 by peah@chromium.org, Apr 5 2018

Yes, that is correct. 
No merges will be necessary for Stable due to this merge. The code is fully beneath a finch flag, and if the merged code would not be performing as it should, it will be deactivated with that flag rather than fixed by merging CLs to Stable.

Comment 12 by cmasso@google.com, Apr 6 2018

Labels: -Hotlist-Merge-Review -Merge-Review-66 Merge-Approved-66
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 7 2018

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

commit 77e98e47cf8d497c086756ee26a316980b70c75c
Author: Per Åhgren <peah@webrtc.org>
Date: Sat Apr 07 21:33:05 2018

Merge to M66: Allow the headset mode to be entered after the call ...

This CL adds a timeout for the detection of the headset mode that
allows it to be entered also for the cases where a headset is
inserted during the call.

(cherry picked from commit 8131eb0667793312004f64fa5548b31c618360a7)

TBR: alessiob@webrtc.org, henrik.lundin@webrtc.org
Bug:  chromium:826720 , webrtc:9083 
Change-Id: Ic3cb4cc0258997a74eccd1bcdf65765e44016ad8
Reviewed-on: https://webrtc-review.googlesource.com/65240
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Per Åhgren <peah@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22658}
Reviewed-on: https://webrtc-review.googlesource.com/67780
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/66@{#18}
Cr-Branched-From: 12c8110e8c717b7f0f87615d3b99caac2a69fa6c-refs/heads/master@{#22215}
[modify] https://crrev.com/77e98e47cf8d497c086756ee26a316980b70c75c/modules/audio_processing/aec3/aec_state.cc
[modify] https://crrev.com/77e98e47cf8d497c086756ee26a316980b70c75c/modules/audio_processing/aec3/aec_state.h

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 7 2018

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

commit e92dc873a5f84ae73ff92b2662948c25e7983f5b
Author: Per Åhgren <peah@webrtc.org>
Date: Sat Apr 07 21:40:52 2018

Merge to M66: Further decrease the AEC3 look window in the nonlinear ...

This CL further decreases the look window size, as well
as the effect of the look window used by AEC3 when is is
in the nonlinear mode.

(cherry picked from commit 85eef49fa2cd44657bd0ce8508ba8b8ce0514fbd)

TBR: alessiob@webrtc.org,henrik.lundin@webrtc.org
Bug:  chromium:826720 , webrtc:9083 
Change-Id: I193539c0af74eea18d2821a3b7e1fae2f783d38a
Reviewed-on: https://webrtc-review.googlesource.com/65161
Commit-Queue: Per Åhgren <peah@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22659}
Reviewed-on: https://webrtc-review.googlesource.com/67800
Reviewed-by: Per Åhgren <peah@webrtc.org>
Cr-Commit-Position: refs/branch-heads/66@{#19}
Cr-Branched-From: 12c8110e8c717b7f0f87615d3b99caac2a69fa6c-refs/heads/master@{#22215}
[modify] https://crrev.com/e92dc873a5f84ae73ff92b2662948c25e7983f5b/api/audio/echo_canceller3_config.h

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

Labels: M-66
Project Member

Comment 16 by sheriffbot@chromium.org, Apr 10 2018

Cc: cmasso@google.com
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

Comment 17 by peah@chromium.org, Apr 10 2018

Labels: -Merge-Approved-66

Sign in to add a comment