From af52b6811621b1b4481e3d973b4b520c0442dd8a Mon Sep 17 00:00:00 2001
From: Danil Chapovalov <danilchap@webrtc.org>
Date: Tue, 27 Nov 2018 10:48:27 +0100
Subject: [PATCH] Populate VideoSendTime extension network2 field when
 configured

before this CL it was only configured when pacer is used.
This CL sets it also when pacer is not used.

Move block for setting TransmissionOffset/AbsoluteTime extensions after pacer_ check
to stress in pacer case there are set(overwritten) in another function.

Bug: None
Change-Id: I06a6dd6ec689a25439a75b3baa71340535cd1ff8
Reviewed-on: https://webrtc-review.googlesource.com/c/112126
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25794}
---
 modules/rtp_rtcp/source/rtp_sender.cc         | 23 +++++++++------
 .../rtp_rtcp/source/rtp_sender_unittest.cc    | 29 +++++++++++++++++--
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc
index e80538bba6..ddf91f5f5a 100644
--- a/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/modules/rtp_rtcp/source/rtp_sender.cc
@@ -919,15 +919,6 @@ bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
   RTC_DCHECK(packet);
   int64_t now_ms = clock_->TimeInMilliseconds();
 
-  // |capture_time_ms| <= 0 is considered invalid.
-  // TODO(holmer): This should be changed all over Video Engine so that negative
-  // time is consider invalid, while 0 is considered a valid time.
-  if (packet->capture_time_ms() > 0) {
-    packet->SetExtension<TransmissionOffset>(
-        kTimestampTicksPerMs * (now_ms - packet->capture_time_ms()));
-  }
-  packet->SetExtension<AbsoluteSendTime>(AbsoluteSendTime::MsTo24Bits(now_ms));
-
   if (video_) {
     BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms,
                                     ActualSendBitrateKbit(), packet->Ssrc());
@@ -971,6 +962,20 @@ bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
   PacketOptions options;
   options.is_retransmit = false;
 
+  // |capture_time_ms| <= 0 is considered invalid.
+  // TODO(holmer): This should be changed all over Video Engine so that negative
+  // time is consider invalid, while 0 is considered a valid time.
+  if (packet->capture_time_ms() > 0) {
+    packet->SetExtension<TransmissionOffset>(
+        kTimestampTicksPerMs * (now_ms - packet->capture_time_ms()));
+
+    if (populate_network2_timestamp_ &&
+        packet->HasExtension<VideoTimingExtension>()) {
+      packet->set_network2_time_ms(now_ms);
+    }
+  }
+  packet->SetExtension<AbsoluteSendTime>(AbsoluteSendTime::MsTo24Bits(now_ms));
+
   bool has_transport_seq_num;
   {
     rtc::CritScope lock(&send_critsect_);
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index f30b383e49..a687bcbb35 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -690,8 +690,8 @@ TEST_P(RtpSenderTest, WritesPacerExitToTimingExtension) {
   EXPECT_EQ(kStoredTimeInMs, video_timing.pacer_exit_delta_ms);
 }
 
-TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtension) {
-  SetUpRtpSender(true, true);
+TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithPacer) {
+  SetUpRtpSender(/*pacer=*/true, /*populate_network2=*/true);
   rtp_sender_->SetStorePacketsStatus(true, 10);
   EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
                    kRtpExtensionVideoTiming, kVideoTimingExtensionId));
@@ -729,6 +729,31 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtension) {
   EXPECT_EQ(kPacerExitMs, video_timing.pacer_exit_delta_ms);
 }
 
+TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithoutPacer) {
+  SetUpRtpSender(/*pacer=*/false, /*populate_network2=*/true);
+  EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
+                   kRtpExtensionVideoTiming, kVideoTimingExtensionId));
+  auto packet = rtp_sender_->AllocatePacket();
+  packet->SetMarker(true);
+  packet->set_capture_time_ms(fake_clock_.TimeInMilliseconds());
+  const VideoSendTiming kVideoTiming = {0u, 0u, 0u, 0u, 0u, 0u, true};
+  packet->SetExtension<VideoTimingExtension>(kVideoTiming);
+  EXPECT_TRUE(rtp_sender_->AssignSequenceNumber(packet.get()));
+
+  const int kPropagateTimeMs = 10;
+  fake_clock_.AdvanceTimeMilliseconds(kPropagateTimeMs);
+
+  EXPECT_TRUE(rtp_sender_->SendToNetwork(std::move(packet),
+                                         kAllowRetransmission,
+                                         RtpPacketSender::kNormalPriority));
+
+  EXPECT_EQ(1, transport_.packets_sent());
+  absl::optional<VideoSendTiming> video_timing =
+      transport_.last_sent_packet().GetExtension<VideoTimingExtension>();
+  ASSERT_TRUE(video_timing);
+  EXPECT_EQ(kPropagateTimeMs, video_timing->network2_timestamp_delta_ms);
+}
+
 TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) {
   EXPECT_CALL(mock_paced_sender_, InsertPacket(RtpPacketSender::kNormalPriority,
                                                kSsrc, kSeqNum, _, _, _));