start/stop the source of the capturer when 1st audiotrack/last audiotrack is added/removed

content/renderer/media/media_stream_dependency_factory.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 "content/renderer/media/media_stream_dependency_factory.h"
 
 #include <vector>
 
 #include "base/command_line.h"
 #include "base/synchronization/waitable_event.h"
@@ skipping 390 matching lines @@
       return false;
     }
   }
 
   WebKit::WebMediaStreamSource::Type type = track.source().type();
   DCHECK(type == WebKit::WebMediaStreamSource::TypeAudio ||
          type == WebKit::WebMediaStreamSource::TypeVideo);
 
   std::string track_id = UTF16ToUTF8(track.id());
   if (source.type() == WebKit::WebMediaStreamSource::TypeAudio) {
     // TODO(henrika,xians): Refactor how an audio track is created to harmonize

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

Is this TODO still vaild?

[no longer working on chromium, 2013/06/05 16:29:45]

I think we can remove this TODO here since it does not tell much.

     // with video tracks.
     scoped_refptr<webrtc::AudioTrackInterface> audio_track(
         CreateLocalAudioTrack(track_id, source_data->local_audio_source()));
     audio_track->set_enabled(track.isEnabled());
-    if (GetWebRtcAudioDevice()) {
-      WebRtcAudioCapturer* capturer = GetWebRtcAudioDevice()->capturer().get();
-      if (!capturer->is_recording())
-        capturer->Start();
-    }
+    if (GetWebRtcAudioDevice() && GetWebRtcAudioDevice()->capturer().get())

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

It is now a bit unclear why this if-statement is needed. It was clear in the old
revision but not now.

[no longer working on chromium, 2013/06/05 16:29:45]

Good catch, it is because we were using MockLocalAudioTrack on our unittest, and
it will crash when we cast it to WebRtcLocalAudioTrack.

I changed the unittest to create real WebRtcLocalAudioTrack instead, now the
checks are removed.

+      static_cast<WebRtcLocalAudioTrack*>(audio_track.get())->Start();

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

Can you add some comments here to explain what actually happens. I assume you
start the source (if not already started) which starts feeding the tracks.
Right?

[no longer working on chromium, 2013/06/05 16:29:45]

Done.

+
     return native_stream->AddTrack(audio_track.get());
   } else {
     scoped_refptr<webrtc::VideoTrackInterface> video_track(
         CreateLocalVideoTrack(track_id, source_data->video_source()));
     video_track->set_enabled(track.isEnabled());
     return native_stream->AddTrack(video_track.get());
   }
 }
 
 bool MediaStreamDependencyFactory::AddNativeVideoMediaTrack(
@@ skipping 252 matching lines @@
 WebRtcAudioDeviceImpl*
 MediaStreamDependencyFactory::GetWebRtcAudioDevice() {
   return audio_device_.get();
 }
 
 void MediaStreamDependencyFactory::StopLocalAudioSource(
     const WebKit::WebMediaStream& description) {
   MediaStreamExtraData* extra_data = static_cast<MediaStreamExtraData*>(
       description.extraData());
   if (extra_data && extra_data->is_local() && extra_data->stream().get() &&
-      !extra_data->stream()->GetAudioTracks().empty()) {
-    if (GetWebRtcAudioDevice()) {
-      scoped_refptr<WebRtcAudioCapturer> capturer =
-          GetWebRtcAudioDevice()->capturer();
-      if (capturer.get())
-        capturer->Stop();
-    }
+      !extra_data->stream()->GetAudioTracks().empty() &&
+      GetWebRtcAudioDevice() &&

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

Same comment as above. It is not clear why you must check for an existing ADM
here. Can't say by looking at the following code why this is needed.

[no longer working on chromium, 2013/06/05 16:29:45]

Removed.

+      GetWebRtcAudioDevice()->capturer().get()) {
+    static_cast<WebRtcLocalAudioTrack*>(

[henrika (OOO until Aug 14), 2013/06/05 09:09:20]

You call start when a track is added and stop when the source is stopped. What
happens when a track is removed then? Feels like there is an asymmetry here.

[no longer working on chromium, 2013/06/05 16:29:45]

StopLocalAudioSource is the wrong name, it should be renamed as
StopLocalAudioTrack, I will create an issue to Tommy.

+        extra_data->stream()->GetAudioTracks()[0].get())->Stop();
   }
 }
 
 void MediaStreamDependencyFactory::InitializeWorkerThread(
     talk_base::Thread** thread,
     base::WaitableEvent* event) {
   jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
   jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
   *thread = jingle_glue::JingleThreadWrapper::current();
   event->Signal();
@@ skipping 87 matching lines @@
       // processed before returning. We wait for the above task to finish before
       // letting the the function continue to avoid any potential race issues.
       chrome_worker_thread_.Stop();
     } else {
       NOTREACHED() << "Worker thread not running.";
     }
   }
 }
 
 }  // namespace content