Removes rtp level keep alive support.
This is not used in practice as there's functionality on
other levels that serves the same purpose.
Bug: None
Change-Id: I0488dc42459b07607363eba0f2b06f4c50f7cda4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/125520
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27061}diff --git a/api/ortc/rtp_transport_interface.h b/api/ortc/rtp_transport_interface.h
index ec71216..ac43831 100644
--- a/api/ortc/rtp_transport_interface.h
+++ b/api/ortc/rtp_transport_interface.h
@@ -26,12 +26,8 @@
struct RtpTransportParameters final {
RtcpParameters rtcp;
- // Enabled periodic sending of keep-alive packets, that help prevent timeouts
- // on the network level, such as NAT bindings. See RFC6263 section 4.6.
- RtpKeepAliveConfig keepalive;
-
bool operator==(const RtpTransportParameters& o) const {
- return rtcp == o.rtcp && keepalive == o.keepalive;
+ return rtcp == o.rtcp;
}
bool operator!=(const RtpTransportParameters& o) const {
return !(*this == o);
diff --git a/api/rtp_headers.h b/api/rtp_headers.h
index 8ab560c..04c3853 100644
--- a/api/rtp_headers.h
+++ b/api/rtp_headers.h
@@ -171,23 +171,6 @@
kNetworkDown,
};
-struct RtpKeepAliveConfig final {
- // If no packet has been sent for |timeout_interval_ms|, send a keep-alive
- // packet. The keep-alive packet is an empty (no payload) RTP packet with a
- // payload type of 20 as long as the other end has not negotiated the use of
- // this value. If this value has already been negotiated, then some other
- // unused static payload type from table 5 of RFC 3551 shall be used and set
- // in |payload_type|.
- int64_t timeout_interval_ms = -1;
- uint8_t payload_type = 20;
-
- bool operator==(const RtpKeepAliveConfig& o) const {
- return timeout_interval_ms == o.timeout_interval_ms &&
- payload_type == o.payload_type;
- }
- bool operator!=(const RtpKeepAliveConfig& o) const { return !(*this == o); }
-};
-
} // namespace webrtc
#endif // API_RTP_HEADERS_H_
diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index 5262cc8..18a6326 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -48,7 +48,6 @@
class RateLimiter;
class RtcpBandwidthObserver;
class RtpPacketSender;
-struct RtpKeepAliveConfig;
class SendDelayStats;
class SendStatisticsProxy;
class TransportFeedbackObserver;
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 9dc3003..4446e64 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -74,7 +74,6 @@
RtcEventLog* event_log,
RateLimiter* retransmission_rate_limiter,
OverheadObserver* overhead_observer,
- RtpKeepAliveConfig keepalive_config,
FrameEncryptorInterface* frame_encryptor,
const CryptoOptions& crypto_options) {
RTC_DCHECK_GT(rtp_config.ssrcs.size(), 0);
@@ -100,7 +99,6 @@
configuration.event_log = event_log;
configuration.retransmission_rate_limiter = retransmission_rate_limiter;
configuration.overhead_observer = overhead_observer;
- configuration.keepalive_config = keepalive_config;
configuration.frame_encryptor = frame_encryptor;
configuration.require_frame_encryption =
crypto_options.sframe.require_frame_encryption;
@@ -247,8 +245,6 @@
event_log,
retransmission_limiter,
this,
- // TODO(srte): Remove this argument.
- RtpKeepAliveConfig(),
frame_encryptor,
crypto_options)),
rtp_config_(rtp_config),
diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h
index 34ccfeb..9ae6ddb 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp.h
@@ -97,8 +97,6 @@
OverheadObserver* overhead_observer = nullptr;
RtcpAckObserver* ack_observer = nullptr;
- RtpKeepAliveConfig keepalive_config;
-
int rtcp_report_interval_ms = 0;
// Update network2 instead of pacer_exit field of video timing extension.
diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
index b648942..a4e75ba 100644
--- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h
@@ -97,7 +97,7 @@
enum KeyFrameRequestMethod { kKeyFrameReqPliRtcp, kKeyFrameReqFirRtcp };
-enum RtpRtcpPacketType { kPacketRtp = 0, kPacketKeepAlive = 1 };
+enum RtpRtcpPacketType { kPacketRtp = 0 };
enum RtxMode {
kRtxOff = 0x0,
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index 6d1c9e2..9a0fc7c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -82,12 +82,10 @@
: kDefaultVideoReportInterval),
this),
clock_(configuration.clock),
- keepalive_config_(configuration.keepalive_config),
last_bitrate_process_time_(clock_->TimeInMilliseconds()),
last_rtt_process_time_(clock_->TimeInMilliseconds()),
next_process_time_(clock_->TimeInMilliseconds() +
kRtpRtcpMaxIdleTimeProcessMs),
- next_keepalive_time_(-1),
packet_overhead_(28), // IPV4 UDP.
nack_last_time_sent_full_ms_(0),
nack_last_seq_number_sent_(0),
@@ -119,11 +117,6 @@
// Make sure rtcp sender use same timestamp offset as rtp sender.
rtcp_sender_.SetTimestampOffset(rtp_sender_->TimestampOffset());
-
- if (keepalive_config_.timeout_interval_ms != -1) {
- next_keepalive_time_ =
- clock_->TimeInMilliseconds() + keepalive_config_.timeout_interval_ms;
- }
}
// Set default packet size limit.
@@ -154,20 +147,6 @@
next_process_time_ =
std::min(next_process_time_, now + kRtpRtcpBitrateProcessTimeMs);
}
- if (keepalive_config_.timeout_interval_ms > 0 &&
- now >= next_keepalive_time_) {
- int64_t last_send_time_ms = rtp_sender_->LastTimestampTimeMs();
- // If no packet has been sent, |last_send_time_ms| will be 0, and so the
- // keep-alive will be triggered as expected.
- if (now >= last_send_time_ms + keepalive_config_.timeout_interval_ms) {
- rtp_sender_->SendKeepAlive(keepalive_config_.payload_type);
- next_keepalive_time_ = now + keepalive_config_.timeout_interval_ms;
- } else {
- next_keepalive_time_ =
- last_send_time_ms + keepalive_config_.timeout_interval_ms;
- }
- next_process_time_ = std::min(next_process_time_, next_keepalive_time_);
- }
}
bool process_rtt = now >= last_rtt_process_time_ + kRtpRtcpRttProcessTimeMs;
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
index 4b1c927..3c7e4ef 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h
@@ -320,11 +320,9 @@
Clock* const clock_;
- const RtpKeepAliveConfig keepalive_config_;
int64_t last_bitrate_process_time_;
int64_t last_rtt_process_time_;
int64_t next_process_time_;
- int64_t next_keepalive_time_;
uint16_t packet_overhead_;
// Send side
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 98067b2..84db79a 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -60,9 +60,7 @@
clock_(nullptr),
delay_ms_(0),
rtp_packets_sent_(0),
- rtcp_packets_sent_(0),
- keepalive_payload_type_(0),
- num_keepalive_sent_(0) {}
+ rtcp_packets_sent_(0) {}
void SetRtpRtcpModule(ModuleRtpRtcpImpl* receiver) { receiver_ = receiver; }
void SimulateNetworkDelay(int64_t delay_ms, SimulatedClock* clock) {
@@ -76,8 +74,6 @@
std::unique_ptr<RtpHeaderParser> parser(RtpHeaderParser::Create());
EXPECT_TRUE(parser->Parse(static_cast<const uint8_t*>(data), len, &header));
++rtp_packets_sent_;
- if (header.payloadType == keepalive_payload_type_)
- ++num_keepalive_sent_;
last_rtp_header_ = header;
return true;
}
@@ -94,10 +90,6 @@
++rtcp_packets_sent_;
return true;
}
- void SetKeepalivePayloadType(uint8_t payload_type) {
- keepalive_payload_type_ = payload_type;
- }
- size_t NumKeepaliveSent() { return num_keepalive_sent_; }
size_t NumRtcpSent() { return rtcp_packets_sent_; }
ModuleRtpRtcpImpl* receiver_;
SimulatedClock* clock_;
@@ -106,8 +98,6 @@
size_t rtcp_packets_sent_;
RTPHeader last_rtp_header_;
std::vector<uint16_t> last_nack_list_;
- uint8_t keepalive_payload_type_;
- size_t num_keepalive_sent_;
};
class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
@@ -127,7 +117,6 @@
RtcpRttStatsTestImpl rtt_stats_;
std::unique_ptr<ModuleRtpRtcpImpl> impl_;
uint32_t remote_ssrc_;
- RtpKeepAliveConfig keepalive_config_;
int rtcp_report_interval_ms_ = 0;
void SetRemoteSsrc(uint32_t ssrc) {
@@ -157,12 +146,6 @@
std::vector<uint16_t> LastNackListSent() {
return transport_.last_nack_list_;
}
- void SetKeepaliveConfigAndReset(const RtpKeepAliveConfig& config) {
- keepalive_config_ = config;
- // Need to create a new module impl, since it's configured at creation.
- CreateModuleImpl();
- transport_.SetKeepalivePayloadType(config.payload_type);
- }
void SetRtcpReportIntervalAndReset(int rtcp_report_interval_ms) {
rtcp_report_interval_ms_ = rtcp_report_interval_ms;
CreateModuleImpl();
@@ -177,7 +160,6 @@
config.receive_statistics = receive_statistics_.get();
config.rtcp_packet_type_counter_observer = this;
config.rtt_stats = &rtt_stats_;
- config.keepalive_config = keepalive_config_;
config.rtcp_report_interval_ms = rtcp_report_interval_ms_;
impl_.reset(new ModuleRtpRtcpImpl(config));
@@ -569,60 +551,6 @@
EXPECT_EQ(75, sender_.RtcpReceived().UniqueNackRequestsInPercent());
}
-TEST_F(RtpRtcpImplTest, SendsKeepaliveAfterTimout) {
- const int kTimeoutMs = 1500;
-
- RtpKeepAliveConfig config;
- config.timeout_interval_ms = kTimeoutMs;
-
- // Recreate sender impl with new configuration, and redo setup.
- sender_.SetKeepaliveConfigAndReset(config);
- SetUp();
-
- // Initial process call.
- sender_.impl_->Process();
- EXPECT_EQ(0U, sender_.transport_.NumKeepaliveSent());
-
- // After one time, a single keep-alive packet should be sent.
- clock_.AdvanceTimeMilliseconds(kTimeoutMs);
- sender_.impl_->Process();
- EXPECT_EQ(1U, sender_.transport_.NumKeepaliveSent());
-
- // Process for the same timestamp again, no new packet should be sent.
- sender_.impl_->Process();
- EXPECT_EQ(1U, sender_.transport_.NumKeepaliveSent());
-
- // Move ahead to the last ms before a keep-alive is expected, no action.
- clock_.AdvanceTimeMilliseconds(kTimeoutMs - 1);
- sender_.impl_->Process();
- EXPECT_EQ(1U, sender_.transport_.NumKeepaliveSent());
-
- // Move the final ms, timeout relative last KA. Should create new keep-alive.
- clock_.AdvanceTimeMilliseconds(1);
- sender_.impl_->Process();
- EXPECT_EQ(2U, sender_.transport_.NumKeepaliveSent());
-
- // Move ahead to the last ms before Christmas.
- clock_.AdvanceTimeMilliseconds(kTimeoutMs - 1);
- sender_.impl_->Process();
- EXPECT_EQ(2U, sender_.transport_.NumKeepaliveSent());
-
- // Send actual payload data, no keep-alive expected.
- SendFrame(&sender_, sender_video_.get(), 0);
- sender_.impl_->Process();
- EXPECT_EQ(2U, sender_.transport_.NumKeepaliveSent());
-
- // Move ahead as far as possible again, timeout now relative payload. No KA.
- clock_.AdvanceTimeMilliseconds(kTimeoutMs - 1);
- sender_.impl_->Process();
- EXPECT_EQ(2U, sender_.transport_.NumKeepaliveSent());
-
- // Timeout relative payload, send new keep-alive.
- clock_.AdvanceTimeMilliseconds(1);
- sender_.impl_->Process();
- EXPECT_EQ(3U, sender_.transport_.NumKeepaliveSent());
-}
-
TEST_F(RtpRtcpImplTest, ConfigurableRtcpReportInterval) {
const int kVideoReportInterval = 3000;
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 197f52b..9f3cefc 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -1229,21 +1229,6 @@
return last_timestamp_time_ms_;
}
-void RTPSender::SendKeepAlive(uint8_t payload_type) {
- std::unique_ptr<RtpPacketToSend> packet = AllocatePacket();
- packet->SetPayloadType(payload_type);
- // Set marker bit and timestamps in the same manner as plain padding packets.
- packet->SetMarker(false);
- {
- rtc::CritScope lock(&send_critsect_);
- packet->SetTimestamp(last_rtp_timestamp_);
- packet->set_capture_time_ms(capture_time_ms_);
- }
- AssignSequenceNumber(packet.get());
- SendToNetwork(std::move(packet), StorageType::kDontRetransmit,
- RtpPacketSender::Priority::kLowPriority);
-}
-
void RTPSender::SetRtt(int64_t rtt_ms) {
packet_history_.SetRtt(rtt_ms);
flexfec_packet_history_.SetRtt(rtt_ms);
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index f08eb89..eca86ee 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -171,7 +171,6 @@
RtpState GetRtxRtpState() const;
int64_t LastTimestampTimeMs() const;
- void SendKeepAlive(uint8_t payload_type);
void SetRtt(int64_t rtt_ms);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 1c74e7e..3f3f221 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -1759,40 +1759,6 @@
SendGenericPacket();
}
-TEST_P(RtpSenderTest, SendsKeepAlive) {
- MockTransport transport;
- rtp_sender_.reset(
- new RTPSender(false, &fake_clock_, &transport, nullptr, absl::nullopt,
- nullptr, nullptr, nullptr, nullptr, &mock_rtc_event_log_,
- nullptr, &retransmission_rate_limiter_, nullptr, false,
- nullptr, false, false, FieldTrialBasedConfig()));
- rtp_sender_->SetSequenceNumber(kSeqNum);
- rtp_sender_->SetTimestampOffset(0);
- rtp_sender_->SetSSRC(kSsrc);
-
- const uint8_t kKeepalivePayloadType = 20;
- RTC_CHECK_NE(kKeepalivePayloadType, kPayload);
-
- EXPECT_CALL(transport, SendRtp(_, _, _))
- .WillOnce(
- Invoke([&kKeepalivePayloadType](const uint8_t* packet, size_t len,
- const PacketOptions& options) {
- webrtc::RTPHeader rtp_header;
- RtpUtility::RtpHeaderParser parser(packet, len);
- EXPECT_TRUE(parser.Parse(&rtp_header, nullptr));
- EXPECT_FALSE(rtp_header.markerBit);
- EXPECT_EQ(0U, rtp_header.paddingLength);
- EXPECT_EQ(kKeepalivePayloadType, rtp_header.payloadType);
- EXPECT_EQ(kSeqNum, rtp_header.sequenceNumber);
- EXPECT_EQ(kSsrc, rtp_header.ssrc);
- EXPECT_EQ(0u, len - rtp_header.headerLength);
- return true;
- }));
-
- rtp_sender_->SendKeepAlive(kKeepalivePayloadType);
- EXPECT_EQ(kSeqNum + 1, rtp_sender_->SequenceNumber());
-}
-
INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead,
RtpSenderTest,
::testing::Bool());
diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc
index 0b076f7..650015e 100644
--- a/pc/rtp_transport.cc
+++ b/pc/rtp_transport.cc
@@ -169,12 +169,6 @@
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_STATE,
"Disabling RTCP muxing is not allowed.");
}
- if (parameters.keepalive != parameters_.keepalive) {
- // TODO(sprang): Wire up support for keep-alive (only ORTC support for now).
- LOG_AND_RETURN_ERROR(
- RTCErrorType::INVALID_MODIFICATION,
- "RTP keep-alive parameters not supported by this channel.");
- }
RtpTransportParameters new_parameters = parameters;
diff --git a/pc/rtp_transport_unittest.cc b/pc/rtp_transport_unittest.cc
index f617445..54f182d 100644
--- a/pc/rtp_transport_unittest.cc
+++ b/pc/rtp_transport_unittest.cc
@@ -52,17 +52,6 @@
EXPECT_EQ(transport.GetParameters().rtcp.cname, kName);
}
-TEST(RtpTransportTest, SetRtpTransportKeepAliveNotSupported) {
- // Tests that we warn users that keep-alive isn't supported yet.
- // TODO(sprang): Wire up keep-alive and remove this test.
- RtpTransport transport(kMuxDisabled);
- RtpTransportParameters params;
- params.keepalive.timeout_interval_ms = 1;
- auto result = transport.SetParameters(params);
- EXPECT_FALSE(result.ok());
- EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
-}
-
class SignalObserver : public sigslot::has_slots<> {
public:
explicit SignalObserver(RtpTransport* transport) {
diff --git a/test/call_test.cc b/test/call_test.cc
index 44df9cb..8d8a9d3 100644
--- a/test/call_test.cc
+++ b/test/call_test.cc
@@ -705,9 +705,6 @@
const uint32_t CallTest::kReceiverLocalAudioSsrc = 0x1234567;
const int CallTest::kNackRtpHistoryMs = 1000;
-const uint8_t CallTest::kDefaultKeepalivePayloadType =
- RtpKeepAliveConfig().payload_type;
-
const std::map<uint8_t, MediaType> CallTest::payload_type_map_ = {
{CallTest::kVideoSendPayloadType, MediaType::VIDEO},
{CallTest::kFakeVideoSendPayloadType, MediaType::VIDEO},
@@ -716,8 +713,7 @@
{CallTest::kRtxRedPayloadType, MediaType::VIDEO},
{CallTest::kUlpfecPayloadType, MediaType::VIDEO},
{CallTest::kFlexfecPayloadType, MediaType::VIDEO},
- {CallTest::kAudioSendPayloadType, MediaType::AUDIO},
- {CallTest::kDefaultKeepalivePayloadType, MediaType::ANY}};
+ {CallTest::kAudioSendPayloadType, MediaType::AUDIO}};
BaseTest::BaseTest() {}
diff --git a/test/call_test.h b/test/call_test.h
index 052d4c6..dcdb03b 100644
--- a/test/call_test.h
+++ b/test/call_test.h
@@ -67,7 +67,6 @@
static const uint32_t kReceiverLocalVideoSsrc;
static const uint32_t kReceiverLocalAudioSsrc;
static const int kNackRtpHistoryMs;
- static const uint8_t kDefaultKeepalivePayloadType;
static const std::map<uint8_t, MediaType> payload_type_map_;
protected:
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index 53846bf..a84b129 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -144,7 +144,6 @@
CallStats call_stats_;
SendStatisticsProxy stats_proxy_;
PacketRouter packet_router_;
- RtpKeepAliveConfig keepalive_config_;
};
TEST_F(VideoSendStreamImplTest, RegistersAsBitrateObserverOnStart) {