Use a pass-through pacer instead of special-cased pacerless mode

This CL removes the old non-paced code path and instead uses a helper
class to just immediately pass the packet through the same code path as
when an actual pacer is used.

Bug: webrtc:10633
Change-Id: Id9a3ee4719829ad07710f5468e5452aa4bc8570b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150530
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28963}
This commit is contained in:
Erik Språng
2019-08-26 19:00:05 +02:00
committed by Commit Bot
parent c15f92aceb
commit 1fbfecd81f
3 changed files with 69 additions and 223 deletions

View File

@ -103,6 +103,21 @@ bool HasBweExtension(const RtpHeaderExtensionMap& extensions_map) {
} // namespace
RTPSender::NonPacedPacketSender::NonPacedPacketSender(RTPSender* rtp_sender)
: transport_sequence_number_(0), rtp_sender_(rtp_sender) {}
RTPSender::NonPacedPacketSender::~NonPacedPacketSender() = default;
void RTPSender::NonPacedPacketSender::EnqueuePacket(
std::unique_ptr<RtpPacketToSend> packet) {
if (!packet->SetExtension<TransportSequenceNumber>(
++transport_sequence_number_)) {
--transport_sequence_number_;
}
packet->ReserveExtension<TransmissionOffset>();
packet->ReserveExtension<AbsoluteSendTime>();
rtp_sender_->TrySendPacket(packet.get(), PacedPacketInfo());
}
RTPSender::RTPSender(const RtpRtcp::Configuration& config)
: clock_(config.clock),
random_(clock_->TimeInMicroseconds()),
@ -110,7 +125,10 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config)
flexfec_ssrc_(config.flexfec_sender
? absl::make_optional(config.flexfec_sender->ssrc())
: absl::nullopt),
paced_sender_(config.paced_sender),
non_paced_packet_sender_(
config.paced_sender ? nullptr : new NonPacedPacketSender(this)),
paced_sender_(config.paced_sender ? config.paced_sender
: non_paced_packet_sender_.get()),
transport_sequence_number_allocator_(
config.transport_sequence_number_allocator),
transport_feedback_observer_(config.transport_feedback_callback),
@ -159,6 +177,7 @@ RTPSender::RTPSender(const RtpRtcp::Configuration& config)
// Random start, 16 bits. Can't be 0.
sequence_number_rtx_ = random_.Rand(1, kMaxInitRtpSeqNumber);
sequence_number_ = random_.Rand(1, kMaxInitRtpSeqNumber);
RTC_DCHECK(paced_sender_);
}
RTPSender::~RTPSender() {
@ -297,56 +316,34 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) {
const int32_t packet_size = static_cast<int32_t>(stored_packet->packet_size);
const bool rtx = (RtxStatus() & kRtxRetransmitted) > 0;
if (paced_sender_) {
std::unique_ptr<RtpPacketToSend> packet =
packet_history_.GetPacketAndMarkAsPending(
packet_id, [&](const RtpPacketToSend& stored_packet) {
// Check if we're overusing retransmission bitrate.
// TODO(sprang): Add histograms for nack success or failure
// reasons.
std::unique_ptr<RtpPacketToSend> retransmit_packet;
if (retransmission_rate_limiter_ &&
!retransmission_rate_limiter_->TryUseRate(packet_size)) {
return retransmit_packet;
}
if (rtx) {
retransmit_packet = BuildRtxPacket(stored_packet);
} else {
retransmit_packet =
absl::make_unique<RtpPacketToSend>(stored_packet);
}
if (retransmit_packet) {
retransmit_packet->set_retransmitted_sequence_number(
stored_packet.SequenceNumber());
}
return retransmit_packet;
});
if (!packet) {
return -1;
}
packet->set_packet_type(RtpPacketToSend::Type::kRetransmission);
paced_sender_->EnqueuePacket(std::move(packet));
return packet_size;
}
// TODO(sprang): Replace this whole code-path with a pass-through pacer.
// Check if we're overusing retransmission bitrate.
// TODO(sprang): Add histograms for nack success or failure reasons.
if (retransmission_rate_limiter_ &&
!retransmission_rate_limiter_->TryUseRate(packet_size)) {
return -1;
}
std::unique_ptr<RtpPacketToSend> packet =
packet_history_.GetPacketAndSetSendTime(packet_id);
packet_history_.GetPacketAndMarkAsPending(
packet_id, [&](const RtpPacketToSend& stored_packet) {
// Check if we're overusing retransmission bitrate.
// TODO(sprang): Add histograms for nack success or failure
// reasons.
std::unique_ptr<RtpPacketToSend> retransmit_packet;
if (retransmission_rate_limiter_ &&
!retransmission_rate_limiter_->TryUseRate(packet_size)) {
return retransmit_packet;
}
if (rtx) {
retransmit_packet = BuildRtxPacket(stored_packet);
} else {
retransmit_packet =
absl::make_unique<RtpPacketToSend>(stored_packet);
}
if (retransmit_packet) {
retransmit_packet->set_retransmitted_sequence_number(
stored_packet.SequenceNumber());
}
return retransmit_packet;
});
if (!packet) {
// Packet could theoretically time out between the first check and this one.
return 0;
}
if (!PrepareAndSendPacket(std::move(packet), rtx, true, PacedPacketInfo()))
return -1;
}
packet->set_packet_type(RtpPacketToSend::Type::kRetransmission);
paced_sender_->EnqueuePacket(std::move(packet));
return packet_size;
}
@ -528,83 +525,6 @@ bool RTPSender::SupportsRtxPayloadPadding() const {
(rtx_ & kRtxRedundantPayloads);
}
bool RTPSender::PrepareAndSendPacket(std::unique_ptr<RtpPacketToSend> packet,
bool send_over_rtx,
bool is_retransmit,
const PacedPacketInfo& pacing_info) {
RTC_DCHECK(packet);
int64_t capture_time_ms = packet->capture_time_ms();
RtpPacketToSend* packet_to_send = packet.get();
std::unique_ptr<RtpPacketToSend> packet_rtx;
if (send_over_rtx) {
packet_rtx = BuildRtxPacket(*packet);
if (!packet_rtx)
return false;
packet_to_send = packet_rtx.get();
}
// Bug webrtc:7859. While FEC is invoked from rtp_sender_video, and not after
// the pacer, these modifications of the header below are happening after the
// FEC protection packets are calculated. This will corrupt recovered packets
// at the same place. It's not an issue for extensions, which are present in
// all the packets (their content just may be incorrect on recovered packets).
// 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.
int64_t now_ms = clock_->TimeInMilliseconds();
int64_t diff_ms = now_ms - capture_time_ms;
packet_to_send->SetExtension<TransmissionOffset>(kTimestampTicksPerMs *
diff_ms);
packet_to_send->SetExtension<AbsoluteSendTime>(
AbsoluteSendTime::MsTo24Bits(now_ms));
if (packet_to_send->HasExtension<VideoTimingExtension>()) {
if (populate_network2_timestamp_) {
packet_to_send->set_network2_time_ms(now_ms);
} else {
packet_to_send->set_pacer_exit_time_ms(now_ms);
}
}
PacketOptions options;
// If we are sending over RTX, it also means this is a retransmission.
// E.g. RTPSender::TrySendRedundantPayloads calls PrepareAndSendPacket with
// send_over_rtx = true but is_retransmit = false.
options.is_retransmit = is_retransmit || send_over_rtx;
bool has_transport_seq_num;
{
rtc::CritScope lock(&send_critsect_);
has_transport_seq_num =
UpdateTransportSequenceNumber(packet_to_send, &options.packet_id);
options.included_in_allocation =
has_transport_seq_num || force_part_of_allocation_;
options.included_in_feedback = has_transport_seq_num;
}
if (has_transport_seq_num) {
AddPacketToTransportFeedback(options.packet_id, *packet_to_send,
pacing_info);
}
options.application_data.assign(packet_to_send->application_data().begin(),
packet_to_send->application_data().end());
if (!is_retransmit && !send_over_rtx) {
UpdateDelayStatistics(packet->capture_time_ms(), now_ms, packet->Ssrc());
UpdateOnSendPacket(options.packet_id, packet->capture_time_ms(),
packet->Ssrc());
}
if (!SendPacketToNetwork(*packet_to_send, options, pacing_info))
return false;
{
rtc::CritScope lock(&send_critsect_);
media_has_been_sent_ = true;
}
UpdateRtpStats(*packet_to_send, send_over_rtx, is_retransmit);
return true;
}
void RTPSender::UpdateRtpStats(const RtpPacketToSend& packet,
bool is_rtx,
bool is_retransmit) {
@ -752,77 +672,18 @@ bool RTPSender::SendToNetwork(std::unique_ptr<RtpPacketToSend> packet,
RTC_DCHECK(packet);
int64_t now_ms = clock_->TimeInMilliseconds();
uint32_t ssrc = packet->Ssrc();
if (paced_sender_) {
auto packet_type = packet->packet_type();
RTC_CHECK(packet_type) << "Packet type must be set before sending.";
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);
}
packet->set_allow_retransmission(storage ==
StorageType::kAllowRetransmission);
paced_sender_->EnqueuePacket(std::move(packet));
return true;
if (packet->capture_time_ms() <= 0) {
packet->set_capture_time_ms(now_ms);
}
PacketOptions options;
options.is_retransmit = false;
packet->set_allow_retransmission(storage ==
StorageType::kAllowRetransmission);
paced_sender_->EnqueuePacket(std::move(packet));
// |capture_time_ms| <= 0 is considered invalid.
// TODO(holmer): This should be changed all over Video Engine so that negative
// time is consider invalid, while 0 is considered a valid time.
if (packet->capture_time_ms() > 0) {
packet->SetExtension<TransmissionOffset>(
kTimestampTicksPerMs * (now_ms - packet->capture_time_ms()));
if (populate_network2_timestamp_ &&
packet->HasExtension<VideoTimingExtension>()) {
packet->set_network2_time_ms(now_ms);
}
}
packet->SetExtension<AbsoluteSendTime>(AbsoluteSendTime::MsTo24Bits(now_ms));
bool has_transport_seq_num;
{
rtc::CritScope lock(&send_critsect_);
has_transport_seq_num =
UpdateTransportSequenceNumber(packet.get(), &options.packet_id);
options.included_in_allocation =
has_transport_seq_num || force_part_of_allocation_;
options.included_in_feedback = has_transport_seq_num;
}
if (has_transport_seq_num) {
AddPacketToTransportFeedback(options.packet_id, *packet.get(),
PacedPacketInfo());
}
options.application_data.assign(packet->application_data().begin(),
packet->application_data().end());
UpdateDelayStatistics(packet->capture_time_ms(), now_ms, packet->Ssrc());
UpdateOnSendPacket(options.packet_id, packet->capture_time_ms(),
packet->Ssrc());
bool sent = SendPacketToNetwork(*packet, options, PacedPacketInfo());
if (sent) {
{
rtc::CritScope lock(&send_critsect_);
media_has_been_sent_ = true;
}
UpdateRtpStats(*packet, false, false);
}
// To support retransmissions, we store the media packet as sent in the
// packet history (even if send failed).
if (storage == kAllowRetransmission) {
RTC_DCHECK_EQ(ssrc, SSRC());
packet_history_.PutRtpPacket(std::move(packet), storage, now_ms);
}
return sent;
return true;
}
void RTPSender::RecomputeMaxSendDelay() {

View File

@ -173,10 +173,19 @@ class RTPSender {
// time.
typedef std::map<int64_t, int> SendDelayMap;
bool PrepareAndSendPacket(std::unique_ptr<RtpPacketToSend> packet,
bool send_over_rtx,
bool is_retransmit,
const PacedPacketInfo& pacing_info);
// Helper class that redirects packets directly to the send part of this class
// without passing through an actual paced sender.
class NonPacedPacketSender : public RtpPacketSender {
public:
explicit NonPacedPacketSender(RTPSender* rtp_sender);
virtual ~NonPacedPacketSender();
void EnqueuePacket(std::unique_ptr<RtpPacketToSend> packet) override;
private:
uint16_t transport_sequence_number_;
RTPSender* const rtp_sender_;
};
std::unique_ptr<RtpPacketToSend> BuildRtxPacket(
const RtpPacketToSend& packet);
@ -215,6 +224,7 @@ class RTPSender {
const absl::optional<uint32_t> flexfec_ssrc_;
const std::unique_ptr<NonPacedPacketSender> non_paced_packet_sender_;
RtpPacketSender* const paced_sender_;
TransportSequenceNumberAllocator* const transport_sequence_number_allocator_;
TransportFeedbackObserver* const transport_feedback_observer_;

View File

@ -64,7 +64,7 @@ const uint16_t kSeqNum = 33;
const uint32_t kSsrc = 725242;
const uint32_t kRtxSsrc = 12345;
const uint32_t kFlexFecSsrc = 45678;
const uint16_t kTransportSequenceNumber = 0xaabbu;
const uint16_t kTransportSequenceNumber = 1;
const uint64_t kStartTime = 123456789;
const size_t kMaxPaddingSize = 224u;
const uint8_t kPayloadData[] = {47, 11, 32, 93, 89};
@ -175,12 +175,6 @@ class MockRtpPacketPacer : public RtpPacketSender {
MOCK_METHOD1(SetAccountForAudioPackets, void(bool account_for_audio));
};
class MockTransportSequenceNumberAllocator
: public TransportSequenceNumberAllocator {
public:
MOCK_METHOD0(AllocateSequenceNumber, uint16_t());
};
class MockSendSideDelayObserver : public SendSideDelayObserver {
public:
MOCK_METHOD4(SendSideDelayUpdated, void(int, int, uint64_t, uint32_t));
@ -232,7 +226,6 @@ class RtpSenderTest : public ::testing::TestWithParam<TestConfig> {
config.local_media_ssrc = kSsrc;
config.rtx_send_ssrc = kRtxSsrc;
config.flexfec_sender = &flexfec_sender_;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@ -246,7 +239,6 @@ class RtpSenderTest : public ::testing::TestWithParam<TestConfig> {
SimulatedClock fake_clock_;
NiceMock<MockRtcEventLog> mock_rtc_event_log_;
MockRtpPacketPacer mock_paced_sender_;
StrictMock<MockTransportSequenceNumberAllocator> seq_num_allocator_;
StrictMock<MockSendPacketObserver> send_packet_observer_;
StrictMock<MockTransportFeedbackObserver> feedback_observer_;
RateLimiter retransmission_rate_limiter_;
@ -468,7 +460,6 @@ TEST_P(RtpSenderTestWithoutPacer,
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
config.local_media_ssrc = kSsrc;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.transport_feedback_callback = &feedback_observer_;
config.event_log = &mock_rtc_event_log_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@ -478,8 +469,6 @@ TEST_P(RtpSenderTestWithoutPacer,
EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId));
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(Return(kTransportSequenceNumber));
const size_t expected_bytes =
GetParam().with_overhead
@ -507,7 +496,6 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) {
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
config.local_media_ssrc = kSsrc;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.transport_feedback_callback = &feedback_observer_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
@ -518,8 +506,6 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) {
kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId));
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(Return(kTransportSequenceNumber));
EXPECT_CALL(send_packet_observer_,
OnSendPacket(kTransportSequenceNumber, _, _))
.Times(1);
@ -549,7 +535,6 @@ TEST_P(RtpSenderTestWithoutPacer, PacketOptionsNoRetransmission) {
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
config.local_media_ssrc = kSsrc;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.transport_feedback_callback = &feedback_observer_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
@ -566,8 +551,6 @@ TEST_P(RtpSenderTestWithoutPacer,
SetUpRtpSender(false, false);
rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId);
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(Return(kTransportSequenceNumber));
EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1);
SendGenericPacket();
EXPECT_TRUE(transport_.last_options_.included_in_feedback);
@ -579,8 +562,6 @@ TEST_P(
SetUpRtpSender(false, false);
rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId);
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(Return(kTransportSequenceNumber));
EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1);
SendGenericPacket();
EXPECT_TRUE(transport_.last_options_.included_in_allocation);
@ -684,8 +665,6 @@ TEST_P(RtpSenderTestWithoutPacer, OnSendPacketUpdated) {
EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionTransportSequenceNumber,
kTransportSequenceNumberExtensionId));
EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber())
.WillOnce(Return(kTransportSequenceNumber));
EXPECT_CALL(send_packet_observer_,
OnSendPacket(kTransportSequenceNumber, _, _))
.Times(1);
@ -699,7 +678,6 @@ TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) {
config.outgoing_transport = &transport_;
config.paced_sender = &mock_paced_sender_;
config.local_media_ssrc = kSsrc;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.transport_feedback_callback = &feedback_observer_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
@ -824,6 +802,7 @@ TEST_P(RtpSenderTest, WritesNetwork2ToTimingExtensionWithoutPacer) {
const VideoSendTiming kVideoTiming = {0u, 0u, 0u, 0u, 0u, 0u, true};
packet->SetExtension<VideoTimingExtension>(kVideoTiming);
EXPECT_TRUE(rtp_sender_->AssignSequenceNumber(packet.get()));
packet->set_packet_type(RtpPacketToSend::Type::kVideo);
const int kPropagateTimeMs = 10;
fake_clock_.AdvanceTimeMilliseconds(kPropagateTimeMs);
@ -1182,7 +1161,6 @@ TEST_P(RtpSenderTest, SendFlexfecPackets) {
config.paced_sender = &mock_paced_sender_;
config.local_media_ssrc = kSsrc;
config.flexfec_sender = &flexfec_sender_;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@ -1268,7 +1246,6 @@ TEST_P(RtpSenderTest, NoFlexfecForTimingFrames) {
config.outgoing_transport = &transport_;
config.paced_sender = &mock_paced_sender_;
config.flexfec_sender = &flexfec_sender;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@ -1394,7 +1371,6 @@ TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
config.outgoing_transport = &transport_;
config.local_media_ssrc = kSsrc;
config.flexfec_sender = &flexfec_sender;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
@ -1663,7 +1639,6 @@ TEST_P(RtpSenderTest, FecOverheadRate) {
config.paced_sender = &mock_paced_sender_;
config.local_media_ssrc = kSsrc;
config.flexfec_sender = &flexfec_sender;
config.transport_sequence_number_allocator = &seq_num_allocator_;
config.event_log = &mock_rtc_event_log_;
config.send_packet_observer = &send_packet_observer_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;