Ensures that WebRTC works for device selection using a different sample rate than default

content/renderer/media/webrtc_audio_capturer.cc

Index: content/renderer/media/webrtc_audio_capturer.cc
diff --git a/content/renderer/media/webrtc_audio_capturer.cc b/content/renderer/media/webrtc_audio_capturer.cc
index 7486e083ccda1410968b7a32d34f30b8ded4b2b8..265fd09fe5589884665d65ac97415cd2ac3b80e1 100644
--- a/content/renderer/media/webrtc_audio_capturer.cc
+++ b/content/renderer/media/webrtc_audio_capturer.cc
@@ -44,7 +44,7 @@ static int GetBufferSizeForSampleRate(int sample_rate) {
   } else {
     buffer_size = (sample_rate / 100);
     DCHECK_EQ(buffer_size * 100, sample_rate) <<
-        "Sample rate not supported. Should have been caught in Init().";
+        "Sample rate not supported";
   }
 #elif defined(OS_LINUX) || defined(OS_OPENBSD)
   // Based on tests using the current ALSA implementation in Chrome, we have
@@ -61,35 +61,85 @@ static int GetBufferSizeForSampleRate(int sample_rate) {
 // static
 scoped_refptr<WebRtcAudioCapturer> WebRtcAudioCapturer::CreateCapturer() {
   scoped_refptr<WebRtcAudioCapturer> capturer = new  WebRtcAudioCapturer();
-  if (capturer->Initialize())
-    return capturer;
+  return capturer;
+}
+
+bool WebRtcAudioCapturer::Initialize(media::ChannelLayout channel_layout,
+                                     int sample_rate) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!sinks_.empty());
+  DVLOG(1) << "WebRtcAudioCapturer::Initialize()";
+
+  media::AudioParameters::Format format =
+      media::AudioParameters::AUDIO_PCM_LOW_LATENCY;
+
+  DVLOG(1) << "Audio input hardware channel layout: " << channel_layout;
+  UMA_HISTOGRAM_ENUMERATION("WebRTC.AudioInputChannelLayout",
+                            channel_layout, media::CHANNEL_LAYOUT_MAX);
+
+  DVLOG(1) << "Audio input hardware sample rate: " << sample_rate;
+  UMA_HISTOGRAM_ENUMERATION("WebRTC.AudioInputSampleRate",
+                            sample_rate, media::kUnexpectedAudioSampleRate);
+
+  // Verify that the reported input hardware sample rate is supported
+  // on the current platform.
+  if (std::find(&kValidInputRates[0],
+                &kValidInputRates[0] + arraysize(kValidInputRates),
+                sample_rate) ==
+          &kValidInputRates[arraysize(kValidInputRates)]) {
+    DLOG(ERROR) << sample_rate << " is not a supported input rate.";
+    return false;
+  }
+
+  int buffer_size = GetBufferSizeForSampleRate(sample_rate);
+
+  // Configure audio parameters for the default source.
+  params_.Reset(format, channel_layout, sample_rate, 16, buffer_size);
+
+  {
+    // Tell all sinks which format we use.
+    base::AutoLock auto_lock(lock_);
+    for (SinkList::const_iterator it = sinks_.begin();

[tommi (sloooow) - chröme, 2013/01/15 17:43:49]

is it possible that other methods can be called on this object before
Inititalize completes? (if not, then the lock isn't needed, right?)

[henrika (OOO until Aug 14), 2013/01/16 16:37:17]

Really good point, thanks. Will make it more clear in the comments as well.

+      it != sinks_.end(); ++it) {

[tommi (sloooow) - chröme, 2013/01/15 17:43:49]

fix indent

[henrika (OOO until Aug 14), 2013/01/16 16:37:17]

Done.

+      (*it)->SetCaptureFormat(params_);
+    }
+  }
+
+  buffer_.reset(new int16[params_.frames_per_buffer() * params_.channels()]);
+
+  // Create and configure the default audio capturing source. The |source_|
+  // will be overwritten if an external client later calls SetCapturerSource()
+  // providing an alternaive media::AudioCapturerSource.
+  SetCapturerSource(AudioDeviceFactory::NewInputDevice());
 
-  return NULL;
+  return true;
 }
 
 WebRtcAudioCapturer::WebRtcAudioCapturer()
     : source_(NULL),
       running_(false),
       buffering_(false) {
+  DVLOG(1) << "WebRtcAudioCapturer::WebRtcAudioCapturer()";
 }
 
 WebRtcAudioCapturer::~WebRtcAudioCapturer() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(sinks_.empty());
   DCHECK(!loopback_fifo_);
+  DVLOG(1) << "WebRtcAudioCapturer::~WebRtcAudioCapturer()";
 }
 
 void WebRtcAudioCapturer::AddCapturerSink(WebRtcAudioCapturerSink* sink) {
-  {
-    base::AutoLock auto_lock(lock_);
-    DCHECK(std::find(sinks_.begin(), sinks_.end(), sink) == sinks_.end());
-    sinks_.push_back(sink);
-  }
-
-  // Tell the |sink| which format we use.
-  sink->SetCaptureFormat(params_);
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "WebRtcAudioCapturer::AddCapturerSink()";
+  DCHECK(std::find(sinks_.begin(), sinks_.end(), sink) == sinks_.end());

[tommi (sloooow) - chröme, 2013/01/15 17:43:49]

move back under lock

[henrika (OOO until Aug 14), 2013/01/16 16:37:17]

Feels much better. Done.

+  base::AutoLock auto_lock(lock_);
+  sinks_.push_back(sink);
 }
 
 void WebRtcAudioCapturer::RemoveCapturerSink(WebRtcAudioCapturerSink* sink) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(1) << "WebRtcAudioCapturer::RemoveCapturerSink()";
   base::AutoLock auto_lock(lock_);
   for (SinkList::iterator it = sinks_.begin(); it != sinks_.end(); ++it) {
     if (sink == *it) {
@@ -101,6 +151,7 @@ void WebRtcAudioCapturer::RemoveCapturerSink(WebRtcAudioCapturerSink* sink) {
 
 void WebRtcAudioCapturer::SetCapturerSource(
     const scoped_refptr<media::AudioCapturerSource>& source) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) <<  "SetCapturerSource()";
   scoped_refptr<media::AudioCapturerSource> old_source;
   {
@@ -122,12 +173,14 @@ void WebRtcAudioCapturer::SetCapturerSource(
 
 void WebRtcAudioCapturer::SetStopCallback(
     const base::Closure& on_device_stopped_cb) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) <<  "WebRtcAudioCapturer::SetStopCallback()";
   base::AutoLock auto_lock(lock_);
   on_device_stopped_cb_ = on_device_stopped_cb;
 }
 
 void WebRtcAudioCapturer::PrepareLoopback() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) <<  "WebRtcAudioCapturer::PrepareLoopback()";
   base::AutoLock auto_lock(lock_);
   DCHECK(!loopback_fifo_);
@@ -146,6 +199,7 @@ void WebRtcAudioCapturer::PrepareLoopback() {
 }
 
 void WebRtcAudioCapturer::CancelLoopback() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) <<  "WebRtcAudioCapturer::CancelLoopback()";
   base::AutoLock auto_lock(lock_);
   buffering_ = false;
@@ -156,12 +210,14 @@ void WebRtcAudioCapturer::CancelLoopback() {
 }
 
 void WebRtcAudioCapturer::PauseBuffering() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) <<  "WebRtcAudioCapturer::PauseBuffering()";
   base::AutoLock auto_lock(lock_);
   buffering_ = false;
 }
 
 void WebRtcAudioCapturer::ResumeBuffering() {
+  DCHECK(thread_checker_.CalledOnValidThread());

[tommi (sloooow) - chröme, 2013/01/15 17:43:49]

thanks for adding all these thread dchecks!

[henrika (OOO until Aug 14), 2013/01/16 16:37:17]

Thanks. I have done lots of manual testing here with all possible sorts of
clients to ensure that it holds.

   DVLOG(1) <<  "WebRtcAudioCapturer::ResumeBuffering()";
   base::AutoLock auto_lock(lock_);
   if (buffering_)
@@ -171,55 +227,6 @@ void WebRtcAudioCapturer::ResumeBuffering() {
   buffering_ = true;
 }
 
-bool WebRtcAudioCapturer::Initialize() {
-  DVLOG(1) << "WebRtcAudioCapturer::Initialize()";
-  // Ask the browser for the default audio input hardware sample-rate.
-  // This request is based on a synchronous IPC message.
-  // TODO(xians): we should ask for the native sample rate of a specific device.
-  int sample_rate = GetAudioInputSampleRate();
-  DVLOG(1) << "Audio input hardware sample rate: " << sample_rate;
-  UMA_HISTOGRAM_ENUMERATION("WebRTC.AudioInputSampleRate",
-                            sample_rate, media::kUnexpectedAudioSampleRate);
-
-  // Verify that the reported input hardware sample rate is supported
-  // on the current platform.
-  if (std::find(&kValidInputRates[0],
-                &kValidInputRates[0] + arraysize(kValidInputRates),
-                sample_rate) ==
-          &kValidInputRates[arraysize(kValidInputRates)]) {
-    DLOG(ERROR) << sample_rate << " is not a supported input rate.";
-    return false;
-  }
-
-  // Ask the browser for the default number of audio input channels.
-  // This request is based on a synchronous IPC message.
-  // TODO(xians): we should ask for the layout of a specific device.
-  media::ChannelLayout channel_layout = GetAudioInputChannelLayout();
-  DVLOG(1) << "Audio input hardware channels: " << channel_layout;
-
-  media::AudioParameters::Format format =
-      media::AudioParameters::AUDIO_PCM_LOW_LATENCY;
-  int buffer_size = GetBufferSizeForSampleRate(sample_rate);
-  if (!buffer_size) {
-    DLOG(ERROR) << "Unsupported platform";
-    return false;
-  }
-
-  params_.Reset(format, channel_layout, sample_rate, 16, buffer_size);
-
-  buffer_.reset(new int16[params_.frames_per_buffer() * params_.channels()]);
-
-  // Create and configure the default audio capturing source. The |source_|
-  // will be overwritten if the client call the source calls
-  // SetCapturerSource().
-  SetCapturerSource(AudioDeviceFactory::NewInputDevice());
-
-  UMA_HISTOGRAM_ENUMERATION("WebRTC.AudioInputChannelLayout",
-                            channel_layout, media::CHANNEL_LAYOUT_MAX);
-
-  return true;
-}
-
 void WebRtcAudioCapturer::ProvideInput(media::AudioBus* dest) {
   base::AutoLock auto_lock(lock_);
   DCHECK(loopback_fifo_.get() != NULL);
@@ -237,7 +244,7 @@ void WebRtcAudioCapturer::ProvideInput(media::AudioBus* dest) {
     dest->Zero();
     // This warning is perfectly safe if it happens for the first audio
     // frames. It should not happen in a steady-state mode.
-    DLOG(WARNING) << "WARNING: loopback FIFO is empty.";
+    DVLOG(2) << "WARNING: loopback FIFO is empty.";
   }
 }
 
@@ -281,12 +288,12 @@ void WebRtcAudioCapturer::Stop() {
 void WebRtcAudioCapturer::SetVolume(double volume) {

[tommi (sloooow) - chröme, 2013/01/15 17:43:49]

can SetVolume be called from multiple threads but not the other methods?

[henrika (OOO until Aug 14), 2013/01/16 16:37:17]

Correct. This one is special since it is called by the AudioInputDevice thread
when it is used in the AGC scheme.
Also, SetAutomaticGainControl is called from a Libjingle thread.

   DVLOG(1) << "WebRtcAudioCapturer::SetVolume()";
   base::AutoLock auto_lock(lock_);
-
   if (source_)
     source_->SetVolume(volume);
 }
 
 void WebRtcAudioCapturer::SetDevice(int session_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "WebRtcAudioCapturer::SetDevice(" << session_id << ")";
   base::AutoLock auto_lock(lock_);
   if (source_)
@@ -300,6 +307,7 @@ void WebRtcAudioCapturer::SetAutomaticGainControl(bool enable) {
 }
 
 bool WebRtcAudioCapturer::IsInLoopbackMode() {
+  DCHECK(thread_checker_.CalledOnValidThread());
   base::AutoLock auto_lock(lock_);
   return (loopback_fifo_ != NULL);
 }