MediaStream audio object graph untangling and clean-ups.

content/renderer/media/webrtc/webrtc_local_audio_track_adapter.h

 // Copyright 2014 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.
 
 #ifndef CONTENT_RENDERER_MEDIA_WEBRTC_WEBRTC_LOCAL_AUDIO_TRACK_ADAPTER_H_
 #define CONTENT_RENDERER_MEDIA_WEBRTC_WEBRTC_LOCAL_AUDIO_TRACK_ADAPTER_H_
 
 #include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_vector.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread_checker.h"
 #include "content/common/content_export.h"
+#include "content/renderer/media/media_stream_audio_level_calculator.h"
 #include "third_party/webrtc/api/mediastreamtrack.h"
 #include "third_party/webrtc/media/base/audiorenderer.h"
 
 namespace cricket {
 class AudioRenderer;
 }
 
 namespace webrtc {
 class AudioSourceInterface;
 class AudioProcessorInterface;
 }
 
 namespace content {
 
 class MediaStreamAudioProcessor;
 class WebRtcAudioSinkAdapter;
 class WebRtcLocalAudioTrack;
 
+// Provides an implementation of the webrtc::AudioTrackInterface that can be
+// bound/unbound to/from a MediaStreamAudioTrack.  In other words, this is an
+// adapter that sits between the media stream object graph and WebRtc's object
+// graph and proxies between the two.
 class CONTENT_EXPORT WebRtcLocalAudioTrackAdapter
     : NON_EXPORTED_BASE(
           public webrtc::MediaStreamTrack<webrtc::AudioTrackInterface>) {
  public:
   static scoped_refptr<WebRtcLocalAudioTrackAdapter> Create(
       const std::string& label,
       webrtc::AudioSourceInterface* track_source);
 
   WebRtcLocalAudioTrackAdapter(
       const std::string& label,
       webrtc::AudioSourceInterface* track_source,
       const scoped_refptr<base::SingleThreadTaskRunner>& signaling_thread);
 
   ~WebRtcLocalAudioTrackAdapter() override;
 
   void Initialize(WebRtcLocalAudioTrack* owner);
 
-  // Called on the audio thread by the WebRtcLocalAudioTrack to set the signal
-  // level of the audio data.
-  void SetSignalLevel(int signal_level);
+  // Set the object that provides shared access to the current audio signal
+  // level.
+  void SetLevel(scoped_refptr<MediaStreamAudioLevelCalculator::Level> level);
 
   // Method called by the WebRtcLocalAudioTrack to set the processor that
   // applies signal processing on the data of the track.
   // This class will keep a reference of the |processor|.
   // Called on the main render thread.
   void SetAudioProcessor(
       const scoped_refptr<MediaStreamAudioProcessor>& processor);
 
   // webrtc::MediaStreamTrack implementation.
   std::string kind() const override;
@@ skipping 13 matching lines @@
 
   // The source of the audio track which handles the audio constraints.
   // TODO(xians): merge |track_source_| to |capturer_| in WebRtcLocalAudioTrack.
   rtc::scoped_refptr<webrtc::AudioSourceInterface> track_source_;
 
   // Libjingle's signaling thread.
   const scoped_refptr<base::SingleThreadTaskRunner> signaling_thread_;
 
   // The audio processsor that applies audio processing on the data of audio
   // track.
   scoped_refptr<MediaStreamAudioProcessor> audio_processor_;

[mcasas, 2016/02/26 01:28:19]

This variable is only set in SetAudioProcessor() and
then read in GetAudioProcessor() (providing a cast
in between) and the latter is called in
third_party/webrtc/api/statscollector_unittest.cc
and 
third_party/webrtc/api/statscollector.cc
which is a code smell...

[miu, 2016/02/27 03:46:36]

Acknowledged.

 
   // A vector of WebRtc VoE channels that the capturer sends data to.
   std::vector<int> voe_channels_;

[miu, 2016/02/27 03:46:36]

Removed this since it's not used at all.

 
   // A vector of the peer connection sink adapters which receive the audio data
   // from the audio track.
   ScopedVector<WebRtcAudioSinkAdapter> sink_adapters_;
 
-  // The amplitude of the signal.
-  int signal_level_;
+  // Thread-safe accessor to current audio signal level.

[mcasas, 2016/02/26 01:28:19]

If this is thread-safe, we don't need to protect it with
the |lock_| in l.109 right? Rename it if so to e.g.
|thread_safe_level_calculator_|.

[miu, 2016/02/27 03:46:36]

Done.  Actually, that's an interesting point.  If this refptr starts out as
null, and is only assigned once, it is completely safe to mutate it and read it
without a lock.

I also noticed, with a little more clean-up in WebRtcAudioCapturer, I was able
to do the same for |audio_processor_|.  So, now we don't need a lock for that.

...and now |lock_| isn't needed at all anymore.  So, I removed that.

+  scoped_refptr<MediaStreamAudioLevelCalculator::Level> level_;
 
   // Thread checker for libjingle's signaling thread.
   base::ThreadChecker signaling_thread_checker_;

[mcasas, 2016/02/26 01:28:19]

We don't need |signaling_thread_checker_| is we have
|signaling_thread_| on which we can call 
BelongsToCurrentThread() and  we assume is not nullptr 
on ctor (needs at least a DCHECK btw).

[miu, 2016/02/27 03:46:36]

Done.  Removed both since the latter is only used in one place (and is detached
there!).

   base::ThreadChecker capture_thread_;
 
   // Protects |voe_channels_|, |audio_processor_| and |signal_level_|.

[mcasas, 2016/02/26 01:28:19]

|signal_level_| is no more.

[miu, 2016/02/27 03:46:36]

Done.  |lock_| is no more too (see above comment).  :)

   mutable base::Lock lock_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_MEDIA_WEBRTC_WEBRTC_LOCAL_AUDIO_TRACK_ADAPTER_H_