Let default device in PulseAudio be the system default device

media/audio/pulse/pulse_util.cc

Index: media/audio/pulse/pulse_util.cc
diff --git a/media/audio/pulse/pulse_util.cc b/media/audio/pulse/pulse_util.cc
index 5044fa4d26eb4ab76dce0811433a8fb11d298299..3138f4ae9e84ed95db7b3da6b6ee4637c52803fe 100644
--- a/media/audio/pulse/pulse_util.cc
+++ b/media/audio/pulse/pulse_util.cc
@@ -211,12 +211,8 @@ bool CreateInputStream(pa_threaded_mainloop* mainloop,
               PA_STREAM_ADJUST_LATENCY |
               PA_STREAM_START_CORKED;
   RETURN_ON_FAILURE(

[Henrik Grunell, 2016/03/07 11:37:17]

Add DCHECK that device_id != AudioManagerBase::kDefaultDeviceId. Ditto for
output.

[rchtara, 2016/03/08 17:28:17]

Done.

-      pa_stream_connect_record(
-          *stream,
-          device_id == AudioManagerBase::kDefaultDeviceId ?
-              NULL : device_id.c_str(),
-          &buffer_attributes,
-          static_cast<pa_stream_flags_t>(flags)) == 0,
+      pa_stream_connect_record(*stream, device_id.c_str(), &buffer_attributes,
+                               static_cast<pa_stream_flags_t>(flags)) == 0,
       "pa_stream_connect_record FAILED ");
 
   // Wait for the stream to be ready.
@@ -241,42 +237,8 @@ bool CreateOutputStream(pa_threaded_mainloop** mainloop,
                         pa_stream_notify_cb_t stream_callback,
                         pa_stream_request_cb_t write_callback,
                         void* user_data) {
-  DCHECK(!*mainloop);
-  DCHECK(!*context);
-
-  *mainloop = pa_threaded_mainloop_new();
-  RETURN_ON_FAILURE(*mainloop, "Failed to create PulseAudio main loop.");
-
-  pa_mainloop_api* pa_mainloop_api = pa_threaded_mainloop_get_api(*mainloop);
-  *context = pa_context_new(pa_mainloop_api,
-                            app_name.empty() ? "Chromium" : app_name.c_str());
-  RETURN_ON_FAILURE(*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(*context, &ContextStateCallback, *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(*mainloop);
-
-  RETURN_ON_FAILURE(pa_threaded_mainloop_start(*mainloop) == 0,
-                    "Failed to start PulseAudio main loop.");
-  RETURN_ON_FAILURE(
-      pa_context_connect(*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(*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(*mainloop);
-  }
+  DCHECK(*mainloop);
+  DCHECK(*context);
 
   // Set sample specifications.

[Henrik Grunell, 2016/03/07 11:37:17]

Do we need to grab the lock here? The old code held on to it throughout the
function, that may or may not have been necessary. The old comment only says
"Lock the main loop while setting up the context." Can you figure out if the
lock is need here?

[rchtara, 2016/03/08 17:28:17]

No sure about this too.
The last version is working. And the lock is held only in
InitializeMainloopAndContext

[Henrik Grunell, 2016/03/09 00:49:16]

Is it guaranteed that we can never get any concurrency issues if the lock isn't
held here? I'm sure the code works, but that's no guarantee.

[rchtara, 2016/03/09 16:16:09]

So let's keep the lock. it safer

[tommi (sloooow) - chröme, 2016/03/19 13:44:17]

We also don't want inefficiency :)  Best is to understand exactly how the code
works.  Having a lock is either going to be needed or not.  We don't want to
grab the lock here if we don't need it.  A way to understand how things work and
to make sure the threading requirements are met, is to use a thread checker.  Is
this function always called on the same thread as other methods that touch the
same data?

[Henrik Grunell, 2016/03/21 09:44:18]

Is the lock anyhow related to PulseAudio internals? Or is it just Chrome code
related? I'm thinking that maybe PA accesses it, and we don't have control over
when.

[Henrik Grunell, 2016/03/23 12:51:27]

Also, while we should sure not grab a lock if we don't need it, in this case
we're just keeping the locking as it was before. I think that's fine and we
could add a todo and bug to look at improvements later.

   pa_sample_spec sample_specifications;
@@ -332,16 +294,12 @@ bool CreateOutputStream(pa_threaded_mainloop** mainloop,
   // and error.
   RETURN_ON_FAILURE(
       pa_stream_connect_playback(
-          *stream,
-          device_id == AudioManagerBase::kDefaultDeviceId ?
-              NULL : device_id.c_str(),
-          &pa_buffer_attributes,
+          *stream, device_id.c_str(), &pa_buffer_attributes,
           static_cast<pa_stream_flags_t>(
               PA_STREAM_INTERPOLATE_TIMING | PA_STREAM_ADJUST_LATENCY |
               PA_STREAM_AUTO_TIMING_UPDATE | PA_STREAM_NOT_MONOTONIC |
               PA_STREAM_START_CORKED),
-          NULL,
-          NULL) == 0,
+          NULL, NULL) == 0,
       "pa_stream_connect_playback FAILED ");
 
   // Wait for the stream to be ready.