From 284542b8827b3379a9be227b3563e2b2c7742b47 Mon Sep 17 00:00:00 2001 From: nisse Date: Tue, 10 Jan 2017 08:58:32 -0800 Subject: [PATCH] Make OverheadObserver::OnOverheadChanged count RTP headers only This lets the RTP code be unaware of lower layers, and the SetTransportOverhead method is deleted from RTPSender and RtpRtcp. Instead, that method is added to CongestionController and TransportFeedbackAdapter, where it is more appropriate. BUG=wertc:6847 Review-Url: https://codereview.webrtc.org/2589743002 Cr-Commit-Position: refs/heads/master@{#15995} --- .../congestion_controller.cc | 6 ++ .../include/congestion_controller.h | 2 + .../transport_feedback_adapter.cc | 11 +++ .../transport_feedback_adapter.h | 2 + webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 39 ++++------- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 8 +-- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 12 ++-- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 4 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 70 ++++--------------- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 13 +--- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 56 +++++---------- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 14 ++-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 23 ++---- .../rtp_rtcp/source/rtp_sender_video.cc | 2 +- .../modules/rtp_rtcp/test/testAPI/test_api.cc | 18 ++--- webrtc/video/video_send_stream.cc | 62 +++++++++++----- webrtc/video/video_send_stream.h | 2 +- webrtc/voice_engine/channel.cc | 23 ++++-- webrtc/voice_engine/channel.h | 6 +- 19 files changed, 163 insertions(+), 210 deletions(-) diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 2fae264156..94b3d8c6da 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -306,6 +306,12 @@ void CongestionController::SignalNetworkState(NetworkState state) { MaybeTriggerOnNetworkChanged(); } +void CongestionController::SetTransportOverhead( + size_t transport_overhead_bytes_per_packet) { + transport_feedback_adapter_.SetTransportOverhead( + transport_overhead_bytes_per_packet); +} + void CongestionController::OnSentPacket(const rtc::SentPacket& sent_packet) { // We're not interested in packets without an id, which may be stun packets, // etc, sent on the same transport. diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 48b9531260..1fcb03dd57 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -81,6 +81,8 @@ class CongestionController : public CallStatsObserver, public Module { int min_bitrate_bps, int max_bitrate_bps); virtual void SignalNetworkState(NetworkState state); + virtual void SetTransportOverhead(size_t transport_overhead_bytes_per_packet); + virtual BitrateController* GetBitrateController() const; virtual RemoteBitrateEstimator* GetRemoteBitrateEstimator( bool send_side_bwe); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc index 7756fee365..7ffbe8d2da 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc @@ -19,6 +19,7 @@ #include "webrtc/modules/congestion_controller/delay_based_bwe.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/modules/utility/include/process_thread.h" +#include "webrtc/system_wrappers/include/field_trial.h" namespace webrtc { @@ -59,6 +60,10 @@ void TransportFeedbackAdapter::AddPacket(uint16_t sequence_number, size_t length, int probe_cluster_id) { rtc::CritScope cs(&lock_); + if (webrtc::field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead") == + "Enabled") { + length += transport_overhead_bytes_per_packet_; + } send_time_history_.AddAndRemoveOld(sequence_number, length, probe_cluster_id); } @@ -73,6 +78,12 @@ void TransportFeedbackAdapter::SetMinBitrate(int min_bitrate_bps) { delay_based_bwe_->SetMinBitrate(min_bitrate_bps); } +void TransportFeedbackAdapter::SetTransportOverhead( + int transport_overhead_bytes_per_packet) { + rtc::CritScope cs(&lock_); + transport_overhead_bytes_per_packet_ = transport_overhead_bytes_per_packet; +} + int64_t TransportFeedbackAdapter::GetProbingIntervalMs() const { rtc::CritScope cs(&bwe_lock_); return delay_based_bwe_->GetProbingIntervalMs(); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.h b/webrtc/modules/congestion_controller/transport_feedback_adapter.h index 09363ec50c..b7085be91d 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter.h +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.h @@ -49,6 +49,7 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; void SetMinBitrate(int min_bitrate_bps); + void SetTransportOverhead(int transport_overhead_bytes_per_packet); int64_t GetProbingIntervalMs() const; @@ -58,6 +59,7 @@ class TransportFeedbackAdapter : public TransportFeedbackObserver, rtc::CriticalSection lock_; rtc::CriticalSection bwe_lock_; + int transport_overhead_bytes_per_packet_ GUARDED_BY(&lock_); SendTimeHistory send_time_history_ GUARDED_BY(&lock_); std::unique_ptr delay_based_bwe_ GUARDED_BY(&bwe_lock_); Clock* const clock_; diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index aa5df5f8cf..325d77ad2d 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -114,36 +114,25 @@ class RtpRtcp : public Module { // Sender // ************************************************************************** - // Sets MTU. - // |size| - Max transfer unit in bytes, default is 1500. - // Returns -1 on failure else 0. - virtual int32_t SetMaxTransferUnit(uint16_t size) = 0; + // TODO(nisse): Deprecated. Kept temporarily, as an alias for the + // new function which has slighly different semantics. Delete as + // soon as known applications are updated. + virtual int32_t SetMaxTransferUnit(uint16_t size) { + SetMaxRtpPacketSize(size); + return 0; + } - // Sets transport overhead. Default is IPv4 and UDP with no encryption. - // |tcp| - true for TCP false UDP. - // |ipv6| - true for IP version 6 false for version 4. - // |authentication_overhead| - number of bytes to leave for an authentication - // header. - // Returns -1 on failure else 0 - // TODO(michaelt): deprecate the function. - virtual int32_t SetTransportOverhead(bool tcp, - bool ipv6, - uint8_t authentication_overhead = 0) = 0; + // Sets the maximum size of an RTP packet, including RTP headers. + virtual void SetMaxRtpPacketSize(size_t size) = 0; - // Sets transport overhead per packet. - virtual void SetTransportOverhead(int transport_overhead_per_packet) = 0; - - // Returns max payload length, which is a combination of the configuration - // MaxTransferUnit and TransportOverhead. + // Returns max payload length. // Does not account for RTP headers and FEC/ULP/RED overhead (when FEC is // enabled). - virtual uint16_t MaxPayloadLength() const = 0; + virtual size_t MaxPayloadSize() const = 0; - // Returns max data payload length, which is a combination of the - // configuration MaxTransferUnit, headers and TransportOverhead. - // Takes into account RTP headers and FEC/ULP/RED overhead (when FEC is - // enabled). - virtual uint16_t MaxDataPayloadLength() const = 0; + // Returns max RTP packet size. Takes into account RTP headers and + // FEC/ULP/RED overhead (when FEC is enabled). + virtual size_t MaxRtpPacketSize() const = 0; // Sets codec name and payload type. Returns -1 on failure else 0. virtual int32_t RegisterSendPayload(const CodecInst& voice_codec) = 0; diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 0e81e4dc6e..fc93655aa5 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -53,12 +53,10 @@ class MockRtpRtcp : public RtpRtcp { uint32_t audio_rtcp_arrival_time_frac)); MOCK_METHOD0(InitSender, int32_t()); MOCK_METHOD1(RegisterSendTransport, int32_t(Transport* outgoing_transport)); - MOCK_METHOD1(SetMaxTransferUnit, int32_t(uint16_t size)); - MOCK_METHOD3(SetTransportOverhead, - int32_t(bool tcp, bool ipv6, uint8_t authentication_overhead)); + MOCK_METHOD1(SetMaxRtpPacketSize, void(size_t size)); MOCK_METHOD1(SetTransportOverhead, void(int transport_overhead_per_packet)); - MOCK_CONST_METHOD0(MaxPayloadLength, uint16_t()); - MOCK_CONST_METHOD0(MaxDataPayloadLength, uint16_t()); + MOCK_CONST_METHOD0(MaxPayloadSize, size_t()); + MOCK_CONST_METHOD0(MaxRtpPacketSize, size_t()); MOCK_METHOD1(RegisterSendPayload, int32_t(const CodecInst& voice_codec)); MOCK_METHOD1(RegisterSendPayload, int32_t(const VideoCodec& video_codec)); MOCK_METHOD2(RegisterVideoSendPayload, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index 744ed625d1..48f7595d77 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -176,7 +176,7 @@ RTCPSender::RTCPSender( tmmbr_send_bps_(0), packet_oh_send_(0), - max_payload_length_(IP_PACKET_SIZE - 28), // IPv4 + UDP by default. + max_packet_size_(IP_PACKET_SIZE - 28), // IPv4 + UDP by default. app_sub_type_(0), app_name_(0), @@ -283,8 +283,8 @@ void RTCPSender::SetTMMBRStatus(bool enable) { } } -void RTCPSender::SetMaxPayloadLength(size_t max_payload_length) { - max_payload_length_ = max_payload_length; +void RTCPSender::SetMaxRtpPacketSize(size_t max_packet_size) { + max_packet_size_ = max_packet_size; } void RTCPSender::SetTimestampOffset(uint32_t timestamp_offset) { @@ -828,7 +828,7 @@ int32_t RTCPSender::SendCompoundRTCP( RTC_DCHECK(AllVolatileFlagsConsumed()); } - size_t bytes_sent = container.SendPackets(max_payload_length_); + size_t bytes_sent = container.SendPackets(max_packet_size_); return bytes_sent == 0 ? -1 : 0; } @@ -1049,9 +1049,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_, IP_PACKET_SIZE); + RTC_DCHECK_LE(max_packet_size_, IP_PACKET_SIZE); uint8_t buffer[IP_PACKET_SIZE]; - return packet.BuildExternalBuffer(buffer, max_payload_length_, &sender) && + return packet.BuildExternalBuffer(buffer, max_packet_size_, &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 3dbbb3e340..96a49a72e0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -133,7 +133,7 @@ class RTCPSender { void SetTMMBRStatus(bool enable); - void SetMaxPayloadLength(size_t max_payload_length); + void SetMaxRtpPacketSize(size_t max_packet_size); void SetTmmbn(std::vector bounding_set); @@ -240,7 +240,7 @@ class RTCPSender { GUARDED_BY(critical_section_rtcp_sender_); uint32_t tmmbr_send_bps_ GUARDED_BY(critical_section_rtcp_sender_); uint32_t packet_oh_send_ GUARDED_BY(critical_section_rtcp_sender_); - size_t max_payload_length_; + size_t max_packet_size_; // 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 b867b9ddbe..3c986e98c2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -121,7 +121,10 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) rtcp_sender_.SetTimestampOffset(rtp_sender_.TimestampOffset()); // Set default packet size limit. - SetMaxTransferUnit(IP_PACKET_SIZE); + // TODO(nisse): Kind-of duplicates + // webrtc::VideoSendStream::Config::Rtp::kDefaultMaxPacketSize. + const size_t kTcpOverIpv4HeaderSize = 40; + SetMaxRtpPacketSize(IP_PACKET_SIZE - kTcpOverIpv4HeaderSize); } // Returns the number of milliseconds until the module want a worker thread @@ -415,65 +418,22 @@ size_t ModuleRtpRtcpImpl::TimeToSendPadding(size_t bytes, return rtp_sender_.TimeToSendPadding(bytes, probe_cluster_id); } -uint16_t ModuleRtpRtcpImpl::MaxPayloadLength() const { - return rtp_sender_.MaxPayloadLength(); +size_t ModuleRtpRtcpImpl::MaxPayloadSize() const { + return rtp_sender_.MaxPayloadSize(); } -uint16_t ModuleRtpRtcpImpl::MaxDataPayloadLength() const { - return rtp_sender_.MaxDataPayloadLength(); +size_t ModuleRtpRtcpImpl::MaxRtpPacketSize() const { + return rtp_sender_.MaxRtpPacketSize(); } -int32_t ModuleRtpRtcpImpl::SetTransportOverhead( - const bool tcp, - const bool ipv6, - const uint8_t authentication_overhead) { - uint16_t packet_overhead = 0; - if (ipv6) { - packet_overhead = 40; - } else { - packet_overhead = 20; - } - if (tcp) { - // TCP. - packet_overhead += 20; - } else { - // UDP. - packet_overhead += 8; - } - packet_overhead += authentication_overhead; +void ModuleRtpRtcpImpl::SetMaxRtpPacketSize(size_t rtp_packet_size) { + RTC_DCHECK_LE(rtp_packet_size, IP_PACKET_SIZE) + << "rtp packet size too large: " << rtp_packet_size; + RTC_DCHECK_GT(rtp_packet_size, packet_overhead_) + << "rtp packet size too small: " << rtp_packet_size; - if (packet_overhead == packet_overhead_) { - // Ok same as before. - return 0; - } - - size_t mtu = rtp_sender_.MaxPayloadLength() + packet_overhead_; - size_t max_payload_length = mtu - packet_overhead; - packet_overhead_ = packet_overhead; - rtcp_sender_.SetMaxPayloadLength(max_payload_length); - rtp_sender_.SetMaxPayloadLength(max_payload_length); - return 0; -} - -void ModuleRtpRtcpImpl::SetTransportOverhead( - int transport_overhead_per_packet) { - RTC_DCHECK_GT(transport_overhead_per_packet, 0); - int mtu = rtp_sender_.MaxPayloadLength() + packet_overhead_; - RTC_DCHECK_LT(transport_overhead_per_packet, mtu); - size_t max_payload_length = mtu - transport_overhead_per_packet; - packet_overhead_ = transport_overhead_per_packet; - rtp_sender_.SetTransportOverhead(packet_overhead_); - rtcp_sender_.SetMaxPayloadLength(max_payload_length); - rtp_sender_.SetMaxPayloadLength(max_payload_length); -} - -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; + rtcp_sender_.SetMaxRtpPacketSize(rtp_packet_size); + rtp_sender_.SetMaxRtpPacketSize(rtp_packet_size); } RtcpMode ModuleRtpRtcpImpl::RTCP() const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 9363a4716a..836f62cebe 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -207,18 +207,11 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { void SetTmmbn(std::vector bounding_set) override; - uint16_t MaxPayloadLength() const override; + size_t MaxPayloadSize() const override; - uint16_t MaxDataPayloadLength() const override; + size_t MaxRtpPacketSize() const override; - int32_t SetMaxTransferUnit(uint16_t size) override; - - // TODO(michaelt): deprecate the function. - int32_t SetTransportOverhead(bool tcp, - bool ipv6, - uint8_t authentication_overhead = 0) override; - - void SetTransportOverhead(int transport_overhead_per_packet) override; + void SetMaxRtpPacketSize(size_t max_packet_size) override; // (NACK) Negative acknowledgment part. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index a55ccc37b9..ae491209b7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -92,8 +92,8 @@ RTPSender::RTPSender( transport_feedback_observer_(transport_feedback_observer), last_capture_time_ms_sent_(0), transport_(transport), - sending_media_(true), // Default to sending media. - max_payload_length_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. + sending_media_(true), // Default to sending media. + max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. payload_type_(-1), payload_type_map_(), rtp_header_extension_map_(), @@ -121,7 +121,6 @@ RTPSender::RTPSender( last_packet_marker_bit_(false), csrcs_(), rtx_(kRtxOff), - transport_overhead_bytes_per_packet_(0), rtp_overhead_bytes_per_packet_(0), retransmission_rate_limiter_(retransmission_rate_limiter), overhead_observer_(overhead_observer) { @@ -297,26 +296,26 @@ int8_t RTPSender::SendPayloadType() const { return payload_type_; } -void RTPSender::SetMaxPayloadLength(size_t max_payload_length) { +void RTPSender::SetMaxRtpPacketSize(size_t max_packet_size) { // Sanity check. - RTC_DCHECK(max_payload_length >= 100 && max_payload_length <= IP_PACKET_SIZE) - << "Invalid max payload length: " << max_payload_length; + RTC_DCHECK(max_packet_size >= 100 && max_packet_size <= IP_PACKET_SIZE) + << "Invalid max payload length: " << max_packet_size; rtc::CritScope lock(&send_critsect_); - max_payload_length_ = max_payload_length; + max_packet_size_ = max_packet_size; } -size_t RTPSender::MaxDataPayloadLength() const { +size_t RTPSender::MaxPayloadSize() const { if (audio_configured_) { - return max_payload_length_ - RtpHeaderLength(); + return max_packet_size_ - RtpHeaderLength(); } else { - return max_payload_length_ - RtpHeaderLength() // RTP overhead. - - video_->FecPacketOverhead() // FEC/ULP/RED overhead. - - (RtxStatus() ? kRtxHeaderSize : 0); // RTX overhead. + return max_packet_size_ - RtpHeaderLength() // RTP overhead. + - video_->FecPacketOverhead() // FEC/ULP/RED overhead. + - (RtxStatus() ? kRtxHeaderSize : 0); // RTX overhead. } } -size_t RTPSender::MaxPayloadLength() const { - return max_payload_length_; +size_t RTPSender::MaxRtpPacketSize() const { + return max_packet_size_; } void RTPSender::SetRtxStatus(int mode) { @@ -483,7 +482,7 @@ size_t RTPSender::SendPadData(size_t bytes, int probe_cluster_id) { // RtpPacketSender, which will make sure we don't send too much padding even // if a single packet is larger than requested. size_t padding_bytes_in_packet = - std::min(MaxDataPayloadLength(), kMaxPaddingLength); + std::min(MaxPayloadSize(), kMaxPaddingLength); size_t bytes_sent = 0; while (bytes_sent < bytes) { int64_t now_ms = clock_->TimeInMilliseconds(); @@ -974,7 +973,7 @@ void RTPSender::GetDataCounters(StreamDataCounters* rtp_stats, std::unique_ptr RTPSender::AllocatePacket() const { rtc::CritScope lock(&send_critsect_); std::unique_ptr packet( - new RtpPacketToSend(&rtp_header_extension_map_, max_payload_length_)); + new RtpPacketToSend(&rtp_header_extension_map_, max_packet_size_)); packet->SetSsrc(ssrc_); packet->SetCsrcs(csrcs_); // Reserve extensions, if registered, RtpSender set in SendToNetwork. @@ -1253,31 +1252,13 @@ RtpState RTPSender::GetRtxRtpState() const { return state; } -void RTPSender::SetTransportOverhead(int transport_overhead) { - if (!overhead_observer_) - return; - size_t overhead_bytes_per_packet = 0; - { - rtc::CritScope lock(&send_critsect_); - if (transport_overhead_bytes_per_packet_ == - static_cast(transport_overhead)) { - return; - } - transport_overhead_bytes_per_packet_ = transport_overhead; - overhead_bytes_per_packet = - rtp_overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_; - } - overhead_observer_->OnOverheadChanged(overhead_bytes_per_packet); -} - void RTPSender::AddPacketToTransportFeedback(uint16_t packet_id, const RtpPacketToSend& packet, int probe_cluster_id) { size_t packet_size = packet.payload_size() + packet.padding_size(); if (webrtc::field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead") == "Enabled") { - rtc::CritScope lock(&send_critsect_); - packet_size = packet.size() + transport_overhead_bytes_per_packet_; + packet_size = packet.size(); } if (transport_feedback_observer_) { @@ -1289,15 +1270,14 @@ void RTPSender::AddPacketToTransportFeedback(uint16_t packet_id, void RTPSender::UpdateRtpOverhead(const RtpPacketToSend& packet) { if (!overhead_observer_) return; - size_t overhead_bytes_per_packet = 0; + size_t overhead_bytes_per_packet; { rtc::CritScope lock(&send_critsect_); if (rtp_overhead_bytes_per_packet_ == packet.headers_size()) { return; } rtp_overhead_bytes_per_packet_ = packet.headers_size(); - overhead_bytes_per_packet = - rtp_overhead_bytes_per_packet_ + transport_overhead_bytes_per_packet_; + overhead_bytes_per_packet = rtp_overhead_bytes_per_packet_; } overhead_observer_->OnOverheadChanged(overhead_bytes_per_packet); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 36acc2a19d..b70b6885e1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -72,8 +72,8 @@ class RTPSender { uint32_t FecOverheadRate() const; uint32_t NackOverheadRate() const; - // Includes size of RTP and FEC headers. - size_t MaxDataPayloadLength() const; + // Excluding size of RTP and FEC headers. + size_t MaxPayloadSize() const; int32_t RegisterPayload(const char* payload_name, const int8_t payload_type, @@ -106,7 +106,7 @@ class RTPSender { void SetCsrcs(const std::vector& csrcs); - void SetMaxPayloadLength(size_t max_payload_length); + void SetMaxRtpPacketSize(size_t max_packet_size); bool SendOutgoingData(FrameType frame_type, int8_t payload_type, @@ -164,7 +164,8 @@ class RTPSender { size_t RtpHeaderLength() const; uint16_t AllocateSequenceNumber(uint16_t packets_to_send); - size_t MaxPayloadLength() const; + // Including RTP headers. + size_t MaxRtpPacketSize() const; uint32_t SSRC() const; @@ -208,8 +209,6 @@ class RTPSender { void SetRtxRtpState(const RtpState& rtp_state); RtpState GetRtxRtpState() const; - void SetTransportOverhead(int transport_overhead); - protected: int32_t CheckPayloadType(int8_t payload_type, RtpVideoCodecTypes* video_type); @@ -272,7 +271,7 @@ class RTPSender { Transport* transport_; bool sending_media_ GUARDED_BY(send_critsect_); - size_t max_payload_length_; + size_t max_packet_size_; int8_t payload_type_ GUARDED_BY(send_critsect_); std::map payload_type_map_; @@ -323,7 +322,6 @@ class RTPSender { uint32_t ssrc_rtx_ GUARDED_BY(send_critsect_); // Mapping rtx_payload_type_map_[associated] = rtx. std::map rtx_payload_type_map_ GUARDED_BY(send_critsect_); - size_t transport_overhead_bytes_per_packet_ GUARDED_BY(send_critsect_); size_t rtp_overhead_bytes_per_packet_ GUARDED_BY(send_critsect_); RateLimiter* const retransmission_rate_limiter_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 254a7d0ae6..ddf6422a9f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1394,21 +1394,16 @@ TEST_F(RtpSenderTest, OnOverheadChanged) { nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); - // Transport overhead is set to 28B. - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(28)).Times(1); - rtp_sender_->SetTransportOverhead(28); - // RTP overhead is 12B. - // 28B + 12B = 40B - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(40)).Times(1); + EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(12)).Times(1); SendGenericPayload(); rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransmissionTimeOffset, kTransmissionTimeOffsetExtensionId); // TransmissionTimeOffset extension has a size of 8B. - // 28B + 12B + 8B = 48B - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(48)).Times(1); + // 12B + 8B = 20B + EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(20)).Times(1); SendGenericPayload(); } @@ -1419,17 +1414,12 @@ TEST_F(RtpSenderTest, DoesNotUpdateOverheadOnEqualSize) { nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); - rtp_sender_->SetTransportOverhead(28); - rtp_sender_->SetTransportOverhead(28); - EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(_)).Times(1); SendGenericPayload(); SendGenericPayload(); } TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { - constexpr int kTransportOverheadBytesPerPacket = 28; constexpr int kRtpOverheadBytesPerPacket = 12 + 8; test::ScopedFieldTrials override_field_trials( "WebRTC-SendSideBwe-WithOverhead/Enabled/"); @@ -1438,7 +1428,6 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { false, &fake_clock_, &transport_, nullptr, nullptr, &seq_num_allocator_, &feedback_observer_, nullptr, nullptr, nullptr, &mock_rtc_event_log_, nullptr, &retransmission_rate_limiter_, &mock_overhead_observer)); - rtp_sender_->SetTransportOverhead(kTransportOverheadBytesPerPacket); EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( kRtpExtensionTransportSequenceNumber, kTransportSequenceNumberExtensionId)); @@ -1447,13 +1436,11 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { EXPECT_CALL(feedback_observer_, AddPacket(kTransportSequenceNumber, sizeof(kPayloadData) + kGenericHeaderLength + - kRtpOverheadBytesPerPacket + - kTransportOverheadBytesPerPacket, + kRtpOverheadBytesPerPacket, PacketInfo::kNotAProbe)) .Times(1); EXPECT_CALL(mock_overhead_observer, - OnOverheadChanged(kTransportOverheadBytesPerPacket + - kRtpOverheadBytesPerPacket)) + OnOverheadChanged(kRtpOverheadBytesPerPacket)) .Times(1); SendGenericPayload(); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 490bfbae4a..7fcc6578fb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -337,7 +337,7 @@ bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type, retransmission_settings = retransmission_settings_; } - size_t packet_capacity = rtp_sender_->MaxPayloadLength() - + size_t packet_capacity = rtp_sender_->MaxRtpPacketSize() - fec_packet_overhead - (rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0); RTC_DCHECK_LE(packet_capacity, rtp_header->capacity()); diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index 2a30043b9f..7c7db4dabc 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -130,20 +130,10 @@ TEST_F(RtpRtcpAPITest, Basic) { EXPECT_TRUE(module_->Sending()); } -TEST_F(RtpRtcpAPITest, MTU) { - EXPECT_EQ(0, module_->SetMaxTransferUnit(1234)); - EXPECT_EQ(1234 - 20 - 8, module_->MaxPayloadLength()); - - EXPECT_EQ(0, module_->SetTransportOverhead(true, true, 12)); - EXPECT_EQ(1234 - 20 - 20 - 20 - 12, module_->MaxPayloadLength()); - - EXPECT_EQ(0, module_->SetTransportOverhead(false, false, 0)); - EXPECT_EQ(1234 - 20 - 8, module_->MaxPayloadLength()); - - module_->SetTransportOverhead(28); - EXPECT_EQ(1234 - 28, module_->MaxPayloadLength()); - module_->SetTransportOverhead(44); - EXPECT_EQ(1234 - 44, module_->MaxPayloadLength()); +TEST_F(RtpRtcpAPITest, PacketSize) { + module_->SetMaxRtpPacketSize(1234); + EXPECT_EQ(1234u, module_->MaxRtpPacketSize()); + EXPECT_EQ(1234u - 12u /* Minimum RTP header */, module_->MaxPayloadSize()); } TEST_F(RtpRtcpAPITest, SSRC) { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 9a3b51e278..6dc68484d1 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -39,6 +39,9 @@ namespace webrtc { static const int kMinSendSidePacketHistorySize = 600; namespace { +// We don't do MTU discovery, so assume that we have the standard ethernet MTU. +const size_t kPathMTU = 1500; + std::vector CreateRtpRtcpModules( Transport* outgoing_transport, RtcpIntraFrameObserver* intra_frame_callback, @@ -326,7 +329,7 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver, void EnableEncodedFrameRecording(const std::vector& files, size_t byte_limit); - void SetTransportOverhead(int transport_overhead_per_packet); + void SetTransportOverhead(size_t transport_overhead_per_packet); private: class CheckEncoderActivityTask; @@ -689,7 +692,8 @@ VideoSendStream::RtpStateMap VideoSendStream::StopPermanentlyAndGetRtpStates() { return state_map; } -void VideoSendStream::SetTransportOverhead(int transport_overhead_per_packet) { +void VideoSendStream::SetTransportOverhead( + size_t transport_overhead_per_packet) { RTC_DCHECK_RUN_ON(&thread_checker_); VideoSendStreamImpl* send_stream = send_stream_.get(); worker_queue_->PostTask([send_stream, transport_overhead_per_packet] { @@ -808,8 +812,7 @@ VideoSendStreamImpl::VideoSendStreamImpl( for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { rtp_rtcp->RegisterRtcpStatisticsCallback(stats_proxy_); rtp_rtcp->RegisterSendChannelRtpStatisticsCallback(stats_proxy_); - rtp_rtcp->SetMaxTransferUnit( - static_cast(config_->rtp.max_packet_size)); + rtp_rtcp->SetMaxRtpPacketSize(config_->rtp.max_packet_size); rtp_rtcp->RegisterVideoSendPayload( config_->encoder_settings.payload_type, config_->encoder_settings.payload_name.c_str()); @@ -1188,14 +1191,27 @@ uint32_t VideoSendStreamImpl::OnBitrateUpdated(uint32_t bitrate_bps, if (webrtc::field_trial::FindFullName("WebRTC-SendSideBwe-WithOverhead") == "Enabled") { - // Substract overhead from bitrate. - rtc::CritScope lock(&overhead_bytes_per_packet_crit_); - int packets_per_second = - std::ceil(bitrate_bps / (8 * (config_->rtp.max_packet_size + - transport_overhead_bytes_per_packet_))); - uint32_t overhead_bps = static_cast( - 8 * overhead_bytes_per_packet_ * packets_per_second); - bitrate_bps = overhead_bps > bitrate_bps ? 0u : bitrate_bps - overhead_bps; + // Subtract total overhead (transport + rtp) from bitrate. + size_t rtp_overhead; + { + rtc::CritScope lock(&overhead_bytes_per_packet_crit_); + rtp_overhead = overhead_bytes_per_packet_; + } + RTC_CHECK_GE(rtp_overhead, 0); + RTC_DCHECK_LT(rtp_overhead, config_->rtp.max_packet_size); + if (rtp_overhead >= config_->rtp.max_packet_size) { + LOG(LS_WARNING) << "RTP overhead (" << rtp_overhead << " bytes)" + << "exceeds maximum packet size (" + << config_->rtp.max_packet_size << " bytes)"; + + bitrate_bps = 0; + } else { + bitrate_bps = + static_cast(static_cast(bitrate_bps) * + (config_->rtp.max_packet_size - rtp_overhead) / + (config_->rtp.max_packet_size + + transport_overhead_bytes_per_packet_)); + } } // Get the encoder target rate. It is the estimated network rate - @@ -1263,15 +1279,23 @@ void VideoSendStreamImpl::OnOverheadChanged(size_t overhead_bytes_per_packet) { } void VideoSendStreamImpl::SetTransportOverhead( - int transport_overhead_bytes_per_packet) { + size_t transport_overhead_bytes_per_packet) { + if (transport_overhead_bytes_per_packet >= static_cast(kPathMTU)) { + LOG(LS_ERROR) << "Transport overhead exceeds size of ethernet frame"; + return; + } + transport_overhead_bytes_per_packet_ = transport_overhead_bytes_per_packet; - RTC_DCHECK_LE(config_->rtp.max_packet_size, - 0xFFFFu + transport_overhead_bytes_per_packet); - const uint16_t mtu = static_cast( - config_->rtp.max_packet_size + transport_overhead_bytes_per_packet); + + congestion_controller_->SetTransportOverhead( + transport_overhead_bytes_per_packet_); + + size_t rtp_packet_size = + std::min(config_->rtp.max_packet_size, + kPathMTU - transport_overhead_bytes_per_packet_); + for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { - rtp_rtcp->SetTransportOverhead(transport_overhead_bytes_per_packet); - rtp_rtcp->SetMaxTransferUnit(mtu); + rtp_rtcp->SetMaxRtpPacketSize(rtp_packet_size); } } diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h index 34b9d5c890..be62a2eaf9 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -92,7 +92,7 @@ class VideoSendStream : public webrtc::VideoSendStream { RtpStateMap StopPermanentlyAndGetRtpStates(); - void SetTransportOverhead(int transport_overhead_per_packet); + void SetTransportOverhead(size_t transport_overhead_per_packet); private: class ConstructionTask; diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc index 100832de3a..84d7a204b1 100644 --- a/webrtc/voice_engine/channel.cc +++ b/webrtc/voice_engine/channel.cc @@ -914,6 +914,8 @@ Channel::Channel(int32_t channelId, _lastLocalTimeStamp(0), _lastPayloadType(0), _includeAudioLevelIndication(false), + transport_overhead_per_packet_(0), + rtp_overhead_per_packet_(0), _outputSpeechType(AudioFrame::kNormalSpeech), restored_packet_in_use_(false), rtcp_observer_(new VoERtcpObserver(this)), @@ -2878,16 +2880,23 @@ void Channel::SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats) { rtcp_rtt_stats_proxy_->SetRtcpRttStats(rtcp_rtt_stats); } -void Channel::SetTransportOverhead(int transport_overhead_per_packet) { - _rtpRtcpModule->SetTransportOverhead(transport_overhead_per_packet); +void Channel::UpdateOverheadForEncoder() { + audio_coding_->ModifyEncoder([&](std::unique_ptr* encoder) { + if (*encoder) { + (*encoder)->OnReceivedOverhead(transport_overhead_per_packet_ + + rtp_overhead_per_packet_); + } + }); +} + +void Channel::SetTransportOverhead(size_t transport_overhead_per_packet) { + transport_overhead_per_packet_ = transport_overhead_per_packet; + UpdateOverheadForEncoder(); } void Channel::OnOverheadChanged(size_t overhead_bytes_per_packet) { - audio_coding_->ModifyEncoder([&](std::unique_ptr* encoder) { - if (*encoder) { - (*encoder)->OnReceivedOverhead(overhead_bytes_per_packet); - } - }); + rtp_overhead_per_packet_ = overhead_bytes_per_packet; + UpdateOverheadForEncoder(); } int Channel::RegisterExternalMediaProcessing(ProcessingTypes type, diff --git a/webrtc/voice_engine/channel.h b/webrtc/voice_engine/channel.h index 11e109e3c8..fca682141d 100644 --- a/webrtc/voice_engine/channel.h +++ b/webrtc/voice_engine/channel.h @@ -415,7 +415,7 @@ class Channel void SetRtcEventLog(RtcEventLog* event_log); void SetRtcpRttStats(RtcpRttStats* rtcp_rtt_stats); - void SetTransportOverhead(int transport_overhead_per_packet); + void SetTransportOverhead(size_t transport_overhead_per_packet); // From OverheadObserver in the RTP/RTCP module void OnOverheadChanged(size_t overhead_bytes_per_packet) override; @@ -443,6 +443,8 @@ class Channel RTPExtensionType type, unsigned char id); + void UpdateOverheadForEncoder(); + int GetRtpTimestampRateHz() const; int64_t GetRTT(bool allow_associate_channel) const; @@ -529,6 +531,8 @@ class Channel uint32_t _lastLocalTimeStamp; int8_t _lastPayloadType; bool _includeAudioLevelIndication; + size_t transport_overhead_per_packet_; + size_t rtp_overhead_per_packet_; // VoENetwork AudioFrame::SpeechType _outputSpeechType; // VoEVideoSync