From 41befcee7d25dabb8f6871dcdef4adc1a0004932 Mon Sep 17 00:00:00 2001 From: danilchap Date: Wed, 30 Mar 2016 11:11:51 -0700 Subject: [PATCH] Make rtcp sender use max transfer unit. Remove packet overhead from rtp sender as unused. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R=philipel, åsapersson Review URL: https://codereview.webrtc.org/1827953002 Cr-Commit-Position: refs/heads/master@{#12165} --- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 16 +++++++++---- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 3 +++ .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 24 ++++++++++--------- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 8 +------ webrtc/modules/rtp_rtcp/source/rtp_sender.h | 5 +--- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index a5fccebad1..440bd50d57 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -99,8 +99,10 @@ class PacketContainer : public rtcp::CompoundPacket, } } - size_t SendPackets() { - rtcp::CompoundPacket::Build(this); + size_t SendPackets(size_t max_payload_length) { + RTC_DCHECK_LE(max_payload_length, static_cast(IP_PACKET_SIZE)); + uint8_t buffer[IP_PACKET_SIZE]; + BuildExternalBuffer(buffer, max_payload_length, this); return bytes_sent_; } @@ -169,6 +171,7 @@ RTCPSender::RTCPSender( tmmbr_help_(), tmmbr_send_(0), packet_oh_send_(0), + max_payload_length_(IP_PACKET_SIZE - 28), // IPv4 + UDP by default. app_sub_type_(0), app_name_(0), @@ -277,6 +280,10 @@ void RTCPSender::SetTMMBRStatus(bool enable) { } } +void RTCPSender::SetMaxPayloadLength(size_t max_payload_length) { + max_payload_length_ = max_payload_length; +} + void RTCPSender::SetStartTimestamp(uint32_t start_timestamp) { rtc::CritScope lock(&critical_section_rtcp_sender_); start_timestamp_ = start_timestamp; @@ -809,7 +816,7 @@ int32_t RTCPSender::SendCompoundRTCP( RTC_DCHECK(AllVolatileFlagsConsumed()); } - size_t bytes_sent = container.SendPackets(); + size_t bytes_sent = container.SendPackets(max_payload_length_); return bytes_sent == 0 ? -1 : 0; } @@ -1030,8 +1037,9 @@ bool RTCPSender::SendFeedbackPacket(const rtcp::TransportFeedback& packet) { // but we can't because of an incorrect warning (C4822) in MVS 2013. } sender(transport_, event_log_); + RTC_DCHECK_LE(max_payload_length_, static_cast(IP_PACKET_SIZE)); uint8_t buffer[IP_PACKET_SIZE]; - return packet.BuildExternalBuffer(buffer, IP_PACKET_SIZE, &sender) && + return packet.BuildExternalBuffer(buffer, max_payload_length_, &sender) && !sender.send_failure_; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index 5183b56b9f..ba6fb70055 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -132,6 +132,8 @@ class RTCPSender { void SetTMMBRStatus(bool enable); + void SetMaxPayloadLength(size_t max_payload_length); + int32_t SetTMMBN(const TMMBRSet* boundingSet); int32_t SetApplicationSpecificData(uint8_t subType, @@ -240,6 +242,7 @@ class RTCPSender { TMMBRHelp tmmbr_help_ GUARDED_BY(critical_section_rtcp_sender_); uint32_t tmmbr_send_ GUARDED_BY(critical_section_rtcp_sender_); uint32_t packet_oh_send_ GUARDED_BY(critical_section_rtcp_sender_); + size_t max_payload_length_; // APP uint8_t app_sub_type_ GUARDED_BY(critical_section_rtcp_sender_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 581aa5111a..5875529cfb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -118,6 +118,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) uint32_t SSRC = rtp_sender_.SSRC(); rtcp_sender_.SetSSRC(SSRC); SetRtcpReceiverSsrcs(SSRC); + SetMaxTransferUnit(IP_PACKET_SIZE); } // Returns the number of milliseconds until the module want a worker thread @@ -479,21 +480,22 @@ int32_t ModuleRtpRtcpImpl::SetTransportOverhead( // Ok same as before. return 0; } - // Calc diff. - int16_t packet_over_head_diff = packet_overhead - packet_overhead_; - // Store new. + size_t mtu = rtp_sender_.MaxPayloadLength() + packet_overhead_; + size_t max_payload_length = mtu - packet_overhead; packet_overhead_ = packet_overhead; - - uint16_t length = - rtp_sender_.MaxPayloadLength() - packet_over_head_diff; - return rtp_sender_.SetMaxPayloadLength(length, packet_overhead_); + rtcp_sender_.SetMaxPayloadLength(max_payload_length); + rtp_sender_.SetMaxPayloadLength(max_payload_length); + return 0; } -int32_t ModuleRtpRtcpImpl::SetMaxTransferUnit(const uint16_t mtu) { - RTC_DCHECK_LE(mtu, IP_PACKET_SIZE) << "Invalid mtu: " << mtu; - return rtp_sender_.SetMaxPayloadLength(mtu - packet_overhead_, - packet_overhead_); +int32_t ModuleRtpRtcpImpl::SetMaxTransferUnit(uint16_t mtu) { + RTC_DCHECK_LE(mtu, IP_PACKET_SIZE) << "MTU too large: " << mtu; + RTC_DCHECK_GT(mtu, packet_overhead_) << "MTU too small: " << mtu; + size_t max_payload_length = mtu - packet_overhead_; + rtcp_sender_.SetMaxPayloadLength(max_payload_length); + rtp_sender_.SetMaxPayloadLength(max_payload_length); + return 0; } RtcpMode ModuleRtpRtcpImpl::RTCP() const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 36d7eb573c..3fbca7b67d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -133,7 +133,6 @@ RTPSender::RTPSender( transport_(transport), sending_media_(true), // Default to sending media. max_payload_length_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. - packet_over_head_(28), payload_type_(-1), payload_type_map_(), rtp_header_extension_map_(), @@ -375,15 +374,12 @@ int RTPSender::SendPayloadFrequency() const { return audio_ != NULL ? audio_->AudioFrequency() : kVideoPayloadTypeFrequency; } -int32_t RTPSender::SetMaxPayloadLength(size_t max_payload_length, - uint16_t packet_over_head) { +void RTPSender::SetMaxPayloadLength(size_t max_payload_length) { // Sanity check. RTC_DCHECK(max_payload_length >= 100 && max_payload_length <= IP_PACKET_SIZE) << "Invalid max payload length: " << max_payload_length; rtc::CritScope lock(&send_critsect_); max_payload_length_ = max_payload_length; - packet_over_head_ = packet_over_head; - return 0; } size_t RTPSender::MaxDataPayloadLength() const { @@ -405,8 +401,6 @@ size_t RTPSender::MaxPayloadLength() const { return max_payload_length_; } -uint16_t RTPSender::PacketOverHead() const { return packet_over_head_; } - void RTPSender::SetRtxStatus(int mode) { rtc::CritScope lock(&send_critsect_); rtx_ = mode; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index b376c380d0..4344df6745 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -67,7 +67,6 @@ class RTPSenderInterface { virtual uint16_t SequenceNumber() const = 0; virtual size_t MaxPayloadLength() const = 0; virtual size_t MaxDataPayloadLength() const = 0; - virtual uint16_t PacketOverHead() const = 0; virtual uint16_t ActualSendBitrateKbit() const = 0; virtual int32_t SendToNetwork(uint8_t* data_buffer, @@ -146,7 +145,7 @@ class RTPSender : public RTPSenderInterface { void SetCsrcs(const std::vector& csrcs); - int32_t SetMaxPayloadLength(size_t length, uint16_t packet_over_head); + void SetMaxPayloadLength(size_t max_payload_length); int32_t SendOutgoingData(FrameType frame_type, int8_t payload_type, @@ -247,7 +246,6 @@ class RTPSender : public RTPSenderInterface { size_t RTPHeaderLength() const override; uint16_t AllocateSequenceNumber(uint16_t packets_to_send) override; size_t MaxPayloadLength() const override; - uint16_t PacketOverHead() const override; // Current timestamp. uint32_t Timestamp() const override; @@ -437,7 +435,6 @@ class RTPSender : public RTPSenderInterface { bool sending_media_ GUARDED_BY(send_critsect_); size_t max_payload_length_; - uint16_t packet_over_head_; int8_t payload_type_ GUARDED_BY(send_critsect_); std::map payload_type_map_;