Set local SSRCs on receivers added before senders.

Addresses bug where a receiver would report SSRC 1 even though the endpoint has sending streams. BUG=4678 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/51099004 Cr-Commit-Position: refs/heads/master@{#9262}
GetStream · May 22, 2015 · 3548dd2 · 3548dd2
1 parent 367c868
commit 3548dd2
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 7 deletions.
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -1053,6 +1053,8 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
 
   if (rtcp_receiver_report_ssrc_ == kDefaultRtcpReceiverReportSsrc) {
     rtcp_receiver_report_ssrc_ = ssrc;
+    for (auto& kv : receive_streams_)
+      kv.second->SetLocalSsrc(ssrc);
   }
   if (default_send_ssrc_ == 0) {
     default_send_ssrc_ = ssrc;
@@ -2312,6 +2314,19 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs(
   RecreateWebRtcStream();
 }
 
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
+    uint32_t local_ssrc) {
+  // TODO(pbos): Consider turning this sanity check into a DCHECK. You should
+  // not be able to create a sender with the same SSRC as a receiver, but right
+  // now this can't be done due to unittests depending on receiving what they
+  // are sending from the same MediaChannel.
+  if (local_ssrc == config_.rtp.remote_ssrc)
+    return;
+
+  config_.rtp.local_ssrc = local_ssrc;
+  RecreateWebRtcStream();
+}
+
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
     bool nack_enabled, bool remb_enabled) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
@@ -2327,8 +2342,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRtpExtensions(
     const std::vector<webrtc::RtpExtension>& extensions) {
   config_.rtp.extensions = extensions;
-  if (stream_ != nullptr)
-    RecreateWebRtcStream();
+  RecreateWebRtcStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {

diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h
@@ -410,6 +410,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
 
     const std::vector<uint32>& GetSsrcs() const;
 
+    void SetLocalSsrc(uint32_t local_ssrc);
     void SetNackAndRemb(bool nack_enabled, bool remb_enabled);
     void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
     void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);

diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -971,6 +971,7 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test,
   }
 
   void TestCpuAdaptation(bool enable_overuse, bool is_screenshare);
+  void TestReceiverLocalSsrcConfiguration(bool receiver_first);
 
   FakeVideoSendStream* SetDenoisingOption(bool enabled) {
     VideoOptions options;
@@ -1312,8 +1313,8 @@ TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) {
 }
 
 TEST_F(WebRtcVideoChannel2Test, NackCanBeEnabledAndDisabled) {
-  FakeVideoReceiveStream* recv_stream = AddRecvStream();
   FakeVideoSendStream* send_stream = AddSendStream();
+  FakeVideoReceiveStream* recv_stream = AddRecvStream();
 
   EXPECT_GT(recv_stream->GetConfig().rtp.nack.rtp_history_ms, 0);
   EXPECT_GT(send_stream->GetConfig().rtp.nack.rtp_history_ms, 0);
@@ -2526,8 +2527,6 @@ TEST_F(WebRtcVideoChannel2Test,
   static const uint32 kOverlappingStreamSsrcs[] = {4, 3, 5};
   EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
 
-  const std::vector<uint32> ssrcs = MAKE_VECTOR(kSsrcs3);
-
   StreamParams sp =
       cricket::CreateSimStreamParams("cname", MAKE_VECTOR(kFirstStreamSsrcs));
 
@@ -2542,12 +2541,44 @@ TEST_F(WebRtcVideoChannel2Test,
 
   // After removing the original stream this should be fine to add (makes sure
   // that RTX ssrcs are not forever taken).
-  EXPECT_TRUE(channel_->RemoveSendStream(ssrcs[0]));
-  EXPECT_TRUE(channel_->RemoveRecvStream(ssrcs[0]));
+  EXPECT_TRUE(channel_->RemoveSendStream(kFirstStreamSsrcs[0]));
+  EXPECT_TRUE(channel_->RemoveRecvStream(kFirstStreamSsrcs[0]));
   EXPECT_TRUE(channel_->AddSendStream(sp));
   EXPECT_TRUE(channel_->AddRecvStream(sp));
 }
 
+void WebRtcVideoChannel2Test::TestReceiverLocalSsrcConfiguration(
+    bool receiver_first) {
+  EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
+
+  const uint32_t kSenderSsrc = 0xC0FFEE;
+  const uint32_t kReceiverSsrc = 0x4711;
+
+  if (receiver_first) {
+    AddRecvStream(StreamParams::CreateLegacy(kReceiverSsrc));
+    std::vector<FakeVideoReceiveStream*> receive_streams =
+        fake_call_->GetVideoReceiveStreams();
+    ASSERT_EQ(1u, receive_streams.size());
+    // Bogus local SSRC when we have no sender.
+    EXPECT_EQ(1, receive_streams[0]->GetConfig().rtp.local_ssrc);
+  }
+  AddSendStream(StreamParams::CreateLegacy(kSenderSsrc));
+  if (!receiver_first)
+    AddRecvStream(StreamParams::CreateLegacy(kReceiverSsrc));
+  std::vector<FakeVideoReceiveStream*> receive_streams =
+      fake_call_->GetVideoReceiveStreams();
+  ASSERT_EQ(1u, receive_streams.size());
+  EXPECT_EQ(kSenderSsrc, receive_streams[0]->GetConfig().rtp.local_ssrc);
+}
+
+TEST_F(WebRtcVideoChannel2Test, ConfiguresLocalSsrc) {
+  TestReceiverLocalSsrcConfiguration(false);
+}
+
+TEST_F(WebRtcVideoChannel2Test, ConfiguresLocalSsrcOnExistingReceivers) {
+  TestReceiverLocalSsrcConfiguration(true);
+}
+
 class WebRtcVideoEngine2SimulcastTest : public testing::Test {
  public:
   WebRtcVideoEngine2SimulcastTest() : engine_(nullptr) {}