Adds a test for an one-way media PeerConnection.

webrtc/api/peerconnection_unittest.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 397 matching lines @@
                                                     &source_constraints);
     std::string label = stream_label + kVideoTrackLabelBase;
     return peer_connection_factory_->CreateVideoTrack(label, source);
   }
 
   DataChannelInterface* data_channel() { return data_channel_; }
   const MockDataChannelObserver* data_observer() const {
     return data_observer_.get();
   }
 
-  webrtc::PeerConnectionInterface* pc() { return peer_connection_.get(); }
+  webrtc::PeerConnectionInterface* pc() const { return peer_connection_.get(); }
 
   void StopVideoCapturers() {
     for (std::vector<cricket::VideoCapturer*>::iterator it =
              video_capturers_.begin();
          it != video_capturers_.end(); ++it) {
       (*it)->Stop();
     }
   }
 
   bool AudioFramesReceivedCheck(int number_of_frames) const {
     return number_of_frames <= fake_audio_capture_module_->frames_received();
   }
 
   int audio_frames_received() const {
     return fake_audio_capture_module_->frames_received();
   }
 
   bool VideoFramesReceivedCheck(int number_of_frames) {
     if (video_decoder_factory_enabled_) {
       const std::vector<FakeWebRtcVideoDecoder*>& decoders
           = fake_video_decoder_factory_->decoders();
       if (decoders.empty()) {
         return number_of_frames <= 0;
       }
-
+      // Note - this checks that EACH decoder has the requisite number
+      // of frames. The video_frames_received() function sums them.
       for (FakeWebRtcVideoDecoder* decoder : decoders) {
         if (number_of_frames > decoder->GetNumFramesReceived()) {
           return false;
         }
       }
       return true;
     } else {
       if (fake_video_renderers_.empty()) {
         return number_of_frames <= 0;
       }
@@ skipping 199 matching lines @@
     return fake_video_renderers_.empty() ? 1 :
         fake_video_renderers_.begin()->second->height();
   }
 
   size_t number_of_remote_streams() {
     if (!pc())
       return 0;
     return pc()->remote_streams()->count();
   }
 
-  StreamCollectionInterface* remote_streams() {
+  StreamCollectionInterface* remote_streams() const {
     if (!pc()) {
       ADD_FAILURE();
       return nullptr;
     }
     return pc()->remote_streams();
   }
 
   StreamCollectionInterface* local_streams() {
     if (!pc()) {
       ADD_FAILURE();
@@ skipping 238 matching lines @@
   P2PTestConductor()
       : pss_(new rtc::PhysicalSocketServer),
         ss_(new rtc::VirtualSocketServer(pss_.get())),
         ss_scope_(ss_.get()) {}
 
   bool SessionActive() {
     return initiating_client_->SessionActive() &&
            receiving_client_->SessionActive();
   }
 
-  // Return true if the number of frames provided have been received or it is
-  // known that that will never occur (e.g. no frames will be sent or
-  // captured).
-  bool FramesNotPending(int audio_frames_to_receive,
-                        int video_frames_to_receive) {
-    return VideoFramesReceivedCheck(video_frames_to_receive) &&
-        AudioFramesReceivedCheck(audio_frames_to_receive);
+  // Return true if the number of frames provided have been received
+  // on the video and audio tracks provided.
+  bool FramesHaveArrived(int audio_frames_to_receive,
+                         int video_frames_to_receive) {
+    bool all_good = true;
+    // Note: We assume that all tracks contain audio and video,
+    // unless one of the parties has can_receive_video() = false.
+    if (initiating_client_->local_streams()->count() > 0) {
+      if (receiving_client_->can_receive_audio()) {
+        all_good &= receiving_client_->AudioFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+      if (receiving_client_->can_receive_video() &&
+          initiating_client_->can_receive_video()) {
+        all_good &= receiving_client_->VideoFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+    }
+    if (receiving_client_->local_streams()->count() > 0) {
+      if (initiating_client_->can_receive_audio()) {
+        all_good &= initiating_client_->AudioFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+      if (initiating_client_->can_receive_video() &&
+          receiving_client_->can_receive_video()) {
+        all_good &= initiating_client_->VideoFramesReceivedCheck(
+            audio_frames_to_receive);
+      }
+    }
+    return all_good;
   }
-  bool AudioFramesReceivedCheck(int frames_received) {
-    return initiating_client_->AudioFramesReceivedCheck(frames_received) &&
-        receiving_client_->AudioFramesReceivedCheck(frames_received);
-  }
-  bool VideoFramesReceivedCheck(int frames_received) {
-    return initiating_client_->VideoFramesReceivedCheck(frames_received) &&
-        receiving_client_->VideoFramesReceivedCheck(frames_received);
-  }
+
   void VerifyDtmf() {
     initiating_client_->VerifyDtmf();
     receiving_client_->VerifyDtmf();
   }
 
   void TestUpdateOfferWithRejectedContent() {
     // Renegotiate, rejecting the video m-line.
     initiating_client_->Negotiate(true, false);
     ASSERT_TRUE_WAIT(SessionActive(), kMaxWaitForActivationMs);
 
@@ skipping 84 matching lines @@
     if (initiating_client_->NumberOfLocalMediaStreams() == 0) {
       initiating_client_->AddMediaStream(true, true);
     }
     initiating_client_->Negotiate();
     // Assert true is used here since next tests are guaranteed to fail and
     // would eat up 5 seconds.
     ASSERT_TRUE_WAIT(SessionActive(), kMaxWaitForActivationMs);
     VerifySessionDescriptions();
 
     int audio_frame_count = kEndAudioFrameCount;
+    int video_frame_count = kEndVideoFrameCount;
     // TODO(ronghuawu): Add test to cover the case of sendonly and recvonly.
     if (!initiating_client_->can_receive_audio() ||

[perkj_webrtc, 2016/03/09 14:44:41]

these can be removed now? 

[hta-webrtc, 2016/03/09 16:37:52]

With one more tweak, yes.

         !receiving_client_->can_receive_audio()) {
       audio_frame_count = -1;
     }
-    int video_frame_count = kEndVideoFrameCount;
     if (!initiating_client_->can_receive_video() ||

[perkj_webrtc, 2016/03/09 14:44:41]

dito?

[hta-webrtc, 2016/03/09 16:37:52]

Done.

         !receiving_client_->can_receive_video()) {
       video_frame_count = -1;
     }
 
-    if (audio_frame_count != -1 || video_frame_count != -1) {
-      // Audio or video is expected to flow, so both clients should reach the
-      // Connected state, and the offerer (ICE controller) should proceed to
-      // Completed.
-      // Note: These tests have been observed to fail under heavy load at
-      // shorter timeouts, so they may be flaky.
-      EXPECT_EQ_WAIT(
-          webrtc::PeerConnectionInterface::kIceConnectionCompleted,
-          initiating_client_->ice_connection_state(),
-          kMaxWaitForFramesMs);
-      EXPECT_EQ_WAIT(
-          webrtc::PeerConnectionInterface::kIceConnectionConnected,
-          receiving_client_->ice_connection_state(),
-          kMaxWaitForFramesMs);
+    if ((!initiating_client_->can_receive_audio() &&
+         !initiating_client_->can_receive_video()) ||
+        (!receiving_client_->can_receive_audio() &&
+         !receiving_client_->can_receive_video())) {
+      // Neither audio nor video will flow, so connections won't be
+      // established. There's nothing more to check.

[perkj_webrtc, 2016/03/09 14:17:17]

Is this really true? What about data channels only?

[hta-webrtc, 2016/03/09 14:31:28]

Datachannels will cause transports to be established, yes. The old logic ignored
that case, so I reproduced that. Need a way to detect the presence of
datachannels.

This whole logic is not valid under Plan A + non-Bundle anyway (we should be
counting media sections of each type, not whether constraints have been set).

[perkj_webrtc, 2016/03/09 14:44:41]

I mean- should the below checks now work even if you just remove all if cases
here and above?

[hta-webrtc, 2016/03/09 16:37:52]

The checks that wait for ICE to complete won't work if ICE doesn't start, and
ICE won't start if there are no transports, and there won't be any transports
when there is no media or datachannel.
So this IF test is necessary. The rest are gone.

I added a TODO about the data channel case.

+      return;
     }
 
-    if (initiating_client_->can_receive_audio() ||
-        initiating_client_->can_receive_video()) {
-      // The initiating client can receive media, so it must produce candidates
-      // that will serve as destinations for that media.
-      // TODO(bemasc): Understand why the state is not already Complete here, as
-      // seems to be the case for the receiving client. This may indicate a bug
-      // in the ICE gathering system.
-      EXPECT_NE(webrtc::PeerConnectionInterface::kIceGatheringNew,
-                initiating_client_->ice_gathering_state());
-    }
-    if (receiving_client_->can_receive_audio() ||
-        receiving_client_->can_receive_video()) {
-      EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
-                     receiving_client_->ice_gathering_state(),
-                     kMaxWaitForFramesMs);
-    }
+    // Audio or video is expected to flow, so both clients should reach the
+    // Connected state, and the offerer (ICE controller) should proceed to
+    // Completed.
+    // Note: These tests have been observed to fail under heavy load at
+    // shorter timeouts, so they may be flaky.
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionCompleted,
+                   initiating_client_->ice_connection_state(),
+                   kMaxWaitForFramesMs);
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
+                   receiving_client_->ice_connection_state(),
+                   kMaxWaitForFramesMs);
 
-    EXPECT_TRUE_WAIT(FramesNotPending(audio_frame_count, video_frame_count),
+    EXPECT_NE(webrtc::PeerConnectionInterface::kIceGatheringNew,

[perkj_webrtc, 2016/03/09 14:17:17]

Should you leave the todo here unless you know that this is the correct
behaviour? 

[hta-webrtc, 2016/03/09 14:31:28]

I know it's not correct behavior according to current spec (and the bug is
already filed a year ago, I think), but the WG seems about to revise its state
machine so that the kIceGatheringComplete state changes meaning, so it doesn't
make sense to fixate upon changing this now.

The check is correct, and will remain correct after spec revision.

[hta-webrtc, 2016/03/09 16:37:52]

Added a TODO and explanatory text.

+              initiating_client_->ice_gathering_state());
+    EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
+                   receiving_client_->ice_gathering_state(),
+                   kMaxWaitForFramesMs);
+
+    // Check that the expected number of frames have arrived.
+    EXPECT_TRUE_WAIT(FramesHaveArrived(audio_frame_count, video_frame_count),
                      kMaxWaitForFramesMs);
   }
 
   void SetupAndVerifyDtlsCall() {
     MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
     FakeConstraints setup_constraints;
     setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
                                    true);
     ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
     LocalP2PTest();
@@ skipping 109 matching lines @@
   LocalP2PTest();
   VerifyRenderedSize(1280, 720);
 }
 
 // This test sets up a call between two endpoints that are configured to use
 // DTLS key agreement. As a result, DTLS is negotiated and used for transport.
 TEST_F(P2PTestConductor, LocalP2PTestDtls) {
   SetupAndVerifyDtlsCall();
 }
 
+// This test sets up an one-way call, with media only from initiator to
+// responder.
+TEST_F(P2PTestConductor, OneWayMediaCall) {
+  ASSERT_TRUE(CreateTestClients());
+  receiving_client()->set_auto_add_stream(false);
+  LocalP2PTest();
+}
+
 // This test sets up a audio call initially and then upgrades to audio/video,
 // using DTLS.
 TEST_F(P2PTestConductor, LocalP2PTestDtlsRenegotiate) {
   MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
   FakeConstraints setup_constraints;
   setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
                                  true);
   ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
   receiving_client()->SetReceiveAudioVideo(true, false);
   LocalP2PTest();
@@ skipping 586 matching lines @@
                  initializing_client()->ice_connection_state(),
                  kMaxWaitForFramesMs);
   EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceConnectionConnected,
                  receiving_client()->ice_connection_state(),
                  kMaxWaitForFramesMs);
   // Now set the tracks, and expect frames to immediately start flowing.
   EXPECT_TRUE(
       audio_sender->SetTrack(initializing_client()->CreateLocalAudioTrack("")));
   EXPECT_TRUE(
       video_sender->SetTrack(initializing_client()->CreateLocalVideoTrack("")));
-  EXPECT_TRUE_WAIT(FramesNotPending(kEndAudioFrameCount, kEndVideoFrameCount),
+  EXPECT_TRUE_WAIT(FramesHaveArrived(kEndAudioFrameCount, kEndVideoFrameCount),
                    kMaxWaitForFramesMs);
 }
 
 class IceServerParsingTest : public testing::Test {
  public:
   // Convenience for parsing a single URL.
   bool ParseUrl(const std::string& url) {
     return ParseUrl(url, std::string(), std::string());
   }
 
@@ skipping 173 matching lines @@
   PeerConnectionInterface::IceServer server;
   server.urls.push_back("turn:hostname");
   server.urls.push_back("turn:hostname2");
   servers.push_back(server);
   EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_servers_, &turn_servers_));
   EXPECT_EQ(2U, turn_servers_.size());
   EXPECT_NE(turn_servers_[0].priority, turn_servers_[1].priority);
 }
 
 #endif // if !defined(THREAD_SANITIZER)