Enable platform echo cancellation through the AudioRecord path.

media/audio/audio_parameters.h

Index: media/audio/audio_parameters.h
diff --git a/media/audio/audio_parameters.h b/media/audio/audio_parameters.h
index 9b86d31186a7c2d33f6f0388b36a344f0855cd97..8c6458c417d3d25fdd58d49b88fb684638284ad7 100644
--- a/media/audio/audio_parameters.h
+++ b/media/audio/audio_parameters.h
@@ -56,6 +56,10 @@ class MEDIA_EXPORT AudioParameters {
              int channels, int input_channels,
              int sample_rate, int bits_per_sample,
              int frames_per_buffer);
+  void Reset(Format format, ChannelLayout channel_layout,

[tommi (sloooow) - chröme, 2013/12/06 12:11:34]

Can you add a TODO here to remove the other Reset() function and update all call
sites?

Can you also ping Dale (dalecurtis@) to have a look at this change?  Just in
case he has other ideas about where native audio effect flags should be.

[Ami GONE FROM CHROMIUM, 2013/12/06 19:22:53]

Better yet, can you delete the old form?

[ajm, 2013/12/10 06:37:16]

I don't actually need to use Reset(), as all of the 16 calls to it but one don't
care about the effects.

Do you think that's dangerous? Someone adding a call to Reset() in the path that
does care about effects is more likely to miss that they need to set it
properly. 

[Ami GONE FROM CHROMIUM, 2013/12/10 19:31:43]

I think it would be better to make use_platform_aec a ctor arg and a const
member and drop set_use_platform_aec, which would allow you to avoid this
question.  (originally commented to this effect in a comment you didn't respond
to at l.94 below).

[ajm, 2013/12/10 20:13:05]

Sorry, missed that comment.

I like your suggestion; the only potential wrinkle I can see is that here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/med...

we'll have to construct a new AudioParameters and copy it to the input
parameter, e.g.

AudioParameters params(...)
*r = params;

Do you think that's fine?

[ajm, 2013/12/11 02:48:04]

Moved to setting through constructors in patch set 5. However, I couldn't make
effects_ a const member, as assignment is used for AudioParameters, including
now in my change to media_param_traits.cc. Do you still prefer the constructor
approach? 

[Ami GONE FROM CHROMIUM, 2013/12/11 05:25:08]

What prevents you from reading into a temporary in traits.cc and using an
explicit constructor for PlatformEffect to make its effects_ member a const?

[ajm, 2013/12/11 17:46:49]

I assume you mean "explicit constructor for AudioParameters". That's what I'm
doing now, right? The problem comes when I try to assign the explicitly
constructed |params| to |*r|. The latter can't be assigned to because effects_
is a const member.

I may be totally missing something here.

[Ami GONE FROM CHROMIUM, 2013/12/11 22:11:38]

Perhaps that something is a copy ctor for PlatformEffects ?

[ajm, 2013/12/12 01:51:27]

This would require an assignment operator, right? And we still can't assign to
the const member.

It's changed to a bitmask now, so things are a bit different, but please let me
know if I'm misinterpreting, and there's some way to use assignment when the
type has a const member.

[Ami GONE FROM CHROMIUM, 2013/12/12 02:01:20]

You're misinterpreting.
"Copy constructor" is a term of art in C++ and is different to assignment
operator.  Because the copy ctor is a ctor it gets to assign to const members.
http://en.wikipedia.org/wiki/Copy_constructor

(I don't actually believe you don't know this; I'm just being explicit to make
sure we're communicating :))

[Ami GONE FROM CHROMIUM, 2013/12/12 02:29:33]

I was failing to process that default-assignment-operator of AudioParameters
(which is used in multiple places, apparently) precludes using the copy ctor
unless placement new is involved, which would be crazytalk.  Fine to keep
mutable, though please add a comment explaining it's only mutable to support
entire-struct (re-)assignment, not individual setting twiddling.

[ajm, 2013/12/12 03:36:03]

Done.

+             int channels, int input_channels,
+             int sample_rate, int bits_per_sample,
+             int frames_per_buffer, bool use_platform_aec);
 
   // Checks that all values are in the expected range. All limits are specified
   // in media::Limits.
@@ -81,9 +85,15 @@ class MEDIA_EXPORT AudioParameters {
   int frames_per_buffer() const { return frames_per_buffer_; }
   int channels() const { return channels_; }
   int input_channels() const { return input_channels_; }
+  bool use_platform_aec() const {
+    return use_platform_aec_;
+  }
 
   // Set to CHANNEL_LAYOUT_DISCRETE with given number of channels.
   void SetDiscreteChannels(int channels);
+  void set_use_platform_aec(bool use) {

[Ami GONE FROM CHROMIUM, 2013/12/06 19:22:53]

Why let this be mutable instead of making it a ctor param?

+    use_platform_aec_ = use;
+  }
 
   // Comparison with other AudioParams.
   bool operator==(const AudioParameters& other) const {
@@ -93,7 +103,8 @@ class MEDIA_EXPORT AudioParameters {
            channels_ == other.channels() &&
            input_channels_ == other.input_channels() &&
            bits_per_sample_ == other.bits_per_sample() &&
-           frames_per_buffer_ == other.frames_per_buffer();
+           frames_per_buffer_ == other.frames_per_buffer() &&
+           use_platform_aec_ == other.use_platform_aec();
   }
 
  private:
@@ -108,6 +119,8 @@ class MEDIA_EXPORT AudioParameters {
   int input_channels_;            // Optional number of input channels.
                                   // Normally 0, but can be set to specify
                                   // synchronized I/O.
+  bool use_platform_aec_;         // Platform AEC is available, or should be
+                                  // enabled, depending on context.
 };
 
 // Comparison is useful when AudioParameters is used with std structures.
@@ -122,7 +135,9 @@ inline bool operator<(const AudioParameters& a, const AudioParameters& b) {
     return a.sample_rate() < b.sample_rate();
   if (a.bits_per_sample() != b.bits_per_sample())
     return a.bits_per_sample() < b.bits_per_sample();
-  return a.frames_per_buffer() < b.frames_per_buffer();
+  if (a.frames_per_buffer() != b.frames_per_buffer())
+    return a.frames_per_buffer() < b.frames_per_buffer();
+  return a.use_platform_aec() < b.use_platform_aec();
 }
 
 }  // namespace media