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

Issue 776327 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Feature

Blocked on:
issue 837661
issue 912847



Sign in to add a comment

Use native echo canceller on Mac for WebRTC

Project Member Reported by grunell@chromium.org, Oct 19 2017

Issue description

Since Sierra (10.12), the Voice-Processing I/O Audio Unit is available on Mac. It features echo cancellation, automatic gain control, ducking of other audio, and avoids the ambient noise reduction.

Use this on Mac with, to begin with, the automatic gain control disabled, instead of the WebRTC echo canceller. The ducking doesn't seem to be controllable.
 
Owner: ossu@chromium.org
Project Member

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

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

commit 853f0dfc7ac8321736a09598e5dc87dd829daf5d
Author: Oskar Sundbom <ossu@chromium.org>
Date: Thu Mar 01 11:45:03 2018

Add experimental support for the native macOS echo canceller

This CL adds platform support for the native echo canceller available in macOS 10.12+, as well as a way to enable it through a runtime flag or Origin Trial.
A native echo canceller has the opportunity to perform better than a general one in that it can be tuned for the hardware platform in question. In this case, it's also closer to the hardware, so it's less likely to be affected by IPC glitches.

Intent to Implement discussion:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/kEld5OudUOM

Bug: chromium:776327
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia828ae2b9e6503ca814de4154dee96c63b838e1d
Reviewed-on: https://chromium-review.googlesource.com/941321
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Reviewed-by: Rick Byers <rbyers@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Tommi <tommi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540130}
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/media_stream_audio_processor_options.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/media_stream_constraints_util_audio.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/media_stream_constraints_util_audio.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/media_stream_constraints_util_audio_unittest.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/content/renderer/media/stream/user_media_processor.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/media/audio/mac/audio_auhal_mac.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/media/audio/mac/audio_low_latency_input_mac.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/media/audio/mac/audio_low_latency_input_mac.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/media/audio/mac/audio_manager_mac.cc
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/media/base/audio_parameters.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/Source/modules/exported/WebUserMediaRequest.cpp
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/Source/modules/mediastream/UserMediaRequest.cpp
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/Source/modules/mediastream/UserMediaRequest.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/Source/platform/runtime_enabled_features.json5
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/public/platform/web_feature.mojom
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/third_party/WebKit/public/web/WebUserMediaRequest.h
[modify] https://crrev.com/853f0dfc7ac8321736a09598e5dc87dd829daf5d/tools/metrics/histograms/enums.xml

Project Member

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

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

commit cbd7febac3151d6f54ce0c5de94c58ba1d6ae8de
Author: Oskar Sundbom <ossu@chromium.org>
Date: Wed Mar 14 11:43:00 2018

Fix origin trial name of ExperimentalHardwareEchoCancellation

Just a simple copy/paste error, I'm afraid.

Intent to Experiment: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/3MYKIKqyaNA

Bug: chromium:776327
Change-Id: I07191b2c1c4a3c3eff69d9b4a5abd604ee6ba01a
Reviewed-on: https://chromium-review.googlesource.com/958906
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543055}
[modify] https://crrev.com/cbd7febac3151d6f54ce0c5de94c58ba1d6ae8de/third_party/WebKit/Source/platform/runtime_enabled_features.json5

Comment 4 by ossu@chromium.org, Mar 14 2018

Labels: Merge-Request-66
Requesting a merge of the change in #3 to M66. It's just a simple one-line string change in a json file: I'd put the wrong name in for the Origin Trial flag. Since it's not a code change, I don't think it should need soaking in Canary first.
Labels: -Merge-Request-66 Merge-Approved-66
Approving M66. Branch:3359
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 15 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4211eabd1238fac3e8b7a4bed58b5229aaf61eec

commit 4211eabd1238fac3e8b7a4bed58b5229aaf61eec
Author: Oskar Sundbom <ossu@chromium.org>
Date: Thu Mar 15 10:07:12 2018

Fix origin trial name of ExperimentalHardwareEchoCancellation

Just a simple copy/paste error, I'm afraid.

Intent to Experiment: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/3MYKIKqyaNA

Bug: chromium:776327
Change-Id: I07191b2c1c4a3c3eff69d9b4a5abd604ee6ba01a
Reviewed-on: https://chromium-review.googlesource.com/958906
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#543055}(cherry picked from commit cbd7febac3151d6f54ce0c5de94c58ba1d6ae8de)
Reviewed-on: https://chromium-review.googlesource.com/964002
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#266}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/4211eabd1238fac3e8b7a4bed58b5229aaf61eec/third_party/WebKit/Source/platform/runtime_enabled_features.json5

Project Member

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

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

commit 5d797d36ef90355fe15a65e6a2f6a2bb46a76814
Author: Oskar Sundbom <ossu@chromium.org>
Date: Wed Mar 28 14:06:56 2018

Mac AEC: Fix two errors in upmixing.

The code was skipping the first sample and calculated output sample
positions slightly wrong. These two issues lead to discontinuities in
the audio.

Bug: chromium:776327
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia39071a1f5f14082f5fcd1cb6032cf45ea5a244a
Reviewed-on: https://chromium-review.googlesource.com/983492
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546476}
[modify] https://crrev.com/5d797d36ef90355fe15a65e6a2f6a2bb46a76814/media/audio/mac/audio_low_latency_input_mac.cc
[modify] https://crrev.com/5d797d36ef90355fe15a65e6a2f6a2bb46a76814/media/audio/mac/audio_low_latency_input_mac.h
[modify] https://crrev.com/5d797d36ef90355fe15a65e6a2f6a2bb46a76814/media/audio/mac/audio_low_latency_input_mac_unittest.cc

Comment 8 by ossu@chromium.org, Apr 3 2018

Labels: Merge-Request-66
Requesting merge of the CL in comment #7.

During beta we discovered a problem with the upmixing in the macOS AEC integration. The CL in #7 fixes that. It's been in Canary for a few days. It's a small bug fix (and some added tests). The functionality is behind a flag, but slated for an origin trial, so risk is very low.
Project Member

Comment 9 by sheriffbot@chromium.org, Apr 3 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
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
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 4 2018

Labels: -merge-approved-66
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4e36026e413ccc2757f7ab4ebb564a16aafc234

commit b4e36026e413ccc2757f7ab4ebb564a16aafc234
Author: Oskar Sundbom <ossu@chromium.org>
Date: Wed Apr 04 08:07:43 2018

Mac AEC: Fix two errors in upmixing.

The code was skipping the first sample and calculated output sample
positions slightly wrong. These two issues lead to discontinuities in
the audio.

(cherry picked from commit 5d797d36ef90355fe15a65e6a2f6a2bb46a76814)

Bug: chromium:776327
Change-Id: I310df4ddf0414c89e5cac548f9ec6fe6b4893da7
Reviewed-on: https://chromium-review.googlesource.com/993572
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#574}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/b4e36026e413ccc2757f7ab4ebb564a16aafc234/media/audio/mac/audio_low_latency_input_mac.cc
[modify] https://crrev.com/b4e36026e413ccc2757f7ab4ebb564a16aafc234/media/audio/mac/audio_low_latency_input_mac.h
[modify] https://crrev.com/b4e36026e413ccc2757f7ab4ebb564a16aafc234/media/audio/mac/audio_low_latency_input_mac_unittest.cc

Comment 12 by hbos@chromium.org, Apr 4 2018

Labels: FoundIn-66 M-67
Is this Fixed or Verified now?

Comment 13 by ossu@chromium.org, Apr 5 2018

Status: Started (was: Assigned)
If anything, it's at least Started. :)

Comment 14 by ossu@chromium.org, Apr 27 2018

Blockedon: 837661
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 6

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

commit 63f2d772b2664b4572190bcd497280afdc6d559f
Author: Henrik Grunell <grunell@chromium.org>
Date: Thu Dec 06 17:25:20 2018

Feature to enable system AEC.

This lets users enable the system AEC for testing purposes.

If there is no system echo canceller available, getUserMedia with echo cancellation enabled will fail.

Bug: 776327
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ie041bc1d205762875f9ad4817d8d3441d967256e
Reviewed-on: https://chromium-review.googlesource.com/c/1113440
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614397}
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/chrome/browser/about_flags.cc
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/chrome/browser/flag-metadata.json
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/content/renderer/media/stream/media_stream_constraints_util_audio.cc
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/media/audio/audio_features.cc
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/media/audio/audio_features.h
[modify] https://crrev.com/63f2d772b2664b4572190bcd497280afdc6d559f/tools/metrics/histograms/enums.xml

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 7

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

commit 73f4f1f9134f280b31646f4891153b5f53181fc9
Author: Armando Miraglia <armax@chromium.org>
Date: Fri Dec 07 08:15:05 2018

[getUserMedia] Fix IsEmpty on EC container.

CL crrev.com/c/1113440 showed that the current definition of IsEmpty in
the EchoCancellationContainer class is incorrect. In fact, the whole
container should **not** be considered empty if the allowed values for
EC types is empty.

BUG=776327
TESTED=./out/Default/content_unittests && manual test with system flag.
R=grunell@chromium.org, guidou@chromium.org

Change-Id: I71a9d6ed73b399951f9e3f3cd9dc55230db9c002
Reviewed-on: https://chromium-review.googlesource.com/c/1365449
Commit-Queue: Armando Miraglia <armax@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614640}
[modify] https://crrev.com/73f4f1f9134f280b31646f4891153b5f53181fc9/content/renderer/media/stream/media_stream_constraints_util_audio.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 10

Labels: merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29aa206f1f4c906f1a604f90a8c049aa5139828f

commit 29aa206f1f4c906f1a604f90a8c049aa5139828f
Author: Henrik Grunell <grunell@chromium.org>
Date: Mon Dec 10 07:49:37 2018

Feature to enable system AEC.

This lets users enable the system AEC for testing purposes.

If there is no system echo canceller available, getUserMedia with echo cancellation enabled will fail.

TBR=grunell@chromium.org

(cherry picked from commit 63f2d772b2664b4572190bcd497280afdc6d559f)

Bug: 776327
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ie041bc1d205762875f9ad4817d8d3441d967256e
Reviewed-on: https://chromium-review.googlesource.com/c/1113440
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614397}
Reviewed-on: https://chromium-review.googlesource.com/c/1369844
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#173}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/chrome/browser/about_flags.cc
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/chrome/browser/flag-metadata.json
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/content/renderer/media/stream/media_stream_constraints_util_audio.cc
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/media/audio/audio_features.cc
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/media/audio/audio_features.h
[modify] https://crrev.com/29aa206f1f4c906f1a604f90a8c049aa5139828f/tools/metrics/histograms/enums.xml

Blockedon: 912847
Labels: CommitLog-Audit-Violation Merge-Without-Approval M-72
Here's a summary of the rules that were executed: 
 - OnlyMergeApprovedChange: Rule Failed -- Revision 29aa206f1f4c906f1a604f90a8c049aa5139828f was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! 
Please explain why this change was merged to the branch!
 - AcknowledgeMerge: Notification Required -- 
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/29aa206f1f4c906f1a604f90a8c049aa5139828f

Commit: 29aa206f1f4c906f1a604f90a8c049aa5139828f
Author: grunell@chromium.org
Commiter: grunell@chromium.org
Date: 2018-12-10 07:49:37 +0000 UTC

Feature to enable system AEC.

This lets users enable the system AEC for testing purposes.

If there is no system echo canceller available, getUserMedia with echo cancellation enabled will fail.

TBR=grunell@chromium.org

(cherry picked from commit 63f2d772b2664b4572190bcd497280afdc6d559f)

Bug: 776327
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ie041bc1d205762875f9ad4817d8d3441d967256e
Reviewed-on: https://chromium-review.googlesource.com/c/1113440
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614397}
Reviewed-on: https://chromium-review.googlesource.com/c/1369844
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#173}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
was there approval for this merge to M72?
Yes, it was approved in  issue 912847 .
Labels: -CommitLog-Audit-Violation -Merge-Without-Approval
M72 merge was approved here - https://bugs.chromium.org/p/chromium/issues/detail?id=912847#c2.

Sign in to add a comment