diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 2ec6b482d3..84f756c000 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -47,6 +47,7 @@ namespace cricket { namespace { const int kMinLayerSize = 16; +constexpr int64_t kUnsignaledSsrcCooldownMs = rtc::kNumMillisecsPerSec / 2; const char* StreamTypeToString( webrtc::VideoSendStream::StreamStats::StreamType type) { @@ -1565,6 +1566,7 @@ void WebRtcVideoChannel::ResetUnsignaledRecvStream() { 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 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, 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 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, 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 e8125e12a0..cd6fbe53fa 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -588,6 +588,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, // 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 last_unsignalled_ssrc_creation_time_ms_ + RTC_GUARDED_BY(thread_checker_); std::set send_ssrcs_ RTC_GUARDED_BY(thread_checker_); std::set 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 4805e9792e..336e1570b3 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 kSsrcs3[] = {1, 2, 3}; 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 @@ class WebRtcVideoChannelTest : public WebRtcVideoEngineTest { 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 @@ TEST_F(WebRtcVideoChannelTest, DefaultReceiveStreamReconfiguresToUseRtx) { 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 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) { 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 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) { // 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { 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 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { 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 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { 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 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { 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 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { 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 @@ TEST_F(WebRtcVideoChannelTest, BaseMinimumPlayoutDelayMsUnsignaledRecvStream) { 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 @@ void WebRtcVideoChannelTest::TestReceiveUnsignaledSsrcPacket( 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 @@ TEST_F(WebRtcVideoChannelTest, ReceiveDifferentUnsignaledSsrc) { 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 @@ TEST_F(WebRtcVideoChannelTest, ReceiveDifferentUnsignaledSsrc) { 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 @@ TEST_F(WebRtcVideoChannelTest, ReceiveDifferentUnsignaledSsrc) { 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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 @@ TEST_F(WebRtcVideoChannelTest, 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();