Chromium Code Reviews
Help | Chromium Project | Sign in
(4)

Issue 2677893002: Adding functionality for activating for the WebRTC EchoCanceller3 functionality in Chrome. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 2 weeks ago by peah
Modified:
2 months ago
CC:
avayvod+watch_chromium.org, chfremer+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, imcheng+watch_chromium.org, isheriff+watch_chromium.org, jam, jasonroberts+watch_google.com, mcasas+watch+vc_chromium.org, miu+watch_chromium.org, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, xjz+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Adding functionality for activating for the WebRTC EchoCanceller3 functionality in Chrome. This CL adds the ability to activate the upcoming WebRTC AEC3 functionality in Chrome. The AEC3 should be possible to activate both via an experiment as a feature, and as a Chrome flag (via chrome://flags and the command line). BUG=688388 Review-Url: https://codereview.chromium.org/2677893002 Cr-Commit-Position: refs/heads/master@{#451251} Committed: https://chromium.googlesource.com/chromium/src/+/1767e59153ee506fe6394efe4018b4cd3f2a2671

Patch Set 1 #

Total comments: 12

Patch Set 2 : Changes in response to reviewer comments #

Total comments: 1

Patch Set 3 : Changed the histogram name #

Total comments: 2

Patch Set 4 : Added WebRtcUseEchoCanceller3:disabled to histograms.xml and corrected the values #

Patch Set 5 : Rebase #

Patch Set 6 : Reverted formatting done by mistake #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -0 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M content/public/common/content_features.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M content/public/common/content_features.cc View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M content/renderer/media/media_stream_audio_processor.cc View 1 2 chunks +6 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 36 (20 generated)
peah
2 months, 2 weeks ago (2017-02-08 18:52:23 UTC) #5
hlundin-chromium
Mostly LG, but I think you've declared the feature too many times. See inline. https://codereview.chromium.org/2677893002/diff/60001/chrome/browser/about_flags.cc ...
2 months, 1 week ago (2017-02-09 10:20:45 UTC) #7
mcasas
Drive-by, don't mind me. Also, BUG=XYZ, don't need crbug: prepended. https://codereview.chromium.org/2677893002/diff/60001/content/public/common/content_features.cc File content/public/common/content_features.cc (right): https://codereview.chromium.org/2677893002/diff/60001/content/public/common/content_features.cc#newcode264 ...
2 months, 1 week ago (2017-02-10 18:13:01 UTC) #9
peah
Hi, I've created this CL for allowing activation in Chrome of the upcoming WebRTC new ...
2 months, 1 week ago (2017-02-15 13:24:25 UTC) #14
tommi - chröme
lgtm
2 months, 1 week ago (2017-02-15 16:18:43 UTC) #16
Alexei Svitkine (slow)
lgtm https://codereview.chromium.org/2677893002/diff/110001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2677893002/diff/110001/tools/metrics/histograms/histograms.xml#newcode96475 tools/metrics/histograms/histograms.xml:96475: + <int value="241711238" label="WebRtcUseEchoCanceller3:disabled"/> I think you need ...
2 months, 1 week ago (2017-02-15 17:16:18 UTC) #17
hlundin-chromium
lgtm with Alexei's comment.
2 months ago (2017-02-16 12:51:58 UTC) #18
peah
https://codereview.chromium.org/2677893002/diff/110001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2677893002/diff/110001/tools/metrics/histograms/histograms.xml#newcode96475 tools/metrics/histograms/histograms.xml:96475: + <int value="241711238" label="WebRtcUseEchoCanceller3:disabled"/> On 2017/02/15 17:16:18, Alexei Svitkine ...
2 months ago (2017-02-16 13:08:57 UTC) #19
jochen
lgtm
2 months ago (2017-02-16 14:47:08 UTC) #20
peah
Thanks everyone for the quick review!
2 months ago (2017-02-16 16:04:37 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2677893002/130001
2 months ago (2017-02-16 16:05:11 UTC) #24
commit-bot: I haz the power
Failed to apply patch for tools/metrics/histograms/histograms.xml: While running git apply --index -p1; error: patch failed: ...
2 months ago (2017-02-16 19:56:27 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2677893002/170001
2 months ago (2017-02-17 01:52:58 UTC) #29
commit-bot: I haz the power
Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on ...
2 months ago (2017-02-17 03:55:37 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2677893002/170001
2 months ago (2017-02-17 05:40:55 UTC) #33
commit-bot: I haz the power
2 months ago (2017-02-17 06:44:28 UTC) #36
Message was sent while issue was closed.
Committed patchset #6 (id:170001) as
https://chromium.googlesource.com/chromium/src/+/1767e59153ee506fe6394efe4018...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46