zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issue with TransmissionOffset using new pacer code path

Browse Source 
This CL fixes two issues related to the TransmissionOffset header
extension and the new (not yet active) pacer mode.

Previously capture time (if unset) would be populated when put into the
packet history before entering the pacer. Since the pacer now owns the
packets, this does not occur until packet is actually sent, if at all.
Capture has really nothing to do with the packet history, this should
be set by the RtpSender pre-pacing instead.

Furthermore, for retransmissions the old path would take the capture
time from the original packet, build the RTX-wrapped retransmission and
set the toffset extension of the RTX packet using that captured capture
time. Since RTX packets are now fully built before the pacer, this does
not work, and we need to transfer the capture time from the original to
the RTX packet instead.

Bug: webrtc:10633
Change-Id: I031e8b6cc4ab20fb094dbd46720829b78951e7f9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146218
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28657}
This commit is contained in:
Erik Språng
2019-07-24 10:47:20 +02:00
committed by Commit Bot
parent 54d9602fe1
commit a57711c941
4 changed files with 95 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
modules/rtp_rtcp/source/rtp_packet_history.cc
View File

@ -142,9 +142,6 @@ void RtpPacketHistory::PutRtpPacket(std::unique_ptr<RtpPacketToSend> 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;

16
modules/rtp_rtcp/source/rtp_packet_history_unittest.cc
View File

@ -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<RtpPacketToSend> 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<RtpPacketToSend> 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();

7
modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -1098,6 +1098,10 @@ bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> 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<RtpPacketToSend> 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;
}

88
modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -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<TransmissionOffset>();
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<TransmissionOffset>();
packet->AllocatePayload(sizeof(kPayloadData));
std::unique_ptr<RtpPacketToSend> packet_to_pace;
EXPECT_CALL(mock_paced_sender_, EnqueuePacket)
.WillOnce([&](std::unique_ptr<RtpPacketToSend> 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<int32_t> transmission_time_extension =
transport_.sent_packets_.back().GetExtension<TransmissionOffset>();
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<RtpPacketToSend> rtx_packet_to_pace;
EXPECT_CALL(mock_paced_sender_, EnqueuePacket)
.WillOnce([&](std::unique_ptr<RtpPacketToSend> 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<TransmissionOffset>();
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},