Dont create RTX receive stream before media SSRC is known
The feauture was added in https://webrtc-review.googlesource.com/c/src/+/291119 in order to ensure RTX packet is part of BWE even before the RTP stream is known.
However, it cause an issue if media is signaled with an SSRC that has this RTX SSRC.
Since BWE is now notified about received packets before demuxing to the correct receive stream, it is not necessary to demux RTX packets before the media SSRC is known.
Note that WebRTC require at least one negotiated SSRC/MID before RTCP feedback can be sent. Ie, for BWE to work, at least one media SSRC must be known after this cl. It can either be unsignaled or signaled.
BWE tested with BweRampupWithInitialProbeTest.
Bug: webrtc:14795, webrtc:14817, b/320258158
Change-Id: Icf2c67bedc352720bf846b9ee38d509346af36f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/340141
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41785}
diff --git a/call/packet_receiver.h b/call/packet_receiver.h
index cdcf7bf..6d223ef 100644
--- a/call/packet_receiver.h
+++ b/call/packet_receiver.h
@@ -23,7 +23,7 @@
// Demux RTCP packets. Must be called on the worker thread.
virtual void DeliverRtcpPacket(rtc::CopyOnWriteBuffer packet) = 0;
- // Invoked once when a packet packet is received that can not be demuxed.
+ // Invoked once when a packet is received that can not be demuxed.
// If the method returns true, a new attempt is made to demux the packet.
using OnUndemuxablePacketHandler =
absl::AnyInvocable<bool(const RtpPacketReceived& parsed_packet)>;
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index a313407..a36b802 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -3117,15 +3117,12 @@
absl::optional<uint32_t> current_default_ssrc = GetUnsignaledSsrc();
if (current_default_ssrc) {
FindReceiveStream(*current_default_ssrc)->UpdateRtxSsrc(packet.Ssrc());
- } else {
- // Received unsignaled RTX packet before a media packet. Create a default
- // stream with a "random" SSRC and the RTX SSRC from the packet. The
- // stream will be recreated on the first media packet, unless we are
- // extremely lucky and used the right media SSRC.
- ReCreateDefaultReceiveStream(/*ssrc =*/14795, /*rtx_ssrc=*/packet.Ssrc());
+ return true;
}
- return true;
- } else {
+ // Default media SSRC not known yet. Drop the packet.
+ // BWE has already been notified of this received packet.
+ return false;
+ }
// Ignore unknown ssrcs if we recently created an unsignalled receive
// stream since this shouldn't happen frequently. Getting into a state
// of creating decoders on every packet eats up processing time (e.g.
@@ -3142,7 +3139,7 @@
return false;
}
}
- }
+
// RTX SSRC not yet known.
ReCreateDefaultReceiveStream(packet.Ssrc(), absl::nullopt);
last_unsignalled_ssrc_creation_time_ms_ = rtc::TimeMillis();
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 4493c9c..4bb795c 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -922,59 +922,6 @@
}
}
-TEST_F(WebRtcVideoEngineTest, SendsFeedbackAfterUnsignaledRtxPacket) {
- // Setup a channel with VP8, RTX and transport sequence number header
- // extension. Receive stream is not explicitly configured.
- AddSupportedVideoCodecType("VP8");
- std::vector<VideoCodec> supported_codecs =
- engine_.recv_codecs(/*include_rtx=*/true);
- ASSERT_EQ(supported_codecs[1].name, "rtx");
- int rtx_payload_type = supported_codecs[1].id;
- MockNetworkInterface network;
- RtcpPacketParser rtcp_parser;
- ON_CALL(network, SendRtcp)
- .WillByDefault(
- testing::DoAll(WithArg<0>([&](rtc::CopyOnWriteBuffer* packet) {
- ASSERT_TRUE(rtcp_parser.Parse(*packet));
- }),
- Return(true)));
- std::unique_ptr<VideoMediaSendChannelInterface> send_channel =
- engine_.CreateSendChannel(call_.get(), GetMediaConfig(), VideoOptions(),
- webrtc::CryptoOptions(),
- video_bitrate_allocator_factory_.get());
- std::unique_ptr<VideoMediaReceiveChannelInterface> receive_channel =
- engine_.CreateReceiveChannel(call_.get(), GetMediaConfig(),
- VideoOptions(), webrtc::CryptoOptions());
- cricket::VideoReceiverParameters parameters;
- parameters.codecs = supported_codecs;
- const int kTransportSeqExtensionId = 1;
- parameters.extensions.push_back(RtpExtension(
- RtpExtension::kTransportSequenceNumberUri, kTransportSeqExtensionId));
- ASSERT_TRUE(receive_channel->SetReceiverParameters(parameters));
- send_channel->SetInterface(&network);
- receive_channel->SetInterface(&network);
- send_channel->OnReadyToSend(true);
- receive_channel->SetReceive(true);
-
- // Inject a RTX packet.
- webrtc::RtpHeaderExtensionMap extension_map(parameters.extensions);
- webrtc::RtpPacketReceived packet(&extension_map);
- packet.SetMarker(true);
- packet.SetPayloadType(rtx_payload_type);
- packet.SetSsrc(999);
- packet.SetExtension<webrtc::TransportSequenceNumber>(7);
- uint8_t* buf_ptr = packet.AllocatePayload(11);
- memset(buf_ptr, 0, 11); // Pass MSAN (don't care about bytes 1-9)
- receive_channel->OnPacketReceived(packet);
-
- // Expect that feedback is sent after a while.
- time_controller_.AdvanceTime(webrtc::TimeDelta::Seconds(1));
- EXPECT_GT(rtcp_parser.transport_feedback()->num_packets(), 0);
-
- send_channel->SetInterface(nullptr);
- receive_channel->SetInterface(nullptr);
-}
-
TEST_F(WebRtcVideoEngineTest, ReceiveBufferSizeViaFieldTrial) {
webrtc::test::ScopedKeyValueConfig override_field_trials(
field_trials_, "WebRTC-ReceiveBufferSize/size_bytes:10000/");
@@ -7475,12 +7422,12 @@
true /* expect_created_receive_stream */);
}
-TEST_F(WebRtcVideoChannelTest, RtxPacketCreateUnsignalledStream) {
+TEST_F(WebRtcVideoChannelTest, RtxPacketDoesntCreateUnsignalledStream) {
AssignDefaultAptRtxTypes();
const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
const int rtx_vp8_payload_type = default_apt_rtx_types_[vp8.id];
TestReceiveUnsignaledSsrcPacket(rtx_vp8_payload_type,
- true /* expect_created_receive_stream */);
+ false /* expect_created_receive_stream */);
}
TEST_F(WebRtcVideoChannelTest, UlpfecPacketDoesntCreateUnsignalledStream) {
@@ -7534,8 +7481,7 @@
EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(rtx_ssrc), 1u);
}
-TEST_F(WebRtcVideoChannelTest,
- MediaPacketAfterRtxImmediatelyRecreatesUnsignalledStream) {
+TEST_F(WebRtcVideoChannelTest, UnsignaledStreamCreatedAfterMediaPacket) {
AssignDefaultAptRtxTypes();
const cricket::VideoCodec vp8 = GetEngineCodec("VP8");
const int payload_type = vp8.id;
@@ -7543,15 +7489,15 @@
const uint32_t ssrc = kIncomingUnsignalledSsrc;
const uint32_t rtx_ssrc = ssrc + 1;
- // Send rtx packet.
+ // Receive rtx packet.
RtpPacketReceived rtx_packet;
rtx_packet.SetPayloadType(rtx_vp8_payload_type);
rtx_packet.SetSsrc(rtx_ssrc);
receive_channel_->OnPacketReceived(rtx_packet);
time_controller_.AdvanceTime(TimeDelta::Zero());
- EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
+ EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
- // Send media packet.
+ // Receive media packet.
RtpPacketReceived packet;
packet.SetPayloadType(payload_type);
packet.SetSsrc(ssrc);
