Adding APM configuration in AEC dump.

webrtc/modules/audio_processing/debug.proto

Index: webrtc/modules/audio_processing/debug.proto
diff --git a/webrtc/modules/audio_processing/debug.proto b/webrtc/modules/audio_processing/debug.proto
index dce2f7920930452f7a512d9bda2f3679893cddc2..edac7af62e1fe29d4eb875faac8fe7144ea760a1 100644
--- a/webrtc/modules/audio_processing/debug.proto
+++ b/webrtc/modules/audio_processing/debug.proto
@@ -2,6 +2,8 @@ syntax = "proto2";
 option optimize_for = LITE_RUNTIME;
 package webrtc.audioproc;
 
+// Contains the format of input/output/reverse audio. An Init message is added
+// when any of the fields is changed.
 message Init {
   optional int32 sample_rate = 1;
   optional int32 device_sample_rate = 2 [deprecated=true];
@@ -39,11 +41,39 @@ message Stream {
   repeated bytes output_channel = 8;
 }
 
+// Contains the configurations of various APM component. A Config message is
+// added when any of the fields is changed.
+message Config {

[Andrew MacDonald, 2015/10/01 04:37:45]

FYI: I just wanted to mention that Michael and I briefly discussed using a
protobuf interface when moving to create-time configuration.

We'd of course want to reuse this message in that case. 

[minyue-webrtc, 2015/10/01 20:18:26]

Acknowledged.

+  // Next field number 17.
+  // Acoustic echo canceler.
+  optional bool aec_enabled = 1;
+  optional bool aec_delay_agnostic = 2;
+  optional bool aec_drift_compensation = 3;
+  optional bool aec_extended_filter = 4;
+  optional int32 aec_suppression_level = 5;
+  // Mobile AEC.
+  optional bool aecm_enabled = 6;
+  optional bool aecm_comfort_noise = 7;
+  optional int32 aecm_routing_mode = 8;
+  // Automatic gain controller.
+  optional bool agc_enabled = 9;
+  optional bool agc_experiment = 10;
+  optional int32 agc_mode = 11;
+  optional bool agc_limiter = 12;
+  // High pass filter.
+  optional bool hpf_enabled = 13;
+  // Noise suppression.
+  optional bool ns_enabled = 14;
+  optional bool ns_experiment = 15;
+  optional int32 ns_level = 16;
+}
+
 message Event {
   enum Type {
     INIT = 0;
     REVERSE_STREAM = 1;
     STREAM = 2;
+    CONFIG = 3;
   }
 
   required Type type = 1;

[minyue-webrtc, 2015/10/01 05:10:21]

I need to mention that adding CONFIG makes Type non-forward-compatible, i.e.,
old unpacker cannot unpack new dump. Ivo, you may suggest me if there is a work
around.

Adding those optional field in existing Event type, i.e., INIT, as I did at the
beginning is of no problem. But is forward compatibility needed?

[ivoc, 2015/10/01 08:55:39]

First off, I'm not sure how important forward compatibility is, does it happen
often that you use old versions of tools to analyze an AEC dump?

What will happen during the parsing is that the old parser will find the unknown
value (CONFIG - 3) and assign it to the default value instead (in this case
INIT, the first one) instead. That can lead to problems if the parser expects an
INIT Event to have a valid init member, which it won't have in this case.

This is the reason that it's recommended to always have an UNKNOWN_EVENT = 0
entry as the first entry in a protobuf enum, so that you can detect unknown
values that are added in the future. Since this enum is already made this way,
it's hard to avoid the problem.

It might be a good idea to add an UNKNOWN_EVENT = 4 field now, and assign it as
the default value of the type (add [default = UNKNOWN_EVENT]), that way we can
at least avoid this problem in the future when adding additional event types, by
allowing the parser to ignore events it doesn't recognize.

[Andrew MacDonald, 2015/10/01 17:35:21]

Thanks for the explanation Ivo, interesting!

Minyue, why not add this now? You still need to make other changes anyway ;)

[minyue-webrtc, 2015/10/01 20:18:26]

Yes, adding it in enum is very simple, my concern is that it cannot be properly
tested in a short time.

I added it. But Ivo, can you help to make  sure that it works the way it is
supposed to?

@@ -51,4 +81,5 @@ message Event {
   optional Init init = 2;
   optional ReverseStream reverse_stream = 3;
   optional Stream stream = 4;
+  optional Config config = 5;
 }