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 rtx_ssrc) { absl::optional 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 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 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 rtx_ssrc) override; rtc::VideoSinkInterface* 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.