Increment RTP timestamps for padding packets

This CL makes the padding packets get their own RTP timestamps,
rather than having the same timestamp as the last sent video
packet. The purpose is to solve Issue 2611, where the overuse-
detector does not react to padding packets.

A test was implemented to verify that the padding packets do
get their own timestamps.

BUG=2611
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/3869004

git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@5125 4adac7df-926f-26a2-2b94-8c16560cd09d

3 files changed, 138 insertions, 5 deletions

diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 5c63b58e..0031a0ee 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -60,9 +60,9 @@ RTPSender::RTPSender(const int32_t id, const bool audio, Clock *clock,
       packets_sent_(0), payload_bytes_sent_(0), start_time_stamp_forced_(false),
       start_time_stamp_(0), ssrc_db_(*SSRCDatabase::GetSSRCDatabase()),
       remote_ssrc_(0), sequence_number_forced_(false), ssrc_forced_(false),
-      timestamp_(0), capture_time_ms_(0), last_packet_marker_bit_(false),
-      num_csrcs_(0), csrcs_(), include_csrcs_(true),
-      rtx_(kRtxOff), payload_type_rtx_(-1) {
+      timestamp_(0), capture_time_ms_(0), last_timestamp_time_ms_(0),
+      last_packet_marker_bit_(false), num_csrcs_(0), csrcs_(),
+      include_csrcs_(true), rtx_(kRtxOff), payload_type_rtx_(-1) {
   memset(nack_byte_count_times_, 0, sizeof(nack_byte_count_times_));
   memset(nack_byte_count_, 0, sizeof(nack_byte_count_));
   memset(csrcs_, 0, sizeof(csrcs_));
@@ -419,6 +419,7 @@ bool RTPSender::SendPaddingAccordingToBitrate(
     timestamp = start_time_stamp_ + capture_timestamp;
     timestamp_ = timestamp;
     capture_time_ms_ = capture_time_ms;
+    last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
   }
   int bytes_sent = SendPadData(payload_type, timestamp, capture_time_ms,
                                bytes, kDontRetransmit, false, false);
@@ -771,6 +772,12 @@ int RTPSender::TimeToSendPadding(int bytes) {
     payload_type = (rtx_ == kRtxOff) ? payload_type_ : payload_type_rtx_;
     timestamp = timestamp_;
     capture_time_ms = capture_time_ms_;
+    if (last_timestamp_time_ms_ > 0) {
+      timestamp +=
+          (clock_->TimeInMilliseconds() - last_timestamp_time_ms_) * 90;
+      capture_time_ms +=
+          (clock_->TimeInMilliseconds() - last_timestamp_time_ms_);
+    }
   }
   return SendPadData(payload_type, timestamp, capture_time_ms, bytes,
                      kDontStore, true, true);
@@ -941,6 +948,7 @@ int32_t RTPSender::BuildRTPheader(
     // timing.
     timestamp_++;
   }
+  last_timestamp_time_ms_ = clock_->TimeInMilliseconds();
   uint32_t sequence_number = sequence_number_++;
   capture_time_ms_ = capture_time_ms;
   last_packet_marker_bit_ = marker_bit;
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index afa57f15..6a3baccd 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -331,6 +331,7 @@ class RTPSender : public Bitrate, public RTPSenderInterface {
   uint32_t ssrc_;
   uint32_t timestamp_;
   int64_t capture_time_ms_;
+  int64_t last_timestamp_time_ms_;
   bool last_packet_marker_bit_;
   uint8_t num_csrcs_;
   uint32_t csrcs_[kRtpCsrcSize];
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index b1f1b50f..4320a0d8 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -15,6 +15,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "webrtc/modules/pacing/include/mock/mock_paced_sender.h"
+#include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h"
 #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_format_video_generic.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_header_extension.h"
@@ -52,6 +53,10 @@ uint16_t GetPayloadDataLength(const RTPHeader& rtp_header,
   return static_cast<uint16_t>(length);
 }
 
+uint64_t ConvertMsToAbsSendTime(int64_t time_ms) {
+  return 0x00fffffful & ((time_ms << 18) / 1000);
+}
+
 class LoopbackTransportTest : public webrtc::Transport {
  public:
   LoopbackTransportTest()
@@ -392,7 +397,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) {
   // Verify transmission time offset.
   EXPECT_EQ(kStoredTimeInMs * 90, rtp_header.extension.transmissionTimeOffset);
   uint64_t expected_send_time =
-      0x00fffffful & ((fake_clock_.TimeInMilliseconds() << 18) / 1000);
+      ConvertMsToAbsSendTime(fake_clock_.TimeInMilliseconds());
   EXPECT_EQ(expected_send_time, rtp_header.extension.absoluteSendTime);
 }
 
@@ -454,7 +459,126 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) {
   // Verify transmission time offset.
   EXPECT_EQ(kStoredTimeInMs * 90, rtp_header.extension.transmissionTimeOffset);
   uint64_t expected_send_time =
-      0x00fffffful & ((fake_clock_.TimeInMilliseconds() << 18) / 1000);
+      ConvertMsToAbsSendTime(fake_clock_.TimeInMilliseconds());
+  EXPECT_EQ(expected_send_time, rtp_header.extension.absoluteSendTime);
+}
+
+// This test sends 1 regular video packet, then 4 padding packets, and then
+// 1 more regular packet.
+TEST_F(RtpSenderTest, SendPadding) {
+  // Make all (non-padding) packets go to send queue.
+  EXPECT_CALL(mock_paced_sender_,
+              SendPacket(PacedSender::kNormalPriority, _, _, _, _, _)).
+                  WillRepeatedly(testing::Return(false));
+
+  uint16_t seq_num = kSeqNum;
+  uint32_t timestamp = kTimestamp;
+  rtp_sender_->SetStorePacketsStatus(true, 10);
+  int rtp_header_len = 12;
+  EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
+      kRtpExtensionTransmissionTimeOffset, kTransmissionTimeOffsetExtensionId));
+  rtp_header_len += 4;  // 4 bytes extension.
+  EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
+      kRtpExtensionAbsoluteSendTime, kAbsoluteSendTimeExtensionId));
+  rtp_header_len += 4;  // 4 bytes extension.
+  rtp_header_len += 4;  // 4 extra bytes common to all extension headers.
+
+  // Create and set up parser.
+  scoped_ptr<webrtc::RtpHeaderParser> rtp_parser(
+      webrtc::RtpHeaderParser::Create());
+  ASSERT_TRUE(rtp_parser.get() != NULL);
+  rtp_parser->RegisterRtpHeaderExtension(kRtpExtensionTransmissionTimeOffset,
+                                         kTransmissionTimeOffsetExtensionId);
+  rtp_parser->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime,
+                                         kAbsoluteSendTimeExtensionId);
+  webrtc::RTPHeader rtp_header;
+
+  rtp_sender_->SetTargetSendBitrate(300000);
+  int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
+  int32_t rtp_length = rtp_sender_->BuildRTPheader(packet_,
+                                                   kPayload,
+                                                   kMarkerBit,
+                                                   timestamp,
+                                                   capture_time_ms);
+
+  // Packet should be stored in a send bucket.
+  EXPECT_EQ(0, rtp_sender_->SendToNetwork(packet_,
+                                          0,
+                                          rtp_length,
+                                          capture_time_ms,
+                                          kAllowRetransmission,
+                                          PacedSender::kNormalPriority));
+
+  int total_packets_sent = 0;
+  EXPECT_EQ(total_packets_sent, transport_.packets_sent_);
+
+  const int kStoredTimeInMs = 100;
+  fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs);
+  rtp_sender_->TimeToSendPacket(seq_num++, capture_time_ms, false);
+  // Packet should now be sent. This test doesn't verify the regular video
+  // packet, since it is tested in another test.
+  EXPECT_EQ(++total_packets_sent, transport_.packets_sent_);
+  timestamp += 90 * kStoredTimeInMs;
+
+  // Send padding 4 times, waiting 50 ms between each.
+  for (int i = 0; i < 4; ++i) {
+    const int kPaddingPeriodMs = 50;
+    const int kPaddingBytes = 100;
+    const int kMaxPaddingLength = 224;  // Value taken from rtp_sender.cc.
+    // Padding will be forced to full packets.
+    EXPECT_EQ(kMaxPaddingLength, rtp_sender_->TimeToSendPadding(kPaddingBytes));
+
+    // Process send bucket. Padding should now be sent.
+    EXPECT_EQ(++total_packets_sent, transport_.packets_sent_);
+    EXPECT_EQ(kMaxPaddingLength + rtp_header_len,
+              transport_.last_sent_packet_len_);
+    // Parse sent packet.
+    ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_, kPaddingBytes,
+                                  &rtp_header));
+
+    // Verify sequence number and timestamp.
+    EXPECT_EQ(seq_num++, rtp_header.sequenceNumber);
+    EXPECT_EQ(timestamp, rtp_header.timestamp);
+    // Verify transmission time offset.
+    EXPECT_EQ(0, rtp_header.extension.transmissionTimeOffset);
+    uint64_t expected_send_time =
+        ConvertMsToAbsSendTime(fake_clock_.TimeInMilliseconds());
+    EXPECT_EQ(expected_send_time, rtp_header.extension.absoluteSendTime);
+    fake_clock_.AdvanceTimeMilliseconds(kPaddingPeriodMs);
+    timestamp += 90 * kPaddingPeriodMs;
+  }
+
+  // Send a regular video packet again.
+  capture_time_ms = fake_clock_.TimeInMilliseconds();
+  rtp_length = rtp_sender_->BuildRTPheader(packet_,
+                                           kPayload,
+                                           kMarkerBit,
+                                           timestamp,
+                                           capture_time_ms);
+
+  // Packet should be stored in a send bucket.
+  EXPECT_EQ(0, rtp_sender_->SendToNetwork(packet_,
+                                          0,
+                                          rtp_length,
+                                          capture_time_ms,
+                                          kAllowRetransmission,
+                                          PacedSender::kNormalPriority));
+
+  rtp_sender_->TimeToSendPacket(seq_num, capture_time_ms, false);
+  // Process send bucket.
+  EXPECT_EQ(++total_packets_sent, transport_.packets_sent_);
+  EXPECT_EQ(rtp_length, transport_.last_sent_packet_len_);
+  // Parse sent packet.
+  ASSERT_TRUE(rtp_parser->Parse(transport_.last_sent_packet_, rtp_length,
+                                &rtp_header));
+
+  // Verify sequence number and timestamp.
+  EXPECT_EQ(seq_num, rtp_header.sequenceNumber);
+  EXPECT_EQ(timestamp, rtp_header.timestamp);
+  // Verify transmission time offset. This packet is sent without delay.
+  EXPECT_EQ(0, rtp_header.extension.transmissionTimeOffset);
+  uint64_t expected_send_time =
+      ConvertMsToAbsSendTime(fake_clock_.TimeInMilliseconds());
   EXPECT_EQ(expected_send_time, rtp_header.extension.absoluteSendTime);
 }