diff --git a/modules/rtp_rtcp/source/rtp_packet_history.cc b/modules/rtp_rtcp/source/rtp_packet_history.cc index 0dd322916e..253e9862be 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history.cc @@ -142,9 +142,6 @@ void RtpPacketHistory::PutRtpPacket(std::unique_ptr packet, : 0)); RTC_DCHECK(it.second) << "Failed to insert packet in history."; StoredPacket& stored_packet = it.first->second; - if (stored_packet.packet_->capture_time_ms() <= 0) { - stored_packet.packet_->set_capture_time_ms(now_ms); - } if (!start_seqno_) { start_seqno_ = rtp_seq_no; diff --git a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc index 25c1f868f7..97e8bc3976 100644 --- a/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_packet_history_unittest.cc @@ -137,22 +137,6 @@ TEST_F(RtpPacketHistoryTest, GetRtpPacket) { EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms()); } -TEST_F(RtpPacketHistoryTest, NoCaptureTime) { - hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); - fake_clock_.AdvanceTimeMilliseconds(1); - int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); - std::unique_ptr packet = CreateRtpPacket(kStartSeqNum); - packet->set_capture_time_ms(-1); - rtc::CopyOnWriteBuffer buffer = packet->Buffer(); - hist_.PutRtpPacket(std::move(packet), kAllowRetransmission, absl::nullopt); - - std::unique_ptr packet_out = - hist_.GetPacketAndSetSendTime(kStartSeqNum); - EXPECT_TRUE(packet_out); - EXPECT_EQ(buffer, packet_out->Buffer()); - EXPECT_EQ(capture_time_ms, packet_out->capture_time_ms()); -} - TEST_F(RtpPacketHistoryTest, DontRetransmit) { hist_.SetStorePacketsStatus(StorageMode::kStoreAndCull, 10); int64_t capture_time_ms = fake_clock_.TimeInMilliseconds(); diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 66d6b71984..870cc2a869 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -1098,6 +1098,10 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, auto packet_type = packet->packet_type(); RTC_CHECK(packet_type) << "Packet type must be set before sending."; + if (packet->capture_time_ms() <= 0) { + packet->set_capture_time_ms(now_ms); + } + if (pacer_legacy_packet_referencing_) { // If |pacer_reference_packets_| then pacer needs to find the packet in // the history when it is time to send, so move packet there. @@ -1574,6 +1578,9 @@ std::unique_ptr RTPSender::BuildRtxPacket( // Add original application data. rtx_packet->set_application_data(packet.application_data()); + // Copy capture time so e.g. TransmissionOffset is correctly set. + rtx_packet->set_capture_time_ms(packet.capture_time_ms()); + return rtx_packet; } diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 75c418fc97..ec12780756 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -2620,6 +2620,94 @@ TEST_P(RtpSenderTest, SupportsPadding) { } } +TEST_P(RtpSenderTest, SetsCaptureTimeAndPopulatesTransmissionOffset) { + rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransmissionTimeOffset, + kTransmissionTimeOffsetExtensionId); + + rtp_sender_->SetSendingMediaStatus(true); + rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads); + rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload); + rtp_sender_->SetStorePacketsStatus(true, 10); + + const int64_t kMissingCaptureTimeMs = 0; + const uint32_t kTimestampTicksPerMs = 90; + const int64_t kOffsetMs = 10; + + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket); + + auto packet = + BuildRtpPacket(kPayload, kMarkerBit, fake_clock_.TimeInMilliseconds(), + kMissingCaptureTimeMs); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + packet->ReserveExtension(); + packet->AllocatePayload(sizeof(kPayloadData)); + EXPECT_TRUE( + rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission)); + + fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); + + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), false, + PacedPacketInfo()); + } else { + auto packet = + BuildRtpPacket(kPayload, kMarkerBit, fake_clock_.TimeInMilliseconds(), + kMissingCaptureTimeMs); + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + packet->ReserveExtension(); + packet->AllocatePayload(sizeof(kPayloadData)); + + std::unique_ptr packet_to_pace; + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .WillOnce([&](std::unique_ptr packet) { + EXPECT_GT(packet->capture_time_ms(), 0); + packet_to_pace = std::move(packet); + }); + + EXPECT_TRUE( + rtp_sender_->SendToNetwork(std::move(packet), kAllowRetransmission)); + + fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); + + rtp_sender_->TrySendPacket(packet_to_pace.get(), PacedPacketInfo()); + } + + EXPECT_EQ(1, transport_.packets_sent()); + absl::optional transmission_time_extension = + transport_.sent_packets_.back().GetExtension(); + ASSERT_TRUE(transmission_time_extension.has_value()); + EXPECT_EQ(*transmission_time_extension, kOffsetMs * kTimestampTicksPerMs); + + // Retransmit packet. The RTX packet should get the same capture time as the + // original packet, so offset is delta from original packet to now. + fake_clock_.AdvanceTimeMilliseconds(kOffsetMs); + + if (GetParam().pacer_references_packets) { + EXPECT_CALL(mock_paced_sender_, InsertPacket); + EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); + rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, + fake_clock_.TimeInMilliseconds(), true, + PacedPacketInfo()); + } else { + std::unique_ptr rtx_packet_to_pace; + EXPECT_CALL(mock_paced_sender_, EnqueuePacket) + .WillOnce([&](std::unique_ptr packet) { + EXPECT_GT(packet->capture_time_ms(), 0); + rtx_packet_to_pace = std::move(packet); + }); + + EXPECT_GT(rtp_sender_->ReSendPacket(kSeqNum), 0); + rtp_sender_->TrySendPacket(rtx_packet_to_pace.get(), PacedPacketInfo()); + } + + EXPECT_EQ(2, transport_.packets_sent()); + transmission_time_extension = + transport_.sent_packets_.back().GetExtension(); + ASSERT_TRUE(transmission_time_extension.has_value()); + EXPECT_EQ(*transmission_time_extension, 2 * kOffsetMs * kTimestampTicksPerMs); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderTest, ::testing::Values(TestConfig{false, false},