MediaStream audio object graph untangling and clean-ups.

content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc

Index: content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc
diff --git a/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc b/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc
index 20a3969d11312d81c0b99354bc08c3f476666884..c7245fef7a5fece857ff53b7e84328007fe7c8f3 100644
--- a/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc
+++ b/content/renderer/media/webrtc/webrtc_local_audio_track_adapter.cc
@@ -23,34 +23,31 @@ WebRtcLocalAudioTrackAdapter::Create(
     webrtc::AudioSourceInterface* track_source) {
   // TODO(tommi): Change this so that the signaling thread is one of the
   // parameters to this method.
-  scoped_refptr<base::SingleThreadTaskRunner> signaling_thread;
+  scoped_refptr<base::SingleThreadTaskRunner> signaling_task_runner;
   RenderThreadImpl* current = RenderThreadImpl::current();
   if (current) {
     PeerConnectionDependencyFactory* pc_factory =
         current->GetPeerConnectionDependencyFactory();
-    signaling_thread = pc_factory->GetWebRtcSignalingThread();
+    signaling_task_runner = pc_factory->GetWebRtcSignalingThread();
+    CHECK(signaling_task_runner);
+  } else {
+    LOG(WARNING) << "Assuming single-threaded operation for unit test.";
   }
 
-  LOG_IF(ERROR, !signaling_thread.get()) << "No signaling thread!";
-
   rtc::RefCountedObject<WebRtcLocalAudioTrackAdapter>* adapter =
       new rtc::RefCountedObject<WebRtcLocalAudioTrackAdapter>(
-          label, track_source, signaling_thread);
+          label, track_source, signaling_task_runner);
   return adapter;
 }
 
 WebRtcLocalAudioTrackAdapter::WebRtcLocalAudioTrackAdapter(
     const std::string& label,
     webrtc::AudioSourceInterface* track_source,
-    const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread)
+    const scoped_refptr<base::SingleThreadTaskRunner>& signaling_task_runner)
     : webrtc::MediaStreamTrack<webrtc::AudioTrackInterface>(label),
       owner_(NULL),
       track_source_(track_source),
-      signaling_thread_(signaling_thread),
-      signal_level_(0) {
-  signaling_thread_checker_.DetachFromThread();
-  capture_thread_.DetachFromThread();
-}
+      signaling_task_runner_(signaling_task_runner) {}
 
 WebRtcLocalAudioTrackAdapter::~WebRtcLocalAudioTrackAdapter() {
 }
@@ -63,15 +60,18 @@ void WebRtcLocalAudioTrackAdapter::Initialize(WebRtcLocalAudioTrack* owner) {
 
 void WebRtcLocalAudioTrackAdapter::SetAudioProcessor(
     const scoped_refptr<MediaStreamAudioProcessor>& processor) {
-  // SetAudioProcessor will be called when a new capture thread has been
-  // initialized, so we need to detach from any current capture thread we're
-  // checking and attach to the current one.
-  capture_thread_.DetachFromThread();
-  DCHECK(capture_thread_.CalledOnValidThread());
-  base::AutoLock auto_lock(lock_);
+  DCHECK(processor.get());
+  DCHECK(!audio_processor_);
   audio_processor_ = processor;
 }
 
+void WebRtcLocalAudioTrackAdapter::SetLevel(
+    scoped_refptr<MediaStreamAudioLevelCalculator::Level> level) {
+  DCHECK(level.get());
+  DCHECK(!level_);
+  level_ = level;

[tommi (sloooow) - chröme, 2016/03/03 11:07:31]

assuming |level| took ref ownership, you probably want to use std::move here

[miu, 2016/03/05 02:55:31]

Done.

+}
+
 std::string WebRtcLocalAudioTrackAdapter::kind() const {
   return kAudioTrackKind;
 }
@@ -79,8 +79,9 @@ std::string WebRtcLocalAudioTrackAdapter::kind() const {
 bool WebRtcLocalAudioTrackAdapter::set_enabled(bool enable) {
   // If we're not called on the signaling thread, we need to post a task to
   // change the state on the correct thread.
-  if (signaling_thread_.get() && !signaling_thread_->BelongsToCurrentThread()) {
-    signaling_thread_->PostTask(FROM_HERE,
+  if (signaling_task_runner_ &&
+      !signaling_task_runner_->BelongsToCurrentThread()) {
+    signaling_task_runner_->PostTask(FROM_HERE,
         base::Bind(
             base::IgnoreResult(&WebRtcLocalAudioTrackAdapter::set_enabled),
             this, enable));
@@ -93,7 +94,8 @@ bool WebRtcLocalAudioTrackAdapter::set_enabled(bool enable) {
 
 void WebRtcLocalAudioTrackAdapter::AddSink(
     webrtc::AudioTrackSinkInterface* sink) {
-  DCHECK(signaling_thread_checker_.CalledOnValidThread());
+  DCHECK(!signaling_task_runner_ ||
+         signaling_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(sink);
 #ifndef NDEBUG
   // Verify that |sink| has not been added.
@@ -112,7 +114,8 @@ void WebRtcLocalAudioTrackAdapter::AddSink(
 
 void WebRtcLocalAudioTrackAdapter::RemoveSink(
     webrtc::AudioTrackSinkInterface* sink) {
-  DCHECK(signaling_thread_checker_.CalledOnValidThread());
+  DCHECK(!signaling_task_runner_ ||
+         signaling_task_runner_->RunsTasksOnCurrentThread());
   DCHECK(sink);
   for (ScopedVector<WebRtcAudioSinkAdapter>::iterator it =
            sink_adapters_.begin();
@@ -126,28 +129,32 @@ void WebRtcLocalAudioTrackAdapter::RemoveSink(
 }
 
 bool WebRtcLocalAudioTrackAdapter::GetSignalLevel(int* level) {
-  DCHECK(signaling_thread_checker_.CalledOnValidThread());
-
-  base::AutoLock auto_lock(lock_);
-  *level = signal_level_;
+  DCHECK(!signaling_task_runner_ ||
+         signaling_task_runner_->RunsTasksOnCurrentThread());
+
+  // |level_| is only set once, so it's safe to read without first acquiring a
+  // mutex.
+  if (!level_)
+    return false;
+  const float signal_level = level_->GetCurrent();
+  DCHECK_GE(signal_level, 0.0f);
+  DCHECK_LE(signal_level, 1.0f);
+  // Convert from float in range [0.0,1.0] to an int in range [0,32767].
+  *level = static_cast<int>(signal_level * std::numeric_limits<int16_t>::max() +
+                            0.5f /* rounding to nearest int */);

[tommi (sloooow) - chröme, 2016/03/03 11:07:31]

Double checking on the behavior...

I see you're maintaining the 'assume non-zero audio' behavior where audio energy
is calculated post processing.  That's great and it solved a situation where we
could frequently get false positives.

Here, if |signal_level| is 1.0f/max() (smallest possible non-zero value), will
it be rounded to 0 or 1?

I'm assuming that before it would have been 1, but I might be mistaken.

[miu, 2016/03/05 02:55:31]

Yes, it will round to 1.  Here's a test program I wrote to double-check this:

#include <stdlib.h>
#include <iostream>
#include <limits>

int main(int argc, char* argv[]) {
  double signal_level = 1.0 / std::numeric_limits<int16_t>::max();
  int level =
      static_cast<int>(signal_level * std::numeric_limits<int16_t>::max() +
                       0.5f /* rounding to nearest int */);

  std::cout << "Result: " << level << std::endl;

  return 0;
}

Output is:

$ g++ test.cc
$ ./a.out
Result: 1

   return true;
 }
 
 rtc::scoped_refptr<webrtc::AudioProcessorInterface>
 WebRtcLocalAudioTrackAdapter::GetAudioProcessor() {
-  DCHECK(signaling_thread_checker_.CalledOnValidThread());
-  base::AutoLock auto_lock(lock_);
+  DCHECK(!signaling_task_runner_ ||
+         signaling_task_runner_->RunsTasksOnCurrentThread());
   return audio_processor_.get();
 }
 
-void WebRtcLocalAudioTrackAdapter::SetSignalLevel(int signal_level) {
-  DCHECK(capture_thread_.CalledOnValidThread());
-  base::AutoLock auto_lock(lock_);
-  signal_level_ = signal_level;
-}
-
 webrtc::AudioSourceInterface* WebRtcLocalAudioTrackAdapter::GetSource() const {
-  DCHECK(signaling_thread_checker_.CalledOnValidThread());
+  DCHECK(!signaling_task_runner_ ||
+         signaling_task_runner_->RunsTasksOnCurrentThread());
   return track_source_;
 }