Let default device in PulseAudio be the system default device

media/audio/pulse/pulse_output.cc

Index: media/audio/pulse/pulse_output.cc
diff --git a/media/audio/pulse/pulse_output.cc b/media/audio/pulse/pulse_output.cc
index 953f9acebfacf10e06b754c2c28b93adb6bd23e8..b7e2cf182c5b159e640970277b0826591a681c6f 100644
--- a/media/audio/pulse/pulse_output.cc
+++ b/media/audio/pulse/pulse_output.cc
@@ -11,11 +11,36 @@
 #include "media/audio/audio_manager_base.h"
 #include "media/audio/pulse/pulse_util.h"
 
+namespace {
+
+// pa_context_get_server_info callback. It's used by
+// GetSystemDefaultOutputDevice to set |default_system_device_name_| to the
+// default system output device.
+void GetSystemDefaultOutputDeviceCallback(pa_context* context,
+                                          const pa_server_info* info,
+                                          void* user_data) {
+  media::PulseAudioOutputStream* stream =
+      reinterpret_cast<media::PulseAudioOutputStream*>(user_data);
+  stream->SetDefaultSystemDeviceName(info->default_sink_name);
+  pa_threaded_mainloop* pa_mainloop = stream->GetPAMainloop();
+  pa_threaded_mainloop_signal(pa_mainloop, 0);

[Henrik Grunell, 2016/03/10 00:57:07]

You could just use stream->GetPAMainloop() directly here, and remove the above
line.

[rchtara, 2016/03/10 13:20:35]

Done.

+}
+}
+
 namespace media {
 
 using pulse::AutoPulseLock;
 using pulse::WaitForOperationCompletion;
 
+// Helper macro to avoid code spam and string bloat.
+#define RETURN_ON_FAILURE(expression, message) \
+  do {                                         \
+    if (!(expression)) {                       \
+      DLOG(ERROR) << message;                  \
+      return false;                            \
+    }                                          \
+  } while (0)
+
 // static, pa_stream_notify_cb
 void PulseAudioOutputStream::StreamNotifyCallback(pa_stream* s, void* p_this) {
   PulseAudioOutputStream* stream = static_cast<PulseAudioOutputStream*>(p_this);
@@ -61,12 +86,75 @@ PulseAudioOutputStream::~PulseAudioOutputStream() {
   DCHECK(!pa_mainloop_);
 }
 
-bool PulseAudioOutputStream::Open() {
+void PulseAudioOutputStream::GetSystemDefaultOutputDevice() {
+  DCHECK(pa_mainloop_);
+  DCHECK(pa_context_);
+  pa_operation* operation = pa_context_get_server_info(
+      pa_context_, GetSystemDefaultOutputDeviceCallback, this);
+  WaitForOperationCompletion(pa_mainloop_, operation);
+}
+
+bool PulseAudioOutputStream::InitializeMainloopAndContext() {
+  DCHECK(!pa_mainloop_);
+  DCHECK(!pa_context_);
   DCHECK(thread_checker_.CalledOnValidThread());
+  std::string app_name = AudioManager::GetGlobalAppName();

[Henrik Grunell, 2016/03/10 00:57:07]

Move this line to just before where it's used (pa_context_ = ...).

[rchtara, 2016/03/10 13:20:35]

Done.

+  pa_mainloop_ = pa_threaded_mainloop_new();
+  RETURN_ON_FAILURE(pa_mainloop_, "Failed to create PulseAudio main loop.");
+
+  pa_mainloop_api* pa_mainloop_api = pa_threaded_mainloop_get_api(pa_mainloop_);
+  pa_context_ = pa_context_new(
+      pa_mainloop_api, app_name.empty() ? "Chromium" : app_name.c_str());
+  RETURN_ON_FAILURE(pa_context_, "Failed to create PulseAudio context.");
+
+  // A state callback must be set before calling pa_threaded_mainloop_lock() or
+  // pa_threaded_mainloop_wait() calls may lead to dead lock.
+  pa_context_set_state_callback(pa_context_, &pulse::ContextStateCallback,
+                                pa_mainloop_);
+  {
+    // Lock the main loop while setting up the context.  Failure to do so may
+    // lead to crashes as the PulseAudio thread tries to run before things are
+    // ready.
+    AutoPulseLock auto_lock(pa_mainloop_);
+
+    RETURN_ON_FAILURE(pa_threaded_mainloop_start(pa_mainloop_) == 0,
+                      "Failed to start PulseAudio main loop.");
+    RETURN_ON_FAILURE(pa_context_connect(pa_context_, NULL,
+                                         PA_CONTEXT_NOAUTOSPAWN, NULL) == 0,
+                      "Failed to connect PulseAudio context.");
+
+    // Wait until |pa_context_| is ready.  pa_threaded_mainloop_wait() must be
+    // called after pa_context_get_state() in case the context is already ready,
+    // otherwise pa_threaded_mainloop_wait() will hang indefinitely.
+    while (true) {
+      pa_context_state_t context_state = pa_context_get_state(pa_context_);
+      RETURN_ON_FAILURE(PA_CONTEXT_IS_GOOD(context_state),
+                        "Invalid PulseAudio context state.");
+      if (context_state == PA_CONTEXT_READY)
+        break;
+      pa_threaded_mainloop_wait(pa_mainloop_);
+    }
+  }
+
+  return true;
+}
+
+bool PulseAudioOutputStream::Open() {
+  std::string app_name = AudioManager::GetGlobalAppName();
+  std::string device_name_to_use;
+  if (!InitializeMainloopAndContext()) {
+    return false;
+  }
+
+  device_name_to_use = device_id_;

[Henrik Grunell, 2016/03/10 00:57:07]

std::string device_name_to_use = device_id_;

Then remove line above.

[rchtara, 2016/03/10 13:20:35]

Done.

+  if (device_id_ == AudioManagerBase::kDefaultDeviceId) {
+    GetSystemDefaultOutputDevice();

[Henrik Grunell, 2016/03/10 00:57:07]

I wonder if we should have the lock when calling this. On the input side we do.
I'm not sure about this.

[rchtara, 2016/03/10 13:20:35]

I tied to use a lock before calling it but the browser crashes.

[Henrik Grunell, 2016/03/10 20:07:34]

OK, what was the crash? And why doesn't it crash on the input side where we have
the lock when getting the default device?

[rchtara, 2016/03/11 10:13:09]

I rechecked it, and now it's working : the crash was due to another issue that
was fixed. 

+    device_name_to_use = default_system_device_name_;
+  }
+
   return pulse::CreateOutputStream(
-      &pa_mainloop_, &pa_context_, &pa_stream_, params_, device_id_,
-      AudioManager::GetGlobalAppName(), &StreamNotifyCallback,
-      &StreamRequestCallback, this);
+      pa_mainloop_, pa_context_, &pa_stream_, params_, device_name_to_use,
+      app_name, &StreamNotifyCallback, &StreamRequestCallback, this);

[Henrik Grunell, 2016/03/10 00:57:07]

Use AudioManager::GetGlobalAppName() here, remove first line in function.

[rchtara, 2016/03/10 13:20:35]

Done.

 }
 
 void PulseAudioOutputStream::Reset() {
@@ -235,4 +323,15 @@ void PulseAudioOutputStream::GetVolume(double* volume) {
   *volume = volume_;
 }
 
+pa_threaded_mainloop* PulseAudioOutputStream::GetPAMainloop() {

[Henrik Grunell, 2016/03/10 00:57:07]

Put the function definitions in the same order as the declaration order in the
header.

[rchtara, 2016/03/10 13:20:35]

They're not ordered as in the header.
But I'm doing my best. 

[Henrik Grunell, 2016/03/10 20:07:34]

Ah, OK. Well, best effort is fine then.

[rchtara, 2016/03/11 10:13:09]

Done.

+  return pa_mainloop_;
+}
+
+void PulseAudioOutputStream::SetDefaultSystemDeviceName(
+    const std::string& name) {
+  default_system_device_name_ = name;
+}
+
+#undef RETURN_ON_FAILURE
+
 }  // namespace media