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}

api/ortc/rtp_transport_interface.h

@@ -26,12 +26,8 @@ class RtpTransportAdapter;
 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);

api/rtp_headers.h

@@ -171,23 +171,6 @@ enum NetworkState {
   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_

call/rtp_transport_controller_send_interface.h

@@ -48,7 +48,6 @@ class RtpVideoSenderInterface;
 class RateLimiter;
 class RtcpBandwidthObserver;
 class RtpPacketSender;
-struct RtpKeepAliveConfig;
 class SendDelayStats;
 class SendStatisticsProxy;
 class TransportFeedbackObserver;

call/rtp_video_sender.cc

@@ -74,7 +74,6 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
     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 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
   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 @@ RtpVideoSender::RtpVideoSender(
                                           event_log,
                                           retransmission_limiter,
                                           this,
-                                          // TODO(srte): Remove this argument.
-                                          RtpKeepAliveConfig(),
                                           frame_encryptor,
                                           crypto_options)),
       rtp_config_(rtp_config),

modules/rtp_rtcp/include/rtp_rtcp.h

@@ -97,8 +97,6 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
     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.

modules/rtp_rtcp/include/rtp_rtcp_defines.h

@@ -97,7 +97,7 @@ enum RTCPPacketType : uint32_t {
 
 enum KeyFrameRequestMethod { kKeyFrameReqPliRtcp, kKeyFrameReqFirRtcp };
 
-enum RtpRtcpPacketType { kPacketRtp = 0, kPacketKeepAlive = 1 };
+enum RtpRtcpPacketType { kPacketRtp = 0 };
 
 enum RtxMode {
   kRtxOff = 0x0,

modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -82,12 +82,10 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
                                                 : 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 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
 
     // 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 @@ void ModuleRtpRtcpImpl::Process() {
       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;

modules/rtp_rtcp/source/rtp_rtcp_impl.h

@@ -320,11 +320,9 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp {
 
   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

modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc

@@ -60,9 +60,7 @@ class SendTransport : public Transport {
         clock_(nullptr),
         delay_ms_(0),
         rtp_packets_sent_(0),
-        rtcp_packets_sent_(0),
+        rtcp_packets_sent_(0) {}
-        keepalive_payload_type_(0),
-        num_keepalive_sent_(0) {}
 
   void SetRtpRtcpModule(ModuleRtpRtcpImpl* receiver) { receiver_ = receiver; }
   void SimulateNetworkDelay(int64_t delay_ms, SimulatedClock* clock) {
@@ -76,8 +74,6 @@ class SendTransport : public Transport {
     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 @@ class SendTransport : public Transport {
     ++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 @@ class SendTransport : public Transport {
   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 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
   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 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
   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 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
     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 @@ TEST_F(RtpRtcpImplTest, UniqueNackRequests) {
   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;
 

modules/rtp_rtcp/source/rtp_sender.cc

@@ -1229,21 +1229,6 @@ int64_t RTPSender::LastTimestampTimeMs() const {
   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);

modules/rtp_rtcp/source/rtp_sender.h

@@ -171,7 +171,6 @@ class RTPSender {
   RtpState GetRtxRtpState() const;
 
   int64_t LastTimestampTimeMs() const;
-  void SendKeepAlive(uint8_t payload_type);
 
   void SetRtt(int64_t rtt_ms);
 

modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -1759,40 +1759,6 @@ TEST_P(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) {
   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());

pc/rtp_transport.cc

@@ -169,12 +169,6 @@ RTCError RtpTransport::SetParameters(const RtpTransportParameters& parameters) {
     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;
 

pc/rtp_transport_unittest.cc

@@ -52,17 +52,6 @@ TEST(RtpTransportTest, SetRtcpParametersEmptyCnameUsesExisting) {
   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) {

test/call_test.cc

@@ -705,9 +705,6 @@ const uint32_t CallTest::kReceiverLocalVideoSsrc = 0x123456;
 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 @@ const std::map<uint8_t, MediaType> CallTest::payload_type_map_ = {
     {CallTest::kRtxRedPayloadType, MediaType::VIDEO},
     {CallTest::kUlpfecPayloadType, MediaType::VIDEO},
     {CallTest::kFlexfecPayloadType, MediaType::VIDEO},
-    {CallTest::kAudioSendPayloadType, MediaType::AUDIO},
+    {CallTest::kAudioSendPayloadType, MediaType::AUDIO}};
-    {CallTest::kDefaultKeepalivePayloadType, MediaType::ANY}};
 
 BaseTest::BaseTest() {}
 

test/call_test.h

@@ -67,7 +67,6 @@ class CallTest : public ::testing::Test {
   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:

video/video_send_stream_impl_unittest.cc

@@ -144,7 +144,6 @@ class VideoSendStreamImplTest : public ::testing::Test {
   CallStats call_stats_;
   SendStatisticsProxy stats_proxy_;
   PacketRouter packet_router_;
-  RtpKeepAliveConfig keepalive_config_;
 };
 
 TEST_F(VideoSendStreamImplTest, RegistersAsBitrateObserverOnStart) {