From a09331a6038bb6191c7662680d8928940463a099 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Fri, 12 May 2023 09:36:34 +0200 Subject: [PATCH] Don't write TransmissionOffset when capture time is not set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RTX padding packets sent before media packets can legitimately have no timestamps set (they are 0). Writing the TransmissionOffset extension with capture time 0 will overflow once current time exceeds ~3 minutes. Bug: webrtc:15172 Change-Id: I4dd1f341802d45016549b330f0e08cd3a00cfa19 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305020 Reviewed-by: Erik Språng Reviewed-by: Per Kjellander Commit-Queue: Danil Chapovalov Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#40055} --- modules/rtp_rtcp/source/rtp_sender_egress.cc | 5 +++-- .../source/rtp_sender_egress_unittest.cc | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_sender_egress.cc b/modules/rtp_rtcp/source/rtp_sender_egress.cc index 22da99b786..a4aa7482f1 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress.cc @@ -208,8 +208,9 @@ void RtpSenderEgress::SendPacket(std::unique_ptr packet, // In case of VideoTimingExtension, since it's present not in every packet, // data after rtp header may be corrupted if these packets are protected by // the FEC. - TimeDelta diff = now - packet->capture_time(); - if (packet->HasExtension()) { + if (packet->HasExtension() && + packet->capture_time() > Timestamp::Zero()) { + TimeDelta diff = now - packet->capture_time(); packet->SetExtension(kTimestampTicksPerMs * diff.ms()); } if (packet->HasExtension()) { diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index 75f18354ec..a31ee37c43 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc @@ -309,6 +309,26 @@ TEST_F(RtpSenderEgressTest, EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } +TEST_F(RtpSenderEgressTest, + DoesntWriteTransmissionOffsetOnRtxPaddingBeforeMedia) { + header_extensions_.RegisterByUri(kTransmissionOffsetExtensionId, + TransmissionOffset::Uri()); + + // Prior to sending media, timestamps are 0. + std::unique_ptr padding = + BuildRtpPacket(/*marker_bit=*/true, /*capture_time_ms=*/0); + padding->set_packet_type(RtpPacketMediaType::kPadding); + padding->SetSsrc(kRtxSsrc); + EXPECT_TRUE(padding->HasExtension()); + + std::unique_ptr sender = CreateRtpSenderEgress(); + sender->SendPacket(std::move(padding), PacedPacketInfo()); + + absl::optional offset = + transport_.last_packet()->packet.GetExtension(); + EXPECT_EQ(offset, 0); +} + TEST_F(RtpSenderEgressTest, OnSendSideDelayUpdated) { StrictMock send_side_delay_observer; RtpRtcpInterface::Configuration config = DefaultConfig();