Recreate default receive stream when unsignaled ssrc for rtx is received

Bug: webrtc:10297
Change-Id: Ie166b778b30e1c178c765f348e38426a6d57e29f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/278682
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38378}

3 files changed, 65 insertions, 7 deletions

diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index bcefa5b2d2..e76f044eed 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -524,7 +524,8 @@ DefaultUnsignalledSsrcHandler::DefaultUnsignalledSsrcHandler()
 
 UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc(
     WebRtcVideoChannel* channel,
-    uint32_t ssrc) {
+    uint32_t ssrc,
+    absl::optional<uint32_t> rtx_ssrc) {
   absl::optional<uint32_t> default_recv_ssrc =
       channel->GetDefaultReceiveStreamSsrc();
 
@@ -536,7 +537,9 @@ UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc(
 
   StreamParams sp = channel->unsignaled_stream_params();
   sp.ssrcs.push_back(ssrc);
-
+  if (rtx_ssrc) {
+    sp.AddFidSsrc(ssrc, *rtx_ssrc);
+  }
   RTC_LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc
                    << ".";
   if (!channel->AddRecvStream(sp, /*default_stream=*/true)) {
@@ -1692,6 +1695,7 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
             break;
         }
 
+        absl::optional<uint32_t> rtx_ssrc;
         uint32_t ssrc = ParseRtpSsrc(packet);
 
         if (unknown_ssrc_packet_buffer_) {
@@ -1711,11 +1715,26 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
         // know what stream it associates with, and we shouldn't ever create an
         // implicit channel for these.
         for (auto& codec : recv_codecs_) {
-          if (payload_type == codec.rtx_payload_type ||
-              payload_type == codec.ulpfec.red_rtx_payload_type ||
+          if (payload_type == codec.ulpfec.red_rtx_payload_type ||
               payload_type == codec.ulpfec.ulpfec_payload_type) {
             return;
           }
+          if (payload_type == codec.rtx_payload_type) {
+            // As we don't support receiving simulcast there can only be one RTX
+            // stream, which will be associated with unsignaled media stream.
+            // It is not possible to update the ssrcs of a receive stream, so we
+            // recreate it insead if found.
+            auto default_ssrc = GetDefaultReceiveStreamSsrc();
+            if (!default_ssrc) {
+              return;
+            }
+            rtx_ssrc = ssrc;
+            ssrc = *default_ssrc;
+            // Allow recreating the receive stream even if the RTX packet is
+            // received just after the media packet.
+            last_unsignalled_ssrc_creation_time_ms_.reset();
+            break;
+          }
         }
         if (payload_type == recv_flexfec_payload_type_) {
           return;
@@ -1745,7 +1764,8 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
           }
         }
         // Let the unsignalled ssrc handler decide whether to drop or deliver.
-        switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc)) {
+        switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc,
+                                                             rtx_ssrc)) {
           case UnsignalledSsrcHandler::kDropPacket:
             return;
           case UnsignalledSsrcHandler::kDeliverPacket:
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index eb95fbecd0..d87a612e72 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -67,7 +67,8 @@ class UnsignalledSsrcHandler {
     kDeliverPacket,
   };
   virtual Action OnUnsignalledSsrc(WebRtcVideoChannel* channel,
-                                   uint32_t ssrc) = 0;
+                                   uint32_t ssrc,
+                                   absl::optional<uint32_t> rtx_ssrc) = 0;
   virtual ~UnsignalledSsrcHandler() = default;
 };
 
@@ -75,7 +76,9 @@ class UnsignalledSsrcHandler {
 class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler {
  public:
   DefaultUnsignalledSsrcHandler();
-  Action OnUnsignalledSsrc(WebRtcVideoChannel* channel, uint32_t ssrc) override;
+  Action OnUnsignalledSsrc(WebRtcVideoChannel* channel,
+                           uint32_t ssrc,
+                           absl::optional<uint32_t> rtx_ssrc) override;
 
   rtc::VideoSinkInterface<webrtc::VideoFrame>* GetDefaultSink() const;
   void SetDefaultSink(WebRtcVideoChannel* channel,
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index c544128571..5cddacac2d 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -6898,6 +6898,41 @@ TEST_F(WebRtcVideoChannelTest, RedRtxPacketDoesntCreateUnsignalledStream) {
                                   false /* expect_created_receive_stream */);
 }
 
+TEST_F(WebRtcVideoChannelTest,
+       RtxAfterMediaPacketRecreatesUnsignalledStream) {
+  AssignDefaultAptRtxTypes();
+  const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
+  const int payload_type = vp8.id;
+  const int rtx_vp8_payload_type = default_apt_rtx_types_[vp8.id];
+  const uint32_t ssrc = kIncomingUnsignalledSsrc;
+  const uint32_t rtx_ssrc = ssrc + 1;
+
+  // Send media packet.
+  RtpPacket packet;
+  packet.SetPayloadType(payload_type);
+  packet.SetSsrc(ssrc);
+  ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1);
+  EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
+      << "Should have created a receive stream for payload type: "
+      << payload_type;
+
+  // Send rtx packet.
+  RtpPacket rtx_packet;
+  rtx_packet.SetPayloadType(rtx_vp8_payload_type);
+  rtx_packet.SetSsrc(rtx_ssrc);
+  ReceivePacketAndAdvanceTime(rtx_packet.Buffer(), /* packet_time_us */ -1);
+  EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
+      << "RTX packet should not have added or removed a receive stream";
+
+  // Check receive stream has been recreated with correct ssrcs.
+  auto recv_stream = fake_call_->GetVideoReceiveStreams().front();
+  auto& config = recv_stream->GetConfig();
+  EXPECT_EQ(config.rtp.remote_ssrc, ssrc)
+      << "Receive stream should have correct media ssrc";
+  EXPECT_EQ(config.rtp.rtx_ssrc, rtx_ssrc)
+      << "Receive stream should have correct rtx ssrc";
+}
+
 // Test that receiving any unsignalled SSRC works even if it changes.
 // The first unsignalled SSRC received will create a default receive stream.
 // Any different unsignalled SSRC received will replace the default.