Adding APM configuration in AEC dump.

webrtc/modules/audio_processing/test/audio_processing_unittest.cc

Index: webrtc/modules/audio_processing/test/audio_processing_unittest.cc
diff --git a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc
index d82ea31c24b1713f6f87ac0e76e00f19df33dc3e..0c99ae9ff564af86c648fca8e6423f7bd29949fa 100644
--- a/webrtc/modules/audio_processing/test/audio_processing_unittest.cc
+++ b/webrtc/modules/audio_processing/test/audio_processing_unittest.cc
@@ -1744,6 +1744,73 @@ void ApmTest::ProcessDebugDump(const std::string& in_filename,
         first_init = false;
       }
 
+    } else if (event_msg.type() == audioproc::Event::CONFIG) {

[Andrew MacDonald, 2015/09/26 02:36:20]

Correct me if I'm wrong, but because the ref03.aecdump won't contain this
message, this isn't testing anything, right?

Instead, add a new focused test where you set a few of these settings, start
debug recording, read the file and verify the config message contains what you
expect.

Too bad StartDebugRecording doesn't take some kind of abstract Writer class so
you wouldn't have to involve the file-system.

[minyue-webrtc, 2015/09/26 16:26:44]

ref03.aecdump doesn't contain Config message. But the test is valid. The test
does the following
1. takes ref03.aecdump and re-process it to ref_xxx, now it contains Config
2. use ref_xxx and re-process it to out_xxx, also contains Config
3. check if ref_xxx === out_xxx

So it checks both backward compatibility and new fields

[Andrew MacDonald, 2015/09/28 06:22:33]

OK, thanks. Still not totally convinced this is a great way to test this. All
components are enabled in this test, and if one of those fails to get logged,
this wouldn't catch it. Right?

Also, we want to remove all these run-time setters anyway. So this feels like a
step backwards.

[minyue-webrtc, 2015/09/29 04:51:49]

True, it does not test all combination of configs,which is an over-kill also.
I may do this
try many combinations offline (just need to change line 1883) , and maybe leave
a test or two in there.

+      const audioproc::Config msg = event_msg.config();
+      Config config;
+      // AEC (5)
+      if (msg.has_aec_enabled()) {
+        apm_->echo_cancellation()->Enable(msg.aec_enabled());
+      }
+      if (msg.has_aec_delay_agnostic()) {
+        config.Set<DelayAgnostic>(new DelayAgnostic(msg.aec_delay_agnostic()));
+      }
+      if (msg.has_aec_drift_compensation()) {
+        apm_->echo_cancellation()->enable_drift_compensation(
+            msg.aec_drift_compensation());
+      }
+      if (msg.has_aec_extended_filter()) {
+        config.Set<ExtendedFilter>(new ExtendedFilter(
+            msg.aec_extended_filter()));
+      }
+      if (msg.has_aec_suppression_level()) {
+        apm_->echo_cancellation()->set_suppression_level(
+            static_cast<webrtc::EchoCancellation::SuppressionLevel>(
+                msg.aec_suppression_level()));
+      }
+      // AEC-M (3)
+      if (msg.has_aecm_enabled()) {
+        apm_->echo_control_mobile()->Enable(msg.aecm_enabled());
+      }
+      if (msg.has_aecm_comfort_noise()) {
+        apm_->echo_control_mobile()->enable_comfort_noise(
+            msg.aecm_comfort_noise());
+      }
+      if (msg.has_aecm_routing_mode()) {
+        apm_->echo_control_mobile()->set_routing_mode(
+            static_cast<webrtc::EchoControlMobile::RoutingMode>(
+            msg.aecm_routing_mode()));
+      }
+      // AGC (4)
+      if (msg.has_agc_enabled()) {
+        apm_->gain_control()->Enable(msg.agc_enabled());
+      }
+      if (msg.has_agc_experiment()) {
+        // agc_experiment can only be set in the ctor of APM. So there should be
+        // nothing to do, unless the |apm_| is wrongly initialized.
+      }
+      if (msg.has_agc_mode()) {
+        apm_->gain_control()->set_mode(
+            static_cast<webrtc::GainControl::Mode>(msg.agc_mode()));
+      }
+      if (msg.has_agc_limiter()) {
+        apm_->gain_control()->enable_limiter(msg.agc_limiter());
+      }
+      // HPF (1)
+      if (msg.has_hpf_enabled()) {
+        apm_->high_pass_filter()->Enable(msg.hpf_enabled());
+      }
+      // NS (3)
+      if (msg.has_ns_enabled()) {
+        apm_->noise_suppression()->Enable(msg.ns_enabled());
+      }
+      if (msg.has_ns_experiment()) {
+        config.Set<ExperimentalNs>(new ExperimentalNs(msg.ns_experiment()));
+      }
+      if (msg.has_ns_level()) {
+        apm_->noise_suppression()->set_level(
+            static_cast<webrtc::NoiseSuppression::Level>(msg.ns_level()));
+      }
+      apm_->SetExtraOptions(config);

[Andrew MacDonald, 2015/09/26 02:36:20]

This API is deprecated. Don't add more calls to it.

[minyue-webrtc, 2015/09/26 16:26:44]

Ok, thanks for the info. But how should one set ns_experiment / da-aec /
aec-extended for now? 

[Andrew MacDonald, 2015/09/28 06:22:33]

Through Create().

[minyue-webrtc, 2015/09/29 04:51:49]

Yes I see, do you suggest recreate APM here?

[Andrew MacDonald, 2015/09/29 05:05:17]

No, I suggest not testing this feature here, as in the other comment.

[minyue-webrtc, 2015/09/29 21:05:05]

Ok. I'd remove these changes.

I will try to add some tests later, preferably in a separate CL. I'd do tests
(without submit) with these modifies offline anyway to verify the new fields.

[Andrew MacDonald, 2015/09/29 23:38:49]

Why in a separate CL? If you're going to add a unit test (and I think you
should) you should do it in this CL.

     } else if (event_msg.type() == audioproc::Event::REVERSE_STREAM) {
       const audioproc::ReverseStream msg = event_msg.reverse_stream();
 
@@ -1839,8 +1906,8 @@ void ApmTest::VerifyDebugDumpTest(Format format) {
   EXPECT_NE(0, feof(out_file));
   ASSERT_EQ(0, fclose(ref_file));
   ASSERT_EQ(0, fclose(out_file));
-  remove(ref_filename.c_str());
-  remove(out_filename.c_str());
+//  remove(ref_filename.c_str());

[ivoc, 2015/09/25 09:24:21]

Why is this commented out? I think it's needed, isn't it?

[minyue-webrtc, 2015/09/25 09:49:09]

sorry sorry, I needed they to help debugging. 

+//  remove(out_filename.c_str());
 }
 
 TEST_F(ApmTest, VerifyDebugDumpInt) {