From 13eb7645fdc8b02380419ea958f4fb99d07f79f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 24 Jun 2019 10:58:48 +0200 Subject: [PATCH] Move towards always using packet type instead of priority in RTPSender MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:10633 Change-Id: I835686f58f9edcf0c7cec8f0b3d54eb93f2920df Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143176 Commit-Queue: Erik Språng Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#28349} --- modules/rtp_rtcp/source/rtp_sender.cc | 52 +++++++++++++++++++-- modules/rtp_rtcp/source/rtp_sender.h | 4 ++ modules/rtp_rtcp/source/rtp_sender_audio.cc | 15 +++--- modules/rtp_rtcp/source/rtp_sender_audio.h | 3 +- modules/rtp_rtcp/source/rtp_sender_video.cc | 21 ++++----- modules/rtp_rtcp/source/rtp_sender_video.h | 3 +- 6 files changed, 71 insertions(+), 27 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index 68a8e21600..ae811a252a 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -87,6 +87,42 @@ constexpr RtpExtensionSize kVideoExtensionSizes[] = { RtpGenericFrameDescriptorExtension01::kMaxSizeBytes}, }; +// TODO(bugs.webrtc.org/10633): Remove when downstream code stops using +// priority. At the time of writing, the priority can be directly mapped to a +// packet type. This is only for a transition period. +RtpPacketToSend::Type PacketPriorityToType(RtpPacketSender::Priority priority) { + switch (priority) { + case RtpPacketSender::Priority::kLowPriority: + return RtpPacketToSend::Type::kVideo; + case RtpPacketSender::Priority::kNormalPriority: + return RtpPacketToSend::Type::kRetransmission; + case RtpPacketSender::Priority::kHighPriority: + return RtpPacketToSend::Type::kAudio; + default: + RTC_NOTREACHED() << "Unexpected priority: " << priority; + return RtpPacketToSend::Type::kVideo; + } +} + +// TODO(bugs.webrtc.org/10633): Remove when packets are always owned by pacer. +RtpPacketSender::Priority PacketTypeToPriority(RtpPacketToSend::Type type) { + switch (type) { + case RtpPacketToSend::Type::kAudio: + return RtpPacketSender::Priority::kHighPriority; + case RtpPacketToSend::Type::kVideo: + return RtpPacketSender::Priority::kLowPriority; + case RtpPacketToSend::Type::kRetransmission: + return RtpPacketSender::Priority::kNormalPriority; + case RtpPacketToSend::Type::kForwardErrorCorrection: + return RtpPacketSender::Priority::kLowPriority; + break; + case RtpPacketToSend::Type::kPadding: + RTC_NOTREACHED() << "Unexpected type for legacy path: kPadding"; + break; + } + return RtpPacketSender::Priority::kLowPriority; +} + } // namespace RTPSender::RTPSender( @@ -802,8 +838,7 @@ size_t RTPSender::TimeToSendPadding(size_t bytes, } bool RTPSender::SendToNetwork(std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority) { + StorageType storage) { RTC_DCHECK(packet); int64_t now_ms = clock_->TimeInMilliseconds(); @@ -813,6 +848,8 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, int64_t capture_time_ms = packet->capture_time_ms(); size_t packet_size = send_side_bwe_with_overhead_ ? packet->size() : packet->payload_size(); + auto packet_type = packet->packet_type(); + RTC_DCHECK(packet_type.has_value()); if (ssrc == FlexfecSsrc()) { // Store FlexFEC packets in the history here, so they can be found // when the pacer calls TimeToSendPacket. @@ -822,8 +859,8 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, packet_history_.PutRtpPacket(std::move(packet), storage, absl::nullopt); } - paced_sender_->InsertPacket(priority, ssrc, seq_no, capture_time_ms, - packet_size, false); + paced_sender_->InsertPacket(PacketTypeToPriority(*packet_type), ssrc, + seq_no, capture_time_ms, packet_size, false); return true; } @@ -884,6 +921,13 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, return sent; } +bool RTPSender::SendToNetwork(std::unique_ptr packet, + StorageType storage, + RtpPacketSender::Priority priority) { + packet->set_packet_type(PacketPriorityToType(priority)); + return SendToNetwork(std::move(packet), storage); +} + void RTPSender::RecomputeMaxSendDelay() { max_delay_it_ = send_delays_.begin(); for (auto it = send_delays_.begin(); it != send_delays_.end(); ++it) { diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 94ef809962..5e86742f0e 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -158,6 +158,10 @@ class RTPSender { absl::optional FlexfecSsrc() const; // Sends packet to |transport_| or to the pacer, depending on configuration. + bool SendToNetwork(std::unique_ptr packet, + StorageType storage); + + // Fallback that infers PacketType from Priority. bool SendToNetwork(std::unique_ptr packet, StorageType storage, RtpPacketSender::Priority priority); diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.cc b/modules/rtp_rtcp/source/rtp_sender_audio.cc index c54cb91cd8..9e5e71c548 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.cc +++ b/modules/rtp_rtcp/source/rtp_sender_audio.cc @@ -255,8 +255,9 @@ bool RTPSenderAudio::SendAudio(AudioFrameType frame_type, TRACE_EVENT_ASYNC_END2("webrtc", "Audio", rtp_timestamp, "timestamp", packet->Timestamp(), "seqnum", packet->SequenceNumber()); - bool send_result = LogAndSendToNetwork( - std::move(packet), kAllowRetransmission, RtpPacketSender::kHighPriority); + packet->set_packet_type(RtpPacketToSend::Type::kAudio); + bool send_result = + LogAndSendToNetwork(std::move(packet), kAllowRetransmission); if (first_packet_sent_()) { RTC_LOG(LS_INFO) << "First audio RTP packet sent to pacer"; } @@ -339,8 +340,8 @@ bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, dtmfbuffer[1] = E | R | volume; ByteWriter::WriteBigEndian(dtmfbuffer + 2, duration); - result = LogAndSendToNetwork(std::move(packet), kAllowRetransmission, - RtpPacketSender::kHighPriority); + packet->set_packet_type(RtpPacketToSend::Type::kAudio); + result = LogAndSendToNetwork(std::move(packet), kAllowRetransmission); send_count--; } while (send_count > 0 && result); @@ -349,8 +350,7 @@ bool RTPSenderAudio::SendTelephoneEventPacket(bool ended, bool RTPSenderAudio::LogAndSendToNetwork( std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority) { + StorageType storage) { #if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE int64_t now_ms = clock_->TimeInMilliseconds(); BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "AudioTotBitrate_kbps", now_ms, @@ -360,8 +360,7 @@ bool RTPSenderAudio::LogAndSendToNetwork( rtp_sender_->NackOverheadRate() / 1000, packet->Ssrc()); #endif - - return rtp_sender_->SendToNetwork(std::move(packet), storage, priority); + return rtp_sender_->SendToNetwork(std::move(packet), storage); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_sender_audio.h b/modules/rtp_rtcp/source/rtp_sender_audio.h index 1c78e92612..d8a61fda21 100644 --- a/modules/rtp_rtcp/source/rtp_sender_audio.h +++ b/modules/rtp_rtcp/source/rtp_sender_audio.h @@ -64,8 +64,7 @@ class RTPSenderAudio { private: bool LogAndSendToNetwork(std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority); + StorageType storage); Clock* const clock_ = nullptr; RTPSender* const rtp_sender_ = nullptr; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 419e938e3c..bbadda80ba 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -251,8 +251,8 @@ 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(); - if (!LogAndSendToNetwork(std::move(packet), storage, - RtpPacketSender::kLowPriority)) { + packet->set_packet_type(RtpPacketToSend::Type::kVideo); + if (!LogAndSendToNetwork(std::move(packet), storage)) { RTC_LOG(LS_WARNING) << "Failed to send video packet " << seq_num; return; } @@ -293,8 +293,8 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( } // Send |red_packet| instead of |packet| for allocated sequence number. size_t red_packet_size = red_packet->size(); - if (LogAndSendToNetwork(std::move(red_packet), media_packet_storage, - RtpPacketSender::kLowPriority)) { + red_packet->set_packet_type(RtpPacketToSend::Type::kVideo); + if (LogAndSendToNetwork(std::move(red_packet), media_packet_storage)) { rtc::CritScope cs(&stats_crit_); video_bitrate_.Update(red_packet_size, clock_->TimeInMilliseconds()); } else { @@ -309,8 +309,7 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( 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(); - if (LogAndSendToNetwork(std::move(rtp_packet), kDontRetransmit, - RtpPacketSender::kLowPriority)) { + if (LogAndSendToNetwork(std::move(rtp_packet), kDontRetransmit)) { rtc::CritScope cs(&stats_crit_); fec_bitrate_.Update(fec_packet->length(), clock_->TimeInMilliseconds()); } else { @@ -337,8 +336,9 @@ void RTPSenderVideo::SendVideoPacketWithFlexfec( for (auto& fec_packet : fec_packets) { size_t packet_length = fec_packet->size(); uint16_t seq_num = fec_packet->SequenceNumber(); - if (LogAndSendToNetwork(std::move(fec_packet), kDontRetransmit, - RtpPacketSender::kLowPriority)) { + fec_packet->set_packet_type( + RtpPacketToSend::Type::kForwardErrorCorrection); + if (LogAndSendToNetwork(std::move(fec_packet), kDontRetransmit)) { rtc::CritScope cs(&stats_crit_); fec_bitrate_.Update(packet_length, clock_->TimeInMilliseconds()); } else { @@ -350,8 +350,7 @@ void RTPSenderVideo::SendVideoPacketWithFlexfec( bool RTPSenderVideo::LogAndSendToNetwork( std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority) { + StorageType storage) { #if BWE_TEST_LOGGING_COMPILE_TIME_ENABLE int64_t now_ms = clock_->TimeInMilliseconds(); BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "VideoTotBitrate_kbps", now_ms, @@ -363,7 +362,7 @@ bool RTPSenderVideo::LogAndSendToNetwork( rtp_sender_->NackOverheadRate() / 1000, packet->Ssrc()); #endif - return rtp_sender_->SendToNetwork(std::move(packet), storage, priority); + return rtp_sender_->SendToNetwork(std::move(packet), storage); } void RTPSenderVideo::SetUlpfecConfig(int red_payload_type, diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 94377bf34f..3555958e3c 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -143,8 +143,7 @@ class RTPSenderVideo { bool protect_media_packet); bool LogAndSendToNetwork(std::unique_ptr packet, - StorageType storage, - RtpPacketSender::Priority priority); + StorageType storage); bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) { return red_payload_type_ >= 0;