Implement writing mic audio input data to file for debugging purposes.

content/browser/renderer_host/media/audio_input_renderer_host.cc

Index: content/browser/renderer_host/media/audio_input_renderer_host.cc
diff --git a/content/browser/renderer_host/media/audio_input_renderer_host.cc b/content/browser/renderer_host/media/audio_input_renderer_host.cc
index 2692f74dc99895a725c6c585be4987fd98cfe7f5..b789178b4bc4140041482f89064fdb612f66d410 100644
--- a/content/browser/renderer_host/media/audio_input_renderer_host.cc
+++ b/content/browser/renderer_host/media/audio_input_renderer_host.cc
@@ -5,22 +5,33 @@
 #include "content/browser/renderer_host/media/audio_input_renderer_host.h"
 
 #include "base/bind.h"
+#include "base/files/file.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/shared_memory.h"
 #include "base/metrics/histogram.h"
 #include "base/numerics/safe_math.h"
 #include "base/process/process.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/media/capture/web_contents_audio_input_stream.h"
 #include "content/browser/media/capture/web_contents_capture_util.h"
 #include "content/browser/media/media_internals.h"
+#include "content/browser/media/webrtc_internals.h"
+#include "content/browser/renderer_host/media/audio_input_debug_writer.h"
 #include "content/browser/renderer_host/media/audio_input_device_manager.h"
 #include "content/browser/renderer_host/media/audio_input_sync_writer.h"
 #include "content/browser/renderer_host/media/media_stream_manager.h"
+#include "content/public/browser/render_process_host.h"
 #include "media/audio/audio_manager_base.h"
 #include "media/base/audio_bus.h"
 
+namespace content {
+
 namespace {
 
+const char kDebugRecordingFileNameAddition[] = "source_input";
+const char kDebugRecordingFileNameExtension[] = "pcm";
+
 void LogMessage(int stream_id, const std::string& msg, bool add_prefix) {
   std::ostringstream oss;
   oss << "[stream_id=" << stream_id << "] ";
@@ -31,9 +42,30 @@ void LogMessage(int stream_id, const std::string& msg, bool add_prefix) {
   DVLOG(1) << oss.str();
 }
 
-}  // namespace
+base::File CreateDebugRecordingFile(base::FilePath file_path) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  base::File recording_file(
+                 file_path,

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

not sure about the indent here... 4 spaces from the variable name? (is git cl
format perhaps messing this up?)

[Henrik Grunell, 2015/08/17 15:05:17]

Indeed. Creative indenting. :) Fixed.

+                 base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_APPEND);
+  if (!recording_file.IsValid()) {
+    VLOG(1) << "Could not open debug recording file, error=" <<

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

nit: could use [P]LOG_IF(ERROR, !recording_file.IsValid()) << "blahh";

[Henrik Grunell, 2015/08/17 15:05:16]

Done.

+               recording_file.error_details();
+  }
+  return recording_file.Pass();
+}
 
-namespace content {
+void CloseFile(base::File* file) {

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

just wondering - could ownership be passed via move semantics and automatically
deleted at end of scope?

[Henrik Grunell, 2015/08/17 15:05:17]

Done.

+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  // |file| must be closed and destroyed on FILE thread.
+  delete file;
+}
+
+void DeleteInputDebugWriterOnFileThread(AudioInputDebugWriter* writer) {

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

same here... wondering if we could use scoped_ptr<>

[Henrik Grunell, 2015/08/17 15:05:17]

Done.

+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+  delete writer;
+}
+
+}  // namespace
 
 struct AudioInputRendererHost::AudioEntry {
   AudioEntry();
@@ -54,42 +86,72 @@ struct AudioInputRendererHost::AudioEntry {
   // ownership of the writer.
   scoped_ptr<media::AudioInputController::SyncWriter> writer;
 
+  // Keep a raw pointer since it must be deleted on the file thread. Must be
+  // posted for deletion and nulled before the AudioEntry is deleted.
+  AudioInputDebugWriter* input_debug_writer;

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

I think you could still use a scoped_ptr even though you pass ownership over

[Henrik Grunell, 2015/08/17 15:05:17]

I think that would signal that it's OK if it's deleted automatically in the
AudioEntry dtor which it isn't since it must be deleted on the file tread. A raw
pointer is clearer imo.

[tommi (sloooow) - chröme, 2015/08/19 11:32:21]

By that logic, it sounds like not deleting it should be OK.  However, I don't
think that's OK either.

On the contrary I'd say that if we delete the object and crash or cause a race
to be triggered (caught by tsan), then it would be preferable.

Since this variable is being converted to a scoped_ptr, then Pass()ed and
eventually set to nullptr, I think it should just be a scoped_ptr.  Not worrying
about setting to nullptr is one less thing to worry about and we can still do a
DCHECK in the dtor.

[Henrik Grunell, 2015/08/19 19:57:15]

Hm, well, not sure I agree. :) But maybe "scoped_ptr primarily means I own it
and shall delete it" logic is commonly used in Chromium. Anyway, I'll change it.
Sure, that will catch it. And that's a pro.
Done.

+
   // Set to true after we called Close() for the controller.
   bool pending_close;
 
   // If this entry's layout has a keyboard mic channel.
-  bool has_keyboard_mic_;
+  bool has_keyboard_mic;
 };
 
 AudioInputRendererHost::AudioEntry::AudioEntry()
     : stream_id(0),
       shared_memory_segment_count(0),
+      input_debug_writer(nullptr),
       pending_close(false),
-      has_keyboard_mic_(false) {
+      has_keyboard_mic(false) {
 }
 
-AudioInputRendererHost::AudioEntry::~AudioEntry() {}
+AudioInputRendererHost::AudioEntry::~AudioEntry() {
+  DCHECK(!input_debug_writer);
+}
 
 AudioInputRendererHost::AudioInputRendererHost(
     int render_process_id,
+    RenderProcessHost* render_process_host,
     media::AudioManager* audio_manager,
     MediaStreamManager* media_stream_manager,
     AudioMirroringManager* audio_mirroring_manager,
     media::UserInputMonitor* user_input_monitor)
     : BrowserMessageFilter(AudioMsgStart),
       render_process_id_(render_process_id),
+      render_process_host_(render_process_host),
       audio_manager_(audio_manager),
       media_stream_manager_(media_stream_manager),
       audio_mirroring_manager_(audio_mirroring_manager),
       user_input_monitor_(user_input_monitor),
       audio_log_(MediaInternals::GetInstance()->CreateAudioLog(
-          media::AudioLogFactory::AUDIO_INPUT_CONTROLLER)) {}
+          media::AudioLogFactory::AUDIO_INPUT_CONTROLLER)),
+      weak_factory_(this) {}
 
 AudioInputRendererHost::~AudioInputRendererHost() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(audio_entries_.empty());
 }
 
+void AudioInputRendererHost::EnableDebugRecording(const base::FilePath& file) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  base::FilePath file_with_extensions =
+      GetDebugRecordingFilePathWithExtensions(file);
+  for (AudioEntryMap::iterator i = audio_entries_.begin();

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

for (const auto& entry : audio_entries_)
  EnabledDebugRecordingForId(file_with_extensions, entry.first);

[Henrik Grunell, 2015/08/17 15:05:17]

Done.

+       i != audio_entries_.end(); ++i) {
+    EnabledDebugRecordingForId(file_with_extensions, i->first);
+  }
+}
+
+void AudioInputRendererHost::DisableDebugRecording() {
+  for (AudioEntryMap::iterator it = audio_entries_.begin();

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

could use a range based loop here as well

[Henrik Grunell, 2015/08/17 15:05:17]

Done.

+       it != audio_entries_.end(); ++it) {
+    it->second->controller->DisableDebugRecording(
+        base::Bind(&AudioInputRendererHost::DeleteDebugWriter,
+                   this,
+                   it->first));
+  }
+}
+
 void AudioInputRendererHost::OnChannelClosing() {
   // Since the IPC sender is gone, close all requested audio streams.
   DeleteEntries();
@@ -409,7 +471,7 @@ void AudioInputRendererHost::DoCreateStream(
 #if defined(OS_CHROMEOS)
   if (config.params.channel_layout() ==
           media::CHANNEL_LAYOUT_STEREO_AND_KEYBOARD_MIC) {
-    entry->has_keyboard_mic_ = true;
+    entry->has_keyboard_mic = true;
   }
 #endif
 
@@ -423,6 +485,13 @@ void AudioInputRendererHost::DoCreateStream(
   audio_log_->OnCreated(stream_id, audio_params, device_id);
   MediaInternals::GetInstance()->SetWebContentsTitleForAudioLogEntry(
       stream_id, render_process_id_, render_frame_id, audio_log_.get());
+
+  if (WebRTCInternals::GetInstance()->audio_debug_recordings_enabled()) {
+    AudioInputRendererHost::EnabledDebugRecordingForId(
+        GetDebugRecordingFilePathWithExtensions(
+            WebRTCInternals::GetInstance()->audio_debug_recordings_file_path()),
+        stream_id);
+  }
 }
 
 void AudioInputRendererHost::OnRecordStream(int stream_id) {
@@ -498,12 +567,22 @@ void AudioInputRendererHost::DeleteEntry(AudioEntry* entry) {
   LogMessage(entry->stream_id, "DeleteEntry: stream is now closed", true);
 
 #if defined(OS_CHROMEOS)
-  if (entry->has_keyboard_mic_) {
+  if (entry->has_keyboard_mic) {
     media_stream_manager_->audio_input_device_manager()
         ->UnregisterKeyboardMicStream();
   }
 #endif
 
+  if (entry->input_debug_writer) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(
+            &DeleteInputDebugWriterOnFileThread,
+            entry->input_debug_writer));

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

would be nice to do input_debug_writer.Pass() :)

[Henrik Grunell, 2015/08/17 15:05:17]

Why not? Made it move-only. :)

EDIT: Argh, can't do that since |file_| should only be accessed on the FILE
thread. Using scoped_ptr anyway.

+    entry->input_debug_writer = nullptr;
+  }
+
   // Delete the entry when this method goes out of scope.
   scoped_ptr<AudioEntry> entry_deleter(entry);
 
@@ -556,4 +635,76 @@ void AudioInputRendererHost::MaybeUnregisterKeyboardMicStream(
 #endif
 }
 
+#if defined(OS_WIN)
+#define IntToStringType base::IntToString16
+#else
+#define IntToStringType base::IntToString
+#endif
+
+base::FilePath AudioInputRendererHost::GetDebugRecordingFilePathWithExtensions(
+    const base::FilePath& file) {
+  return file.AddExtension(kDebugRecordingFileNameAddition).AddExtension(
+      IntToStringType(base::GetProcId(render_process_host_->GetHandle())));
+}
+
+void AudioInputRendererHost::EnabledDebugRecordingForId(
+    const base::FilePath& file,
+    int stream_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTaskAndReplyWithResult(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(
+          &CreateDebugRecordingFile,
+          file.AddExtension(IntToStringType(stream_id))
+              .AddExtension(
+                  FILE_PATH_LITERAL(kDebugRecordingFileNameExtension))),
+      base::Bind(
+          &AudioInputRendererHost::DoEnableDebugRecording,
+          weak_factory_.GetWeakPtr(),
+          stream_id));
+}
+
+#undef IntToStringType
+
+void AudioInputRendererHost::DoEnableDebugRecording(
+    int stream_id,
+    base::File file) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!file.IsValid())
+    return;

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

should we dcheck here or is this an expected case?

[Henrik Grunell, 2015/08/17 15:05:17]

It could happen if the user selects a folder without write access for example.

+  AudioEntry* entry = LookupById(stream_id);
+  if (!entry) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(
+            &CloseFile,
+            base::Unretained(new base::File(file.Pass()))));

[tommi (sloooow) - chröme, 2015/08/07 12:09:51]

is there no way we could just do
file.Pass() here?

I would have expected base::Passed(file.Pass()) to work if CloseFile() simply
accepted a base::File parameter (by value that is).

[Henrik Grunell, 2015/08/17 15:05:17]

Done.

+    return;
+  }
+  entry->input_debug_writer = new AudioInputDebugWriter(file.Pass());
+  entry->controller->EnableDebugRecording(entry->input_debug_writer);
+}
+
+void AudioInputRendererHost::DeleteDebugWriter(int stream_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  AudioEntry* entry = LookupById(stream_id);
+  if (!entry) {
+    // This happens if DisableDebugRecording is called right after
+    // DeleteEntries.
+    return;
+  }
+
+  if (entry->input_debug_writer) {
+    BrowserThread::PostTask(
+        BrowserThread::FILE,
+        FROM_HERE,
+        base::Bind(
+            &DeleteInputDebugWriterOnFileThread,
+            entry->input_debug_writer));
+      entry->input_debug_writer = nullptr;
+  }
+}
+
 }  // namespace content