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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks ago by peah
Modified:
1 week, 3 days 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 weeks, 4 days 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 weeks, 4 days 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 weeks, 3 days 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 ...
1 week, 5 days ago (2017-02-15 13:24:25 UTC) #14
tommi (krómíum)
lgtm
1 week, 5 days 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 ...
1 week, 5 days ago (2017-02-15 17:16:18 UTC) #17
hlundin-chromium
lgtm with Alexei's comment.
1 week, 4 days 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 ...
1 week, 4 days ago (2017-02-16 13:08:57 UTC) #19
jochen (OOO until Mar 6)
lgtm
1 week, 4 days ago (2017-02-16 14:47:08 UTC) #20
peah
Thanks everyone for the quick review!
1 week, 4 days 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
1 week, 4 days 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: ...
1 week, 3 days 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
1 week, 3 days 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 ...
1 week, 3 days 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
1 week, 3 days ago (2017-02-17 05:40:55 UTC) #33
commit-bot: I haz the power
1 week, 3 days 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 f8e48bd