Reland "Prepares RtpSenderVideo for batch forwarding of generated packets"
This is a reland of a21d50c1f3eab29fd9026cc67c8cb4017efda5e3
Original change's description:
> Prepares RtpSenderVideo for batch forwarding of generated packets
>
> In order to reduce contention, this CL avoids taking locks per packet
> and prepares for forwarding all packets for a frame in one call, rather
> than one at a time. This will especially reduce contention in the paced
> sender during very high packet rates.
>
> Bug: webrtc:10809
> Change-Id: Ifc5fe3759b76a2a45f418b69d29c329e876f96d0
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154358
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#29323}
Bug: webrtc:10809
Change-Id: I50e0a27eb3b0b1afa39f250febdd564e1e1f06eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/155362
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29367}
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index a0fd668..37dcdf2 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -267,24 +267,10 @@
}
}
-void RTPSenderVideo::SendVideoPacket(std::unique_ptr<RtpPacketToSend> packet) {
- // Remember some values about the packet before sending it away.
- size_t packet_size = packet->size();
- uint16_t seq_num = packet->SequenceNumber();
- packet->set_packet_type(RtpPacketToSend::Type::kVideo);
- if (!LogAndSendToNetwork(std::move(packet))) {
- RTC_LOG(LS_WARNING) << "Failed to send video packet " << seq_num;
- return;
- }
- rtc::CritScope cs(&stats_crit_);
- video_bitrate_.Update(packet_size, clock_->TimeInMilliseconds());
-}
-
-void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec(
+void RTPSenderVideo::AppendAsRedMaybeWithUlpfec(
std::unique_ptr<RtpPacketToSend> media_packet,
- bool protect_media_packet) {
- uint16_t media_seq_num = media_packet->SequenceNumber();
-
+ bool protect_media_packet,
+ std::vector<std::unique_ptr<RtpPacketToSend>>* packets) {
std::unique_ptr<RtpPacketToSend> red_packet(
new RtpPacketToSend(*media_packet));
BuildRedPayload(*media_packet, red_packet.get());
@@ -327,16 +313,12 @@
}
}
}
+
// Send |red_packet| instead of |packet| for allocated sequence number.
- size_t red_packet_size = red_packet->size();
red_packet->set_packet_type(RtpPacketToSend::Type::kVideo);
red_packet->set_allow_retransmission(media_packet->allow_retransmission());
- if (LogAndSendToNetwork(std::move(red_packet))) {
- rtc::CritScope cs(&stats_crit_);
- video_bitrate_.Update(red_packet_size, clock_->TimeInMilliseconds());
- } else {
- RTC_LOG(LS_WARNING) << "Failed to send RED packet " << media_seq_num;
- }
+ packets->emplace_back(std::move(red_packet));
+
for (const auto& fec_packet : fec_packets) {
// TODO(danilchap): Make ulpfec_generator_ generate RtpPacketToSend to avoid
// reparsing them.
@@ -345,61 +327,71 @@
RTC_CHECK(rtp_packet->Parse(fec_packet->data(), fec_packet->length()));
rtp_packet->set_capture_time_ms(media_packet->capture_time_ms());
rtp_packet->set_packet_type(RtpPacketToSend::Type::kForwardErrorCorrection);
- uint16_t fec_sequence_number = rtp_packet->SequenceNumber();
rtp_packet->set_allow_retransmission(false);
- if (LogAndSendToNetwork(std::move(rtp_packet))) {
- rtc::CritScope cs(&stats_crit_);
- fec_bitrate_.Update(fec_packet->length(), clock_->TimeInMilliseconds());
- } else {
- RTC_LOG(LS_WARNING) << "Failed to send ULPFEC packet "
- << fec_sequence_number;
- }
+ RTC_DCHECK_EQ(fec_packet->length(), rtp_packet->size());
+ packets->emplace_back(std::move(rtp_packet));
}
}
-void RTPSenderVideo::SendVideoPacketWithFlexfec(
- std::unique_ptr<RtpPacketToSend> media_packet,
- bool protect_media_packet) {
+void RTPSenderVideo::GenerateAndAppendFlexfec(
+ std::vector<std::unique_ptr<RtpPacketToSend>>* packets) {
RTC_DCHECK(flexfec_sender_);
- if (protect_media_packet)
- flexfec_sender_->AddRtpPacketAndGenerateFec(*media_packet);
-
- SendVideoPacket(std::move(media_packet));
-
if (flexfec_sender_->FecAvailable()) {
std::vector<std::unique_ptr<RtpPacketToSend>> fec_packets =
flexfec_sender_->GetFecPackets();
for (auto& fec_packet : fec_packets) {
- size_t packet_length = fec_packet->size();
- uint16_t seq_num = fec_packet->SequenceNumber();
fec_packet->set_packet_type(
RtpPacketToSend::Type::kForwardErrorCorrection);
fec_packet->set_allow_retransmission(false);
- if (LogAndSendToNetwork(std::move(fec_packet))) {
- rtc::CritScope cs(&stats_crit_);
- fec_bitrate_.Update(packet_length, clock_->TimeInMilliseconds());
- } else {
- RTC_LOG(LS_WARNING) << "Failed to send FlexFEC packet " << seq_num;
- }
+ packets->emplace_back(std::move(fec_packet));
}
}
}
-bool RTPSenderVideo::LogAndSendToNetwork(
- std::unique_ptr<RtpPacketToSend> packet) {
-#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE
+void RTPSenderVideo::LogAndSendToNetwork(
+ std::vector<std::unique_ptr<RtpPacketToSend>> packets,
+ size_t unpacketized_payload_size) {
int64_t now_ms = clock_->TimeInMilliseconds();
- BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms,
- rtp_sender_->ActualSendBitrateKbit(),
- packet->Ssrc());
- BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms,
- FecOverheadRate() / 1000, packet->Ssrc());
- BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoNackBitrate_kbps", now_ms,
- rtp_sender_->NackOverheadRate() / 1000,
- packet->Ssrc());
+#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE
+ for (const auto& packet : packets) {
+ const uint32_t ssrc = packet->Ssrc();
+ BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms,
+ rtp_sender_->ActualSendBitrateKbit(), ssrc);
+ BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoFecBitrate_kbps", now_ms,
+ FecOverheadRate() / 1000, ssrc);
+ BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoNackBitrate_kbps", now_ms,
+ rtp_sender_->NackOverheadRate() / 1000,
+ ssrc);
+ }
#endif
- return rtp_sender_->SendToNetwork(std::move(packet));
+
+ {
+ rtc::CritScope cs(&stats_crit_);
+ size_t packetized_payload_size = 0;
+ for (const auto& packet : packets) {
+ switch (*packet->packet_type()) {
+ case RtpPacketToSend::Type::kVideo:
+ video_bitrate_.Update(packet->size(), now_ms);
+ packetized_payload_size += packet->payload_size();
+ break;
+ case RtpPacketToSend::Type::kForwardErrorCorrection:
+ fec_bitrate_.Update(packet->size(), clock_->TimeInMilliseconds());
+ break;
+ default:
+ continue;
+ }
+ }
+ RTC_DCHECK_GE(packetized_payload_size, unpacketized_payload_size);
+ packetization_overhead_bitrate_.Update(
+ packetized_payload_size - unpacketized_payload_size,
+ clock_->TimeInMilliseconds());
+ }
+
+ // TODO(sprang): Replace with bulk send method.
+ for (auto& packet : packets) {
+ rtp_sender_->SendToNetwork(std::move(packet));
+ }
}
void RTPSenderVideo::SetUlpfecConfig(int red_payload_type,
@@ -681,13 +673,13 @@
} else {
unpacketized_payload_size = payload_size;
}
- size_t packetized_payload_size = 0;
if (num_packets == 0)
return false;
uint16_t first_sequence_number;
bool first_frame = first_frame_sent_();
+ std::vector<std::unique_ptr<RtpPacketToSend>> rtp_packets;
for (size_t i = 0; i < num_packets; ++i) {
std::unique_ptr<RtpPacketToSend> packet;
int expected_payload_capacity;
@@ -714,7 +706,6 @@
RTC_DCHECK_LE(packet->payload_size(), expected_payload_capacity);
if (!rtp_sender_->AssignSequenceNumber(packet.get()))
return false;
- packetized_payload_size += packet->payload_size();
if (rtp_sequence_number_map_ && i == 0) {
first_sequence_number = packet->SequenceNumber();
@@ -741,14 +732,21 @@
protect_packet = false;
}
- if (flexfec_enabled()) {
- // TODO(brandtr): Remove the FlexFEC code path when FlexfecSender
- // is wired up to PacedSender instead.
- SendVideoPacketWithFlexfec(std::move(packet), protect_packet);
- } else if (red_enabled) {
- SendVideoPacketAsRedMaybeWithUlpfec(std::move(packet), protect_packet);
+ if (red_enabled) {
+ AppendAsRedMaybeWithUlpfec(std::move(packet), protect_packet,
+ &rtp_packets);
} else {
- SendVideoPacket(std::move(packet));
+ packet->set_packet_type(RtpPacketToSend::Type::kVideo);
+ const RtpPacketToSend& media_packet = *packet;
+ rtp_packets.emplace_back(std::move(packet));
+ if (flexfec_enabled()) {
+ // TODO(brandtr): Remove the FlexFEC code path when FlexfecSender
+ // is wired up to PacedSender instead.
+ if (protect_packet) {
+ flexfec_sender_->AddRtpPacketAndGenerateFec(media_packet);
+ }
+ GenerateAndAppendFlexfec(&rtp_packets);
+ }
}
if (first_frame) {
@@ -770,11 +768,7 @@
timestamp);
}
- rtc::CritScope cs(&stats_crit_);
- RTC_DCHECK_GE(packetized_payload_size, unpacketized_payload_size);
- packetization_overhead_bitrate_.Update(
- packetized_payload_size - unpacketized_payload_size,
- clock_->TimeInMilliseconds());
+ LogAndSendToNetwork(std::move(rtp_packets), unpacketized_payload_size);
TRACE_EVENT_ASYNC_END1("webrtc", "Video", capture_time_ms, "timestamp",
rtp_timestamp);
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index 65f2b48..1ee8e73 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -132,18 +132,19 @@
size_t CalculateFecPacketOverhead() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_);
- void SendVideoPacket(std::unique_ptr<RtpPacketToSend> packet);
-
- void SendVideoPacketAsRedMaybeWithUlpfec(
+ void AppendAsRedMaybeWithUlpfec(
std::unique_ptr<RtpPacketToSend> media_packet,
- bool protect_media_packet);
+ bool protect_media_packet,
+ std::vector<std::unique_ptr<RtpPacketToSend>>* packets);
// TODO(brandtr): Remove the FlexFEC functions when FlexfecSender has been
// moved to PacedSender.
- void SendVideoPacketWithFlexfec(std::unique_ptr<RtpPacketToSend> media_packet,
- bool protect_media_packet);
+ void GenerateAndAppendFlexfec(
+ std::vector<std::unique_ptr<RtpPacketToSend>>* packets);
- bool LogAndSendToNetwork(std::unique_ptr<RtpPacketToSend> packet);
+ void LogAndSendToNetwork(
+ std::vector<std::unique_ptr<RtpPacketToSend>> packets,
+ size_t unpacketized_payload_size);
bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) {
return red_payload_type_ >= 0;
