Replaced the data logging functionality in the AEC with a generic logging functionality

webrtc/modules/audio_processing/logging/apm_data_dumper.h

+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_PROCESSING_LOGGING_APM_DATA_DUMPER_H_
+#define WEBRTC_MODULES_AUDIO_PROCESSING_LOGGING_APM_DATA_DUMPER_H_
+
+#include <map>
+#include <string>
+
+#include "webrtc/base/constructormagic.h"
+#include "webrtc/base/array_view.h"

[hlundin-webrtc, 2016/04/11 11:58:29]

Order of includes.

[peah-webrtc, 2016/04/12 21:46:39]

Done.

+#include "webrtc/common_audio/wav_file.h"
+
+namespace webrtc {
+
+// Class that handles dumping of variables into files.
+class ApmDataDumper {
+ public:
+  // Constructor that takes an instance index that may
+  // be used to distinguish data dumped from different
+  // instances of the code.
+  explicit ApmDataDumper(int instance_index)
+      : instance_index_(instance_index) {}
+  ~ApmDataDumper();
+
+  // (Re)Initializes the data dumping such that new versions
+  // of all files being dumped to are created.
+  void Initialize();

[kwiberg-webrtc, 2016/04/11 10:43:37]

The name of this function suggests that it has to be called before the object is
ready. If that's the case, I'd like to object. If it's not the case, I'd like to
suggest a different name.

[hlundin-webrtc, 2016/04/11 11:58:29]

Acknowledged.

[peah-webrtc, 2016/04/12 21:46:39]

Done.

[peah-webrtc, 2016/04/12 21:46:39]

Good point! Did not think of that. I modified this a bit. PTAL.

+
+  // Methods for performing dumping of data of various types into
+  // various formats.
+  void DumpRaw(const std::string& name, int v_length, float* v);
+  void DumpRaw(const std::string& name, rtc::ArrayView<const float> v);
+  void DumpWav(const std::string& name,
+               int v_length,
+               float* v,
+               int sample_rate_hz);
+  void DumpWav(const std::string& name,
+               rtc::ArrayView<const float> v,
+               int sample_rate_hz);

[kwiberg-webrtc, 2016/04/11 10:43:37]

By making these methods not inline, you'll prevent the compiler from omitting
the calls altogether when not logging.

[peah-webrtc, 2016/04/12 21:46:39]

Good point! 
Done.

+
+ private:

[kwiberg-webrtc, 2016/04/11 10:43:37]

Consider #ifdeffing the members away too.

[peah-webrtc, 2016/04/12 21:46:39]

Done.

+  const int instance_index_;
+  int reinit_counter_ = -1;
+  std::map<std::string, FILE*> raw_files_;
+  std::map<std::string, rtc_WavWriter*> wav_files_;
+
+  FILE* GetRawFile(const std::string& name);

[kwiberg-webrtc, 2016/04/11 10:43:37]

You don't include anything that provides FILE except by accident.

[hlundin-webrtc, 2016/04/11 11:58:29]

Forward declare?

[peah-webrtc, 2016/04/12 21:46:39]

Good find!

[peah-webrtc, 2016/04/12 21:46:39]

I'm not sure what you mean with this. 

[hlundin-webrtc, 2016/04/25 11:29:59]

What I think I meant was that since your previous version only referenced this
as a pointer in the .h file, it would suffice to forward declare it here, and
then do the full include in the .cc file. But now that is water under the
bridges.

+  rtc_WavWriter* GetWavFile(const std::string& name, int sample_rate_hz);

[kwiberg-webrtc, 2016/04/11 10:43:37]

It looks like you're using the C API for the wav writer. Use the C++ API
instead.

[peah-webrtc, 2016/04/12 21:46:39]

Good point!
Done.

+
+  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(ApmDataDumper);
+};
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_PROCESSING_LOGGING_APM_DATA_DUMPER_H_