Fix issue where padding is sent before media with undefined timestamps if not abs-send-time is enabled.

This broke bandwidth estimation for calls without abs-send-time is enabled, but where RTX was.

BUG=
R=pbos@webrtc.org

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

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

5 files changed, 41 insertions, 4 deletions

diff --git a/modules/rtp_rtcp/interface/rtp_rtcp_defines.h b/modules/rtp_rtcp/interface/rtp_rtcp_defines.h
index e1bec5fc..99808439 100644
--- a/modules/rtp_rtcp/interface/rtp_rtcp_defines.h
+++ b/modules/rtp_rtcp/interface/rtp_rtcp_defines.h
@@ -198,12 +198,14 @@ struct RtpState {
         start_timestamp(0),
         timestamp(0),
         capture_time_ms(-1),
-        last_timestamp_time_ms(-1) {}
+        last_timestamp_time_ms(-1),
+        media_has_been_sent(false) {}
   uint16_t sequence_number;
   uint32_t start_timestamp;
   uint32_t timestamp;
   int64_t capture_time_ms;
   int64_t last_timestamp_time_ms;
+  bool media_has_been_sent;
 };
 
 class RtpData
diff --git a/modules/rtp_rtcp/source/rtp_header_extension.cc b/modules/rtp_rtcp/source/rtp_header_extension.cc
index 2e72d750..9a1836e1 100644
--- a/modules/rtp_rtcp/source/rtp_header_extension.cc
+++ b/modules/rtp_rtcp/source/rtp_header_extension.cc
@@ -65,6 +65,16 @@ int32_t RtpHeaderExtensionMap::Deregister(const RTPExtensionType type) {
   return 0;
 }
 
+bool RtpHeaderExtensionMap::IsRegistered(RTPExtensionType type) const {
+  std::map<uint8_t, HeaderExtension*>::const_iterator it =
+    extensionMap_.begin();
+  for (; it != extensionMap_.end(); ++it) {
+    if (it->second->type == type)
+      return true;
+  }
+  return false;
+}
+
 int32_t RtpHeaderExtensionMap::GetType(const uint8_t id,
                                        RTPExtensionType* type) const {
   assert(type);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 930778c3..e4507c34 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -413,6 +413,10 @@ TEST_F(RtpSendingTest, RoundRobinPadding) {
 TEST_F(RtpSendingTest, RoundRobinPaddingRtx) {
   // Enable RTX to allow padding to be sent prior to media.
   for (int i = 1; i < codec_.numberOfSimulcastStreams + 1; ++i) {
+    // Abs-send-time is needed to be allowed to send padding prior to media,
+    // as otherwise the timestmap used for BWE will be broken.
+    senders_[i]->RegisterSendRtpHeaderExtension(kRtpExtensionAbsoluteSendTime,
+                                                1);
     senders_[i]->SetRtxSendPayloadType(96);
     senders_[i]->SetRtxSsrc(kSenderRtxSsrc + i);
     senders_[i]->SetRTXSendStatus(kRtxRetransmitted);
diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index 0d465d93..eb88be7c 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -87,6 +87,7 @@ RTPSender::RTPSender(const int32_t id,
       timestamp_(0),
       capture_time_ms_(0),
       last_timestamp_time_ms_(0),
+      media_has_been_sent_(false),
       last_packet_marker_bit_(false),
       num_csrcs_(0),
       csrcs_(),
@@ -520,6 +521,11 @@ int RTPSender::SendPadData(int payload_type,
         ++sequence_number_;
         over_rtx = false;
       } else {
+        // Without abs-send-time a media packet must be sent before padding so
+        // that the timestamps used for estimation are correct.
+        if (!media_has_been_sent_ && !rtp_header_extension_map_.IsRegistered(
+            kRtpExtensionAbsoluteSendTime))
+          return bytes_sent;
         ssrc = ssrc_rtx_;
         sequence_number = sequence_number_rtx_;
         ++sequence_number_rtx_;
@@ -603,10 +609,13 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id, uint32_t min_resend_time) {
       return length;
     }
   }
-
-  CriticalSectionScoped lock(send_critsect_);
+  int rtx = kRtxOff;
+  {
+    CriticalSectionScoped lock(send_critsect_);
+    rtx = rtx_;
+  }
   return PrepareAndSendPacket(data_buffer, length, capture_time_ms,
-                              (rtx_ & kRtxRetransmitted) > 0, true) ?
+                              (rtx & kRtxRetransmitted) > 0, true) ?
       length : -1;
 }
 
@@ -799,6 +808,10 @@ bool RTPSender::PrepareAndSendPacket(uint8_t* buffer,
                                diff_ms);
   UpdateAbsoluteSendTime(buffer_to_send_ptr, length, rtp_header, now_ms);
   bool ret = SendPacketToNetwork(buffer_to_send_ptr, length);
+  if (ret) {
+    CriticalSectionScoped lock(send_critsect_);
+    media_has_been_sent_ = true;
+  }
   UpdateRtpStats(buffer_to_send_ptr, length, rtp_header, send_over_rtx,
                  is_retransmit);
   return ret;
@@ -936,6 +949,11 @@ int32_t RTPSender::SendToNetwork(
   uint32_t length = payload_length + rtp_header_length;
   if (!SendPacketToNetwork(buffer, length))
     return -1;
+  assert(payload_length - rtp_header.paddingLength > 0);
+  {
+    CriticalSectionScoped lock(send_critsect_);
+    media_has_been_sent_ = true;
+  }
   UpdateRtpStats(buffer, length, rtp_header, false, false);
   return 0;
 }
@@ -1670,6 +1688,7 @@ void RTPSender::SetRtpState(const RtpState& rtp_state) {
   timestamp_ = rtp_state.timestamp;
   capture_time_ms_ = rtp_state.capture_time_ms;
   last_timestamp_time_ms_ = rtp_state.last_timestamp_time_ms;
+  media_has_been_sent_ = rtp_state.media_has_been_sent;
 }
 
 RtpState RTPSender::GetRtpState() const {
@@ -1681,6 +1700,7 @@ RtpState RTPSender::GetRtpState() const {
   state.timestamp = timestamp_;
   state.capture_time_ms = capture_time_ms_;
   state.last_timestamp_time_ms = last_timestamp_time_ms_;
+  state.media_has_been_sent = media_has_been_sent_;
 
   return state;
 }
diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h
index 198c6103..f3a2bdc1 100644
--- a/modules/rtp_rtcp/source/rtp_sender.h
+++ b/modules/rtp_rtcp/source/rtp_sender.h
@@ -387,6 +387,7 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer {
   uint32_t timestamp_ GUARDED_BY(send_critsect_);
   int64_t capture_time_ms_ GUARDED_BY(send_critsect_);
   int64_t last_timestamp_time_ms_ GUARDED_BY(send_critsect_);
+  bool media_has_been_sent_ GUARDED_BY(send_critsect_);
   bool last_packet_marker_bit_ GUARDED_BY(send_critsect_);
   uint8_t num_csrcs_ GUARDED_BY(send_critsect_);
   uint32_t csrcs_[kRtpCsrcSize] GUARDED_BY(send_critsect_);