Reject StreamParams with RTX SSRCs not in ssrcs.

BUG=1788, chromium:470122
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/44859004

Cr-Commit-Position: refs/heads/master@{#8855}
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index 496c439..ae14c64 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -86,6 +86,40 @@
   return true;
 }
 
+static bool ValidateStreamParams(const StreamParams& sp) {
+  if (sp.ssrcs.empty()) {
+    LOG(LS_ERROR) << "No SSRCs in stream parameters: " << sp.ToString();
+    return false;
+  }
+
+  std::vector<uint32> primary_ssrcs;
+  sp.GetPrimarySsrcs(&primary_ssrcs);
+  std::vector<uint32> rtx_ssrcs;
+  sp.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs);
+  for (uint32_t rtx_ssrc : rtx_ssrcs) {
+    bool rtx_ssrc_present = false;
+    for (uint32_t sp_ssrc : sp.ssrcs) {
+      if (sp_ssrc == rtx_ssrc) {
+        rtx_ssrc_present = true;
+        break;
+      }
+    }
+    if (!rtx_ssrc_present) {
+      LOG(LS_ERROR) << "RTX SSRC '" << rtx_ssrc
+                    << "' missing from StreamParams ssrcs: " << sp.ToString();
+      return false;
+    }
+  }
+  if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
+    LOG(LS_ERROR)
+        << "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
+        << sp.ToString();
+    return false;
+  }
+
+  return true;
+}
+
 static std::string RtpExtensionsToString(
     const std::vector<RtpHeaderExtension>& extensions) {
   std::stringstream out;
@@ -799,10 +833,8 @@
 
 bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
   LOG(LS_INFO) << "AddSendStream: " << sp.ToString();
-  if (sp.ssrcs.empty()) {
-    LOG(LS_ERROR) << "No SSRCs in stream parameters.";
+  if (!ValidateStreamParams(sp))
     return false;
-  }
 
   uint32 ssrc = sp.first_ssrc();
   assert(ssrc != 0);
@@ -814,17 +846,6 @@
     return false;
   }
 
-  std::vector<uint32> primary_ssrcs;
-  sp.GetPrimarySsrcs(&primary_ssrcs);
-  std::vector<uint32> rtx_ssrcs;
-  sp.GetFidSsrcs(primary_ssrcs, &rtx_ssrcs);
-  if (!rtx_ssrcs.empty() && primary_ssrcs.size() != rtx_ssrcs.size()) {
-    LOG(LS_ERROR)
-        << "RTX SSRCs exist, but don't cover all SSRCs (unsupported): "
-        << sp.ToString();
-    return false;
-  }
-
   WebRtcVideoSendStream* stream =
       new WebRtcVideoSendStream(call_.get(),
                                 external_encoder_factory_,
@@ -889,8 +910,10 @@
 
 bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
                                         bool default_stream) {
-  LOG(LS_INFO) << "AddRecvStream: " << sp.ToString();
-  assert(sp.ssrcs.size() > 0);
+  LOG(LS_INFO) << "AddRecvStream" << (default_stream ? " (default stream)" : "")
+               << ": " << sp.ToString();
+  if (!ValidateStreamParams(sp))
+    return false;
 
   uint32 ssrc = sp.first_ssrc();
   assert(ssrc != 0);  // TODO(pbos): Is this ever valid?
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index d7176eb..f07f281 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -2353,37 +2353,26 @@
             recv_stream->GetConfig().rtp.rtx.begin()->second.ssrc);
 }
 
+TEST_F(WebRtcVideoChannel2Test, RejectsAddingStreamsWithMissingSsrcsForRtx) {
+  EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
+
+  const std::vector<uint32> ssrcs = MAKE_VECTOR(kSsrcs1);
+  const std::vector<uint32> rtx_ssrcs = MAKE_VECTOR(kRtxSsrcs1);
+
+  StreamParams sp =
+      cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs);
+  sp.ssrcs = ssrcs;  // Without RTXs, this is the important part.
+
+  EXPECT_FALSE(channel_->AddSendStream(sp));
+  EXPECT_FALSE(channel_->AddRecvStream(sp));
+}
+
 class WebRtcVideoEngine2SimulcastTest : public testing::Test {
  public:
-  WebRtcVideoEngine2SimulcastTest()
-      : engine_(nullptr), engine_codecs_(engine_.codecs()) {
-    assert(!engine_codecs_.empty());
-
-    bool codec_set = false;
-    for (size_t i = 0; i < engine_codecs_.size(); ++i) {
-      if (engine_codecs_[i].name == "red") {
-        default_red_codec_ = engine_codecs_[i];
-      } else if (engine_codecs_[i].name == "ulpfec") {
-        default_ulpfec_codec_ = engine_codecs_[i];
-      } else if (engine_codecs_[i].name == "rtx") {
-        default_rtx_codec_ = engine_codecs_[i];
-      } else if (!codec_set) {
-        default_codec_ = engine_codecs_[i];
-        codec_set = true;
-      }
-    }
-
-    assert(codec_set);
-  }
+  WebRtcVideoEngine2SimulcastTest() : engine_(nullptr) {}
 
  protected:
   WebRtcVideoEngine2 engine_;
-  VideoCodec default_codec_;
-  VideoCodec default_red_codec_;
-  VideoCodec default_ulpfec_codec_;
-  VideoCodec default_rtx_codec_;
-  // TODO(pbos): Remove engine_codecs_ unless used a lot.
-  std::vector<VideoCodec> engine_codecs_;
 };
 
 class WebRtcVideoChannel2SimulcastTest : public WebRtcVideoEngine2SimulcastTest,