From 5a0763564bb2a41f63065bbe14b5f6e2d4da12a2 Mon Sep 17 00:00:00 2001
From: Evan Shrubsole <eshr@webrtc.org>
Date: Tue, 13 Dec 2022 11:20:15 +0000
Subject: [PATCH] Don't send abs capture time when capture time unset

Bug: b/217301555
Change-Id: Ibd55c4af586aa1ee19af9e35c25607b6a64de8b7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/287940
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38881}
---
 modules/rtp_rtcp/source/rtp_sender_video.cc   | 29 ++++++++++++-------
 .../source/rtp_sender_video_unittest.cc       | 18 ++++++++++++
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 3a583c684c..e1ac4e41c3 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -525,32 +525,41 @@ bool RTPSenderVideo::SendVideo(
                         (use_fec ? FecPacketOverhead() : 0) -
                         (rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0);
 
+  absl::optional<Timestamp> capture_time;
+  if (capture_time_ms > 0) {
+    capture_time = Timestamp::Millis(capture_time_ms);
+  }
+
   std::unique_ptr<RtpPacketToSend> single_packet =
       rtp_sender_->AllocatePacket();
   RTC_DCHECK_LE(packet_capacity, single_packet->capacity());
   single_packet->SetPayloadType(payload_type);
   single_packet->SetTimestamp(rtp_timestamp);
-  single_packet->set_capture_time(Timestamp::Millis(capture_time_ms));
+  if (capture_time)
+    single_packet->set_capture_time(*capture_time);
 
   // Construct the absolute capture time extension if not provided.
-  if (!video_header.absolute_capture_time.has_value()) {
+  if (!video_header.absolute_capture_time.has_value() &&
+      capture_time.has_value()) {
     video_header.absolute_capture_time.emplace();
     video_header.absolute_capture_time->absolute_capture_timestamp =
         Int64MsToUQ32x32(
-            clock_->ConvertTimestampToNtpTimeInMilliseconds(capture_time_ms));
+            clock_->ConvertTimestampToNtpTime(*capture_time).ToMs());
     if (include_capture_clock_offset_) {
       video_header.absolute_capture_time->estimated_capture_clock_offset = 0;
     }
   }
 
   // Let `absolute_capture_time_sender_` decide if the extension should be sent.
-  video_header.absolute_capture_time =
-      absolute_capture_time_sender_.OnSendPacket(
-          AbsoluteCaptureTimeSender::GetSource(single_packet->Ssrc(),
-                                               single_packet->Csrcs()),
-          single_packet->Timestamp(), kVideoPayloadTypeFrequency,
-          video_header.absolute_capture_time->absolute_capture_timestamp,
-          video_header.absolute_capture_time->estimated_capture_clock_offset);
+  if (video_header.absolute_capture_time.has_value()) {
+    video_header.absolute_capture_time =
+        absolute_capture_time_sender_.OnSendPacket(
+            AbsoluteCaptureTimeSender::GetSource(single_packet->Ssrc(),
+                                                 single_packet->Csrcs()),
+            single_packet->Timestamp(), kVideoPayloadTypeFrequency,
+            video_header.absolute_capture_time->absolute_capture_timestamp,
+            video_header.absolute_capture_time->estimated_capture_clock_offset);
+  }
 
   auto first_packet = std::make_unique<RtpPacketToSend>(*single_packet);
   auto middle_packet = std::make_unique<RtpPacketToSend>(*single_packet);
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index e2d1a73828..72dfd0238d 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -1204,6 +1204,24 @@ TEST_F(RtpSenderVideoTest, AbsoluteCaptureTime) {
       absolute_capture_time->estimated_capture_clock_offset.has_value());
 }
 
+TEST_F(RtpSenderVideoTest,
+       AbsoluteCaptureTimeNotForwardedWhenImageHasNoCaptureTime) {
+  uint8_t kFrame[kMaxPacketLength];
+  rtp_module_->RegisterRtpHeaderExtension(AbsoluteCaptureTimeExtension::Uri(),
+                                          kAbsoluteCaptureTimeExtensionId);
+
+  RTPVideoHeader hdr;
+  hdr.frame_type = VideoFrameType::kVideoFrameKey;
+  rtp_sender_video_->SendVideo(kPayload, kType, kTimestamp,
+                               /*capture_time_ms=*/0, kFrame, hdr,
+                               kDefaultExpectedRetransmissionTimeMs);
+  // No absolute capture time should be set as the capture_time_ms was the
+  // default value.
+  for (const RtpPacketReceived& packet : transport_.sent_packets()) {
+    EXPECT_FALSE(packet.HasExtension<AbsoluteCaptureTimeExtension>());
+  }
+}
+
 // Essentially the same test as AbsoluteCaptureTime but with a field trial.
 // After the field trial is experimented, we will remove AbsoluteCaptureTime.
 TEST_F(RtpSenderVideoTest, AbsoluteCaptureTimeWithCaptureClockOffset) {