From 8b47ea459e849e2d9d13c2792a8fafe7fc726f64 Mon Sep 17 00:00:00 2001
From: Dan Tan <dwtan@google.com>
Date: Wed, 23 Nov 2022 01:02:02 -0800
Subject: [PATCH] Fixed timestamp_offset for RtpSenderEgress during
 initialization and SetRtpState call
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The constructor and SetRtpState calls for ModuleRtpRtcpImpl2 class fail to propagate the RTP timestamp offset of RtpSender class to RtpSenderEgress class. This results in wrong RTP timestamps being propagated in LossNotification messages.

Change-Id: I1d293289a4815de29d9dd15208bb7fd1a682be82
Bug: webrtc:14719
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/284824
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Dan Tan <dwtan@google.com>
Cr-Commit-Position: refs/heads/main@{#38768}
---
 modules/rtp_rtcp/source/rtp_rtcp_impl2.cc     |  3 ++
 .../source/rtp_rtcp_impl2_unittest.cc         | 30 +++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
index 4329a423cb..ecc05ca9d0 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2.cc
@@ -89,6 +89,8 @@ ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
     // Make sure rtcp sender use same timestamp offset as rtp sender.
     rtcp_sender_.SetTimestampOffset(
         rtp_sender_->packet_generator.TimestampOffset());
+    rtp_sender_->packet_sender.SetTimestampOffset(
+        rtp_sender_->packet_generator.TimestampOffset());
   }
 
   // Set default packet size limit.
@@ -186,6 +188,7 @@ void ModuleRtpRtcpImpl2::SetRtpState(const RtpState& rtp_state) {
   rtp_sender_->packet_generator.SetRtpState(rtp_state);
   rtp_sender_->sequencer.SetRtpState(rtp_state);
   rtcp_sender_.SetTimestampOffset(rtp_state.start_timestamp);
+  rtp_sender_->packet_sender.SetTimestampOffset(rtp_state.start_timestamp);
 }
 
 void ModuleRtpRtcpImpl2::SetRtxState(const RtpState& rtp_state) {
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
index 4dece662ec..f5c6311ded 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -709,6 +709,36 @@ TEST_P(RtpRtcpImpl2Test, ConfigurableRtcpReportInterval) {
   EXPECT_EQ(sender_.transport_.NumRtcpSent(), 2u);
 }
 
+TEST_P(RtpRtcpImpl2Test, RtpSenderEgressTimestampOffset) {
+  // RTP timestamp offset not explicitly set, default to random value.
+  uint16_t seqno = sender_.impl_->GetRtpState().sequence_number;
+  uint32_t media_rtp_ts = 1001;
+  uint32_t rtp_ts = media_rtp_ts + sender_.impl_->StartTimestamp();
+  EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid, rtp_ts,
+                        /*capture_time_ms=*/0));
+  AdvanceTime(kOneWayNetworkDelay);
+  EXPECT_THAT(
+      sender_.impl_->GetSentRtpPacketInfos(std::vector<uint16_t>{seqno}),
+      ElementsAre(Field(&RtpSequenceNumberMap::Info::timestamp, media_rtp_ts)));
+
+  RtpState saved_rtp_state = sender_.impl_->GetRtpState();
+
+  // Change RTP timestamp offset.
+  sender_.impl_->SetStartTimestamp(2000);
+
+  // Restores RtpState and make sure the old timestamp offset is in place.
+  sender_.impl_->SetRtpState(saved_rtp_state);
+  seqno = sender_.impl_->GetRtpState().sequence_number;
+  media_rtp_ts = 1031;
+  rtp_ts = media_rtp_ts + sender_.impl_->StartTimestamp();
+  EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid, rtp_ts,
+                        /*capture_time_ms=*/0));
+  AdvanceTime(kOneWayNetworkDelay);
+  EXPECT_THAT(
+      sender_.impl_->GetSentRtpPacketInfos(std::vector<uint16_t>{seqno}),
+      ElementsAre(Field(&RtpSequenceNumberMap::Info::timestamp, media_rtp_ts)));
+}
+
 TEST_P(RtpRtcpImpl2Test, StoresPacketInfoForSentPackets) {
   const uint32_t kStartTimestamp = 1u;
   SetUp();