Add sender controlled playout delay limits

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

Index: webrtc/modules/rtp_rtcp/source/rtp_sender.cc
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index cda776bf22b2cdcef3510233fe94acda11286aa4..d87fc69a940e1645e6d1b5dc4a7c903360c4e9aa 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -22,6 +22,7 @@
 #include "webrtc/call/rtc_event_log.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_cvo.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
+#include "webrtc/modules/rtp_rtcp/source/playout_delay_oracle.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_sender_audio.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_sender_video.h"
 #include "webrtc/modules/rtp_rtcp/source/time_util.h"
@@ -491,10 +492,12 @@ int32_t RTPSender::SendOutgoingData(FrameType frame_type,
                                     const RTPFragmentationHeader* fragmentation,
                                     const RTPVideoHeader* rtp_hdr) {
   uint32_t ssrc;
+  uint16_t sequence_number;
   {
     // Drop this packet if we're not sending media packets.
     rtc::CritScope lock(&send_critsect_);
     ssrc = ssrc_;
+    sequence_number = sequence_number_;
     if (!sending_media_) {
       return 0;
     }
@@ -523,10 +526,13 @@ int32_t RTPSender::SendOutgoingData(FrameType frame_type,
     if (frame_type == kEmptyFrame)
       return 0;
 
-    ret_val =
-        video_->SendVideo(video_type, frame_type, payload_type,
-                          capture_timestamp, capture_time_ms, payload_data,
-                          payload_size, fragmentation, rtp_hdr);
+    if (rtp_hdr)

[stefan-webrtc, 2016/05/28 05:01:44]

Add {} since this is a multi-line if (>2 lines).

[Irfan, 2016/06/01 08:38:33]

Done.

+      playout_delay_oracle_.UpdateRequest(ssrc, rtp_hdr->playout_delay,
+                                          sequence_number);
+
+    ret_val = video_->SendVideo(
+        video_type, frame_type, payload_type, capture_timestamp,
+        capture_time_ms, payload_data, payload_size, fragmentation, rtp_hdr);
   }
 
   rtc::CritScope cs(&statistics_crit_);
@@ -821,6 +827,12 @@ void RTPSender::OnReceivedNACK(const std::list<uint16_t>& nack_sequence_numbers,
   }
 }
 
+void RTPSender::OnReceivedRtcpReceiverReport(
+    const ReportBlockList& report_blocks) {
+  rtc::CritScope lock(&send_critsect_);

[stefan-webrtc, 2016/05/28 05:01:44]

Do you really have to lock this? Had you not already protected the internals of
the oracle?

[Irfan, 2016/06/01 08:38:33]

Good catch. Fixed.

+  playout_delay_oracle_.OnReceivedRtcpReceiverReport(report_blocks);
+}
+
 bool RTPSender::ProcessNACKBitRate(uint32_t now) {
   uint32_t num = 0;
   size_t byte_count = 0;
@@ -1034,6 +1046,11 @@ int32_t RTPSender::SendToNetwork(uint8_t* buffer,
 
   UpdateAbsoluteSendTime(buffer, length, rtp_header, now_ms);
 
+  if (playout_delay_oracle_.send_playout_delay())

[stefan-webrtc, 2016/05/28 05:01:44]

{}

[Irfan, 2016/06/01 08:38:33]

Done.

+    UpdatePlayoutDelayLimits(buffer, length, rtp_header,
+                             playout_delay_oracle_.min_playout_delay_ms(),
+                             playout_delay_oracle_.max_playout_delay_ms());

[stefan-webrtc, 2016/05/28 05:01:44]

Are you sure you have to update here? Can the values change after you did Build?

[Irfan, 2016/06/01 08:38:33]

This is indeed redundant. removed.

+
   // Used for NACK and to spread out the transmission of packets.
   if (packet_history_.PutRTPPacket(buffer, length, capture_time_ms, storage) !=
       0) {
@@ -1270,6 +1287,13 @@ uint16_t RTPSender::BuildRTPHeaderExtension(uint8_t* data_buffer,
         block_length = BuildTransportSequenceNumberExtension(
             extension_data, transport_sequence_number_);
         break;
+      case kRtpExtensionPlayoutDelay:
+        if (playout_delay_oracle_.send_playout_delay()) {
+          block_length = BuildPlayoutDelayExtension(
+              extension_data, playout_delay_oracle_.min_playout_delay_ms(),
+              playout_delay_oracle_.max_playout_delay_ms());
+        }
+        break;
       default:
         assert(false);
     }
@@ -1445,6 +1469,36 @@ uint8_t RTPSender::BuildTransportSequenceNumberExtension(
   return kTransportSequenceNumberLength;
 }
 
+uint8_t RTPSender::BuildPlayoutDelayExtension(
+    uint8_t* data_buffer,
+    uint16_t min_playout_delay_ms,
+    uint16_t max_playout_delay_ms) const {
+  RTC_DCHECK_LE(min_playout_delay_ms, kPlayoutDelayMaxMs);
+  RTC_DCHECK_LE(max_playout_delay_ms, kPlayoutDelayMaxMs);

[stefan-webrtc, 2016/05/28 05:01:44]

Maybe also check that min <= max?

[Irfan, 2016/06/01 08:38:33]

Done.

+  //   0                   1                   2                   3
+  //   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+  //  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+  //  |  ID   | len=2 |   MIN delay           |   MAX delay           |
+  //  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+  uint8_t id;
+  if (rtp_header_extension_map_.GetId(kRtpExtensionPlayoutDelay, &id) != 0) {
+    // Not registered.
+    return 0;
+  }
+  size_t pos = 0;
+  const uint8_t len = 2;
+  // Convert MS to value to be sent on extension header.
+  uint16_t min_playout = min_playout_delay_ms / kPlayoutDelayGranularityMs;
+  uint16_t max_playout = max_playout_delay_ms / kPlayoutDelayGranularityMs;
+
+  data_buffer[pos++] = (id << 4) + len;
+  data_buffer[pos++] = min_playout >> 4;
+  data_buffer[pos++] = ((min_playout & 0xf) << 4) | (max_playout >> 8);
+  data_buffer[pos++] = max_playout & 0xff;

[stefan-webrtc, 2016/05/28 05:01:44]

I think you can use ByteWriter for this?

[sprang, 2016/05/30 12:13:15]

That's mostly useful for integers with full byte sizes, not 12 bit ones...
Maybe rtc::BitBufferWriter could be used though?

[Irfan, 2016/06/01 08:38:33]

The current code is explicit about how bytes are filled up alongside the
illustration. Not sure the overhead of BitBufferWriter helps. Let me know if you
feel strongly.

+  assert(pos == kPlayoutDelayLength);
+  return kPlayoutDelayLength;
+}
+
 bool RTPSender::FindHeaderExtensionPosition(RTPExtensionType type,
                                             const uint8_t* rtp_packet,
                                             size_t rtp_packet_length,
@@ -1638,6 +1692,33 @@ bool RTPSender::UpdateTransportSequenceNumber(
   return true;
 }
 
+void RTPSender::UpdatePlayoutDelayLimits(uint8_t* rtp_packet,
+                                         size_t rtp_packet_length,
+                                         const RTPHeader& rtp_header,
+                                         uint16_t min_playout_delay_ms,
+                                         uint16_t max_playout_delay_ms) const {
+  size_t offset;
+  rtc::CritScope lock(&send_critsect_);
+
+  switch (VerifyExtension(kRtpExtensionPlayoutDelay, rtp_packet,
+                          rtp_packet_length, rtp_header, kPlayoutDelayLength,
+                          &offset)) {
+    case ExtensionStatus::kNotRegistered:
+      return;
+    case ExtensionStatus::kError:
+      LOG(LS_WARNING) << "Failed to update playout delay limits";
+      return;
+    case ExtensionStatus::kOk:
+      break;
+    default:
+      RTC_NOTREACHED();
+  }
+
+  BuildPlayoutDelayExtension(rtp_packet + offset, min_playout_delay_ms,
+                             max_playout_delay_ms);
+  return;
+}
+
 bool RTPSender::AllocateTransportSequenceNumber(int* packet_id) const {
   if (!transport_sequence_number_allocator_)
     return false;