commit b9b0890541f245c5ab672c530e44776e7dbaef87
author Taylor Brandstetter <deadbeef@webrtc.org> Fri Sep 10 01:34:06 2021
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 10 21:28:23 2021
tree b0c68e01005a4f0b27ec394c9331323c5cf69b73
parent 3a9640a5aa939b137fde2f39baeed07245c85647

Fix error adding receive stream after packet received from non-primary SSRC.

The non-primary SSRC being RTX, for example. Normally a default stream
wouldn't be created from RTX packets, but there is a window of time
where packets can be received before the video engine has receive
parameters/payload type mappings, so it creates one anyway.

Then in AddRecvStream, normally the default stream would be destroyed
before creating a new one, but this only happens for sp.first_ssrc().
Resulting in the error "Receive stream with SSRC 'X' already exists".

Fixed by simply iterating over all SSRCs.

Bug: webrtc:13171
Change-Id: Iaf4e4a3ceafddee3d9b2d1e24af68be56f9695de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231633
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34971}

media/engine/webrtc_video_engine.cc

diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index a630b45..a20b37e 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1452,18 +1452,18 @@
   if (!ValidateStreamParams(sp))
     return false;
 
-  uint32_t ssrc = sp.first_ssrc();
-
-  // Remove running stream if this was a default stream.
-  const auto& prev_stream = receive_streams_.find(ssrc);
-  if (prev_stream != receive_streams_.end()) {
-    if (default_stream || !prev_stream->second->IsDefaultStream()) {
-      RTC_LOG(LS_ERROR) << "Receive stream for SSRC '" << ssrc
-                        << "' already exists.";
-      return false;
+  for (uint32_t ssrc : sp.ssrcs) {
+    // Remove running stream if this was a default stream.
+    const auto& prev_stream = receive_streams_.find(ssrc);
+    if (prev_stream != receive_streams_.end()) {
+      if (default_stream || !prev_stream->second->IsDefaultStream()) {
+        RTC_LOG(LS_ERROR) << "Receive stream for SSRC '" << ssrc
+                          << "' already exists.";
+        return false;
+      }
+      DeleteReceiveStream(prev_stream->second);
+      receive_streams_.erase(prev_stream);
     }
-    DeleteReceiveStream(prev_stream->second);
-    receive_streams_.erase(prev_stream);
   }
 
   if (!ValidateReceiveSsrcAvailability(sp))
@@ -1486,7 +1486,7 @@
   if (unsignaled_frame_transformer_ && !config.frame_transformer)
     config.frame_transformer = unsignaled_frame_transformer_;
 
-  receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
+  receive_streams_[sp.first_ssrc()] = new WebRtcVideoReceiveStream(
       this, call_, sp, std::move(config), default_stream, recv_codecs_,
       flexfec_config);
 

media/engine/webrtc_video_engine_unittest.cc

diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index be431cb..96598f5 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -8623,6 +8623,24 @@
   EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
 }
 
+// Test that if a default stream is created for a non-primary stream (for
+// example, RTX before we know it's RTX), we are still able to explicitly add
+// the stream later.
+TEST_F(WebRtcVideoChannelTest,
+       AddReceiveStreamAfterReceivingNonPrimaryUnsignaledSsrc) {
+  // Receive VP8 RTX packet.
+  RtpPacket rtp_packet;
+  const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
+  rtp_packet.SetPayloadType(default_apt_rtx_types_[vp8.id]);
+  rtp_packet.SetSsrc(2);
+  ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
+  EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
+
+  cricket::StreamParams params = cricket::StreamParams::CreateLegacy(1);
+  params.AddFidSsrc(1, 2);
+  EXPECT_TRUE(channel_->AddRecvStream(params));
+}
+
 void WebRtcVideoChannelTest::TestReceiverLocalSsrcConfiguration(
     bool receiver_first) {
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));