AecDump interface

webrtc/modules/audio_processing/include/audio_processing.h

Index: webrtc/modules/audio_processing/include/audio_processing.h
diff --git a/webrtc/modules/audio_processing/include/audio_processing.h b/webrtc/modules/audio_processing/include/audio_processing.h
index a1cd398b022f225adc233f7ddaa6abcb66ccd985..9232936ebd8312555f73bb41e5b063ce2bf591c5 100644
--- a/webrtc/modules/audio_processing/include/audio_processing.h
+++ b/webrtc/modules/audio_processing/include/audio_processing.h
@@ -29,6 +29,7 @@ namespace webrtc {
 
 struct AecCore;
 
+class AecDump;
 class AudioFrame;
 
 class NonlinearBeamformer;
@@ -447,6 +448,20 @@ class AudioProcessing {
   virtual void set_delay_offset_ms(int offset) = 0;
   virtual int delay_offset_ms() const = 0;
 
+  // Attaches provided webrtc::AecDump for recording debugging
+  // information. Log file and maximum file size logic is supposed to
+  // be handled by implementing instance of AecDump. Calling this
+  // method when another AecDump is attached resets the active AecDump
+  // with a new one. This causes the d-tor of the earlier AecDump to
+  // be called. The d-tor call may block until all pending logging

[peah-webrtc, 2017/05/15 05:32:51]

The blocking part is not good at all. If a new aecdump is created when there is
another existing aecdump, this may render the aecdump unusable (as it will cause
glitches).

Will the d-tor really block? What is causing the need of the d-tor blocking?

[aleloi, 2017/05/15 13:20:52]

After offline discussions d-tor still blocks, but not while holding both the
render and capture locks. If StopDebugDump or DetachAecDump is called from
something which is not the real-time audio thread, audio processing should
continue without glitches.

Short version of why the d-tor blocks:

When an AecDumpImpl object is destoryed, worker threads must not try to access
pointers to it. The WriteToFile tasks have access to 'int64_t
num_bytes_left_for_log_' and, 'FileWrapper* debug_file_', which are owned by
AecDumpImpl. A task queue has no 'flushing' functionality.

We could possibly add a way to notify pending tasks that their AecDumpImpl
pointers are stale and that the task should not be executed. That would imply 
* rewriting a number of AecDump tests, because the current ones rely on every
task being executed
* adding extra locks and complicating the memory pattern.

Another comment: a blocking d-tor is the pattern used in most other places with
a task queue/working thread in WebRTC: AudioSendStream, RTC event log,
VideoSendStream, voice engine channel.

+  // tasks are completed.
+  virtual void AttachAecDump(std::unique_ptr<AecDump> aec_dump) = 0;
+
+  // If no AecDump is attached, this has no effect. If an AecDump is
+  // attached, it's destructor is called. The d-tor may block until
+  // all pending logging tasks are completed.
+  virtual void DetachAecDump() = 0;
+
   // Starts recording debugging information to a file specified by |filename|,
   // a NULL-terminated string. If there is an ongoing recording, the old file
   // will be closed, and recording will continue in the newly specified file.