Let default device in PulseAudio be the system default device

media/audio/pulse/pulse_util.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/pulse/pulse_util.h"
 
 #include <stdint.h>
 
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 143 matching lines @@
 
 // Helper macro for CreateInput/OutputStream() to avoid code spam and
 // string bloat.
 #define RETURN_ON_FAILURE(expression, message) do { \
   if (!(expression)) { \
     DLOG(ERROR) << message; \
     return false; \
   } \
 } while (0)
 
 bool CreateInputStream(pa_threaded_mainloop* mainloop,

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

Add a todo comment in the header file for this and the output function that they
should be moved to the input and output classes. (Since they're only used
there.)

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

Done.

                        pa_context* context,
                        pa_stream** stream,
                        const AudioParameters& params,
                        const std::string& device_id,
                        pa_stream_notify_cb_t stream_callback,
                        void* user_data) {
   DCHECK(mainloop);
   DCHECK(context);
 
   // Set sample specifications.
@@ skipping 28 matching lines @@
   const unsigned int buffer_size = params.GetBytesPerBuffer();
   buffer_attributes.maxlength = static_cast<uint32_t>(-1);
   buffer_attributes.tlength = buffer_size;
   buffer_attributes.minreq = buffer_size;
   buffer_attributes.prebuf = static_cast<uint32_t>(-1);
   buffer_attributes.fragsize = buffer_size;
   int flags = PA_STREAM_AUTO_TIMING_UPDATE |
               PA_STREAM_INTERPOLATE_TIMING |
               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.
   while (true) {
     pa_stream_state_t stream_state = pa_stream_get_state(*stream);
     RETURN_ON_FAILURE(
         PA_STREAM_IS_GOOD(stream_state), "Invalid PulseAudio stream state");
     if (stream_state == PA_STREAM_READY)
         break;
     pa_threaded_mainloop_wait(mainloop);
   }
 
   return true;
 }
 
 bool CreateOutputStream(pa_threaded_mainloop** mainloop,

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

Change to take pa_threaded_mainloop* and pa_context*.

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

Done.

                         pa_context** context,
                         pa_stream** stream,
                         const AudioParameters& params,
                         const std::string& device_id,
                         const std::string& app_name,
                         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;
   sample_specifications.format = BitsToPASampleFormat(
       params.bits_per_sample());
   sample_specifications.rate = params.sample_rate();
   sample_specifications.channels = params.channels();
 
   // Get channel mapping.
   pa_channel_map* map = NULL;
   pa_channel_map source_channel_map = ChannelLayoutToPAChannelMap(
       params.channel_layout());
@@ skipping 33 matching lines @@
   pa_buffer_attributes.minreq = params.GetBytesPerBuffer() / 2;
   pa_buffer_attributes.prebuf = static_cast<uint32_t>(-1);
   pa_buffer_attributes.tlength = params.GetBytesPerBuffer() * 3;
   pa_buffer_attributes.fragsize = static_cast<uint32_t>(-1);
 
   // Connect playback stream.  Like pa_buffer_attr, the pa_stream_flags have a
   // huge impact on the performance of the stream and were chosen through trial
   // 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.
   while (true) {
     pa_stream_state_t stream_state = pa_stream_get_state(*stream);
     RETURN_ON_FAILURE(
         PA_STREAM_IS_GOOD(stream_state), "Invalid PulseAudio stream state");
     if (stream_state == PA_STREAM_READY)
       break;
     pa_threaded_mainloop_wait(*mainloop);
   }
 
   return true;
 }
 
 #undef RETURN_ON_FAILURE
 
 }  // namespace pulse
 
 }  // namespace media