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

webrtc/modules/audio_processing/logging/apm_data_dumper.cc

+/*
+ *  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.
+ */
+
+#include "webrtc/modules/audio_processing/logging/apm_data_dumper.h"
+
+namespace webrtc {
+namespace {
+
+std::string FormRawFileName(std::string name,

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

const std::string&

[kwiberg-webrtc, 2016/04/11 12:59:11]

+1. Good point.

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

Good find!
Done.

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

Acknowledged.

+                            int instance_index,
+                            int reinit_index) {
+  return name + "_" + std::to_string(instance_index) + "-" +

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

std::to_string is currently not allowed in Chrome. See
https://chromium-cpp.appspot.com/#library-review.

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

Ah, great find! Too bad. Will do this in another way instead.

Done.

+         std::to_string(reinit_index) + ".dat";
+}
+
+std::string FormWavFileName(std::string name,
+                            int instance_index,
+                            int reinit_index) {
+  return name + "_" + std::to_string(instance_index) + "-" +
+         std::to_string(reinit_index) + ".wav";
+}

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

You can probably unify these two.

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

Agree. I see (at least) two options:
1. std::string MakeFileName(std::string name,
                            int instance_index,
                            int reinit_index,
                            const std::string& suffix) {...}
called as MakeFileName("basename", 17, 4711, "dat");

Or
2. std::string MakeFileName(std::string name,
                            int instance_index,
                            int reinit_index) {...}
called as MakeFileName("basename", 17, 4711) + ".dat";

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

Done.

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

Done.

+
+}  // namespace
+
+void ApmDataDumper::Initialize() {
+  ++reinit_counter_;
+}
+
+ApmDataDumper::~ApmDataDumper() {
+#ifdef WEBRTC_AEC_DEBUG_DUMP
+  for (auto& raw_files_element : raw_files_) {
+    FILE** file = &raw_files_element.second;

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

Can you not just call
fclose(raw_files_element.second);
?

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

True.
Done.

+    fclose(*file);
+  }
+
+  for (auto& wav_files_element : wav_files_) {
+    rtc_WavWriter** file = &wav_files_element.second;

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

Similar as above?

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

Done.

+    rtc_WavClose(*file);
+  }
+#endif
+}
+
+FILE* ApmDataDumper::GetRawFile(const std::string& name) {
+  std::string filename =
+      FormRawFileName(name, instance_index_, reinit_counter_);
+  auto search = raw_files_.find(filename);
+  if (search != raw_files_.end()) {
+    return search->second;
+  }
+  FILE* file = fopen(filename.c_str(), "wb");
+  raw_files_[filename] = file;
+  return file;
+}
+
+rtc_WavWriter* ApmDataDumper::GetWavFile(const std::string& name,
+                                         int sample_rate_hz) {
+  std::string filename =
+      FormWavFileName(name, instance_index_, reinit_counter_);
+  auto search = wav_files_.find(filename);
+  if (search != wav_files_.end()) {
+    return search->second;
+  }
+  rtc_WavWriter* file = rtc_WavOpen(filename.c_str(), sample_rate_hz, 1);
+  wav_files_[filename] = file;
+  return file;
+}
+
+void ApmDataDumper::DumpRaw(const std::string& name,
+                            rtc::ArrayView<const float> v) {

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

Is "pass-by-value" just as good as passing const rtc::ArrayView<const float>& v
?

[kwiberg-webrtc, 2016/04/11 12:59:11]

Yes (ArrayView is just a pointer + a length), and IIRC, the docs for that class
even say so explicitly.

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

Acknowledged.

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

Acknowledged.

+#ifdef WEBRTC_AEC_DEBUG_DUMP
+  FILE* file = GetRawFile(name);
+  fwrite(v.data(), sizeof(v[0]), v.size(), file);

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

Do floats suffer from the same endian problems as integers, or do they always
use the same byte order?

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

Yes, they do suffer from the same problem. But I don't think that is a problem
here as these files are only to be generated in an offline manner and will never
need to be converted to another endianness.

+#endif
+}
+
+void ApmDataDumper::DumpRaw(const std::string& name, int v_length, float* v) {

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

const float* v

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

Order of methods should be the same in h and cpp files.

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

Good point! I now anyway moved these to the header file in order to allow the
possibility of inlining.
Done.

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

Done.

+#ifdef WEBRTC_AEC_DEBUG_DUMP
+  FILE* file = GetRawFile(name);
+  fwrite(v, sizeof(v[0]), v_length, file);
+#endif

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

Implement one DumpRaw in terms of the other?

[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]

Done.

+}
+
+void ApmDataDumper::DumpWav(const std::string& name,
+                            rtc::ArrayView<const float> v,
+                            int sample_rate_hz) {
+#ifdef WEBRTC_AEC_DEBUG_DUMP
+  rtc_WavWriter* file = GetWavFile(name, sample_rate_hz);
+  rtc_WavWriteSamples(file, v.data(), v.size());
+#endif
+}
+
+void ApmDataDumper::DumpWav(const std::string& name,

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

Order of methods should be the same in h and cpp files.

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

Good point! I now anyway moved these to the header file in order to allow the
possibility of inlining.
Done.

+                            int v_length,
+                            float* v,

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

const float* v

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

Done.

+                            int sample_rate_hz) {
+#ifdef WEBRTC_AEC_DEBUG_DUMP
+  rtc_WavWriter* file = GetWavFile(name, sample_rate_hz);
+  rtc_WavWriteSamples(file, v, v_length);
+#endif

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

Implement one DumpWav in terms of the other?

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

Done.

+}
+
+}  // namespace webrtc