Replaced the wav file dumping functionality in aec_core.c with the newly added corresponding macros

webrtc/modules/audio_processing/logging/aec_logging_file_handling.c

Index: webrtc/modules/audio_processing/logging/aec_logging_file_handling.c
diff --git a/webrtc/modules/audio_processing/logging/aec_logging_file_handling.c b/webrtc/modules/audio_processing/logging/aec_logging_file_handling.c
new file mode 100644
index 0000000000000000000000000000000000000000..c562d128ffea9abebcafc70db2c422215432d041
--- /dev/null
+++ b/webrtc/modules/audio_processing/logging/aec_logging_file_handling.c
@@ -0,0 +1,62 @@
+/*
+ *  Copyright (c) 2015 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.
+ */
+
+
+#ifdef WEBRTC_AEC_DEBUG_DUMP
+#include <assert.h>
+#include <stdio.h>
+#include <stdint.h>

[hlundin-webrtc, 2015/08/10 13:04:40]

Sort includes alphabetically. stdint < stdio

[peah-webrtc, 2015/08/12 20:25:51]

Done.

+#include "webrtc/common_audio/wav_file.h"

[hlundin-webrtc, 2015/08/10 13:04:40]

I usually insert a blank line between system files and project files.

[peah-webrtc, 2015/08/12 20:25:51]

Done.

+#include "webrtc/typedefs.h"
+#include "aec_logging_file_handling.h"

[hlundin-webrtc, 2015/08/10 13:04:40]

Use full path.

[peah-webrtc, 2015/08/12 20:25:50]

Done.

+#endif
+
+// To enable AEC logging, run this command from trunk/ :

[hlundin-webrtc, 2015/08/10 13:04:40]

This comment is the same as in the .h file.

Function comments at the declaration should describe what the function does.
Function comments at the definition should (if necessary) describe how it does
it.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Comm...

[peah-webrtc, 2015/08/12 20:25:51]

Done.

+// python webrtc/build/gyp_webrtc -Daec_debug_dump=1
+#ifdef WEBRTC_AEC_DEBUG_DUMP
+// Open a new Wav file for writing. If it was already open with a different
+// sample frequency, close it first.
+void WebRtcAec_ReopenWav(rtc_WavWriter** wav_file,

[hlundin-webrtc, 2015/08/10 13:04:40]

wav_file is both input and output. Input-only goes before other parameters.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Para...

[Andrew MacDonald, 2015/08/10 15:28:44]

Agreed. I actually would prefer returning a rtc_WavWriter* (and a FILE* below).

[peah-webrtc, 2015/08/12 20:25:51]

I agree that that would be preferrable. My concern with that, however, is that
then the calls to the macros would need to be placed within #ifdef's as the
receiving output variable cannot be removed from the code using the compiler
flag specific #define selections that are used in aec_logging.h. (My goal with
these macros was to allow permanent logpoints in the code that are not built
into the released product and neither clutters the code with extra #ifdef's).

I'm not that good with macros so maybe any of you knows a way to achieve this
using macros with nonvoid return values?

Is it ok to keep it as it is (no pointer return values)?

[peah-webrtc, 2015/08/12 20:25:51]

Done.

[Andrew MacDonald, 2015/08/12 20:37:58]

Ah, good point. Yes I think it's reasonable to keep it as-is.

+                         const char* const name,
+                         const int seq1,
+                         const int seq2,
+                         const int sample_rate) {
+  int written ATTRIBUTE_UNUSED;
+  char filename[64];
+  if (*wav_file) {
+    if (rtc_WavSampleRate(*wav_file) == sample_rate)
+      return;
+    rtc_WavClose(*wav_file);
+  }
+  written = snprintf(filename, sizeof(filename), "%s%d-%d.wav",
+                     name, seq1, seq2);
+  assert(written >= 0);                        // no output error
+  assert((size_t)written < sizeof(filename));  // buffer was large enough
+  *wav_file = rtc_WavOpen(filename, sample_rate, 1);
+}
+
+
+// Open a new file for writing, named with a instance specific
+// suffix
+void WebRtcAec_RawFileOpen(FILE** file,
+                           const char * const name,
+                           const int16_t instanceCtr) {
+int written ATTRIBUTE_UNUSED;

[hlundin-webrtc, 2015/08/10 13:04:40]

Indentation. (Run "git cl format" on your patch.)

[peah-webrtc, 2015/08/12 20:25:51]

Done.

+  char filename[64];
+
+  written = sprintf(filename, "%s_%d.dat", name, instanceCtr);

[hlundin-webrtc, 2015/08/10 13:04:40]

snprintf?

[peah-webrtc, 2015/08/12 20:25:51]

Done.

+  assert(written >= 0);                        // no output error
+  assert((size_t)written < sizeof(filename));  // buffer was large enough
+
+  *file = fopen(filename, "wb");

[hlundin-webrtc, 2015/08/10 13:04:40]

Why not return a FILE* instead, and avoid the FILE** parameter?

[peah-webrtc, 2015/08/12 20:25:51]

I agree that would be a better solution, if only it could work without extra
enclosing #ifdef's (pls see reply above).

Is it ok to keep it as it is?

+}
+
+
+#endif  // WEBRTC_AEC_DEBUG_DUMP