From f1e97b9ebd23c12d12ffd6b18bdf3eb4951153b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Thu, 26 Sep 2019 12:48:47 +0200 Subject: [PATCH] Reland "Prepares RtpSenderVideo for batch forwarding of generated packets" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 > Commit-Queue: Erik Språng > 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 Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#29367} --- modules/rtp_rtcp/source/rtp_sender_video.cc | 142 ++++++++++---------- modules/rtp_rtcp/source/rtp_sender_video.h | 15 ++- 2 files changed, 76 insertions(+), 81 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index a0fd668fe3..37dcdf229f 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::RegisterPayloadType(int8_t payload_type, } } -void RTPSenderVideo::SendVideoPacket(std::unique_ptr 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 media_packet, - bool protect_media_packet) { - uint16_t media_seq_num = media_packet->SequenceNumber(); - + bool protect_media_packet, + std::vector>* packets) { std::unique_ptr red_packet( new RtpPacketToSend(*media_packet)); BuildRedPayload(*media_packet, red_packet.get()); @@ -327,16 +313,12 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( } } } + // 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 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( 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 media_packet, - bool protect_media_packet) { +void RTPSenderVideo::GenerateAndAppendFlexfec( + std::vector>* 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> 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 packet) { -#if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE +void RTPSenderVideo::LogAndSendToNetwork( + std::vector> 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 @@ bool RTPSenderVideo::SendVideo( } 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> rtp_packets; for (size_t i = 0; i < num_packets; ++i) { std::unique_ptr packet; int expected_payload_capacity; @@ -714,7 +706,6 @@ bool RTPSenderVideo::SendVideo( 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 @@ bool RTPSenderVideo::SendVideo( 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 @@ bool RTPSenderVideo::SendVideo( 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 65f2b488ee..1ee8e73b5a 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -132,18 +132,19 @@ class RTPSenderVideo { size_t CalculateFecPacketOverhead() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); - void SendVideoPacket(std::unique_ptr packet); - - void SendVideoPacketAsRedMaybeWithUlpfec( + void AppendAsRedMaybeWithUlpfec( std::unique_ptr media_packet, - bool protect_media_packet); + bool protect_media_packet, + std::vector>* packets); // TODO(brandtr): Remove the FlexFEC functions when FlexfecSender has been // moved to PacedSender. - void SendVideoPacketWithFlexfec(std::unique_ptr media_packet, - bool protect_media_packet); + void GenerateAndAppendFlexfec( + std::vector>* packets); - bool LogAndSendToNetwork(std::unique_ptr packet); + void LogAndSendToNetwork( + std::vector> packets, + size_t unpacketized_payload_size); bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) { return red_payload_type_ >= 0;