Add small cooldown to unsignalled ssrc stream creation.
This CL adds a cooldown of 0.5 seconds where if the WebRtcVideoChannel
created an unsignalled receive stream within that amount of time, if we
receive even more unknown ssrcs we simply drop those RTP packets.
This prevents getting into a state of spawning new decoders on every
single packet which could happen e.g. if PT based demuxing is enabled
and MIDs are missing from the packets.
Bug: webrtc:12815
Change-Id: Id7675fb0cbfbc72281dcfe030d1a35629df3eb9f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221520
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34263}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 2ec6b48..84f756c 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -47,6 +47,7 @@
namespace {
const int kMinLayerSize = 16;
+constexpr int64_t kUnsignaledSsrcCooldownMs = rtc::kNumMillisecsPerSec / 2;
const char* StreamTypeToString(
webrtc::VideoSendStream::StreamStats::StreamType type) {
@@ -1565,6 +1566,7 @@
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_LOG(LS_INFO) << "ResetUnsignaledRecvStream.";
unsignaled_stream_params_ = StreamParams();
+ last_unsignalled_ssrc_creation_time_ms_ = absl::nullopt;
// Delete any created default streams. This is needed to avoid SSRC collisions
// in Call's RtpDemuxer, in the case that |this| has created a default video
@@ -1767,7 +1769,23 @@
if (demuxer_criteria_id_ != demuxer_criteria_completed_id_) {
return;
}
-
+ // 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.
+ // https://crbug.com/1069603) and this cooldown prevents that.
+ if (last_unsignalled_ssrc_creation_time_ms_.has_value()) {
+ int64_t now_ms = rtc::TimeMillis();
+ if (now_ms - last_unsignalled_ssrc_creation_time_ms_.value() <
+ kUnsignaledSsrcCooldownMs) {
+ // We've already created an unsignalled ssrc stream within the last
+ // 0.5 s, ignore with a warning.
+ RTC_LOG(LS_WARNING)
+ << "Another unsignalled ssrc packet arrived shortly after the "
+ << "creation of an unsignalled ssrc stream. Dropping packet.";
+ return;
+ }
+ }
+ // Let the unsignalled ssrc handler decide whether to drop or deliver.
switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc)) {
case UnsignalledSsrcHandler::kDropPacket:
return;
@@ -1780,6 +1798,7 @@
webrtc::PacketReceiver::DELIVERY_OK) {
RTC_LOG(LS_WARNING) << "Failed to deliver RTP packet on re-delivery.";
}
+ last_unsignalled_ssrc_creation_time_ms_ = rtc::TimeMillis();
}));
}
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index e8125e1..cd6fbe5 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -588,6 +588,8 @@
// is a risk of receiving ssrcs for other, recently added m= sections.
uint32_t demuxer_criteria_id_ RTC_GUARDED_BY(thread_checker_) = 0;
uint32_t demuxer_criteria_completed_id_ RTC_GUARDED_BY(thread_checker_) = 0;
+ absl::optional<int64_t> last_unsignalled_ssrc_creation_time_ms_
+ RTC_GUARDED_BY(thread_checker_);
std::set<uint32_t> send_ssrcs_ RTC_GUARDED_BY(thread_checker_);
std::set<uint32_t> receive_ssrcs_ RTC_GUARDED_BY(thread_checker_);
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 4805e97..336e157 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -97,6 +97,7 @@
static const uint32_t kRtxSsrcs1[] = {4};
static const uint32_t kFlexfecSsrc = 5;
static const uint32_t kIncomingUnsignalledSsrc = 0xC0FFEE;
+static const int64_t kUnsignalledReceiveStreamCooldownMs = 500;
constexpr uint32_t kRtpHeaderSize = 12;
@@ -2565,6 +2566,16 @@
cricket::VideoCodec DefaultCodec() { return GetEngineCodec("VP8"); }
+ // After receciving and processing the packet, enough time is advanced that
+ // the unsignalled receive stream cooldown is no longer in effect.
+ void ReceivePacketAndAdvanceTime(rtc::CopyOnWriteBuffer packet,
+ int64_t packet_time_us) {
+ channel_->OnPacketReceived(packet, packet_time_us);
+ rtc::Thread::Current()->ProcessMessages(0);
+ fake_clock_.AdvanceTime(
+ webrtc::TimeDelta::Millis(kUnsignalledReceiveStreamCooldownMs));
+ }
+
protected:
FakeVideoSendStream* AddSendStream() {
return AddSendStream(StreamParams::CreateLegacy(++last_ssrc_));
@@ -6297,8 +6308,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], ssrcs[0]);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
<< "No default receive stream created.";
@@ -6459,8 +6469,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// The stream should now be created with the appropriate sync label.
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
@@ -6475,16 +6484,14 @@
// Until the demuxer criteria has been updated, we ignore in-flight ssrcs of
// the recently removed unsignaled receive stream.
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
// After the demuxer criteria has been updated, we should proceed to create
// unsignalled receive streams. This time when a default video receive stream
// is created it won't have a sync_group.
channel_->OnDemuxerCriteriaUpdateComplete();
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
EXPECT_TRUE(
fake_call_->GetVideoReceiveStreams()[0]->GetConfig().sync_group.empty());
@@ -6501,8 +6508,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// Default receive stream created.
const auto& receivers1 = fake_call_->GetVideoReceiveStreams();
@@ -6552,7 +6558,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc1);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
{
// Receive a packet for kSsrc2.
@@ -6561,9 +6567,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc2);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
// No unsignaled ssrc for kSsrc2 should have been created, but kSsrc1 should
// arrive since it already has a stream.
@@ -6585,7 +6590,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc1);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
{
// Receive a packet for kSsrc2.
@@ -6594,9 +6599,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc2);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
// An unsignalled ssrc for kSsrc2 should be created and the packet counter
// should increase for both ssrcs.
@@ -6637,7 +6641,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc1);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
{
// Receive a packet for kSsrc2.
@@ -6646,9 +6650,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc2);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
// No unsignaled ssrc for kSsrc1 should have been created, but the packet
// count for kSsrc2 should increase.
@@ -6669,7 +6672,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc1);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
{
// Receive a packet for kSsrc2.
@@ -6678,9 +6681,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc2);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
// An unsignalled ssrc for kSsrc1 should be created and the packet counter
// should increase for both ssrcs.
@@ -6717,9 +6719,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 1u);
// Signal that the demuxer knows about the first update: the removal.
@@ -6734,9 +6735,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u);
// Remove the kSsrc again while previous demuxer updates are still pending.
@@ -6752,9 +6752,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 0u);
EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u);
@@ -6770,9 +6769,8 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
}
- rtc::Thread::Current()->ProcessMessages(0);
EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 0u);
EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u);
@@ -6788,11 +6786,72 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
+ }
+ EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 3u);
+}
+
+TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) {
+ const uint32_t kSsrc1 = 1;
+ const uint32_t kSsrc2 = 2;
+
+ // Send packets for kSsrc1, creating an unsignalled receive stream.
+ {
+ // Receive a packet for kSsrc1.
+ const size_t kDataLength = 12;
+ uint8_t data[kDataLength];
+ memset(data, 0, sizeof(data));
+ rtc::SetBE32(&data[8], kSsrc1);
+ rtc::CopyOnWriteBuffer packet(data, kDataLength);
channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
}
rtc::Thread::Current()->ProcessMessages(0);
+ fake_clock_.AdvanceTime(
+ webrtc::TimeDelta::Millis(kUnsignalledReceiveStreamCooldownMs - 1));
+
+ // We now have an unsignalled receive stream for kSsrc1.
EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 1u);
- EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 3u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc1), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc2), 0u);
+
+ {
+ // Receive a packet for kSsrc2.
+ const size_t kDataLength = 12;
+ uint8_t data[kDataLength];
+ memset(data, 0, sizeof(data));
+ rtc::SetBE32(&data[8], kSsrc2);
+ rtc::CopyOnWriteBuffer packet(data, kDataLength);
+ channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ }
+ rtc::Thread::Current()->ProcessMessages(0);
+
+ // Not enough time has passed to replace the unsignalled receive stream, so
+ // the kSsrc2 should be ignored.
+ EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc1), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc2), 0u);
+
+ // After 500 ms, kSsrc2 should trigger a new unsignalled receive stream that
+ // replaces the old one.
+ fake_clock_.AdvanceTime(webrtc::TimeDelta::Millis(1));
+ {
+ // Receive a packet for kSsrc2.
+ const size_t kDataLength = 12;
+ uint8_t data[kDataLength];
+ memset(data, 0, sizeof(data));
+ rtc::SetBE32(&data[8], kSsrc2);
+ rtc::CopyOnWriteBuffer packet(data, kDataLength);
+ channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
+ }
+ rtc::Thread::Current()->ProcessMessages(0);
+
+ // The old unsignalled receive stream was destroyed and replaced, so we still
+ // only have one unsignalled receive stream. But tha packet counter for kSsrc2
+ // has now increased.
+ EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc1), 1u);
+ EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc2), 1u);
}
// Test BaseMinimumPlayoutDelayMs on receive streams.
@@ -6828,8 +6887,7 @@
memset(data, 0, sizeof(data));
rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
recv_stream = fake_call_->GetVideoReceiveStream(kIncomingUnsignalledSsrc);
EXPECT_EQ(recv_stream->base_mininum_playout_delay_ms(), 200);
@@ -6866,8 +6924,7 @@
rtc::Set8(data, 1, payload_type);
rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc);
rtc::CopyOnWriteBuffer packet(data, kDataLength);
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
if (expect_created_receive_stream) {
EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
@@ -6954,8 +7011,7 @@
rtpHeader.ssrc = kIncomingUnsignalledSsrc + 1;
cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// VP8 packet should create default receive stream.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
FakeVideoReceiveStream* recv_stream = fake_call_->GetVideoReceiveStreams()[0];
@@ -6976,8 +7032,7 @@
rtpHeader.ssrc = kIncomingUnsignalledSsrc + 2;
cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
rtc::CopyOnWriteBuffer packet2(data, sizeof(data));
- channel_->OnPacketReceived(packet2, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet2, /* packet_time_us */ -1);
// VP9 packet should replace the default receive SSRC.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
recv_stream = fake_call_->GetVideoReceiveStreams()[0];
@@ -6999,8 +7054,7 @@
rtpHeader.ssrc = kIncomingUnsignalledSsrc + 3;
cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
rtc::CopyOnWriteBuffer packet3(data, sizeof(data));
- channel_->OnPacketReceived(packet3, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet3, /* packet_time_us */ -1);
// H264 packet should replace the default receive SSRC.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
recv_stream = fake_call_->GetVideoReceiveStreams()[0];
@@ -7039,8 +7093,7 @@
rtp_header.ssrc = kSsrcs3[0];
cricket::SetRtpHeader(data, sizeof(data), rtp_header);
rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// Default receive stream should be created.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
FakeVideoReceiveStream* recv_stream0 =
@@ -7058,8 +7111,7 @@
rtp_header.ssrc = kSsrcs3[1];
cricket::SetRtpHeader(data, sizeof(data), rtp_header);
packet.SetData(data, sizeof(data));
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// New default receive stream should be created, but old stream should remain.
ASSERT_EQ(2u, fake_call_->GetVideoReceiveStreams().size());
EXPECT_EQ(recv_stream0, fake_call_->GetVideoReceiveStreams()[0]);
@@ -8672,8 +8724,7 @@
rtpHeader.ssrc = kIncomingUnsignalledSsrc;
cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- channel_->OnPacketReceived(packet, /* packet_time_us */ -1);
- rtc::Thread::Current()->ProcessMessages(0);
+ ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
// The |ssrc| member should still be unset.
rtp_parameters = channel_->GetDefaultRtpReceiveParameters();
