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

content/renderer/media/webrtc_local_audio_track_unittest.cc

Index: content/renderer/media/webrtc_local_audio_track_unittest.cc
diff --git a/content/renderer/media/webrtc_local_audio_track_unittest.cc b/content/renderer/media/webrtc_local_audio_track_unittest.cc
index 7f7c16dc1bc2686e22bcceae0fd6859cd6cda4a5..51d868d3fcfe3341617d1f25243d05edb7a446c8 100644
--- a/content/renderer/media/webrtc_local_audio_track_unittest.cc
+++ b/content/renderer/media/webrtc_local_audio_track_unittest.cc
@@ -13,6 +13,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
+using ::testing::AnyNumber;
 using ::testing::AtLeast;
 using ::testing::Return;
 
@@ -115,10 +116,8 @@ class WebRtcLocalAudioTrackTest : public ::testing::Test {
                                  media::CHANNEL_LAYOUT_STEREO,
                                  48000);
 
-    EXPECT_CALL(*capturer_source_.get(), Start()).WillOnce(Return());
     EXPECT_CALL(*capturer_source_.get(), SetAutomaticGainControl(false))
         .WillOnce(Return());
-    capturer_->Start();
     audio_thread_.reset(new FakeAudioThread(capturer_));
     audio_thread_->Start();
   }
@@ -126,8 +125,6 @@ class WebRtcLocalAudioTrackTest : public ::testing::Test {
   virtual void TearDown() {
     audio_thread_->Stop();
     audio_thread_.reset();
-    EXPECT_CALL(*capturer_source_.get(), Stop()).WillOnce(Return());
-    capturer_->Stop();
   }
 
   scoped_refptr<MockCapturerSource> capturer_source_;
@@ -140,8 +137,10 @@ class WebRtcLocalAudioTrackTest : public ::testing::Test {
 // get data callback when the track is connected to the capturer but not when
 // the track is disconnected from the capturer.
 TEST_F(WebRtcLocalAudioTrackTest, ConnectAndDisconnectOneSink) {
+  EXPECT_CALL(*capturer_source_.get(), Start()).WillOnce(Return());
   scoped_refptr<WebRtcLocalAudioTrack> track =
       WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track->Start();
   EXPECT_TRUE(track->enabled());
   scoped_ptr<MockWebRtcAudioCapturerSink> sink(
       new MockWebRtcAudioCapturerSink());
@@ -155,6 +154,9 @@ TEST_F(WebRtcLocalAudioTrackTest, ConnectAndDisconnectOneSink) {
 
   EXPECT_TRUE(event.TimedWait(TestTimeouts::tiny_timeout()));
   track->RemoveSink(sink.get());
+
+  EXPECT_CALL(*capturer_source_.get(), Stop()).WillOnce(Return());
+  track->Stop();

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

What happens if you call Stop before RemoveSink?

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

Nothing will happen.

   track = NULL;
 }
 
@@ -165,8 +167,10 @@ TEST_F(WebRtcLocalAudioTrackTest, ConnectAndDisconnectOneSink) {
 // TODO(xians): Enable this test after resolving the racing issue that TSAN
 // reports on MediaStreamTrack::enabled();
 TEST_F(WebRtcLocalAudioTrackTest, DISABLED_DisableEnableAudioTrack) {
+  EXPECT_CALL(*capturer_source_.get(), Start()).WillOnce(Return());
   scoped_refptr<WebRtcLocalAudioTrack> track =
     WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track->Start();
   EXPECT_TRUE(track->enabled());
   EXPECT_TRUE(track->set_enabled(false));
   scoped_ptr<MockWebRtcAudioCapturerSink> sink(
@@ -187,6 +191,9 @@ TEST_F(WebRtcLocalAudioTrackTest, DISABLED_DisableEnableAudioTrack) {
   EXPECT_TRUE(track->set_enabled(true));
   EXPECT_TRUE(event.TimedWait(TestTimeouts::tiny_timeout()));
   track->RemoveSink(sink.get());
+
+  EXPECT_CALL(*capturer_source_.get(), Stop()).WillOnce(Return());
+  track->Stop();
   track = NULL;
 }
 
@@ -194,8 +201,10 @@ TEST_F(WebRtcLocalAudioTrackTest, DISABLED_DisableEnableAudioTrack) {
 // callbacks appear/disappear.
 // TODO(xians): Enable the test after the racing problem is resolved.
 TEST_F(WebRtcLocalAudioTrackTest, DISABLED_MultipleAudioTracks) {
+  EXPECT_CALL(*capturer_source_.get(), Start()).WillOnce(Return());

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

Who is default here and what is the difference when no default is used?

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

It does not have the default_sink here.

The default_sink is used by the capturer to send data to WebRtcAudioDeviceImpl,
so it does not matter for this unittest.

   scoped_refptr<WebRtcLocalAudioTrack> track_1 =
     WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track_1->Start();
   EXPECT_TRUE(track_1->enabled());
   scoped_ptr<MockWebRtcAudioCapturerSink> sink_1(
       new MockWebRtcAudioCapturerSink());
@@ -209,6 +218,7 @@ TEST_F(WebRtcLocalAudioTrackTest, DISABLED_MultipleAudioTracks) {
 
   scoped_refptr<WebRtcLocalAudioTrack> track_2 =
     WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track_2->Start();
   EXPECT_TRUE(track_2->set_enabled(false));
   scoped_ptr<MockWebRtcAudioCapturerSink> sink_2(
       new MockWebRtcAudioCapturerSink());
@@ -234,9 +244,59 @@ TEST_F(WebRtcLocalAudioTrackTest, DISABLED_MultipleAudioTracks) {
   EXPECT_TRUE(event_2.TimedWait(TestTimeouts::tiny_timeout()));
 
   track_1->RemoveSink(sink_1.get());
+  track_1->Stop();
+  track_1 = NULL;
+
+  EXPECT_CALL(*capturer_source_.get(), Stop()).WillOnce(Return());
   track_2->RemoveSink(sink_2.get());
+  track_2->Stop();
+  track_2 = NULL;
+}
+
+// Start/Stop tracks and verify the capturer is correctly starting/stopping
+// its source.
+TEST_F(WebRtcLocalAudioTrackTest, StartAndStopCapturer) {
+  // SetDefaultSink() should not trigger the Start() on |capturer_source_|.
+  base::WaitableEvent event(false, false);
+  scoped_ptr<MockWebRtcAudioCapturerSink> default_sink(
+      new MockWebRtcAudioCapturerSink());
+  EXPECT_CALL(*default_sink, SetCaptureFormat(_)).WillOnce(Return());
+  EXPECT_CALL(*default_sink, CaptureData(
+      _, _, _, 0, 0))
+      .Times(AnyNumber()).WillRepeatedly(Return());
+  capturer_->SetDefaultSink(default_sink.get());
+  EXPECT_CALL(*capturer_source_.get(), Start()).Times(0);
+  EXPECT_FALSE(event.TimedWait(TestTimeouts::tiny_timeout()));
+
+  // Starting the first audio track will start the |capturer_source_|.
+  event.Reset();
+  EXPECT_CALL(*capturer_source_.get(), Start()).WillOnce(SignalEvent(&event));
+  scoped_refptr<WebRtcLocalAudioTrack> track_1 =
+      WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track_1->Start();

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

The sequence is track calls AddSink on default sink and it therefore calls
Start, right?

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

It does not matter on the order, but mostly it is
WebRtcAudioDeviceImpl adds itself as default sink to the capturer,
track->Start(), then track->AddSink

+  EXPECT_TRUE(event.TimedWait(TestTimeouts::tiny_timeout()));
+
+  // Start the second audio track will not start the |capturer_source_|
+  // since it has been started.
+  EXPECT_CALL(*capturer_source_.get(), Start()).Times(0);

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

Should you also verify that default start is not called?

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

It is done here:
Note that it is Times(0);

+  scoped_refptr<WebRtcLocalAudioTrack> track_2 =
+      WebRtcLocalAudioTrack::Create(std::string(), capturer_, NULL);
+  track_2->Start();
+
+  // Stop the first audio track will not stop the |capturer_source_|.
+  EXPECT_CALL(*capturer_source_.get(), Stop()).Times(0);
+  track_1->Stop();

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

Any difference if you stop #2 first?

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

No difference.

   track_1 = NULL;
+
+  // Stop the last audio track will stop the |capturer_source_|.
+  event.Reset();
+  EXPECT_CALL(*capturer_source_.get(), Stop())
+      .Times(1).WillOnce(SignalEvent(&event));
+  track_2->Stop();
   track_2 = NULL;
+  EXPECT_TRUE(event.TimedWait(TestTimeouts::tiny_timeout()));
+
+  capturer_->SetDefaultSink(NULL);

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

Is this required?

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

Yes, ADM does it this way.

 }
 
 }  // namespace content