Added a chrome flag to enable Delay Agnostic AEC in WebRTC

Added a chrome flag to enable Delay Agnostic AEC in WebRTC

There has been echo issues on Mac when using WebRTC for quite some time and a possible solution is to use a delay agnostic AEC. It has been turned on under a Finch experiment (crbug/385073).
Recently, there has been a lot of echo issues on CrOS of which a delay agnostic AEC would have helped the user to get rid of the echo. The need for a chrome flag, to help in such situations and also to simplify the test and verification process of delay agnostic AEC itself, is clear.

This CL adds a flag and adds a help function that checks if delay agnostic AEC is enabled or not, either through a flag or the above mentioned Finch experiment.

Once the experiment is fully launched on all platforms, the flag will be removed.

BUG=N/A
TESTED=verified locally on Mac
TBR=cpu@chromium.org

Committed: https://crrev.com/3a1fa641e17c3293c8df42a9abf6e858b6935014
Cr-Commit-Position: refs/heads/master@{#311241}

[bjornv, 2015-01-06 19:11:37]

bjornv@chromium.org changed reviewers:
+ asvitkine@chromium.org, perkj@chromium.org

[bjornv, 2015-01-06 19:26:26]

Dear reviewers,

PTAL at this chrome flag CL.

TBR=cpu@chromium.org (for generated_resources.grd)
Alexei for histograms.xml
perkj for audio processing in chrome

Let me know if I should add more/other reviewers.

Regards
Björn

[Alexei Svitkine (slow), 2015-01-06 21:28:50]

histograms lgtm

[perkj_chrome, 2015-01-09 07:37:08]

I am only an owner of media. You will have to find someone else for the rest of
the files.

https://codereview.chromium.org/795793003/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/795793003/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:79: #if
defined(OS_MACOSX)
Are these if defs necessary? How about:

base::CommandLine* command_line =base::CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(switches::kEnableDelayAgnosticAec))
  return true;

const std::string group_name =
  base::FieldTrialList::FindFullName("NoReportedDelayOnMac");
return (group_name == "Enabled");

[bjornv, 2015-01-10 10:06:20]

bjornv@chromium.org changed reviewers:
+ avi@chromium.org

[bjornv, 2015-01-10 10:08:41]

perkj: PTAL

avi: I added you as a reviewer of content/public/common/content_switches.*
If you're not an appropriate reviewer with owners right, could you please point
me to someone?

https://codereview.chromium.org/795793003/diff/1/content/renderer/media/media...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/795793003/diff/1/content/renderer/media/media...
content/renderer/media/media_stream_audio_processor.cc:79: #if
defined(OS_MACOSX)
On 2015/01/09 07:37:08, perkj wrote:
> Are these if defs necessary? How about:
> 
> base::CommandLine* command_line =base::CommandLine::ForCurrentProcess();
> if (command_line->HasSwitch(switches::kEnableDelayAgnosticAec))
>   return true;
> 
> const std::string group_name =
>   base::FieldTrialList::FindFullName("NoReportedDelayOnMac");
> return (group_name == "Enabled");

You have to acquire the Finch group name first, but not evaluate the result if
you want to exclude flag users from the stats; See go/finch-and-flags

I kept the defs since it was under def before, but how about this change?

[Avi (use Gerrit), 2015-01-10 17:36:40]

I am the person who can do that for you. LGTM, with the question of how
permanent this is. We're on a quest to remove flags since we have 800 or so.

[bjornv, 2015-01-12 05:49:18]

On 2015/01/10 17:36:40, Avi wrote:
> I am the person who can do that for you. LGTM, with the question of how
> permanent this is. We're on a quest to remove flags since we have 800 or so.

The flag will be removed once the delay agnostic AEC has been fully rolled out
on all platforms. Should I add a TODO in content_switches as a reminder?

[perkj_chrome, 2015-01-12 07:50:37]

lgtm

https://codereview.chromium.org/795793003/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/795793003/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_audio_processor.cc:77: const std::string
group_name =
nit: move to where its used.

[bjornv, 2015-01-12 21:50:50]

https://codereview.chromium.org/795793003/diff/20001/content/renderer/media/m...
File content/renderer/media/media_stream_audio_processor.cc (right):

https://codereview.chromium.org/795793003/diff/20001/content/renderer/media/m...
content/renderer/media/media_stream_audio_processor.cc:77: const std::string
group_name =
On 2015/01/12 07:50:37, perkj wrote:
> nit: move to where its used.

If I understand the directions on using flags in Finch correctly, the query
order is important. I added the same note here as in the example code in
finch-and-flags.

[bjornv, 2015-01-13 07:17:22]

The CQ bit was checked by bjornv@chromium.org

[commit-bot: I haz the power, 2015-01-13 07:17:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/795793003/60001

[commit-bot: I haz the power, 2015-01-13 09:19:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-13 09:20:41]

Patchset 4 (id:??) landed as
https://crrev.com/3a1fa641e17c3293c8df42a9abf6e858b6935014
Cr-Commit-Position: refs/heads/master@{#311241}